BUG: simple attack when control channel muxing is used (was: Re: ControlMaster question)

[mancha]

On Mon, Nov 10, 2014 at 05:00:16AM +0100, Christoph Anton Mitterer
wrote:
> Hey.
> 
> Interesting that you bring this up now... I've actually looked into
> this a week ago but forgot to write a bug report.
> 
> A simple test showed, that ssh doesn't employ any security checks...
> when it is able to open the socket, it'll use it apparently:
> 
> I tried last week something like this: user at hostA:~$ ssh -o
> ControlMaster=yes -o ControlPath=/tmp/sshmux hostB
> 
> and then: root at hostA:~$ ssh -o ControlMaster=no -o
> ControlPath=/tmp/sshmux hostC
> 
> As you can see, the socket is created by user, and root "accidentally"
> uses it, even trying to connect to another node.  ssh will just do so
> without any complains.
> 
> And even when one uses something like %h, %p or that like, an attacker
> can easily guess these.
> 
> 
> Since it doesn't seem to be documented that the socket must be created
> in a secure location and since neither there are any owner checks like
> sshd's StrictMode... I'd probably consider that a security hole.

The socket is created with a umask of 0177 so you should end up with
socket perms of 0600 or thereabouts. Standard ACLs kick in. If the
threat model includes an evil root though, there's not much to do (and
in fact a lot more to worry about: trojaned ssh binary, tapped tty,
etc.). Abandon ship.

Regarding possible racey mischief, the socket is created "pseudo
atomically".

That said, an ownership check that prevents, among other things, root
from accidentally falling through a wormhole wouldn't be bad. Attached
patch against 6.7p1 should do.

--mancha

PS Patch also at:
http://sf.net/projects/mancha/files/misc/openssh-6.7p1_socket-owner.diff
-------------- next part --------------
From 4f9aabcbf3b633f6d0e066efaf28349fc610d2b6 Mon Sep 17 00:00:00 2001
From: mancha <mancha1 AT zoho DOT com>
Date: Mon, 10 Nov 2014
Subject: Check shared connection socket

Before allowing a new session to share an existing connection make sure
user owns the file specified by ControlPath. 

---
 mux.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- a/mux.c
+++ b/mux.c
@@ -2087,6 +2087,7 @@ muxclient(const char *path)
 	socklen_t sun_len;
 	int sock;
 	u_int pid;
+	struct stat filestat;
 
 	if (muxclient_command == 0) {
 		if (stdio_forward_host != NULL)
@@ -2106,6 +2107,11 @@ muxclient(const char *path)
 		return;
 	}
 
+	/* Check file ownership */
+	if (stat(path, &filestat) == 0 && filestat.st_uid != geteuid())
+		fatal("You do not own the file specified by ControlPath "
+		     "\"%.100s\"", path);
+
 	memset(&addr, '\0', sizeof(addr));
 	addr.sun_family = AF_UNIX;
 	sun_len = offsetof(struct sockaddr_un, sun_path) +
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20141110/8a5b435e/attachment.bin>