|
|
ca10a9 |
diff -up ./src/polkitbackend/polkitbackendinteractiveauthority.c.ori ./src/polkitbackend/polkitbackendinteractiveauthority.c
|
|
|
ca10a9 |
--- ./src/polkitbackend/polkitbackendinteractiveauthority.c.ori 2019-01-21 17:30:43.468115782 +0100
|
|
|
ca10a9 |
+++ ./src/polkitbackend/polkitbackendinteractiveauthority.c 2019-01-21 17:31:03.220029178 +0100
|
|
|
ca10a9 |
@@ -2904,6 +2904,43 @@ temporary_authorization_store_free (Temp
|
|
|
ca10a9 |
g_free (store);
|
|
|
ca10a9 |
}
|
|
|
ca10a9 |
|
|
|
ca10a9 |
+/* See the comment at the top of polkitunixprocess.c */
|
|
|
ca10a9 |
+static gboolean
|
|
|
ca10a9 |
+subject_equal_for_authz (PolkitSubject *a,
|
|
|
ca10a9 |
+ PolkitSubject *b)
|
|
|
ca10a9 |
+{
|
|
|
ca10a9 |
+ if (!polkit_subject_equal (a, b))
|
|
|
ca10a9 |
+ return FALSE;
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+ /* Now special case unix processes, as we want to protect against
|
|
|
ca10a9 |
+ * pid reuse by including the UID.
|
|
|
ca10a9 |
+ */
|
|
|
ca10a9 |
+ if (POLKIT_IS_UNIX_PROCESS (a) && POLKIT_IS_UNIX_PROCESS (b)) {
|
|
|
ca10a9 |
+ PolkitUnixProcess *ap = (PolkitUnixProcess*)a;
|
|
|
ca10a9 |
+ int uid_a = polkit_unix_process_get_uid ((PolkitUnixProcess*)a);
|
|
|
ca10a9 |
+ PolkitUnixProcess *bp = (PolkitUnixProcess*)b;
|
|
|
ca10a9 |
+ int uid_b = polkit_unix_process_get_uid ((PolkitUnixProcess*)b);
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+ if (uid_a != -1 && uid_b != -1)
|
|
|
ca10a9 |
+ {
|
|
|
ca10a9 |
+ if (uid_a == uid_b)
|
|
|
ca10a9 |
+ {
|
|
|
ca10a9 |
+ return TRUE;
|
|
|
ca10a9 |
+ }
|
|
|
ca10a9 |
+ else
|
|
|
ca10a9 |
+ {
|
|
|
ca10a9 |
+ g_printerr ("denying slowfork; pid %d uid %d != %d!\n",
|
|
|
ca10a9 |
+ polkit_unix_process_get_pid (ap),
|
|
|
ca10a9 |
+ uid_a, uid_b);
|
|
|
ca10a9 |
+ return FALSE;
|
|
|
ca10a9 |
+ }
|
|
|
ca10a9 |
+ }
|
|
|
ca10a9 |
+ /* Fall through; one of the uids is unset so we can't reliably compare */
|
|
|
ca10a9 |
+ }
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+ return TRUE;
|
|
|
ca10a9 |
+}
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
static gboolean
|
|
|
ca10a9 |
temporary_authorization_store_has_authorization (TemporaryAuthorizationStore *store,
|
|
|
ca10a9 |
PolkitSubject *subject,
|
|
|
ca10a9 |
@@ -2946,7 +2983,7 @@ temporary_authorization_store_has_author
|
|
|
ca10a9 |
TemporaryAuthorization *authorization = l->data;
|
|
|
ca10a9 |
|
|
|
ca10a9 |
if (strcmp (action_id, authorization->action_id) == 0 &&
|
|
|
ca10a9 |
- polkit_subject_equal (subject_to_use, authorization->subject))
|
|
|
ca10a9 |
+ subject_equal_for_authz (subject_to_use, authorization->subject))
|
|
|
ca10a9 |
{
|
|
|
ca10a9 |
ret = TRUE;
|
|
|
ca10a9 |
if (out_tmp_authz_id != NULL)
|
|
|
ca10a9 |
diff -up ./src/polkit/polkitsubject.c.ori ./src/polkit/polkitsubject.c
|
|
|
ca10a9 |
--- ./src/polkit/polkitsubject.c.ori 2013-05-29 16:51:37.000000000 +0200
|
|
|
ca10a9 |
+++ ./src/polkit/polkitsubject.c 2019-01-21 17:31:03.218029187 +0100
|
|
|
ca10a9 |
@@ -99,6 +99,8 @@ polkit_subject_hash (PolkitSubject *subj
|
|
|
ca10a9 |
* @b: A #PolkitSubject.
|
|
|
ca10a9 |
*
|
|
|
ca10a9 |
* Checks if @a and @b are equal, ie. represent the same subject.
|
|
|
ca10a9 |
+ * However, avoid calling polkit_subject_equal() to compare two processes;
|
|
|
ca10a9 |
+ * for more information see the `PolkitUnixProcess` documentation.
|
|
|
ca10a9 |
*
|
|
|
ca10a9 |
* This function can be used in e.g. g_hash_table_new().
|
|
|
ca10a9 |
*
|
|
|
ca10a9 |
diff -up ./src/polkit/polkitunixprocess.c.ori ./src/polkit/polkitunixprocess.c
|
|
|
ca10a9 |
--- ./src/polkit/polkitunixprocess.c.ori 2019-01-21 17:30:43.477115743 +0100
|
|
|
ca10a9 |
+++ ./src/polkit/polkitunixprocess.c 2019-01-21 17:31:03.219029182 +0100
|
|
|
ca10a9 |
@@ -44,13 +44,82 @@
|
|
|
ca10a9 |
* @title: PolkitUnixProcess
|
|
|
ca10a9 |
* @short_description: Unix processs
|
|
|
ca10a9 |
*
|
|
|
ca10a9 |
- * An object for representing a UNIX process.
|
|
|
ca10a9 |
+ * An object for representing a UNIX process. NOTE: This object as
|
|
|
ca10a9 |
+ * designed is now known broken; a mechanism to exploit a delay in
|
|
|
ca10a9 |
+ * start time in the Linux kernel was identified. Avoid
|
|
|
ca10a9 |
+ * calling polkit_subject_equal() to compare two processes.
|
|
|
ca10a9 |
*
|
|
|
ca10a9 |
* To uniquely identify processes, both the process id and the start
|
|
|
ca10a9 |
* time of the process (a monotonic increasing value representing the
|
|
|
ca10a9 |
* time since the kernel was started) is used.
|
|
|
ca10a9 |
*/
|
|
|
ca10a9 |
|
|
|
ca10a9 |
+/* See https://gitlab.freedesktop.org/polkit/polkit/issues/75
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+ But quoting the original email in full here to ensure it's preserved:
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+ From: Jann Horn <jannh@google.com>
|
|
|
ca10a9 |
+ Subject: [SECURITY] polkit: temporary auth hijacking via PID reuse and non-atomic fork
|
|
|
ca10a9 |
+ Date: Wednesday, October 10, 2018 5:34 PM
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+When a (non-root) user attempts to e.g. control systemd units in the system
|
|
|
ca10a9 |
+instance from an active session over DBus, the access is gated by a polkit
|
|
|
ca10a9 |
+policy that requires "auth_admin_keep" auth. This results in an auth prompt
|
|
|
ca10a9 |
+being shown to the user, asking the user to confirm the action by entering the
|
|
|
ca10a9 |
+password of an administrator account.
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+After the action has been confirmed, the auth decision for "auth_admin_keep" is
|
|
|
ca10a9 |
+cached for up to five minutes. Subject to some restrictions, similar actions can
|
|
|
ca10a9 |
+then be performed in this timespan without requiring re-auth:
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+ - The PID of the DBus client requesting the new action must match the PID of
|
|
|
ca10a9 |
+ the DBus client requesting the old action (based on SO_PEERCRED information
|
|
|
ca10a9 |
+ forwarded by the DBus daemon).
|
|
|
ca10a9 |
+ - The "start time" of the client's PID (as seen in /proc/$pid/stat, field 22)
|
|
|
ca10a9 |
+ must not have changed. The granularity of this timestamp is in the
|
|
|
ca10a9 |
+ millisecond range.
|
|
|
ca10a9 |
+ - polkit polls every two seconds whether a process with the expected start time
|
|
|
ca10a9 |
+ still exists. If not, the temporary auth entry is purged.
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+Without the start time check, this would obviously be buggy because an attacker
|
|
|
ca10a9 |
+could simply wait for the legitimate client to disappear, then create a new
|
|
|
ca10a9 |
+client with the same PID.
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+Unfortunately, the start time check is bypassable because fork() is not atomic.
|
|
|
ca10a9 |
+Looking at the source code of copy_process() in the kernel:
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+ p->start_time = ktime_get_ns();
|
|
|
ca10a9 |
+ p->real_start_time = ktime_get_boot_ns();
|
|
|
ca10a9 |
+ [...]
|
|
|
ca10a9 |
+ retval = copy_thread_tls(clone_flags, stack_start, stack_size, p, tls);
|
|
|
ca10a9 |
+ if (retval)
|
|
|
ca10a9 |
+ goto bad_fork_cleanup_io;
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+ if (pid != &init_struct_pid) {
|
|
|
ca10a9 |
+ pid = alloc_pid(p->nsproxy->pid_ns_for_children);
|
|
|
ca10a9 |
+ if (IS_ERR(pid)) {
|
|
|
ca10a9 |
+ retval = PTR_ERR(pid);
|
|
|
ca10a9 |
+ goto bad_fork_cleanup_thread;
|
|
|
ca10a9 |
+ }
|
|
|
ca10a9 |
+ }
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+The ktime_get_boot_ns() call is where the "start time" of the process is
|
|
|
ca10a9 |
+recorded. The alloc_pid() call is where a free PID is allocated. In between
|
|
|
ca10a9 |
+these, some time passes; and because the copy_thread_tls() call between them can
|
|
|
ca10a9 |
+access userspace memory when sys_clone() is invoked through the 32-bit syscall
|
|
|
ca10a9 |
+entry point, an attacker can even stall the kernel arbitrarily long at this
|
|
|
ca10a9 |
+point (by supplying a pointer into userspace memory that is associated with a
|
|
|
ca10a9 |
+userfaultfd or is backed by a custom FUSE filesystem).
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+This means that an attacker can immediately call sys_clone() when the victim
|
|
|
ca10a9 |
+process is created, often resulting in a process that has the exact same start
|
|
|
ca10a9 |
+time reported in procfs; and then the attacker can delay the alloc_pid() call
|
|
|
ca10a9 |
+until after the victim process has died and the PID assignment has cycled
|
|
|
ca10a9 |
+around. This results in an attacker process that polkit can't distinguish from
|
|
|
ca10a9 |
+the victim process.
|
|
|
ca10a9 |
+*/
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
+
|
|
|
ca10a9 |
/**
|
|
|
ca10a9 |
* PolkitUnixProcess:
|
|
|
ca10a9 |
*
|