ssh-agent allowing access to other users?

[Daniel Kahn Gillmor]

hi openssh folks--

thanks to openbsd-compat/getpeereuid.c, ssh-agent refuses to allow
connections from other users. This is great in the general case, but in
some cases, i would like to use the agent to mask off access to the raw
public key material but make it available for use by other user accounts.

For an example use case, see what Tollef (cc'ed here) was trying to do
in [0]. running the agent as one user and permitting access from
another user via filesystem permissions currently fails due to the euid
check in ssh-agent.c.

Is it unreasonable to propose a slight weakening of this check when
deliberately configured? If so, what interface would seem most
reasonable to permit?

Some example interface choices:

A) ssh-agent could take a new -g option indicating the name of a unix
group; if a peer's euid is a member of that group (based on the same
logic used by sshd's AllowGroups option) then access would be granted.

B) the SSH_ALLOWED_EUIDS environment variable for the ssh-agent process
could be read as a whitespace-separated list of acceptable numeric uids
to allow connections from?

C) some other configuration interface/authorization interface? I'm
open to suggestions...

I'd be happy to write up a patch for A or B if folks think either would
be reasonable.

Feedback? Thoughts?

Regards,

--dkg


[0]
http://err.no/personal/blog/tech/2013-03-22-09-45_sharing_an_ssh_key_securely.html

[Ángel González]

On 01/04/13 23:36, Daniel Kahn Gillmor wrote:
> hi openssh folks--
>
> thanks to openbsd-compat/getpeereuid.c, ssh-agent refuses to allow
> connections from other users. This is great in the general case, but in
> some cases, i would like to use the agent to mask off access to the raw
> public key material but make it available for use by other user accounts.
>
> For an example use case, see what Tollef (cc'ed here) was trying to do
> in [0]. running the agent as one user and permitting access from
> another user via filesystem permissions currently fails due to the euid
> check in ssh-agent.c.
>
> Is it unreasonable to propose a slight weakening of this check when
> deliberately configured? If so, what interface would seem most
> reasonable to permit?

Add a --allow-other-users or --give-keys-to-anyone parameter?
Given that there will be little use of such feature, it's hard to make
an interface
which serves everybody and someone disabling that check should know what
they are doing (and thus properly secure the fs permissions).

As for the problem of Tollef due to an evil ssh-agent (clever trick!), a
solution
would be to make /usr/bin/ssh-agent sgid to another group (eg.
‘good-agent’), then check
in the sudo snippet (you better make a script...) that $SSH_AUTH_SOCK
belongs
to that group (and thus was created by the trusted code).

There are a few caveats for BSD:
* Some BSD flavours ignore socket permissions (but the containing folder
should still make the initial --allow-other-users socket safe).
* It may be possible to reattach to the socket under BSD using SO_REUSEADDR
in the second process (anyone wants to test with AF_UNIX?)

Under Linux (which is what Tollef seems to be using) it would apparently
work.
Anyone seeing a hole in that method?

_______________________________________________
openssh-unix-dev mailing list
openssh-...@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

[Daniel Kahn Gillmor]

On 04/01/2013 06:57 PM, Ángel González wrote:
> Add a --allow-other-users or --give-keys-to-anyone parameter?

i was thinking that the semantics would be more like
"allow-certain-other-users-to-make-ssh-agent-requests". since ssh-agent
doesn't give its keys to anyone under any circumstance, there would be
no give-keys-to-anyone, and any constraints the ssh-agent owner wanted
to impose would still be honored for requests coming from other users.

So, for example, i could run an ssh-agent in my main X11 session and ask
it to prompt me for every request. Then i could allow a designated user
account access to that agent's socket, and i could selectively allow or
deny requests made by that user account.

The idea of live modifications of the access list suggests a third
implementation option:

C) extend the ssh-agent protocol to include enabling access to other
users with a separate protocol command type.

I'm not sure i like it, but it's worth having on the table while
brainstorming, i guess.

> Given that there will be little use of such feature, it's hard to make
> an interface
> which serves everybody and someone disabling that check should know what
> they are doing (and thus properly secure the fs permissions).

Nothing about this proposal mentions adjusting the fs permissions. The
current code is already more restrictive than the fs permissions. If we
wanted to trust the user to know what to do with fs permissions, we
could just do that directly by dropping the getpeereuid check entirely.
But i'm proposing to selectively loosen the getpeereuid check, not to
tweak the fs permissions at this point.

And yes, a sophisticated user can work around this (as i am currently,
for example, by using a socat invocation as the original ssh-agent user
to launder connections from another less-restrictive socket through to
the original ssh-agent socket). But this is crufty and more dangerous
and more complex than just having the logic where it makes more sense:
in the agent itself.

> would be to make /usr/bin/ssh-agent sgid to another group (eg.
> ‘good-agent’), then check
> in the sudo snippet (you better make a script...) that $SSH_AUTH_SOCK
> belongs
> to that group (and thus was created by the trusted code).

on the system i'm currently using (Debian GNU/Linux), /usr/bin/ssh-agent
is already setgid root, to avoid arbitrary memory access by
non-privileged users. I think this isn't compatible with the change
you're suggesting.

Thanks for the thought and feedback!

Regards,

--dkg

[Tollef Fog Heen]

]] Ángel González

Daniel, thanks a lot for following up on this, I've been pondering how
to do it over Easter, but not gotten around to actually writing any
mails.

> As for the problem of Tollef due to an evil ssh-agent (clever trick!),

> a solution would be to make /usr/bin/ssh-agent sgid to another group

> (eg. ‘good-agent’), then check in the sudo snippet (you better make a
> script...) that $SSH_AUTH_SOCK belongs to that group (and thus was
> created by the trusted code).

I had an idea along those lines, yes, and it will mostly work, except:

ssh-agent is currently (in my setup) sgid ssh. Almost as the first
thing in main, it does a setegid(getgid()); setgid(getgid()), so the
socket itself is gid tfheen, not gid ssh. I am not entirely sure why it
drops those privileges (gid ssh is, as Daniel explains, merely used to
prevent ptracing the process, thereby preventing leaked keys caused by
an exploit of other processes belonging to the user), or if it could be
taught not to.

> Under Linux (which is what Tollef seems to be using) it would apparently
> work.
> Anyone seeing a hole in that method?

Correct, I'm on Linux (Debian to be precise).

Cheers,
--
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are

[Daniel Kahn Gillmor]

On 04/02/2013 02:23 PM, Ángel González wrote:

> As such parameter would have been equivalent to removing the getpeereid
> check, the access would have to be filtered by the permissions provided by
> the filesystem.

i'm not convinced by the idea that they're equivalent, but this is a
reasonable fourth interaction proposal:

D) ssh-agent takes a new initial argument (e.g. --no-peer-id-check).
if this argument is present, then the getpeereid() check is skipped.
Users of this flag are expected to ensure that filesystem permissions
are set up properly to limit access.

This is probably the simplest/cleanest of the existing proposals for
approaching this use case so far.

> While brainstormin about changing the protocol, I'd like a change where
> the (claimed) target was provided (in a stackable fashion) to the agent.

This probably deserves a separate thread. I don't know what you mean by
"stackable fashion" -- but if you start a separate thread about this,
i'd be happy to read and comment on it.

--dkg

[Ángel González]

On 02/04/13 01:20, Daniel Kahn Gillmor wrote:
>> Given that there will be little use of such feature, it's hard to make
>> > an interface
>> > which serves everybody and someone disabling that check should know what
>> > they are doing (and thus properly secure the fs permissions).
> Nothing about this proposal mentions adjusting the fs permissions. The
> current code is already more restrictive than the fs permissions. If we
> wanted to trust the user to know what to do with fs permissions, we
> could just do that directly by dropping the getpeereuid check entirely.
> But i'm proposing to selectively loosen the getpeereuid check, not to
> tweak the fs permissions at this point.

As such parameter would have been equivalent to removing the getpeereid
check, the access would have to be filtered by the permissions provided by
the filesystem.

I'm not a fan of adding dynamic permissions to ssh-agent either. I don't
think
it's worth the complexity added to the simple-to-understand ssh-agent tool.
However, getpeereid() returns the effective gid, so the privileged group
should
be the primary one (the non-standard getgrouplist could be used). Access
control
to the agent by secondary groups is still easier done by fs checks :/

While brainstormin about changing the protocol, I'd like a change where
the (claimed)
target was provided (in a stackable fashion) to the agent.

On 02/04/13 07:29, Tollef Fog Heen wrote:
> I had an idea along those lines, yes, and it will mostly work, except:
>
> ssh-agent is currently (in my setup) sgid ssh. Almost as the first
> thing in main, it does a setegid(getgid()); setgid(getgid()), so the
> socket itself is gid tfheen, not gid ssh. I am not entirely sure why it
> drops those privileges (gid ssh is, as Daniel explains, merely used to
> prevent ptracing the process, thereby preventing leaked keys caused by
> an exploit of other processes belonging to the user), or if it could be
> taught not to.

I had been looking at that piece of code (thinking about a possible race
condition before the prctl call, which the sgid avoids) and yet failed
to notice
that. :(
You could insert a library that replaces getgid() with getegid(), but it
seems
too hacky.