Bug 1750 - Sftp hangs if stderr is used.
Summary: Sftp hangs if stderr is used.
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 5.4p1
Hardware: Other All
: P2 major
Assignee: Assigned to nobody
URL:
Keywords:
: 1755 (view as bug list)
Depends on:
Blocks: V_5_6
  Show dependency treegraph
 
Reported: 2010-04-08 20:02 AEST by jchadima
Modified: 2023-01-13 13:36 AEDT (History)
3 users (show)

See Also:


Attachments
session-subsys-stderr-devnull.diff (4.04 KB, patch)
2010-04-23 10:50 AEST, Damien Miller
no flags Details | Diff
/home/djm/sshd-ignore-subsys-stderr.diff (6.46 KB, patch)
2010-06-23 21:51 AEST, Damien Miller
no flags Details | Diff
/home/djm/sshd-ignore-subsys-stderr.diff (6.21 KB, patch)
2010-06-25 12:13 AEST, Damien Miller
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jchadima 2010-04-08 20:02:36 AEST
According to SSH File Transfer Protocol draft-ietf-secsh-filexfer-13 the usage of stderr may not harm the transfer. 
The chapter 3.1 says: 

"Data sent on stderr by the server SHOULD be considered free format  debug or supplemental error information, and MAY be displayed to the user." 

Actually any try to write to stderr causes immediate server hangup.

The test case is use bash as the login shell of the user and add line
echo "Hello" >&2
into ~/.bashrc

In the openssh with session.c before version 1.237 it works, later it hangs.
Comment 1 Damien Miller 2010-04-08 21:08:15 AEST
When you say "session.c before version 1.237" do you mean that 1.237 is affected? Also, are you using the version from portable OpenSSH CVS or from OpenBSD CVS?
Comment 2 jchadima 2010-04-08 22:20:06 AEST
(In reply to comment #1)
> When you say "session.c before version 1.237" do you mean that 1.237 is
> affected?
yes all versions from 1.237 are affected, all older aren't



 Also, are you using the version from portable OpenSSH CVS or
> from OpenBSD CVS?

portable openssh.

The problem is in this chunk of the patch:

@@ -507,6 +507,10 @@
*/
if (compat20) {
session_set_fds(s, inout[1], inout[1], s->is_subsystem ? -1 : err[1]);
+ /* close err[1] to not leak the socket if this inside a subsystem */
+ if (s->is_subsystem) {
+ close(err[1]);
+ }
} else {
server_loop(pid, inout[1], inout[1], err[1]);
/* server_loop has closed inout[1] and err[1]. */


and another one for pipes also.


the stderr is closed and any attempt to write to it causes the hang.
Comment 3 Damien Miller 2010-04-23 10:50:53 AEST
Created attachment 1840 [details]
session-subsys-stderr-devnull.diff

set stderr to /dev/null for subsystems
Comment 4 Damien Miller 2010-04-23 11:00:30 AEST
This should fix it.
Comment 5 Damien Miller 2010-04-24 08:42:21 AEST
patch applied - will be in 5.6. Thanks!
Comment 6 jchadima 2010-06-23 16:42:16 AEST
This solutions is not 100% correct because breaks the chroot environment with internal-sftp. Typical setup:

subsystem sftp internal-sftp

match group sftponly
    chroot directory /home/%u
    x11forwarding no
    allowtcpforwarding no
    forcecommand internal-sftp

the /home/%u directories are root owned with some subdirs owned by an user, but there should not be the copy of dev directory inside.

The patch wants to have at least /dev/null inside the chroot.
Comment 7 jchadima 2010-06-23 16:55:49 AEST
The possible sollution is to open /dev/null before chroot.
Comment 8 Damien Miller 2010-06-23 21:51:01 AEST
Created attachment 1884 [details]
/home/djm/sshd-ignore-subsys-stderr.diff

Ignore stderr from subsystems in channels code (warning: completely untested diff)
Comment 9 jchadima 2010-06-23 23:41:21 AEST
Seems to be working for me, thx.
Comment 10 Damien Miller 2010-06-25 12:13:18 AEST
Created attachment 1885 [details]
/home/djm/sshd-ignore-subsys-stderr.diff

improved patch
Comment 11 Damien Miller 2010-06-25 17:20:16 AEST
patch applied - will be in 5.6. Thanks.
Comment 12 Damien Miller 2010-06-25 21:33:55 AEST
*** Bug 1755 has been marked as a duplicate of this bug. ***
Comment 13 Damien Miller 2011-01-24 12:33:42 AEDT
Move resolved bugs to CLOSED after 5.7 release