ControlMaster question

[Philippe Cerfon]

Hello.

I'd like to spread the use of control channel muxing at our institute,
so the plan was to change the defaults for our ssh to, e.g.

ControlMaster auto
ControlPath /tmp/%C.ssh-mux
ControlPersist 0

The idea was:
- use opportunistic muxing
- not all users may have actually homedirs, therefore something like
/tmp needs to be used
- the mux process should be backgrounded (and not block the original
ssh process to remain open) and it should immediately close when it's
no longer used by anyone


Some problems/questions:
1) Is it a security issue, when the sockets are created in /tmp? E.g.
could a malevolent user create such a socket and intercept the other
user's connection? Or does ssh check whether the socket is owned by
BOTH it's own user/group?
I think normally there shouldn't be a way for a user a to create a
file owned by user b,... it may be possible that user a creates a file
which is owned by group b... but if ssh checks BOTH user and group, it
should be fine, right? So does it do such checks?
Are there any other things to obey (thinking of all the different
kinds of /tmp-dir hacks)?

2) Apparently ControlPersist 0 is actually the same as yes and the mux
process isn't stopped 0s (i.e. immediately) after the last connection
has gone, but never.
Is this a bug?


Sincerely,
Philippe

[Christoph Anton Mitterer]

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:
***@hostA:~$ ssh -o ControlMaster=yes -o ControlPath=/tmp/sshmux hostB

and then:
***@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.

upstream what do you think?


Cheers,
Chris.

btw: I cannot answer your second question, perhaps one of the developers
knows more about that.

[Christoph Anton Mitterer]

I've just filed https://bugzilla.mindrot.org/show_bug.cgi?id=2311 to
keep track on this.


Cheers,
Chris.

[mancha]

On Mon, Nov 10, 2014 at 05:00:16AM +0100, Christoph Anton Mitterer

Post by Christoph Anton Mitterer
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...
ControlMaster=yes -o ControlPath=/tmp/sshmux hostB
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

[Christoph Anton Mitterer]

Post by mancha
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.

Sure,... but that alone doesn't help...

Post by mancha
If the
threat model includes an evil root though

Uhm... no I talk an about evil user,...
As long as the user can create the socket, he can at least trick root
into using it (if that uses the socket from a location as /tmp), just as
my example has shown.
The socket there was owned by user:user and had mode 600... yet root
connected to it without noting.


If one does it the other way round, i.e. root creates the mux:
I tried last week something like this:
***@hostA:~$ ssh -o ControlMaster=yes -o ControlPath=/tmp/sshmux hostC

and then:
***@hostA:~$ ssh -o ControlMaster=no -o ControlPath=/tmp/sshmux hostB

It "fails" of course, at least it cannot open the socket and then goes
on an connects normally.

Now assuming root wouldn't be evil, but accidentally set o+rw:
***@hostA:~$ chmod o+rw /tmp/sshmux

and then trying:
***@hostA:~$ ssh -o ControlMaster=no -o ControlPath=/tmp/sshmux hostB

ssh exits with $?=141 but not giving any further warning/error

Post by mancha
debug1: multiplexing control connection
multiplex uid mismatch: peer euid 1000 != uid 0

to stderr


The same seems to be the case when trying with two normal users, i.e.
***@hostA:~$ ssh -o ControlMaster=yes -o ControlPath=/tmp/sshmux hostC
***@hostA:~$ chmod o+rw /tmp/sshmux

***@hostA:~$ ssh -o ControlMaster=no -o ControlPath=/tmp/sshmux hostB

debug1: multiplexing control connection
multiplex uid mismatch: peer euid 1000 != uid 1001


So it only seems to work that a normal user tricks root into using an
alien socket.
That being said,... I haven't checked for any racey hacks...

Post by mancha
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.

Wouldn't it be the enough to simply check whether
- the socket is owned by the same user
- has mode 600
- and directory permissions are such, that another user couldn't have
changed this (thinking of ACLs for that)

Post by mancha
http://sf.net/projects/mancha/files/misc/openssh-6.7p1_socket-owner.diff

mhh I get a HTTP 500 internal server error?!


Cheers,
Chris.

[Stephen Frost]

Post by Christoph Anton Mitterer

Post by mancha
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.

Wouldn't it be the enough to simply check whether
- the socket is owned by the same user
- has mode 600
- and directory permissions are such, that another user couldn't have
changed this (thinking of ACLs for that)

Should there be a hard-link count check also..? Haven't really thought
it all the way through, but that's a common thing to check also..

Thanks,

Stephen

[Christoph Anton Mitterer]

Post by Stephen Frost
Should there be a hard-link count check also..? Haven't really thought
it all the way through, but that's a common thing to check also..

hmm not sure if that helps anything...

A normal user cannot create hardlinks on files owned by other users,
right?

So if the owner check already shows that the socket belongs to the
current user, then no on (but a evil root) could have created such
hardlink, except the user itself.

And since no one but root can chown the owern of a file, it should
neither work, that a evil userB creates a mux socket, hardlinks it and
then changes the owner to good userA of one of the hardlinks.

Or am I wrong? (I'm truly no expert in these kind of filesystem level
hacks)


Cheers,
Chris.

[Daniel Kahn Gillmor]

Post by Christoph Anton Mitterer
A normal user cannot create hardlinks on files owned by other users,
right?

This depends on your kernel.

Some recent Linux kernels prohibit this kind of behavior, but many older
Linux kernels (and i'm sure some non-Linux kernels) freely allow the
creation of hardlinks to files not owned by the linking user.

--dkg

[Ángel González]

Post by Christoph Anton Mitterer
Uhm... no I talk an about evil user,...
As long as the user can create the socket, he can at least trick root
into using it (if that uses the socket from a location as /tmp), just as
my example has shown.
The socket there was owned by user:user and had mode 600... yet root
connected to it without noting.
It "fails" of course, at least it cannot open the socket and then goes
on an connects normally.

Note that Linux enforces the discretionary permissions set on unix
sockets but other
Unix (including the BSD family) doesn't.

The ssh check

Post by Christoph Anton Mitterer
if ((euid != 0) && (getuid() != euid)) { error("multiplex uid
mismatch: peer euid %u != uid %u", (u_int)euid, (u_int)getuid());

seems precisely to be trying to protect from this (euid is the peer euid).

I guess the (euid != 0) checkis there in case ssh was root setuid? It
should probably be
changed to if ((euid != 0 && (getuid() != uid)) && (getuid() != euid))
not to make it so easy
for a malicious root to use your remote connections (yes, it would need
receiving the peer ruid).

However, for the attack shown, there's not so much to win from improving
the check at the
socket server side. It should be the connecting ssh (ie. root's) the one
verifying that the socket
is owned by himself.

Post by Christoph Anton Mitterer
That being said,... I haven't checked for any racey hacks...

Sadly, the provided patch has a race condition between the stat and the
connect. And I don't think
fstat()ing a connected unix socket would give us the owner of the socket
inode... :(


Regards

[Christoph Anton Mitterer]

Post by Ángel González
Note that Linux enforces the discretionary permissions set on unix
sockets but other

What exactly does that mean?

Post by Ángel González
I guess the (euid != 0) checkis there in case ssh was root setuid?

Or it's really to exclude root from the check, as Damien mentioned.

Post by Ángel González
It
should probably be
changed to if ((euid != 0 && (getuid() != uid)) && (getuid() != euid))
not to make it so easy
for a malicious root to use your remote connections (yes, it would need
receiving the peer ruid).

Let's see what upstream thinks

Post by Ángel González
However, for the attack shown, there's not so much to win from improving
the check at the
socket server side. It should be the connecting ssh (ie. root's) the one
verifying that the socket
is owned by himself.

Yes... let's see what upstream replies :)



Cheers,
Chris.

[Damien Miller]

Post by Christoph Anton Mitterer
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...
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.
upstream what do you think?

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"

[Christoph Anton Mitterer]

Post by Damien Miller
This behaviour is intentional. root is allowed to connect to users'
control sockets for a number of reasons.

Even if,... shouldn't it then be properly documented or better:
the checks should be in place per default for root as well, and only
with some additional option ControlMasterConnectUnownedSockets=yes (or
something like this), which defaults to no, root should be allowed to do
this?
I mean most people will likely never need that features you mentioned,
but it happens rather easy that people place such things in /tmp
or /run .

Apart from that, have you seen Ángel's post where he says the check
would happen on the socket server side?
That would of course make any user (not just root) attackable.

Post by Damien Miller
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"

Or there should be a StrictModes option like on the sshd side, which
prohibits taking sockets from insecure locations per default.


Cheers,
Chris.

[Christoph Anton Mitterer]

I've also just noted that %C is usually not enough to prevent collisions
when used in multi-user locations:

%C is the hash hover (local host, remote user, hostname, port)

I'd guess local host is needed in case of shared homedirs,.. but when it
comes to ControlPaths in locations used by multiple users, one obviously
needs local user as well.


This has of course less todo to with helping against the attacks
described above,... but more with preventing accidental collisions.


Cheers,
Chris.

[mancha]

[SNIP]

Post by Damien Miller
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

[shawn wilson]

Post by mancha
[SNIP]

Post by Damien Miller
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.

Great for general use. However there should be an option to turn the owner
and perms check off. I like single use accounts (I haven't done this but
now that I'm thinking about it) so a repo user who handles repo
interactions. I wouldn't want it to have access to my ssh private keys but
would setup a ControlMaster for it to use.

Also, it'd be cool if I could report on the parameters a ControlMaster was
initialized with (host, port, user, key, etc) - if this information could
be kept in memory and be retrieved via the file that might help with this
issue (and its something that's been on my mind besides :P ).

[Damien Miller]

Post by Philippe Cerfon
1) Is it a security issue, when the sockets are created in /tmp? E.g.
could a malevolent user create such a socket and intercept the other
user's connection? Or does ssh check whether the socket is owned by
BOTH it's own user/group?

It allows the user who created the socket and root (subject to file
permissions). It's best not to mix users' control sockets in the same
directory. Could you arrange a per-user temporary directory be created
at login time? (e.g. via PAM) If so, then you could put the sockets
there.

Post by Philippe Cerfon
2) Apparently ControlPersist 0 is actually the same as yes and the mux
process isn't stopped 0s (i.e. immediately) after the last connection
has gone, but never.
Is this a bug?

Kind of - '0' is used internally to implement ControlPersist=yes and this
leaked through to the UI. It's probably not a good idea to ban it
retrospectively, so I'll add a note to the manual page.

-d

[Philippe Cerfon]

Hello Damien.

Okay I'll see. So the recipe right now is to depend on something to
not mix user's sockets.
Since this sounds a bit error prone, though, and I've also read the
other user's comments now, wouldn't it be better to fix this in a way
proposed there? I saw some patches and someone suggested to either
apply the check for root as well or make something like StrictModes
for the ~/.ssh for the sockets.
I've also seem that claim that this user ID check would happen on the
socket server side, which would be the one trying to attack, right? So
can that be copied to the socket client side as well?


Good, thanks for the note. Do you think it's easy to write a patch
that makes 0s behave like "immediately exit after the last one is
gone"? Would sound like a compelling default :-)

Best wishes,
Philippe

[Christoph Anton Mitterer]

Hey folks.

Damien added a commit[0] which already suggests users to place their
sockets in safe directories only.

But I think this is only part of a proper solution, so I made some
effort and collected all the information from this thread, and placed it
into the ticket[1] I've had created previously:

I've also reported ticket #2318[2] which is loosely related

Ideally, any further discussions, patches, would be placed there.


Cheers,
Chris.


[0] https://github.com/openssh/openssh-portable/commit/fc302561369483bb755b17f671f70fb894aec01d
[1] https://bugzilla.mindrot.org/show_bug.cgi?id=2311#c1
[2] https://bugzilla.mindrot.org/show_bug.cgi?id=2318