Ticket #512 (closed defect: fixed)

Opened 12 years ago

Last modified 10 years ago

sftp subsystem should not return stdout/stderr from dotfiles

Reported by: geofft Owned by: jdreed
Priority: normal Milestone: Natty Beta
Component: -- Keywords:
Cc: Fixed in version:
Upstream bug:

Description

(this should be discussed with upstream OpenSSH)

Surely there must be some way to write the code such that when you make an sftp subsystem request on an SSH connection, sshd doesn't display output from the dotfiles and only sends output from the sftp process to the client, right?

I think we can't simply get by with not running your dotfiles in case they do something like aklog sipb.

Change History

comment:1 Changed 11 years ago by jdreed

  • Priority changed from low to major
  • Type changed from enhancement to defect
  • Milestone changed from Upstream Utopia to The Distant Future

This is a behavior change from Athena 9 (the Suns, anyway). boojum noted today that a user who attached (without -q) a locker in their .environment was losing when trying to sftp (from a Mac, if that's relevant):

       Connecting to athena.dialup.mit.edu...
       Received message too long 1114795883

Changing it to an "attach -q" solved the problem, however this does seem to be a behavior change, as there is no problem sftp'ing to the Suns with a .environment that generates output.

I don't know if there's an easy fix (do something clever in the dotfiles?), but I don't quite want it to fall off our radar.

comment:2 Changed 11 years ago by jdreed

  • Milestone changed from The Distant Future to IAP 2011

comment:3 Changed 11 years ago by geofft

Okay, it looks like ssh is just calling $SHELL -c /usr/lib/openssh/sftp-server, which would make it particularly hard for upstream OpenSSH to be able to fix this. (There's some evidence of an internal sftp server that's not a subprocess that appears not in use in the Debian build as far as I can see, which probably explains the difference in the Suns' behavior). However, we can work around it: I propose that around the call to sourcing .environment or .bash_environment, we check if "$2" is /usr/lib/openssh/sftp-server, and if so redirect stdout and stderr to /dev/null for the duration of the call and reset them when done.

Since this is not being called as a login (argv[0][0] == '-') shell, it's our awesome dotfiles that are at fault here for calling .{bash_,}environment everywhere, and so I don't think this is really an upstream issue, and I don't think we need to care about dotfiles other than .{bash_,}environment.

comment:4 Changed 10 years ago by jdreed

I'm fine with geofft's proposed plan in the most recent comment. Is there a reason not to do this?

comment:5 follow-up: ↓ 6 Changed 10 years ago by jdreed

  • Status changed from new to accepted
  • Owner set to jdreed

Having tested this a bit more, the "$2" trick doesn't work, because $2 doesn't get set, AFAICT.

So we have a couple of options:
a) Conditionalize on echo $0 | grep -q "^-"
b) Conditionalize on [ -z "$PS1" ] or (! $?prompt)
c) Conditionalize on [ -z "$SSH_TTY" ] && [ -n "$SSH_CONNECTION" ]

(a) is probably "right", though I'm concerned about unintended consequences*
(b) also seems reasonable, but I'm also concerned about unintended consequences*
(c) will fix this specific problem only.

  • There exist people who still use kerberized rsh and rlogin, and I don't recall under which circumstances those are login shells and under which circumstances prompt/PS1 are set. If someone has a canonical answer, then that makes this easier.

Thoughts?

comment:6 in reply to: ↑ 5 Changed 10 years ago by andersk

(a) would trigger when someone interactively runs a second bash, which is wrong.

All of your proposals are about changing the behavior of ssh HOST command for every command. I’m worried about doing that, because it would confuse someone who tries to use echo to figure out which dotfiles get run in that case, and dotfiles are confusing enough already.

comment:7 Changed 10 years ago by jdreed

  • Status changed from accepted to committed

Anders pointed out the existence of $BASH_EXECUTION_STRING and tcsh's $command.
Committed in r25097-r25100

comment:8 Changed 10 years ago by jdreed

I'm going to build this today. For maximum hilarity, this bug also prevents people from using file transfer when they're over quota, because /usr/share/debathena-bash-config/bashrc.d/10-dialup-environment spews errors about ~/.lastdialup.

comment:9 Changed 10 years ago by jdreed

  • Status changed from committed to development

Fixed in dotfiles 10.0.28-0debathena1

comment:10 Changed 10 years ago by jdreed

  • Status changed from development to closed
  • Resolution set to fixed

This was moved a while ago.

comment:11 Changed 10 years ago by jdreed

  • Status changed from closed to reopened
  • Resolution fixed deleted

To bad we never actually tested this. The value of $command on the dialups is, at least:

/usr/bin/nice -5 /usr/lib/openssh/sftp-server

which does not match the case statement

comment:12 Changed 10 years ago by jdreed

can we get away with echo $command | grep -q /usr/lib/openssh/sftp-server?

It's not like anyone is ever going to run legit commands with that in the command line and not expect an SFTP session. It's also unclear to me that the second part of the case statement (scp *) is ever true.

comment:13 Changed 10 years ago by jdreed

In fact, no, the value of $command and $BASH_EXECUTION_STRING is "" when scp is being run. scp also doesn't barf on dotfiles.

comment:14 Changed 10 years ago by jdreed

  • Status changed from reopened to committed

Re-committed in r25364.

comment:15 Changed 10 years ago by jdreed

  • Status changed from committed to development

10.0.29-0debathena1 -> dev

comment:16 Changed 10 years ago by jdreed

In production. We should poke Ops to take this on the dialups.

comment:17 Changed 10 years ago by jdreed

  • Status changed from development to closed
  • Resolution set to fixed

Ops has been poked.

Note: See TracTickets for help on using tickets.