Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ssh-agent: remove all keys upon SIGUSR1.. #297

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sdaoden
Copy link

@sdaoden sdaoden commented Jan 22, 2022

With the advent of per-user temporary directories it became
hard for an administrator to remove all keys from all running
ssh-agent instances; what formerly could be done like so

if command -v ssh-add >/dev/null 2>&1; then
for a in /tmp/ssh-/agent.; do
[ -e "$a" ] || continue
act "SSH_AUTH_SOCK="$a" ssh-add -D </dev/null >/dev/null 2>&1 &"
inc
done
fi

has become a major undertaking, especially with even more
containerization. Being able to remove all keys from all agents
with a single command seems so desirable that it is available in
other agents in the software world.

@sdaoden
Copy link
Author

sdaoden commented Jan 22, 2022

Ah. Merde! Sorry, i thought "mergeable" meant the pull request!

@sdaoden sdaoden closed this Jan 22, 2022
@sdaoden sdaoden reopened this Feb 1, 2022
@sdaoden
Copy link
Author

sdaoden commented Feb 1, 2022

..and one more sorry, this was not meant to be closed. I might become a github expert.

@djmdjm
Copy link
Contributor

djmdjm commented Apr 29, 2022

This looks good to me and the better signal handling could allow us to make cleanup_handler() a regular call in future (i.e. no longer calling it in signal context).

Would appreciate @daztucker taking a look at this too

@sdaoden
Copy link
Author

sdaoden commented Apr 29, 2022

(It is all taken from sshd.c of course. Only to mention it. I .. do not implement this cleanup like it is done in sshd.c in this context here now!?!)
Thank you for looking into this.

@sdaoden
Copy link
Author

sdaoden commented Sep 29, 2022

But surely not because of this changeset, no?
Never used CI as it is here; but there is no log at all for the failure, not even a build one?

@sdaoden
Copy link
Author

sdaoden commented Dec 23, 2023

(i do not understand this CI-fuzz github thing which fails in one out of dozens of things. this should have nothing to do with the little patch, i would think.)

With the advent of per-user temporary directories it became
hard for an administrator to remove all keys from all running
ssh-agent instances; what formerly could be done like so

   if command -v ssh-add >/dev/null 2>&1; then
      for a in /tmp/ssh-*/agent.*; do
         [ -e "$a" ] || continue
         act "SSH_AUTH_SOCK=\"$a\" ssh-add -D </dev/null >/dev/null 2>&1 &"
         inc
      done
   fi

has become a major undertaking, especially with even more
containerization.  Being able to remove all keys from all agents
with a single command seems so desirable that it is available in
other agents in the software world.
@sdaoden
Copy link
Author

sdaoden commented Mar 11, 2024

Interestingly "the ppoll part has just recently been committed" almost as stated here.
Please let me allow one sentence: what a crappy attitude to leave that truly desired piece of code laying around for two years. git allows selective staging via "git add -p", and you would have had it 22 months ago.
Anyhow, this little pull request is now solely the SIGUSR1 handling, and all tests ran.

@sdaoden
Copy link
Author

sdaoden commented Mar 11, 2024

P.S.: "you would have had it 22 months ago": within less than 30 seconds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants