On Thu 2024-04-04 15:28:34 -0400, Daniel Kahn Gillmor wrote: > ssh-agent is a critical piece of infrastructure for my workflow, and i > want it better integrated with my user session, which is managed by > systemd's per-user login manager (`systemd --user`). I'm attaching an updated set of patches. These replace the earlier set of patches. In particular, it irons out the following bugs: - the systemd socket-activation now sets up the systemd activation environment, but doesn't try to use dbus-update-activation-environment, as i found that trying to talk to dbus during early login wasn't reliable. - the socket is set up with mode 0600 - the socket is actually installed with the debian package - properly initialize `sock` variable in ssh-agent to -1, so that "ssh-agent <command>" works properly again. I'd be happy to hear some feedback if anyone else wants to try it. --dkg
From b3051efa59b6a3802f2b10048f35640cf8d2011c Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Date: Thu, 4 Apr 2024 13:09:10 -0400
Subject: [PATCH 1/2] ssh-agent: automatically detect when socket-activated via
systemd
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.
---
ssh-agent.1 | 8 ++++++++
ssh-agent.c | 57 ++++++++++++++++++++++++++++++++++-------------------
2 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/ssh-agent.1 b/ssh-agent.1
index 1ab7d729d..0ce0cd781 100644
--- a/ssh-agent.1
+++ b/ssh-agent.1
@@ -247,6 +247,14 @@ starts, it creates a
socket and stores its pathname in this variable.
It is accessible only to the current user,
but is easily abused by root or another instance of the same user.
+.It Ev SD_LISTEN_FDS
+When
+.Nm
+starts, if no socket path has been specified, but this variable and the
+accompanying SD_LISTEN_PID are set as described in
+.Xr sd_listen_fds 3 ,
+.Nm
+uses the socket passed in by the systemd supervisor.
.El
.Pp
In Debian,
diff --git a/ssh-agent.c b/ssh-agent.c
index d35741a86..54a8978cc 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -2193,8 +2193,8 @@ int
main(int ac, char **av)
{
int c_flag = 0, d_flag = 0, D_flag = 0, k_flag = 0, s_flag = 0;
- int sock, ch, result, saved_errno;
- char *shell, *format, *pidstr, *agentsocket = NULL;
+ int sock = -1, ch, result, saved_errno;
+ char *shell, *format, *pidstr, *listen, *agentsocket = NULL;
#ifdef HAVE_SETRLIMIT
struct rlimit rlim;
#endif
@@ -2339,14 +2339,27 @@ main(int ac, char **av)
parent_pid = getpid();
if (agentsocket == NULL) {
- /* Create private directory for agent socket */
- mktemp_proto(socket_dir, sizeof(socket_dir));
- if (mkdtemp(socket_dir) == NULL) {
- perror("mkdtemp: private socket dir");
- exit(1);
+ /* Check if we are socket-activated with a single socket, as
+ * described in sd_listen_fds(3) */
+ if ((D_flag || d_flag) &&
+ (listen = getenv("LISTEN_PID")) &&
+ strtol(listen, NULL, 10) == (long)parent_pid &&
+ (listen = getenv("LISTEN_FDS")) &&
+ strcmp(listen, "1") == 0) {
+ socket_name[0] = '\0';
+ socket_dir[0] = '\0';
+ /* this is SD_LISTEN_FDS_START */
+ sock = 3;
+ } else {
+ /* Create private directory for agent socket */
+ mktemp_proto(socket_dir, sizeof(socket_dir));
+ if (mkdtemp(socket_dir) == NULL) {
+ perror("mkdtemp: private socket dir");
+ exit(1);
+ }
+ snprintf(socket_name, sizeof socket_name, "%s/agent.%ld", socket_dir,
+ (long)parent_pid);
}
- snprintf(socket_name, sizeof socket_name, "%s/agent.%ld", socket_dir,
- (long)parent_pid);
} else {
/* Try to use specified agent socket */
socket_dir[0] = '\0';
@@ -2357,14 +2370,16 @@ main(int ac, char **av)
* Create socket early so it will exist before command gets run from
* the parent.
*/
- prev_mask = umask(0177);
- sock = unix_listener(socket_name, SSH_LISTEN_BACKLOG, 0);
if (sock < 0) {
- /* XXX - unix_listener() calls error() not perror() */
- *socket_name = '\0'; /* Don't unlink any existing file */
- cleanup_exit(1);
+ prev_mask = umask(0177);
+ sock = unix_listener(socket_name, SSH_LISTEN_BACKLOG, 0);
+ if (sock < 0) {
+ /* XXX - unix_listener() calls error() not perror() */
+ *socket_name = '\0'; /* Don't unlink any existing file */
+ cleanup_exit(1);
+ }
+ umask(prev_mask);
}
- umask(prev_mask);
/*
* Fork, and have the parent execute the command, if any, or present
@@ -2374,11 +2389,13 @@ main(int ac, char **av)
log_init(__progname,
d_flag ? SYSLOG_LEVEL_DEBUG3 : SYSLOG_LEVEL_INFO,
SYSLOG_FACILITY_AUTH, 1);
- format = c_flag ? "setenv %s %s;\n" : "%s=%s; export %s;\n";
- printf(format, SSH_AUTHSOCKET_ENV_NAME, socket_name,
- SSH_AUTHSOCKET_ENV_NAME);
- printf("echo Agent pid %ld;\n", (long)parent_pid);
- fflush(stdout);
+ if (socket_name[0]) {
+ format = c_flag ? "setenv %s %s;\n" : "%s=%s; export %s;\n";
+ printf(format, SSH_AUTHSOCKET_ENV_NAME, socket_name,
+ SSH_AUTHSOCKET_ENV_NAME);
+ printf("echo Agent pid %ld;\n", (long)parent_pid);
+ fflush(stdout);
+ }
goto skip;
}
pid = fork();
--
2.43.0
From 039e522d6b8735432bb28e77c7d20cc980ab4a2d Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Date: Thu, 4 Apr 2024 14:17:36 -0400
Subject: [PATCH 2/2] ssh-agent -k: kill via systemctl --user if SSH_AGENT_PID
is unset
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.
---
ssh-agent.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/ssh-agent.c b/ssh-agent.c
index 54a8978cc..0bc6b95e1 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -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);
+ fprintf(stderr, "%s not set, cannot kill agent directly.\n"
+ "'systemctl --user stop ssh-agent.service' did not run: %d (%s).\n",
+ SSH_AGENTPID_ENV_NAME,
+ errno, strerror(errno));
exit(1);
}
pid = (int)strtonum(pidstr, 2, INT_MAX, &errstr);
--
2.43.0
From 4337ce14ceeaee3f67d959e2e72c19db2cda8e44 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor <dkg@fifthhorseman.net> Date: Thu, 4 Apr 2024 13:08:55 -0400 Subject: [PATCH] update systemd unit files for socket-activated ssh-agent --- debian/openssh-client.install | 1 + debian/systemd/ssh-agent.service | 10 +++------- debian/systemd/ssh-agent.socket | 13 +++++++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) create mode 100644 debian/systemd/ssh-agent.socket diff --git a/debian/openssh-client.install b/debian/openssh-client.install index 96c8deae7..2a33dc93b 100755 --- a/debian/openssh-client.install +++ b/debian/openssh-client.install @@ -34,3 +34,4 @@ debian/openssh-client.apport => usr/share/apport/package-hooks/openssh-client.py # systemd user unit (only used under sessions) debian/systemd/ssh-agent.service usr/lib/systemd/user +debian/systemd/ssh-agent.socket usr/lib/systemd/user diff --git a/debian/systemd/ssh-agent.service b/debian/systemd/ssh-agent.service index 68273bd75..72e0a3e46 100644 --- a/debian/systemd/ssh-agent.service +++ b/debian/systemd/ssh-agent.service @@ -1,17 +1,13 @@ [Unit] Description=OpenSSH Agent Documentation=man:ssh-agent(1) -Before=graphical-session-pre.target -ConditionPathExists=/etc/X11/Xsession.options -Wants=dbus.socket -After=dbus.socket [Service] +Environment=SSH_ASKPASS_REQUIRE=force # If you need to pass extra arguments to ssh-agent, you can use "systemctl # --user edit ssh-agent.service" to add a drop-in unit with contents along # these lines: # [Service] # ExecStart= -# ExecStart=/usr/lib/openssh/agent-launch start -- -t 1200 -ExecStart=/usr/lib/openssh/agent-launch start -ExecStopPost=/usr/lib/openssh/agent-launch stop +# ExecStart=/usr/bin/ssh-agent -D -t 1200 +ExecStart=/usr/bin/ssh-agent -D diff --git a/debian/systemd/ssh-agent.socket b/debian/systemd/ssh-agent.socket new file mode 100644 index 000000000..9980c36f0 --- /dev/null +++ b/debian/systemd/ssh-agent.socket @@ -0,0 +1,13 @@ +[Unit] +Description=OpenSSH Agent socket +Documentation=man:ssh-agent(1) +Before=graphical-session-pre.target + +[Socket] +SocketMode=0600 +ListenStream=%t/openssh_agent +ExecStartPost=/usr/bin/systemctl --user set-environment SSH_AUTH_SOCK=%t/openssh_agent +ExecStopPre=/usr/bin/systemctl --user unset-environment SSH_AUTH_SOCK + +[Install] +WantedBy=sockets.target -- 2.43.0
Attachment:
signature.asc
Description: PGP signature