[Bug 1775] RFE: Would like to use 'abstract' unix sockets for ControlPath

[bugzilla-daemon at bugzilla.mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=1775

--- Comment #2 from David Woodhouse <dwmw2 at infradead.org>  ---
Indeed. See my private mail to you of June 5th 2010, Message-Id:
<1275740006.17903.1724.camel at macbook.infradead.org>...

After filing the patch in bug #1775 (which I now realise works only on
Linux) I started looking more closely at the security implications. I
was about to file this in bugzilla, but figured it does no harm to
mention it in private first...

It looks like nothing prevents us from using a control socket which is
actually owned by an attacker. The attacker then gets to take over our
terminal, can give us a fake password prompt and basically has a field
day. If we're using 'ControlMaster auto' we may never notice.

Using the abstract namespace only exacerbates an existing problem --
with path-based sockets it's somewhat mitigated by the fact that you
_can_ put your control sockets in a private directory where no attacker
could create their own.

But we don't _enforce_ that, despite being quite anal about path
security in all other places. Perhaps we should insist on control
sockets being in a directory which isn't writeable by anyone else?

Another way of dealing with the problem is to check that the control
socket is owned by the current user, before trusting it.

This patch does so on Linux and other systems which implement
SO_PEERCRED; if you think it's the right approach, I'll extend it to
use
getpeereid() on *BSD and whatever Solaris needs...

--- mux.c~      2010-06-05 03:27:21.000000000 +0100
+++ mux.c       2010-06-05 12:51:54.000000000 +0100
@@ -1708,6 +1708,10 @@ void
 muxclient(const char *path)
 {
        struct sockaddr_un addr;
+#ifdef SO_PEERCRED
+       struct ucred peer;
+       socklen_t peer_len;
+#endif
        socklen_t sun_len;
        int sock;
        u_int pid;
@@ -1767,6 +1771,23 @@ muxclient(const char *path)
                close(sock);
                return;
        }
+#ifdef SO_PEERCRED
+       peer_len = sizeof(peer);
+       if (getsockopt(sock, SOL_SOCKET, SO_PEERCRED, (void *)&peer,
&peer_len) ||
+           peer_len != sizeof(peer)) {
+               error("Failed to obtain peer credentials on control
socket");
+               close(sock);
+               return;
+       }
+       
+       debug2("%s: peer uid %d gid %d pid %d", __func__, peer.uid,
peer.gid, peer.pid);
+       if (peer.uid != getuid()) {
+               error("Control socket \"%.100s\" owned by user %d; not
using",
+                     path, peer.uid);
+               close(sock);
+               return;
+       }
+#endif
        set_nonblock(sock);

        if (mux_client_hello_exchange(sock) != 0) {

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.