4357052 1999-10-01 20:45 /137 rader/ Postmaster Mottagare: Bugtraq (import) <8027> Ärende: Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM X-Accept-Language: en MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------EE7E52CF326C0771F6E0B66B" Message-ID: <37F3E326.8A5407A8@kestrel.cc.ukans.edu> Date: Thu, 30 Sep 1999 17:24:38 -0500 Reply-To: Jeff Long <long@KESTREL.CC.UKANS.EDU> Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Jeff Long <long@KESTREL.CC.UKANS.EDU> Organization: #f X-To: BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM This is a multi-part message in MIME format. --------------EE7E52CF326C0771F6E0B66B Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Dan Astoorian wrote: > > On Wed, 29 Sep 1999 16:59:48 EDT, Sylvain Robitaille writes: > > I don't promise the most impressive code, but it has been tested (on > > Digital Unix) and I believe it works correctly. Comments are of course > > welcome... > > I have a couple of serious concerns about this patch. > > 1) It leaves behind a race condition; a symlink created between the > lstat() and the bind() will still get happily followed. The race > condition could be minimized by moving the lstat() and the bind() > closer together, but it can't be eliminated this way. This is why > it's important for the check to be made in the kernel, where it can > be done atomically. <snip> > The race condition is a hard problem; if bind() follows symlinks, it is > *impossible* to safely use it in a directory writable by anyone other > than geteuid(). > > I haven't looked into what would be involved in creating a proper patch, > but appropriate ways to fix the problem *might* include: > > - changing the process's effective userid/groupid/credentials to match > the target user before doing the bind(), so that directories not > writable by the user are also not writable by the code doing the > bind(); or <snip> Seeing the race problems with the previous two patches I thought I would take a shot at one. It changes the effective uid/gid to the user logging in before doing the bind() (and then resets them after) which seems to take care of the problem. It also chown's the /tmp/ssh-<username> directory before doing the bind in the case that it did not already exist so that the user owns it before performing the bind. On Digital Unix 4.0D this seems to take care of the problem. The bind() will fail if a symlink exists to a file that the user would normally not be able to write to (such as /etc/nologin). The only other difference after logging in is that the socket will now be owned by the user instead of root but I can't think of a reason why this would be bad. If anyone sees problems in this patch please let me know. Jeff Long --------------EE7E52CF326C0771F6E0B66B Content-Type: text/plain; charset=us-ascii; name="newchannels.c.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="newchannels.c.patch" *** newchannels.c.original Thu Sep 30 16:58:22 1999 --- newchannels.c Thu Sep 30 17:13:24 1999 *************** *** 2262,2267 **** --- 2262,2269 ---- struct stat st, st2, parent_st; mode_t old_umask; char *last_dir; + uid_t saved_euid = 0; + gid_t saved_egid = 0; if (auth_get_socket_name() != NULL) fatal("Protocol error: authentication forwarding requested twice."); *************** *** 2411,2427 **** creating unix-domain sockets, you might not be able to use ssh-agent connections on your system */ old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); ! ! if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0) ! packet_disconnect("Agent socket bind failed: %.100s", strerror(errno)); ! ! umask(old_umask); ! if (directory_created) if (chown(".", pw->pw_uid, pw->pw_gid) < 0) packet_disconnect("Agent socket directory chown failed: %.100s", strerror(errno)); /* Start listening on the socket. */ if (listen(sock, 5) < 0) packet_disconnect("Agent socket listen failed: %.100s", strerror(errno)); --- 2413,2442 ---- creating unix-domain sockets, you might not be able to use ssh-agent connections on your system */ old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); ! if (directory_created) if (chown(".", pw->pw_uid, pw->pw_gid) < 0) packet_disconnect("Agent socket directory chown failed: %.100s", strerror(errno)); + saved_euid = geteuid(); + saved_egid = getegid(); + + if (setegid(pw->pw_gid) < 0) + packet_disconnect("Agent socket setegid failed: %.100s", strerror(errno)); + if (seteuid(pw->pw_uid) < 0) + packet_disconnect("Agent socket seteuid failed: %.100s", strerror(errno)); + + if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0) + packet_disconnect("Agent socket bind failed: %.100s", strerror(errno)); + + if (seteuid(saved_euid) < 0) + packet_disconnect("Agent socket re-seteuid failed: %.100s", strerror(errno)); + if (setegid(saved_egid) < 0) + packet_disconnect("Agent socket re-setegid failed: %.100s", strerror(errno)); + + umask(old_umask); + /* Start listening on the socket. */ if (listen(sock, 5) < 0) packet_disconnect("Agent socket listen failed: %.100s", strerror(errno)); --------------EE7E52CF326C0771F6E0B66B-- (4357052) ----------------------------------- 4357077 1999-10-01 20:54 /144 rader/ Postmaster Mottagare: Bugtraq (import) <8029> Ärende: Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM X-Accept-Language: en MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------AD6746A5F112E9AFFCB93E5F" Message-ID: <37F3E416.F07E7A7@kestrel.cc.ukans.edu> Date: Thu, 30 Sep 1999 17:28:38 -0500 Reply-To: Jeff Long <long@KESTREL.CC.UKANS.EDU> Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Jeff Long <long@KESTREL.CC.UKANS.EDU> Organization: #f X-To: BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM This is a multi-part message in MIME format. --------------AD6746A5F112E9AFFCB93E5F Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Oh crud, this wasn't tested on Digital Unix 4.0D. It was tested before and after on Compaq Tru64 5.0. Jeff Long Jeff Long wrote: > > Dan Astoorian wrote: > > > > On Wed, 29 Sep 1999 16:59:48 EDT, Sylvain Robitaille writes: > > > I don't promise the most impressive code, but it has been tested (on > > > Digital Unix) and I believe it works correctly. Comments are of course > > > welcome... > > > > I have a couple of serious concerns about this patch. > > > > 1) It leaves behind a race condition; a symlink created between the > > lstat() and the bind() will still get happily followed. The race > > condition could be minimized by moving the lstat() and the bind() > > closer together, but it can't be eliminated this way. This is why > > it's important for the check to be made in the kernel, where it can > > be done atomically. > <snip> > > The race condition is a hard problem; if bind() follows symlinks, it is > > *impossible* to safely use it in a directory writable by anyone other > > than geteuid(). > > > > I haven't looked into what would be involved in creating a proper patch, > > but appropriate ways to fix the problem *might* include: > > > > - changing the process's effective userid/groupid/credentials to match > > the target user before doing the bind(), so that directories not > > writable by the user are also not writable by the code doing the > > bind(); or > <snip> > > Seeing the race problems with the previous two patches I thought I would > take a shot at one. It changes the effective uid/gid to the user > logging in before doing the bind() (and then resets them after) which > seems to take care of the problem. It also chown's the > /tmp/ssh-<username> directory before doing the bind in the case that it > did not already exist so that the user owns it before performing the > bind. On Digital Unix 4.0D this seems to take care of the problem. The > bind() will fail if a symlink exists to a file that the user would > normally not be able to write to (such as /etc/nologin). The only other > difference after logging in is that the socket will now be owned by the > user instead of root but I can't think of a reason why this would be > bad. > > If anyone sees problems in this patch please let me know. > > Jeff Long --------------AD6746A5F112E9AFFCB93E5F Content-Type: text/plain; charset=us-ascii; name="newchannels.c.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="newchannels.c.patch" *** newchannels.c.original Thu Sep 30 16:58:22 1999 --- newchannels.c Thu Sep 30 17:13:24 1999 *************** *** 2262,2267 **** --- 2262,2269 ---- struct stat st, st2, parent_st; mode_t old_umask; char *last_dir; + uid_t saved_euid = 0; + gid_t saved_egid = 0; if (auth_get_socket_name() != NULL) fatal("Protocol error: authentication forwarding requested twice."); *************** *** 2411,2427 **** creating unix-domain sockets, you might not be able to use ssh-agent connections on your system */ old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); ! ! if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0) ! packet_disconnect("Agent socket bind failed: %.100s", strerror(errno)); ! ! umask(old_umask); ! if (directory_created) if (chown(".", pw->pw_uid, pw->pw_gid) < 0) packet_disconnect("Agent socket directory chown failed: %.100s", strerror(errno)); /* Start listening on the socket. */ if (listen(sock, 5) < 0) packet_disconnect("Agent socket listen failed: %.100s", strerror(errno)); --- 2413,2442 ---- creating unix-domain sockets, you might not be able to use ssh-agent connections on your system */ old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); ! if (directory_created) if (chown(".", pw->pw_uid, pw->pw_gid) < 0) packet_disconnect("Agent socket directory chown failed: %.100s", strerror(errno)); + saved_euid = geteuid(); + saved_egid = getegid(); + + if (setegid(pw->pw_gid) < 0) + packet_disconnect("Agent socket setegid failed: %.100s", strerror(errno)); + if (seteuid(pw->pw_uid) < 0) + packet_disconnect("Agent socket seteuid failed: %.100s", strerror(errno)); + + if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0) + packet_disconnect("Agent socket bind failed: %.100s", strerror(errno)); + + if (seteuid(saved_euid) < 0) + packet_disconnect("Agent socket re-seteuid failed: %.100s", strerror(errno)); + if (setegid(saved_egid) < 0) + packet_disconnect("Agent socket re-setegid failed: %.100s", strerror(errno)); + + umask(old_umask); + /* Start listening on the socket. */ if (listen(sock, 5) < 0) packet_disconnect("Agent socket listen failed: %.100s", strerror(errno)); --------------AD6746A5F112E9AFFCB93E5F-- (4357077) ----------------------------------- 4357101 1999-10-01 21:00 /42 rader/ Postmaster Mottagare: Bugtraq (import) <8030> Ärende: Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM Message-ID: <99Sep30.150614edt.96306-2339@jane.cs.toronto.edu> Date: Thu, 30 Sep 1999 15:06:12 -0400 Reply-To: Dan Astoorian <djast@CS.TORONTO.EDU> Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Dan Astoorian <djast@CS.TORONTO.EDU> X-To: BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM [To Aleph1: please kill my previous reply to Eric Griffis's patch; it contains mostly the same content as my reply to Sylvain Robitaille's patch, which I'd assumed you'd rejected.] Eric Griffis's patch suffers from the same race condition as Sylvain Robitaille's: the link could be created between the lstat() and the bind(). It's better than nothing, but it doesn't get rid of the whole problem. As I said before, I haven't done any testing, so I don't know if this would a) work, or b) be effective against the flaw, but: has anyone considered an approach like adding this sort of code: if (setregid(-1, pw->pw_gid) < 0 || setreuid(-1, pw->pw_uid) < 0) { ... /*error*/ } before the bind() call, and: if (setreuid(-1, 0) < 0) { ... /*error*/ }; after? (In case it's not clear, what I'm trying to do is assume the user's uid/gid for the duration of the bind(), and reacquire root privs afterwards.) -- People shouldn't think that it's better to have Dan Astoorian loved and lost than never loved at all. It's Sysadmin, CS Lab not, it's better to have loved and won. All djast@cs.toronto.edu the other options really suck. --Dan Redican (4357101) ----------------------------------- 4360722 1999-10-04 04:57 /198 rader/ Postmaster Mottagare: Bugtraq (import) <8040> Ärende: Fix for ssh-1.2.27 symlink/bind problem ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM X-Authentication-Warning: sgifford.tir.com: sgifford set sender t sgifford@tir.com using -f Mime-Version: 1.0 (generated by tm-edit 7.106) Content-Type: text/plain; charset=US-ASCII Message-ID: <m3hfk9l761.fsf@sgifford.tir.com> Date: Sat, 2 Oct 1999 18:38:46 -0400 Reply-To: Scott Gifford <sgifford@TIR.COM> Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Scott Gifford <sgifford@TIR.COM> X-To: BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM I've put together a patch that lets ssh work around the OS bug that allows bind to follow symlinks. It makes a safe directory with mkdir(), which doesn't follow symlinks, makes the socket in there, then moves it to its real location with rename(), which also doesn't follow symlinks. It depends on being able to rename a UNIX socket with the standard rename() system call, which may not be portable, but it works on the Linux 2.2.12 system I tested it on. I'm not sure if it offers any advantages over Jeff Long's patch which simply dropped root privileges before the bind(), but it preserves the previous ssh behavior of the socket being owned by root. Plus, it was already 90% done before I saw Jeff's message. :) If anybody sees any problems in this patch, please let me know... -----Scott. diff -c ssh-1.2.27/newchannels.c ssh-1.2.27-sg/newchannels.c *** ssh-1.2.27/newchannels.c Wed May 12 07:19:27 1999 --- ssh-1.2.27-sg/newchannels.c Sat Oct 2 18:09:05 1999 *************** *** 2262,2267 **** --- 2262,2269 ---- struct stat st, st2, parent_st; mode_t old_umask; char *last_dir; + char tmpbinddir[1024]; + int broke=0; if (auth_get_socket_name() != NULL) fatal("Protocol error: authentication forwarding requested twice."); *************** *** 2401,2419 **** packet_disconnect("Agent socket creation failed: %.100s", strerror(errno)); /* Bind it to the name. */ - memset(&sunaddr, 0, AF_UNIX_SIZE(sunaddr)); - sunaddr.sun_family = AF_UNIX; - strncpy(sunaddr.sun_path, channel_forwarded_auth_socket_name, - sizeof(sunaddr.sun_path)); - /* Use umask to get desired permissions, chmod is too dangerous NOTE: If your system doesn't handle umask correctly when creating unix-domain sockets, you might not be able to use ssh-agent connections on your system */ old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0) ! packet_disconnect("Agent socket bind failed: %.100s", strerror(errno)); umask(old_umask); --- 2403,2516 ---- packet_disconnect("Agent socket creation failed: %.100s", strerror(errno)); /* Bind it to the name. */ /* Use umask to get desired permissions, chmod is too dangerous NOTE: If your system doesn't handle umask correctly when creating unix-domain sockets, you might not be able to use ssh-agent connections on your system */ old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); + /* OK, this is tricky. --sg + * + * We're going to make a directory that root owns, make the socket + * in there, then rename it to where we need to be. This makes all + * the *important things atomic. We are already in the socket + * directory. + **/ + + tmpbinddir[1023]=0; + snprintf(tmpbinddir,1023,"tempbinddir-%ld-%d",time(0),getpid()); + if (mkdir(tmpbinddir,0700) == -1) + { + packet_send_debug("* Remote error: Agent socket creation: couldn't make temp bind dir"); + packet_send_debug("* Remote error: Authentication forwarding disabled."); + return 0; + } + + if (lstat(tmpbinddir,&st) == -1) + { + packet_send_debug("* Remote error: Agent socket creation: couldn't stat temp bind dir"); + packet_send_debug("* Remote error: Authentication forwarding disabled."); + return 0; + } + + /* Are we owned by root? */ + if (st.st_uid != 0) + { + packet_send_debug("* Remote error: Agent socket creation: ownership changed on temp bind dir"); + broke=1; + goto abort_after_mkdir; + } + + if (chdir(tmpbinddir) == -1) + { + packet_send_debug("* Remote error: Agent socket creation: couldn't chdir to temp bind dir"); + broke=1; + goto abort_after_mkdir; + } + if (lstat(".", &st2) == -1) + { + packet_send_debug("* Remote error: Agent socket creation: couldn't stat . in temp bind dir"); + broke=1; + goto abort_after_mkdir; + } + if ( (st2.st_uid != 0) || (st2.st_ino != st.st_ino) || (st2.st_dev != st.st_dev) || (st2.st_mode != st.st_mode)) + { + packet_send_debug("* Remote error: Agent socket creation: Permissions changed in temp bind dir"); + broke=1; + goto abort_after_mkdir; + } + + /* OK, now we know we're in the directory we created. Nobody can + * rmdir() this because we are in it. Nobody besides root can have + * made a symlink in here, because they wouldn't have permission. + * Lookin' good... + **/ + + memset(&sunaddr, 0, AF_UNIX_SIZE(sunaddr)); + sunaddr.sun_family = AF_UNIX; + strncpy(sunaddr.sun_path, "tempauthsock", sizeof(sunaddr.sun_path)); + if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0) ! { ! packet_send_debug("Agent socket bind failed: %.100s",strerror(errno)); ! broke=1; ! goto abort_after_mkdir; ! } ! ! /* Change the relative socket name to absolute */ ! snprintf(channel_forwarded_auth_socket_name, ! strlen(SSH_AGENT_SOCKET_DIR) + strlen(SSH_AGENT_SOCKET) + ! strlen(pw->pw_name) + 10, ! SSH_AGENT_SOCKET_DIR"/"SSH_AGENT_SOCKET, ! pw->pw_name, (int)getpid()); ! ! if (rename("tempauthsock",channel_forwarded_auth_socket_name) == -1) ! { ! packet_send_debug("* Remote error: Agent socket bind rename failed: %.100s",strerror(errno)); ! if (unlink("tempauthsock") == -1) ! { ! /* Not much we can really do about it... */ ! packet_send_debug("* Remote error: Couldn't remove temp auth sock: %.100s",strerror(errno)); ! } ! broke=1; ! goto abort_after_mkdir; ! } ! ! abort_after_mkdir: ! if (chdir(channel_forwarded_auth_socket_dir_name) == -1) ! { ! packet_send_debug("* Remote error: Couldn't chdir out of temp bind dir"); ! } ! if (rmdir(tmpbinddir) == -1) ! { ! packet_send_debug("* Remote error: Couldn't remove temp bind dir"); ! } ! if (broke) ! { ! packet_send_debug("* Remote error: Authentication forwarding disabled."); ! return 0; ! } ! /* And we're in the same state as the old code. */ umask(old_umask); *************** *** 2426,2438 **** if (listen(sock, 5) < 0) packet_disconnect("Agent socket listen failed: %.100s", strerror(errno)); - /* Change the relative socket name to absolute */ - snprintf(channel_forwarded_auth_socket_name, - strlen(SSH_AGENT_SOCKET_DIR) + strlen(SSH_AGENT_SOCKET) + - strlen(pw->pw_name) + 10, - SSH_AGENT_SOCKET_DIR"/"SSH_AGENT_SOCKET, - pw->pw_name, (int)getpid()); - /* Allocate a channel for the authentication agent socket. */ newch = channel_allocate(SSH_CHANNEL_AUTH_LISTENER, sock, xstrdup("auth socket")); --- 2523,2528 ---- (4360722) ----------------------------------- 4360726 1999-10-04 05:14 /107 rader/ Postmaster Mottagare: Bugtraq (import) <8042> Ärende: Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM Message-ID: <99Oct1.145214edt.96305-2339@jane.cs.toronto.edu> Date: Fri, 1 Oct 1999 14:52:09 -0400 Reply-To: Dan Astoorian <djast@CS.TORONTO.EDU> Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Dan Astoorian <djast@CS.TORONTO.EDU> X-To: Eric Griffis <egriffis@COMMONTECH.COM> Jeff Long <long@KESTREL.CC.UKANS.EDU> X-cc: BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM In-Reply-To: Your message of "Thu, 30 Sep 1999 15:04:14 EDT." <003501bf0b76$9684df40$0701a8c0@grayface.commontech.com> On Thu, 30 Sep 1999 15:04:14 EDT, Eric Griffis writes: > This race condition was pointed out to me a little while before my message > made it to the list, and I am still puzzled as to how one would get the > timing right to perform such a maneuvre. Is there a way to somehow detect > that there's been an lstat performed without being superuser? It *might* be theoretically possible to detect the lstat(), if you could make /tmp/ssh-username point to a remote filesystem on a server that's under the user's control, but even if it's possible, it's probably not easy. A hacked ssh client/agent might also be able to have inside information about when it's about to send the request for agent forwarding. However, suppose the program merely guesses at the timing. Even if you lose the race, you can just keep retrying until you win the race; you only have to get it right once. In other words, if the odds of getting it right are one in a million, try it a few million times. > Also, I think the amount of processor time it takes to create a symbolic > link is multiple times larger than the amount of time between the return of > lstat and actual socket creation, All it takes is for the scheduler to decide to end sshd's time slice at an inconvenient time--and the context switches relating to the system calls are probably good candidates for that. > Okay, I see a few other messages about popen, permissions and such... At the > moment, I believe disabling remote agent services entirely is the only sure > way to remedy the whole issue, which will require password authentication. > And sshd needs to be run as root to perform authentication. I don't think > there's an easy way around that one. Disabling agent forwarding is probably overkill. (It would nevertheless have been nice if sshd had a way to disable agent forwarding from /etc/sshd_config, but that's neither here nor there.) Until we apply the kernel patch which Alan Cox is producing (has produced?) for Linux (the only vulnerable OS I personally have to deal with), I'm employing the following workaround: if someone uses this DoS on one of our systems, I'm going to go kick his ass. The system logs and the timestamp on the /etc/nologin file created by the denial of service makes it fairly straightforward to track down the idiot responsible. Jeff Long's patch is the sort of method I had in mind for fixing the problem properly. I'm not sure whether there are other credentials which the code should also be giving away--for instance, if root belongs to supplementary groups, then perhaps directories that are writable by those groups might still be attacked by the exploit. It would be worth testing whether this is the case. If so, perhaps an appropriate "initgroups(...)" would help? One could also include Eric Griffis's code to do the lstat() beforehand, in conjunction with Jeff's fix, if one wanted to. Adding the lstat() check is safe and effective, and there's no reason not to do it in addition to the other mechanism for fixing the flaw. And here's yet another option. I have *not* actually tried the following, but from a quick reading of the code, I *think* the problem could be eliminated by adding this quick hack to newchannels.c, anywhere after the #include's but before the definition of auth_input_request_forwarding(): #undef SSH_AGENT_SOCKET_DIR #define SSH_AGENT_SOCKET_DIR "/var/run/ssh-%.50s" This overrides the definition in ssh.h (for sshd, but not for ssh-agent), and *should* cause SSH agent sockets to be created under /var/run instead of /tmp when the agent forwarding is being done by sshd (but not when it's being done by ssh-agent). I'm assuming here, of course, that /var/run exists and is writable only by root; feel free to substitute a more appropriate directory name if you don't like this one. Try this fix at your own risk. People with more spare time than I have should feel free to test whether this fixes the problem, and report back to the list whether or not it does any good. This "undef/define" fix is *not* compatible with Jeff's patch; if you try to use both fixes, you'll definitely break agent forwarding. Eric's code change is compatible with both Jeff's and mine. Hope this helps somebody.... -- People shouldn't think that it's better to have Dan Astoorian loved and lost than never loved at all. It's Sysadmin, CS Lab not, it's better to have loved and won. All djast@cs.toronto.edu the other options really suck. --Dan Redican (4360726) ----------------------------------- 4360728 1999-10-04 05:20 /34 rader/ Postmaster Mottagare: Bugtraq (import) <8043> Ärende: Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM Message-ID: <199910011933.VAA25840@romulus> Date: Fri, 1 Oct 1999 21:33:02 +0200 Reply-To: Casper Dik <casper@HOLLAND.SUN.COM> Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Casper Dik <casper@HOLLAND.SUN.COM> X-To: BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM In-Reply-To: Your message of "Thu, 30 Sep 1999 15:06:12 EDT." <99Sep30.150614edt.96306-2339@jane.cs.toronto.edu> So, what about: char tmpl[] = "/tmp/dirXXXXXXX"; char dir[sizeof(tmpl)]; do { strcpy(x, tmpl); mktemp(x); } while (mkdir(x, 0700) != 0); bind(somesocket in dir x) rename(nameof socket, desired name of socket); rmdir(x); Under proper uids; I think most UNIX domain sockets can stand renaming; not sure if they all do. Casper (4360728) ----------------------------------- 4360731 1999-10-04 05:26 /47 rader/ Postmaster Mottagare: Bugtraq (import) <8044> Ärende: Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Message-ID: <19991002172926.27E6.0@bobanek.nowhere.cz> Date: Sat, 2 Oct 1999 18:11:42 +0200 Reply-To: Pavel Kankovsky <peak@ARGO.TROJA.MFF.CUNI.CZ> Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Pavel Kankovsky <peak@ARGO.TROJA.MFF.CUNI.CZ> X-To: Eric Griffis <egriffis@COMMONTECH.COM> X-cc: BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM In-Reply-To: <003501bf0b76$9684df40$0701a8c0@grayface.commontech.com> On Thu, 30 Sep 1999, Eric Griffis wrote: > This race condition was pointed out to me a little while before my message > made it to the list, and I am still puzzled as to how one would get the > timing right to perform such a maneuvre... I am afraid there is no way to "get the timing right" with stat() or lstat(). Unless you make the directory where the things happen immutable for a while---at least for the potential attacker. Perhaps this code in auth_input_request_forwarding() would be safe (with all the checks making sure "." is the right directory): chown(".", 0, 0); chmod(".", 700); lstat(...) etc. bind(...) etc. chown(".", pw->pw_uid, pw->pw_gid); > Also, I think the amount of processor time it takes to create a symbolic > link is multiple times larger than the amount of time between the return of > lstat and actual socket creation, which would require the sshd process to > hang temporarily or be seriously slowed down. Is that feasible? The context switch can happen anytime (unless the process in question is scheduled in some non-preemptive way). The probability of success is small but not zero, and it increases when many attempts are done. On the other hand, the risk may be acceptable if every failed attempt triggers a loud alarm and the odds the attacker can reset the alarm before it is noticed are small. --Pavel Kankovsky aka Peak [ Boycott Microsoft--http://www.vcnet.com/bms ] "Resistance is futile. Open your source code and prepare for assimilation." (4360731) ----------------------------------- 4360747 1999-10-04 06:47 /82 rader/ Postmaster Mottagare: Bugtraq (import) <8051> Ärende: Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM X-Url: http://black-ice.cc.vt.edu/~valdis/ X-Face: 34C9$Ewd2zeX+\!i1BA\j{ex+$/V'JBG#;3_noWWYPa"|,I#`R"{n@w>#:{)FXyiAS7(8t ^*w5O*!8O9YTe[r{e%7(yVRb|qxsRYw`7J!`AM}m_SHaj}f8eb@d^L>BrX7iO[<!v4-0bVIpaxF#- %9#a9h6JXI|T|8o6t\V?kGl]Q!1V]GtNliUtz:3},0"hkPeBuu%E,j(:\iOX-P,t7lRR# Mime-Version: 1.0 Content-Type: multipart/signed; boundary="==_Exmh_-581632224P"; micalg=pgp-md5 protocol="application/pgp-signature" Content-Transfer-Encoding: 7bit Message-ID: <199910012038.d91Kcw324754@black-ice.cc.vt.edu> Date: Fri, 1 Oct 1999 16:38:57 -0400 Reply-To: Valdis.Kletnieks@VT.EDU Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Valdis.Kletnieks@VT.EDU X-To: Eric Griffis <egriffis@COMMONTECH.COM> X-cc: BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM In-Reply-To: Your message of "Thu, 30 Sep 1999 12:04:14 PDT." <003501bf0b76$9684df40$0701a8c0@grayface.commontech.com> --==_Exmh_-581632224P Content-Type: text/plain; charset=us-ascii On Thu, 30 Sep 1999 12:04:14 PDT, Eric Griffis <egriffis@COMMONTECH.COM> said: > Also, I think the amount of processor time it takes to create a symbolic > link is multiple times larger than the amount of time between the return of > lstat and actual socket creation, which would require the sshd process to > hang temporarily or be seriously slowed down. Is that feasible? > > How would these things be done, or is there something I missed? I'm very > familiar with C and the unix environment, but the security-related aspects cat >> slowmedown.c main() { for(;;)} ^D cc -o slowmedown slowmedown.c for i in 1 2 3 4 5 6 7 8 9; do ./slowmedown &; done Or apply yuor favorite fork bomb. It's easy to slow things down as much as needed - you get that load average up to 60 or 80 the window you're trying to hit will get REAL wide. I'f you're REALLY smart, you'll have all the 'slowmedown' processes trying to hit the window while they bog things down. -- Valdis Kletnieks Computer Systems Senior Engineer Virginia Tech --==_Exmh_-581632224P Content-Type: application/pgp-signature -----BEGIN PGP MESSAGE----- Version: 2.6.2 owFdU11oXEUUbmgUu7ragkVEhIMPboO72910m+xmm8S42ZqlqSlmkSpUmL333L3D zs91Zm421+JjH/xBLCj13RcRrCCiPqigPvmgCAoWfNAHFRRBDeqTFs/cTTB04F7m 55zvnO+bb5498MrBm6bm/7rn+6+nmr/MvHtHb2rqATh4/b6n/72zcJr98dalz9+/ fGn6x3fqqzuLpfkXnp/vrneM2amc/vXQP1e+/KbQ/f2TxsrVF91Hh1976KcjH5QX iz+8/d0t8tTl4Lm1aOfi+Q+fuPvoG3+6W7+6/cpL5S9OvnftXPOq/e3mu45cSKab +uOfg/Mvv3rv8ua310vys7+Pnnl9+7Zrm59eaL05LdzhWlXaYXW2PteYreK2jA/Q 6GjlULlKP0twARxuu+OJYFy1IYiZsegWU1thNuC8WCgWNhT047QMJ2qwiQnUW60W 1GcXao2FegPOrfbL0DU8gIcNjyJu4RQOJ7MHOxtnz2480u921qo0XQKwjIcLxcIS rAiry9ADF3M1oj8CkzpVDnQEidEBWqsNOC4RuAPHRmjBaQgMMkexYDM50IIHHkt4 CKorU+F4IjBPsyCYGSJhxEzdUCCHHaAbI06ODLrUKDrK4axjDpgKgQUuZQKsDkbo JrW5VmUYxzyIYaxTEVLqUyk3mMNYG4d73VO3HoyKD0lfmWjDDBcZEKsBRaLhOrW0 tkKPMYRQj1UVeta36yBCZvlA4LKH8N+aHu/WozoWc9mG1iOFWmHZo3Kfi9SJ1RLz c5JXcmsxXIZeScIWmsxjRUxywZmBMXcxdHKmvvtU8W1AtcWNVpLMUYZB6ia8MEgN d1nFoCD5SRibYOCs90ZA7S4t5TQk5izoTiRZ6dgMXIRIm2Pt9swzxcKTqxQbQEXv C70hK/IsgCuowyycgAachDmYhya02kQTqsf3Zd7fzpnn7jTAkoSkzFICiNiWpl7R lx7BQMtBFaDnShZI08x7yKPsCZhjMW8dulBGhBRiSAQrkOkUhugm9yE0I9IkIBuS TolHmat50Zu1XKAxV4Tkc0reCiYj8GKBomIy75gLkUM92l1Zp1WIvqNStBfut9cf ByuZIc39JsXHVA0YTQi+WCj9z7y0ZzD/IPJCsFtnXyNkUJFbMiMFhvvJVr1ixUKl QrY6ROMxJkJyzhmBTnEc2cluR8skdfR4NjNL5rX07BUnul015KSQ2c3lhpacQR+D 2KP+Bw== =VWS3 -----END PGP MESSAGE----- --==_Exmh_-581632224P-- (4360747) ----------------------------------- 4366803 1999-10-05 19:33 /42 rader/ Postmaster Mottagare: Bugtraq (import) <8053> Ärende: Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM X-Accept-Language: en MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <37F8D498.6194523B@kestrel.cc.ukans.edu> Date: Mon, 4 Oct 1999 11:23:52 -0500 Reply-To: Jeff Long <long@KESTREL.CC.UKANS.EDU> Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Jeff Long <long@KESTREL.CC.UKANS.EDU> Organization: #f X-To: Chris Keane <Chris.Keane@COMLAB.OX.AC.UK> X-cc: BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM Chris Keane wrote: > > >>>>> On Thu, 30 Sep 1999, "JL" = Jeff Long wrote: > > JL> Seeing the race problems with the previous two patches I thought I > JL> would take a shot at one. It changes the effective uid/gid to the > JL> user logging in before doing the bind() (and then resets them after) > JL> which seems to take care of the problem. [ ... ] The bind() will > JL> fail if a symlink exists to a file that the user would normally not > JL> be able to write to (such as /etc/nologin). > > Surely this still isn't ideal, though? It now won't overwrite root-owned > files, so the security hazard isn't there, but anyone on the system can > still fool a user into overwriting one of his own files, which is not > great. From looking at the code it appears that it checks to make sure the directory the socket is created in is owned by the logging in user. Thus other users shouldn't be able to cause this problem. If the directory doesn't exist the patched version creates the directory (as root) then chowns the directory to the logging in user so I believe only the user will be able to overwrite their own files (i.e. they would have to create the symlink themselves to erase their own file). Jeff Long (4366803) ----------------------------------- 4367252 1999-10-05 22:57 /133 rader/ Postmaster Mottagare: Bugtraq (import) <8072> Ärende: Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy] ------------------------------------------------------------ Approved-By: aleph1@SECURITYFOCUS.COM Delivered-To: bugtraq@lists.securityfocus.com Delivered-To: BUGTRAQ@SECURITYFOCUS.COM X-Accept-Language: en MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------929FEACD82C86314874AE1C7" Message-ID: <37F8E6D5.7772EA34@kestrel.cc.ukans.edu> Date: Mon, 4 Oct 1999 12:41:41 -0500 Reply-To: Jeff Long <long@KESTREL.CC.UKANS.EDU> Sender: Bugtraq List <BUGTRAQ@SECURITYFOCUS.COM> From: Jeff Long <long@KESTREL.CC.UKANS.EDU> Organization: #f X-To: Dan Astoorian <djast@cs.toronto.edu> X-cc: Eric Griffis <egriffis@COMMONTECH.COM>, BUGTRAQ@SECURITYFOCUS.COM To: BUGTRAQ@SECURITYFOCUS.COM This is a multi-part message in MIME format. --------------929FEACD82C86314874AE1C7 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Dan Astoorian wrote: > > On Thu, 30 Sep 1999 15:04:14 EDT, Eric Griffis writes: > Jeff Long's patch is the sort of method I had in mind for fixing the > problem properly. > > I'm not sure whether there are other credentials which the code should > also be giving away--for instance, if root belongs to supplementary > groups, then perhaps directories that are writable by those groups might > still be attacked by the exploit. It would be worth testing whether > this is the case. If so, perhaps an appropriate "initgroups(...)" would > help? There's been a flurry of patches for this problem but since I'd already sent out a partially correct patch I figured I better follow up with a more(?) correct patch. I've added the code to fixup the concurrent groups before and after doing the bind(). This was tested on Tru64 5.0. Since I guess some OS'es don't have initgroups() they will need to add the #ifdef for initgroups() to avoid it. Also I'm not really sure if the calls to setegid() are still necessary but I felt better just leaving them in. Jeff Long --------------929FEACD82C86314874AE1C7 Content-Type: text/plain; charset=us-ascii; name="newchannels.c.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="newchannels.c.patch" *** newchannels.c.original Thu Sep 30 16:58:22 1999 --- newchannels.c Mon Oct 4 12:23:48 1999 *************** *** 2262,2268 **** struct stat st, st2, parent_st; mode_t old_umask; char *last_dir; ! if (auth_get_socket_name() != NULL) fatal("Protocol error: authentication forwarding requested twice."); --- 2262,2272 ---- struct stat st, st2, parent_st; mode_t old_umask; char *last_dir; ! uid_t saved_euid = 0; ! gid_t saved_egid = 0; ! gid_t saved_groups[NGROUPS_MAX]; ! int saved_groups_count = 0; ! if (auth_get_socket_name() != NULL) fatal("Protocol error: authentication forwarding requested twice."); *************** *** 2411,2427 **** creating unix-domain sockets, you might not be able to use ssh-agent connections on your system */ old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); ! ! if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0) ! packet_disconnect("Agent socket bind failed: %.100s", strerror(errno)); ! ! umask(old_umask); ! if (directory_created) if (chown(".", pw->pw_uid, pw->pw_gid) < 0) packet_disconnect("Agent socket directory chown failed: %.100s", strerror(errno)); /* Start listening on the socket. */ if (listen(sock, 5) < 0) packet_disconnect("Agent socket listen failed: %.100s", strerror(errno)); --- 2415,2450 ---- creating unix-domain sockets, you might not be able to use ssh-agent connections on your system */ old_umask = umask(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); ! if (directory_created) if (chown(".", pw->pw_uid, pw->pw_gid) < 0) packet_disconnect("Agent socket directory chown failed: %.100s", strerror(errno)); + saved_euid = geteuid(); + saved_egid = getegid(); + + if ((saved_groups_count = getgroups(NGROUPS_MAX, saved_groups)) < 0) + packet_disconnect("Agent socket getgroups failed: %s.100s", strerror(errno)); + if (initgroups(pw->pw_name, pw->pw_gid)) + packet_disconnect("Agent socket initgroups failed: %s.100s", strerror(errno)); + + if (setegid(pw->pw_gid) < 0) + packet_disconnect("Agent socket setegid failed: %.100s", strerror(errno)); + if (seteuid(pw->pw_uid) < 0) + packet_disconnect("Agent socket seteuid failed: %.100s", strerror(errno)); + if (bind(sock, (struct sockaddr *)&sunaddr, AF_UNIX_SIZE(sunaddr)) < 0) + packet_disconnect("Agent socket bind failed: %.100s", strerror(errno)); + + if (seteuid(saved_euid) < 0) + packet_disconnect("Agent socket re-seteuid failed: %.100s", strerror(errno)); + if (setegid(saved_egid) < 0) + packet_disconnect("Agent socket re-setegid failed: %.100s", strerror(errno)); + if (setgroups(saved_groups_count, saved_groups) < 0) + packet_disconnect("Agent socket setgroups failed: %s.100s", strerror(errno)); + + umask(old_umask); + /* Start listening on the socket. */ if (listen(sock, 5) < 0) packet_disconnect("Agent socket listen failed: %.100s", strerror(errno)); --------------929FEACD82C86314874AE1C7-- (4367252) -----------------------------------