ssh-agent: perform AGENTC_REMOVE_ALL_IDENTITIES on SIGUSR1

Peter Stuge peter at stuge.se
Fri Sep 10 08:57:08 AEST 2021


Hi Steffen,

Steffen Nurpmeso wrote:
>  |I think the lack of feedback about removal success/failure inherent
>  |with signals is problematic.
> 
> Bah.  What hair-splitting is that?  Sorry.

To clarify, my comment is about using a signal to trigger the deletion;
with a signal the sender can not know whether removal completed successfully,
failed or is ongoing. They can merel hope for the best. That's a very weak
security promise.

I for one would expect suspend to wait for successful removal and
that the suspend is cancelled with an error message should removal
fail, to know the state of agents.


>  |> Since _i think_ the poll(2) will not immediately tick due to
>  |> SA_RESTART if the "timeout" parameter is not -1,
..
>  |"The details vary across UNIX systems" suggests that you may need to
>  |do research on this.
> 
> Wouldn't you agree that the approach that was chosen covers exactly that?

Yes! Another fd can make poll() return reliably. I'd probably choose a pipe.


>  |There's no error handling after socket(), which is a problem in itself
>  |but also ties into the feedback problem. Since this is intended to be
> 
> To put that straight you mean the opposite, that _if_ socket(2)
> fails we will not be able to connect(2), and therefore the loop
> will not tick.

Sorry, I wasn't so clear - the patch did test socket() success, but
neither connect() nor close() success, at a minimum connect() must
succeed for poll() to return.


> If it wakes up the next time it _will_ remove the keys.

Without feedback (blocking removal) that could be post resume.

Such an addition would merely provide a false sense of security.


>  |a security feature there really needs to be some feedback, making
..
> call the cleanup_handler() and then fatal_r()ly abort the program instead?

I find that a good addition in error paths but it doesn't help with
the race condition inherent to using a signal.


>  |direct interaction with the agents look a lot better, with the
> 
> How do you do this, Mister, from an ACPI script running as root?

for SSH_AUTH_SOCK in /tmp/ssh-*/agent.*; do ssh-add -D; done # maybe?


>  |significant additional advantage of needing zero new agent code.
> 
> Oh nooo!  Spoiler!!!

When in doubt, keep the trusted codebase small.


//Peter


More information about the openssh-unix-dev mailing list