diff options
author | Qi.Chen@windriver.com <Qi.Chen@windriver.com> | 2019-05-21 10:02:22 +0800 |
---|---|---|
committer | Armin Kuster <akuster808@gmail.com> | 2019-06-16 08:15:44 -0700 |
commit | cd933df57dc567d4a35e215398dc677a67d78c09 (patch) | |
tree | af0e06d32c8fdc58185e7c06494e02d44b3997d6 | |
parent | 9e862a8f34653a760c47665eaeaf09b48bcf6b8c (diff) | |
download | meta-openembedded-cd933df57dc567d4a35e215398dc677a67d78c09.tar.gz |
polkit: fix CVE-2019-6133
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Armin Kuster <akuster808@gmail.com>
-rw-r--r-- | meta-oe/recipes-extended/polkit/polkit/0001-backend-Compare-PolkitUnixProcess-uids-for-temporary.patch | 186 | ||||
-rw-r--r-- | meta-oe/recipes-extended/polkit/polkit_0.115.bb | 7 |
2 files changed, 190 insertions, 3 deletions
diff --git a/meta-oe/recipes-extended/polkit/polkit/0001-backend-Compare-PolkitUnixProcess-uids-for-temporary.patch b/meta-oe/recipes-extended/polkit/polkit/0001-backend-Compare-PolkitUnixProcess-uids-for-temporary.patch new file mode 100644 index 000000000..ae314e3c1 --- /dev/null +++ b/meta-oe/recipes-extended/polkit/polkit/0001-backend-Compare-PolkitUnixProcess-uids-for-temporary.patch | |||
@@ -0,0 +1,186 @@ | |||
1 | From eb1f1336e8e49b4db6243b543e0a71f7c0c9b5b1 Mon Sep 17 00:00:00 2001 | ||
2 | From: Colin Walters <walters@verbum.org> | ||
3 | Date: Fri, 4 Jan 2019 14:24:48 -0500 | ||
4 | Subject: [PATCH] backend: Compare PolkitUnixProcess uids for temporary | ||
5 | authorizations | ||
6 | |||
7 | It turns out that the combination of `(pid, start time)` is not | ||
8 | enough to be unique. For temporary authorizations, we can avoid | ||
9 | separate users racing on pid reuse by simply comparing the uid. | ||
10 | |||
11 | https://bugs.chromium.org/p/project-zero/issues/detail?id=1692 | ||
12 | |||
13 | And the above original email report is included in full in a new comment. | ||
14 | |||
15 | Reported-by: Jann Horn <jannh@google.com> | ||
16 | |||
17 | Closes: https://gitlab.freedesktop.org/polkit/polkit/issues/75 | ||
18 | |||
19 | Upstream-Status: Backport | ||
20 | CVE: CVE-2019-6133 | ||
21 | Signed-off-by: Chen Qi <Qi.Chen@windriver.com> | ||
22 | --- | ||
23 | src/polkit/polkitsubject.c | 2 + | ||
24 | src/polkit/polkitunixprocess.c | 71 +++++++++++++++++++++- | ||
25 | .../polkitbackendinteractiveauthority.c | 39 +++++++++++- | ||
26 | 3 files changed, 110 insertions(+), 2 deletions(-) | ||
27 | |||
28 | diff --git a/src/polkit/polkitsubject.c b/src/polkit/polkitsubject.c | ||
29 | index d4c1182..ccabd0a 100644 | ||
30 | --- a/src/polkit/polkitsubject.c | ||
31 | +++ b/src/polkit/polkitsubject.c | ||
32 | @@ -99,6 +99,8 @@ polkit_subject_hash (PolkitSubject *subject) | ||
33 | * @b: A #PolkitSubject. | ||
34 | * | ||
35 | * Checks if @a and @b are equal, ie. represent the same subject. | ||
36 | + * However, avoid calling polkit_subject_equal() to compare two processes; | ||
37 | + * for more information see the `PolkitUnixProcess` documentation. | ||
38 | * | ||
39 | * This function can be used in e.g. g_hash_table_new(). | ||
40 | * | ||
41 | diff --git a/src/polkit/polkitunixprocess.c b/src/polkit/polkitunixprocess.c | ||
42 | index 972b777..7a6d48b 100644 | ||
43 | --- a/src/polkit/polkitunixprocess.c | ||
44 | +++ b/src/polkit/polkitunixprocess.c | ||
45 | @@ -51,7 +51,10 @@ | ||
46 | * @title: PolkitUnixProcess | ||
47 | * @short_description: Unix processs | ||
48 | * | ||
49 | - * An object for representing a UNIX process. | ||
50 | + * An object for representing a UNIX process. NOTE: This object as | ||
51 | + * designed is now known broken; a mechanism to exploit a delay in | ||
52 | + * start time in the Linux kernel was identified. Avoid | ||
53 | + * calling polkit_subject_equal() to compare two processes. | ||
54 | * | ||
55 | * To uniquely identify processes, both the process id and the start | ||
56 | * time of the process (a monotonic increasing value representing the | ||
57 | @@ -66,6 +69,72 @@ | ||
58 | * polkit_unix_process_new_for_owner() with trusted data. | ||
59 | */ | ||
60 | |||
61 | +/* See https://gitlab.freedesktop.org/polkit/polkit/issues/75 | ||
62 | + | ||
63 | + But quoting the original email in full here to ensure it's preserved: | ||
64 | + | ||
65 | + From: Jann Horn <jannh@google.com> | ||
66 | + Subject: [SECURITY] polkit: temporary auth hijacking via PID reuse and non-atomic fork | ||
67 | + Date: Wednesday, October 10, 2018 5:34 PM | ||
68 | + | ||
69 | +When a (non-root) user attempts to e.g. control systemd units in the system | ||
70 | +instance from an active session over DBus, the access is gated by a polkit | ||
71 | +policy that requires "auth_admin_keep" auth. This results in an auth prompt | ||
72 | +being shown to the user, asking the user to confirm the action by entering the | ||
73 | +password of an administrator account. | ||
74 | + | ||
75 | +After the action has been confirmed, the auth decision for "auth_admin_keep" is | ||
76 | +cached for up to five minutes. Subject to some restrictions, similar actions can | ||
77 | +then be performed in this timespan without requiring re-auth: | ||
78 | + | ||
79 | + - The PID of the DBus client requesting the new action must match the PID of | ||
80 | + the DBus client requesting the old action (based on SO_PEERCRED information | ||
81 | + forwarded by the DBus daemon). | ||
82 | + - The "start time" of the client's PID (as seen in /proc/$pid/stat, field 22) | ||
83 | + must not have changed. The granularity of this timestamp is in the | ||
84 | + millisecond range. | ||
85 | + - polkit polls every two seconds whether a process with the expected start time | ||
86 | + still exists. If not, the temporary auth entry is purged. | ||
87 | + | ||
88 | +Without the start time check, this would obviously be buggy because an attacker | ||
89 | +could simply wait for the legitimate client to disappear, then create a new | ||
90 | +client with the same PID. | ||
91 | + | ||
92 | +Unfortunately, the start time check is bypassable because fork() is not atomic. | ||
93 | +Looking at the source code of copy_process() in the kernel: | ||
94 | + | ||
95 | + p->start_time = ktime_get_ns(); | ||
96 | + p->real_start_time = ktime_get_boot_ns(); | ||
97 | + [...] | ||
98 | + retval = copy_thread_tls(clone_flags, stack_start, stack_size, p, tls); | ||
99 | + if (retval) | ||
100 | + goto bad_fork_cleanup_io; | ||
101 | + | ||
102 | + if (pid != &init_struct_pid) { | ||
103 | + pid = alloc_pid(p->nsproxy->pid_ns_for_children); | ||
104 | + if (IS_ERR(pid)) { | ||
105 | + retval = PTR_ERR(pid); | ||
106 | + goto bad_fork_cleanup_thread; | ||
107 | + } | ||
108 | + } | ||
109 | + | ||
110 | +The ktime_get_boot_ns() call is where the "start time" of the process is | ||
111 | +recorded. The alloc_pid() call is where a free PID is allocated. In between | ||
112 | +these, some time passes; and because the copy_thread_tls() call between them can | ||
113 | +access userspace memory when sys_clone() is invoked through the 32-bit syscall | ||
114 | +entry point, an attacker can even stall the kernel arbitrarily long at this | ||
115 | +point (by supplying a pointer into userspace memory that is associated with a | ||
116 | +userfaultfd or is backed by a custom FUSE filesystem). | ||
117 | + | ||
118 | +This means that an attacker can immediately call sys_clone() when the victim | ||
119 | +process is created, often resulting in a process that has the exact same start | ||
120 | +time reported in procfs; and then the attacker can delay the alloc_pid() call | ||
121 | +until after the victim process has died and the PID assignment has cycled | ||
122 | +around. This results in an attacker process that polkit can't distinguish from | ||
123 | +the victim process. | ||
124 | +*/ | ||
125 | + | ||
126 | + | ||
127 | /** | ||
128 | * PolkitUnixProcess: | ||
129 | * | ||
130 | diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c | ||
131 | index de3f752..098d343 100644 | ||
132 | --- a/src/polkitbackend/polkitbackendinteractiveauthority.c | ||
133 | +++ b/src/polkitbackend/polkitbackendinteractiveauthority.c | ||
134 | @@ -3035,6 +3035,43 @@ temporary_authorization_store_free (TemporaryAuthorizationStore *store) | ||
135 | g_free (store); | ||
136 | } | ||
137 | |||
138 | +/* See the comment at the top of polkitunixprocess.c */ | ||
139 | +static gboolean | ||
140 | +subject_equal_for_authz (PolkitSubject *a, | ||
141 | + PolkitSubject *b) | ||
142 | +{ | ||
143 | + if (!polkit_subject_equal (a, b)) | ||
144 | + return FALSE; | ||
145 | + | ||
146 | + /* Now special case unix processes, as we want to protect against | ||
147 | + * pid reuse by including the UID. | ||
148 | + */ | ||
149 | + if (POLKIT_IS_UNIX_PROCESS (a) && POLKIT_IS_UNIX_PROCESS (b)) { | ||
150 | + PolkitUnixProcess *ap = (PolkitUnixProcess*)a; | ||
151 | + int uid_a = polkit_unix_process_get_uid ((PolkitUnixProcess*)a); | ||
152 | + PolkitUnixProcess *bp = (PolkitUnixProcess*)b; | ||
153 | + int uid_b = polkit_unix_process_get_uid ((PolkitUnixProcess*)b); | ||
154 | + | ||
155 | + if (uid_a != -1 && uid_b != -1) | ||
156 | + { | ||
157 | + if (uid_a == uid_b) | ||
158 | + { | ||
159 | + return TRUE; | ||
160 | + } | ||
161 | + else | ||
162 | + { | ||
163 | + g_printerr ("denying slowfork; pid %d uid %d != %d!\n", | ||
164 | + polkit_unix_process_get_pid (ap), | ||
165 | + uid_a, uid_b); | ||
166 | + return FALSE; | ||
167 | + } | ||
168 | + } | ||
169 | + /* Fall through; one of the uids is unset so we can't reliably compare */ | ||
170 | + } | ||
171 | + | ||
172 | + return TRUE; | ||
173 | +} | ||
174 | + | ||
175 | static gboolean | ||
176 | temporary_authorization_store_has_authorization (TemporaryAuthorizationStore *store, | ||
177 | PolkitSubject *subject, | ||
178 | @@ -3077,7 +3114,7 @@ temporary_authorization_store_has_authorization (TemporaryAuthorizationStore *st | ||
179 | TemporaryAuthorization *authorization = l->data; | ||
180 | |||
181 | if (strcmp (action_id, authorization->action_id) == 0 && | ||
182 | - polkit_subject_equal (subject_to_use, authorization->subject)) | ||
183 | + subject_equal_for_authz (subject_to_use, authorization->subject)) | ||
184 | { | ||
185 | ret = TRUE; | ||
186 | if (out_tmp_authz_id != NULL) | ||
diff --git a/meta-oe/recipes-extended/polkit/polkit_0.115.bb b/meta-oe/recipes-extended/polkit/polkit_0.115.bb index 13c4b0259..562a754b2 100644 --- a/meta-oe/recipes-extended/polkit/polkit_0.115.bb +++ b/meta-oe/recipes-extended/polkit/polkit_0.115.bb | |||
@@ -23,9 +23,10 @@ PACKAGECONFIG[consolekit] = ",,,consolekit" | |||
23 | 23 | ||
24 | PAM_SRC_URI = "file://polkit-1_pam.patch" | 24 | PAM_SRC_URI = "file://polkit-1_pam.patch" |
25 | SRC_URI = "http://www.freedesktop.org/software/polkit/releases/polkit-${PV}.tar.gz \ | 25 | SRC_URI = "http://www.freedesktop.org/software/polkit/releases/polkit-${PV}.tar.gz \ |
26 | file://0001-make-netgroup-support-configurable.patch \ | 26 | file://0001-make-netgroup-support-configurable.patch \ |
27 | ${@bb.utils.contains('DISTRO_FEATURES', 'pam', '${PAM_SRC_URI}', '', d)} \ | 27 | ${@bb.utils.contains('DISTRO_FEATURES', 'pam', '${PAM_SRC_URI}', '', d)} \ |
28 | " | 28 | file://0001-backend-Compare-PolkitUnixProcess-uids-for-temporary.patch \ |
29 | " | ||
29 | SRC_URI[md5sum] = "f03b055d6ae5fc8eac76838c7d83d082" | 30 | SRC_URI[md5sum] = "f03b055d6ae5fc8eac76838c7d83d082" |
30 | SRC_URI[sha256sum] = "2f87ecdabfbd415c6306673ceadc59846f059b18ef2fce42bac63fe283f12131" | 31 | SRC_URI[sha256sum] = "2f87ecdabfbd415c6306673ceadc59846f059b18ef2fce42bac63fe283f12131" |
31 | 32 | ||