Fix for ssh-1.2.27 symlink/bind problem

From: Scott Gifford (sgiffordat_private)
Date: Sat Oct 02 1999 - 15:38:46 PDT

  • Next message: Casper Dik: "Re: [Fwd: Truth about ssh 1.2.27 vulnerabiltiy]"

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



    This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 15:06:27 PDT