ssh-agent allowing access to other users?

[Á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.