[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1075942: marked as done (bookworm-pu: package cockpit/287.1-0+deb12u3)



Your message dated Sat, 31 Aug 2024 12:34:14 +0100
with message-id <9e3e8b8cd0db3b52d4adb2cfad04baa007c8e3e8.camel@adam-barratt.org.uk>
and subject line Closing bugs for 12.7
has caused the Debian Bug report #1075942,
regarding bookworm-pu: package cockpit/287.1-0+deb12u3
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1075942: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1075942
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Control: affects -1 + src:cockpit
X-Debbugs-Cc: Salvatore Bonaccorso <carnil@debian.org>
User: release.debian.org@packages.debian.org
Usertags: pu
Tags: bookworm
Severity: normal

[ Reason ]
CVE-2024-6126

[ Impact ]
Local authenticated DoS via privilege escalation: kill arbitrary process with
root privileges.

See https://cockpit-project.org/blog/cockpit-320.html for details about the
vulnerability.

[ Fix ]
https://github.com/cockpit-project/cockpit/commit/08965365ac311f906a520cbf65427742d5f84ba4

This is included in version 320, which is in unstable, testing, and
bookworm-backports now.

[ Tests ]
The patch includes an integration test which reproduces the attack scenario,
and it passes.

[ Risks ]
The patch is fairly small and easy to read, and has been tested in the field in
Debian, Ubuntu devel series, and Fedora 39/40 since last Wednesday. The worst
possible impact is that pam_ssh_add.so stops working, which would break
automatic SSH agent key preloading in Cockpit -- that's mostly a convenience
feature, the key can always be unlocked inside of Cockpit by re-entering the
password.

[ Checklist ]
  [x] *all* changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in stable-security
  [x] the issue is verified as fixed in unstable

[ Changes ]
See attached debdiff, just a backport of the upstream patch.

[ Other info ]
Security team (Salvatore Bonaccorso <carnil@debian.org>) asked for this to go
via updates, not DSA.

I already uploaded the update to the review queue.

Thanks,

Martin
diff -Nru cockpit-287.1/debian/changelog cockpit-287.1/debian/changelog
--- cockpit-287.1/debian/changelog	2024-04-16 09:20:17.000000000 +0200
+++ cockpit-287.1/debian/changelog	2024-07-05 06:15:50.000000000 +0200
@@ -1,3 +1,16 @@
+cockpit (287.1-0+deb12u3) bookworm; urgency=medium
+
+  * Add 0002-pam-ssh-add-Fix-insecure-killing-of-session-ssh-agen.patch:
+    Cockpit’s pam_ssh_add module had a vulnerability when user_readenv is
+    enabled in /etc/pam.d/cockpit (which is the default on Debian). This could
+    cause a Denial of Service if a locally-authenticated user crafted a
+    ~/.pam_environment file: it would kill an arbitrary process on the
+    system with root privileges when logging out of a Cockpit session.
+    Patch cherry-picked from upstream (08965365ac311f906a5).
+    [CVE-2024-6126]
+
+ -- Martin Pitt <mpitt@debian.org>  Fri, 05 Jul 2024 06:15:50 +0200
+
 cockpit (287.1-0+deb12u2) bookworm-security; urgency=medium
 
   * Add 0001-ssh-Use-valid-host-name-in-test-sshbridge.patch:
diff -Nru cockpit-287.1/debian/patches/0002-pam-ssh-add-Fix-insecure-killing-of-session-ssh-agen.patch cockpit-287.1/debian/patches/0002-pam-ssh-add-Fix-insecure-killing-of-session-ssh-agen.patch
--- cockpit-287.1/debian/patches/0002-pam-ssh-add-Fix-insecure-killing-of-session-ssh-agen.patch	1970-01-01 01:00:00.000000000 +0100
+++ cockpit-287.1/debian/patches/0002-pam-ssh-add-Fix-insecure-killing-of-session-ssh-agen.patch	2024-07-05 06:15:35.000000000 +0200
@@ -0,0 +1,158 @@
+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
+---
+ src/pam-ssh-add/pam-ssh-add.c | 46 ++++++++++++++++++++++++++++-------
+ test/verify/check-session     | 33 +++++++++++++++++++++++++
+ 2 files changed, 70 insertions(+), 9 deletions(-)
+
+diff --git a/src/pam-ssh-add/pam-ssh-add.c b/src/pam-ssh-add/pam-ssh-add.c
+index a9159d710..839b797d2 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 56a0fc08c..21812f325 100755
+--- a/test/verify/check-session
++++ b/test/verify/check-session
+@@ -86,6 +86,39 @@ class TestSession(testlib.MachineCase):
+         b.logout()
+         wait_session(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__':
+     test_main()
+--
+2.45.2
diff -Nru cockpit-287.1/debian/patches/series cockpit-287.1/debian/patches/series
--- cockpit-287.1/debian/patches/series	2024-04-16 09:20:00.000000000 +0200
+++ cockpit-287.1/debian/patches/series	2024-07-05 06:11:56.000000000 +0200
@@ -1 +1,2 @@
 0001-ssh-Use-valid-host-name-in-test-sshbridge.patch
+0002-pam-ssh-add-Fix-insecure-killing-of-session-ssh-agen.patch

--- End Message ---
--- Begin Message ---
Package: release.debian.org
Version: 12.7

Hi,

Each of these bugs relates to an update including in today's bookworm
12.7 point release.

Regards,

Adam

--- End Message ---

Reply to: