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) -----------------------------------