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: enable systemd-style socket activation #502

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dkg
Copy link

@dkg dkg commented Jun 26, 2024

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 added 2 commits June 26, 2024 16:34
Socket activation for the ssh-agent is useful for several reasons:

- the systemd user session can choose and reserve the socket before
  the session has fully started.

- systemd can inject SSH_AUTH_SOCK directly into the dbus and systemd
  environment, even before the agent is started.

- the agent will be started lazily, on-demand.  This consumes no
  system resources (not even a pid) if the user never tries to talk to
  the agent, and the agent will inherit the systemd service activation
  environment when it is first accessed (for example, if it is first
  accessed from a Wayland session, $WAYLAND_DISPLAY will be set; if it
  is first accessed from an X11 session, $DISPLAY will be set).

We only enter this mode if the following conditions are true:

 - One of the -D or -d flags are given, and no command arguments are
   present (ssh-agent when supervised by systemd must be run in the
   foreground and not as a subprocess), and

 - The environment variable conditions described in
   sd_listen_fds(3) are met, and only a single socket is provided.
When ssh-agent is socket-activated and managed for the session by
systemd, $SSH_AGENT_PID will not be explicitly set.

In this case, ssh-agent -k should try to talk to the local systemd
user session manager before giving up.
@dkg
Copy link
Author

dkg commented Aug 1, 2024

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
Copy link

jrollins commented Aug 3, 2024

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.

@@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this introduces a footgun that currently doesn't exist: if a user logs into a system via SSH with agent forwarding enabled then they'll get a $SSH_AUTH_SOCK but no $SSH_AGENT_PID. If they then run ssh-agent -k, they they might kill a different unrelated agent running on the host (i.e. the one started by systemd)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

djmdjm added a commit to djmdjm/openssh-portable-wip that referenced this pull request Oct 9, 2024
@djmdjm
Copy link
Contributor

djmdjm commented Oct 9, 2024

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

djmdjm/openssh-portable-wip@b0e44a0

@djmdjm
Copy link
Contributor

djmdjm commented Oct 18, 2024

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

@djmdjm
Copy link
Contributor

djmdjm commented Oct 22, 2024 via email

@dkg
Copy link
Author

dkg commented Oct 22, 2024

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.

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.

3 participants