diff --git a/src/pam-ssh-add/pam-ssh-add.c b/src/pam-ssh-add/pam-ssh-add.c index a9159d710046..839b797d215a 100644 --- a/src/pam-ssh-add/pam-ssh-add.c +++ b/src/pam-ssh-add/pam-ssh-add.c @@ -54,6 +54,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" @@ -866,6 +869,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); @@ -952,19 +974,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; } diff --git a/test/verify/check-session b/test/verify/check-session index 56a0fc08c04f..21812f325072 100755 --- a/test/verify/check-session +++ b/test/verify/check-session @@ -86,6 +86,39 @@ class TestSession(testlib.MachineCase): b.logout() wait_session(should_exist=False) + # try to pwn $SSH_AGENT_PID via pam_env's user_readenv=1 (CVE-2024-6126) + + if m.image in ["fedora-39", "fedora-40", "centos-10", "rhel-10-0"]: + # pam_env user_readenv crashes in Fedora/RHEL 10, skip the test + # https://bugzilla.redhat.com/show_bug.cgi?id=2293045 + return + if m.ostree_image: + # not using cockpit's PAM config + return + + # this is enabled by default in tools/cockpit.debian.pam, as well as + # Debian/Ubuntu's /etc/pam.d/sshd; but not in Fedora/RHEL + if "debian" not in m.image and "ubuntu" not in m.image: + self.write_file("/etc/pam.d/cockpit", "session required pam_env.so user_readenv=1\n", append=True) + victim_pid = m.spawn("sleep infinity", "sleep.log") + self.addCleanup(m.execute, f"kill {victim_pid} || true") + self.write_file("/home/admin/.pam_environment", f"SSH_AGENT_PID={victim_pid}\n", owner="admin") + + b.login_and_go() + wait_session(should_exist=True) + # starts ssh-agent in session + m.execute("pgrep -u admin ssh-agent") + # but the session has the modified SSH_AGENT_PID + bridge = m.execute("pgrep -u admin cockpit-bridge").strip() + agent = m.execute(f"grep --null-data SSH_AGENT_PID /proc/{bridge}/environ | xargs -0 | sed 's/.*=//'").strip() + self.assertEqual(agent, str(victim_pid)) + + # logging out still kills the actual ssh-agent, not the victim pid + b.logout() + wait_session(should_exist=False) + m.execute("while pgrep -u admin ssh-agent; do sleep 1; done", timeout=10) + m.execute(f"test -e /proc/{victim_pid}") + if __name__ == '__main__': testlib.test_main()