Bug 1775 - RFE: Would like to use 'abstract' unix sockets for ControlPath
Summary: RFE: Would like to use 'abstract' unix sockets for ControlPath
Status: NEW
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 5.5p1
Hardware: All All
: P2 enhancement
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-04 19:58 AEST by David Woodhouse
Modified: 2014-03-27 02:07 AEDT (History)
2 users (show)

See Also:


Attachments
Path to allow abstract socket addresses. (1.61 KB, application/octet-stream)
2010-06-04 19:58 AEST, David Woodhouse
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Woodhouse 2010-06-04 19:58:28 AEST
Created attachment 1853 [details]
Path to allow abstract socket addresses.

Using sockets with a real pathname is problematic (see bug #1329). It works much better if we can use 'abstract' sockets. They don't pollute the file system, and thus they disappear as soon as the listening program exits.

This patch makes OpenSSH interpret a leading '@' in the ControlPath to mean that an abstract socket should be used -- it'll replace the @ with a zero byte. Now I don't need the (original) patch in bug #1329 and everything works without leaving stale sockets behind.
Comment 1 Damien Miller 2010-07-02 13:20:36 AEST
I just added this comment to bug #1789, I think it applies here too:

> Isn't the solution for SELinux rules breaking /tmp to fix the SELinux 
> rules? Abstract sockets look like a complete trainwreck waiting to 
> happen: a brand new, completely unstructured but shared namespace, with 
> zero intrinsic security protections (not even filesystem permissions) 
> where every consumer application must implement security controls 
> correctly, rather than letting the kernel do it.
> 
> At the very least, I think we will wait a while before rushing to add 
> support for this to OpenSSH.
Comment 2 David Woodhouse 2010-07-02 15:26:53 AEST
Indeed. See my private mail to you of June 5th 2010, Message-Id: <1275740006.17903.1724.camel@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) {