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--
This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 15:06:18 PDT