ssh-agent: enable systemd-style socket activation

[dkg]

The systemd service manager on many Linux-based OSes provides a handy socket activation mechanism: the service manager opens the relevant sockets, and then automatically spawns the correct daemon when someone connects to it. The service manager also reaps relevant processes when the session lifecycle demands it.

ssh-agent needs a few small changes to enable socket activation on such a system. This only involves a single user, as it is not a system service. systemd's "user" session manager handles single-user services in the same way as the system-wide, multi-user service manager handles system services, including socket-activation.

The changes in this series take the following two steps:

• recognize a socket passed in as a file descriptor a la sd_listen_fds(3) when launched.
• enable killing the agent via systemd --user in case $SSH_AGENT_PID is unset.

Neither step should have any effect on a non-systemd system, and the two steps together enable a user who is already running a systemd user session manager to have a socket-activated, session-manager-governed ssh-agent process.

I've been running ssh-agent from a systemd-supervised, socket-activated user session with these patches for about 2½ months now and have had no problems with it.

[dkg]

any chance of a review on this? I've mentioned it on the openssh-unix-dev mailing list as well but it hasn't shown up in the archives yet.

[jrollins]

I have also been using these patches for months and they work very well. I agree that socket activation of the agent service is the way to go, and this is the simplest way to do it, so I fully support these improvements. Thanks.

[djmdjm]

I think the socket activation detection logic could be simplified here, e.g.

https://github.com/djmdjm/openssh-portable-wip/pull/6/commits/b0e44a0b474886b8315ec76bd2c0a562e22b0f7d

[djmdjm]

Also not sure whether socket activation should be magically auto-enabled or should require a flag, e.g. -A is free

[djmdjm]

IMO it's perfectly fine for -k not to work in this case

On Tue, 22 Oct 2024, 16:50 dkg, @.***> wrote:

@.**** commented on this pull request.

In ssh-agent.c https://github.com/openssh/openssh-portable/pull/502#discussion_r1809989079 :

@@ -2303,8 +2303,11 @@ main(int ac, char **av)

  pidstr = getenv(SSH_AGENTPID_ENV_NAME);
  if (pidstr == NULL) {

• 	fprintf(stderr, "%s not set, cannot kill agent\n",
  

• 	    SSH_AGENTPID_ENV_NAME);
  

• 	execlp("systemctl", "systemctl", "--user", "stop", "ssh-agent.service", (char*)NULL);
  

That's a good point. what do you think should be done instead? should we just expect ssh-agent -k to not work for socket-activated agents? I've never used ssh-agent -k in any of my regular workflows (i also don't generally forward agents, i prefer a star topology where possible), so i'm not sure what the typical expectations are.

— Reply to this email directly, view it on GitHub https://github.com/openssh/openssh-portable/pull/502#discussion_r1809989079, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJSKPZPRUFDPWWK5OJWCTZ4XRR3AVCNFSM6AAAAABJ6T67YWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOBTHE4DGMJVG4 . You are receiving this because you commented.Message ID: @.***>

[dkg]

I'm OK with the proposed socket activation detection logic changes on the agent-socket-activation branch, modulo the comments i made there about LISTEN_PID (it should look at the current process ID, not at the current process's parent's pid).

It seems to me that auto-detecting socket activation is safe -- i don't understand the circumstances where it would necessarily fail. The autodetection already requires running the agent in foregrounded mode, with no explicit socket address, and no command arguments. Is there any other case where someone user might be doing that while LISTEN_FDS is set?

Of course, if you feel more comfortable putting it behind an -A flag, i wouldn't object, but if that's the case then i guess we would also have to decide what to do if -A is present when any of these other conditions are not true.

[djmdjm]

Ok, that's reasonable. I've pushed some fixes to https://github.com/djmdjm/openssh-portable-wip/pull/6 - are you able to test that they still work?

[dkg]

@djmdjm thanks i've followed up over on that MR.

[djmdjm]

This has been merged and will be in openssh-10.0, due in a couple of months. Thanks for writing this and your feedback along the way.