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

[mancha]

On Tue, Nov 11, 2014 at 08:00:04AM +1100, Damien Miller wrote:
> On Mon, 10 Nov 2014, Christoph Anton Mitterer wrote:

[SNIP]

> This behaviour is intentional. root is allowed to connect to users'
> control sockets for a number of reasons. These include making them
> work across sudo and it being mostly pointless to restrict root on a
> system.
> 
> If you want to avoid root connecting to a suspect socket, then ensure
> root's sockets are created in a directory that is not writable by
> untrusted users. I use "ControlPath ~/.ssh/ctl-%C"

Before I got Damien's response I had already cooked up a new patch that
imposes three restrictions on control socket usage: 1. must be owned by
user, 2. perms must be 600, and 3. hard link count can't exceed one.

Those who want the more stringent conditions are welcome to it. Modify
to your heart's content.

It's a bit less racey but if you have a more atomic (and still portable)
approach, go for it. I won't be spending any more time on this.

--mancha

Patch attached and mirrored at:
http://sf.net/projects/mancha/files/misc/openssh-6.7p1_socket-hardening.diff

-------------- next part --------------
From d08ff5729992bf628932565f4ca45867f04be6f8 Mon Sep 17 00:00:00 2001
From: mancha <mancha1 AT zoho DOT com>
Date: Mon, 10 Nov 2014
Subject: Stricter conditions on control socket

Before allowing access to a control socket make sure: a) user owns the file;
b) it has perms 600; and c) its hard link count is not greater than one.  

---
 mux.c |   14 ++++++++++++++
 1 file changed, 14 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)
@@ -2118,6 +2119,19 @@ muxclient(const char *path)
 	if ((sock = socket(PF_UNIX, SOCK_STREAM, 0)) < 0)
 		fatal("%s socket(): %s", __func__, strerror(errno));
 
+	/* Check file perms, hard link count, and ownership */
+	if (stat(path, &filestat) == 0) {
+		if (filestat.st_uid != geteuid())
+			fatal("You do not own the file specified by "
+			     "ControlPath \"%.100s\"", path);
+		if (filestat.st_mode & 0177)	
+			fatal("File specified by ControlPath \"%.100s\" "
+			     "must have permissions 600", path);
+		if (filestat.st_nlink > 1)	
+			fatal("File specified by ControlPath \"%.100s\" "
+			     "has more than one hard link", path);
+	}
+
 	if (connect(sock, (struct sockaddr *)&addr, sun_len) == -1) {
 		switch (muxclient_command) {
 		case SSHMUX_COMMAND_OPEN:
-------------- 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/20141111/fbd8a94b/attachment.bin>