123 lines
4.2 KiB
Diff
123 lines
4.2 KiB
Diff
From 08965365ac311f906a520cbf65427742d5f84ba4 Mon Sep 17 00:00:00 2001
|
|
From: Martin Pitt <mpitt@redhat.com>
|
|
Date: Mon, 10 Jun 2024 10:49:56 +0200
|
|
Subject: [PATCH] pam-ssh-add: Fix insecure killing of session ssh-agent
|
|
[CVE-2024-6126]
|
|
|
|
Some distributions like Debian 12, or possibly some administrators
|
|
enable pam_env's deprecated `user_readenv` option [1]. The user session
|
|
can change the `$SSH_AGENT_PID`, so that it can pass an arbitrary pid to
|
|
`pam_sm_close_session()`. This is a local authenticated DoS.
|
|
|
|
Avoid this by storing the agent pid in a global variable. The
|
|
cockpit-session process stays around for the entire session time, so we
|
|
don't need to put the pid into the PAM data.
|
|
|
|
It can also happen that the user session's ssh-agent gets killed, and
|
|
some other process later on recycles the PID. Temporarily drop
|
|
privileges to the target user so that we at least don't kill anyone
|
|
else's process.
|
|
|
|
Add an integration test which checks that changing the env variable
|
|
works, pointing it to a different process doesn't kill that, and
|
|
ssh-agent (the original pid) is still cleaned up correctly. However, as
|
|
pam_so.env in Fedora crashes hard, skip the test there.
|
|
|
|
Many thanks to Paolo Perego <paolo.perego@suse.com> for discovering,
|
|
and Luna Dragon <luna.dragon@suse.com> for reporting this issue!
|
|
|
|
[1] https://man7.org/linux/man-pages/man8/pam_env.8.html
|
|
|
|
CVE-2024-6126
|
|
https://bugzilla.redhat.com/show_bug.cgi?id=2290859
|
|
|
|
Reference:https://github.com/cockpit-project/cockpit/commit/08965365ac311f906a520cbf65427742d5f84ba4
|
|
Conflict:include limits.h for ULONG_MAX;remove test
|
|
---
|
|
src/pam-ssh-add/pam-ssh-add.c | 47 ++++++++++++++++++++++++++++-------
|
|
1 file changed, 38 insertions(+), 9 deletions(-)
|
|
|
|
diff --git a/src/pam-ssh-add/pam-ssh-add.c b/src/pam-ssh-add/pam-ssh-add.c
|
|
index d63f06c..71223b7 100644
|
|
--- a/src/pam-ssh-add/pam-ssh-add.c
|
|
+++ b/src/pam-ssh-add/pam-ssh-add.c
|
|
@@ -33,6 +33,7 @@
|
|
#include <signal.h>
|
|
#include <assert.h>
|
|
#include <errno.h>
|
|
+#include <limits.h>
|
|
#include <stdio.h>
|
|
#include <stdlib.h>
|
|
#include <stdarg.h>
|
|
@@ -54,6 +55,9 @@ const char *pam_ssh_agent_arg = NULL;
|
|
const char *pam_ssh_add_program = PATH_SSH_ADD;
|
|
const char *pam_ssh_add_arg = NULL;
|
|
|
|
+static unsigned long ssh_agent_pid;
|
|
+static uid_t ssh_agent_uid;
|
|
+
|
|
/* Environment */
|
|
#define ENVIRON_SIZE 5
|
|
#define PATH "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
|
|
@@ -938,6 +942,25 @@ start_agent (pam_handle_t *pamh,
|
|
error ("couldn't set agent environment: %s",
|
|
pam_strerror (pamh, res));
|
|
}
|
|
+
|
|
+ /* parse and store the agent pid for later cleanup */
|
|
+ if (strncmp (auth_pid, "SSH_AGENT_PID=", 14) == 0)
|
|
+ {
|
|
+ unsigned long pid = strtoul (auth_pid + 14, NULL, 10);
|
|
+ if (pid > 0 && pid != ULONG_MAX)
|
|
+ {
|
|
+ ssh_agent_pid = pid;
|
|
+ ssh_agent_uid = auth_pwd->pw_uid;
|
|
+ }
|
|
+ else
|
|
+ {
|
|
+ error ("invalid SSH_AGENT_PID value: %s", auth_pid);
|
|
+ }
|
|
+ }
|
|
+ else
|
|
+ {
|
|
+ error ("unexpected agent pid format: %s", auth_pid);
|
|
+ }
|
|
}
|
|
|
|
free (auth_socket);
|
|
@@ -1024,19 +1047,25 @@ pam_sm_close_session (pam_handle_t *pamh,
|
|
int argc,
|
|
const char *argv[])
|
|
{
|
|
- const char *s_pid;
|
|
- int pid = 0;
|
|
parse_args (argc, argv);
|
|
|
|
/* Kill the ssh agent we started */
|
|
- s_pid = pam_getenv (pamh, "SSH_AGENT_PID");
|
|
- if (s_pid)
|
|
- pid = atoi (s_pid);
|
|
-
|
|
- if (pid > 0)
|
|
+ if (ssh_agent_pid > 0)
|
|
{
|
|
- debug ("Closing %d", pid);
|
|
- kill (pid, SIGTERM);
|
|
+ debug ("Closing %lu", ssh_agent_pid);
|
|
+ /* kill as user to guard against crashing ssh-agent and PID reuse */
|
|
+ if (setresuid (ssh_agent_uid, ssh_agent_uid, -1) < 0)
|
|
+ {
|
|
+ error ("could not drop privileges for killing ssh agent: %m");
|
|
+ return PAM_SESSION_ERR;
|
|
+ }
|
|
+ if (kill (ssh_agent_pid, SIGTERM) < 0 && errno != ESRCH)
|
|
+ message ("could not kill ssh agent %lu: %m", ssh_agent_pid);
|
|
+ if (setresuid (0, 0, -1) < 0)
|
|
+ {
|
|
+ error ("could not restore privileges after killing ssh agent: %m");
|
|
+ return PAM_SESSION_ERR;
|
|
+ }
|
|
}
|
|
return PAM_SUCCESS;
|
|
}
|