wok-next view polkit105/stuff/patches/CVE-2015-4625.patch @ rev 21449
updated slock (1.1 -> 1.4)
| author | Hans-G?nter Theisgen |
|---|---|
| date | Thu May 07 07:51:56 2020 +0100 (2020-05-07) |
| parents | |
| children |
line source
1 From ea544ffc18405237ccd95d28d7f45afef49aca17 Mon Sep 17 00:00:00 2001
2 From: Colin Walters <walters@redhat.com>
3 Date: Thu, 4 Jun 2015 12:15:18 -0400
4 Subject: CVE-2015-4625: Use unpredictable cookie values, keep them secret
5 MIME-Version: 1.0
6 Content-Type: text/plain; charset=UTF-8
7 Content-Transfer-Encoding: 8bit
9 Tavis noted that it'd be possible with a 32 bit counter for someone to
10 cause the cookie to wrap by creating Authentication requests in a
11 loop.
13 Something important to note here is that wrapping of signed integers
14 is undefined behavior in C, so we definitely want to fix that. All
15 counter integers used in this patch are unsigned.
17 See the comment above `authentication_agent_generate_cookie` for
18 details, but basically we're now using a cookie of the form:
20 ```
21 <agent serial> - <agent random id> - <session serial> - <session
22 random id>
23 ```
25 Which has multiple 64 bit counters, plus unpredictable random 128 bit
26 integer ids (effectively UUIDs, but we're not calling them that
27 because we don't need to be globally unique.
29 We further ensure that the cookies are not visible to other processes
30 by changing the setuid helper to accept them over standard input. This
31 means that an attacker would have to guess both ids.
33 In any case, the security hole here is better fixed with the other
34 change to bind user id (uid) of the agent with cookie lookups, making
35 cookie guessing worthless.
37 Nevertheless, I think it's worth doing this change too, for defense in
38 depth.
40 Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90832
41 CVE: CVE-2015-4625
42 Reported-by: Tavis Ormandy <taviso@google.com>
43 Reviewed-by: Miloslav Trmač <mitr@redhat.com>
44 Signed-off-by: Colin Walters <walters@redhat.com>
46 diff --git a/src/polkitagent/polkitagenthelper-pam.c b/src/polkitagent/polkitagenthelper-pam.c
47 index 937386e..19062aa 100644
48 --- a/src/polkitagent/polkitagenthelper-pam.c
49 +++ b/src/polkitagent/polkitagenthelper-pam.c
50 @@ -65,7 +65,7 @@ main (int argc, char *argv[])
51 {
52 int rc;
53 const char *user_to_auth;
54 - const char *cookie;
55 + char *cookie = NULL;
56 struct pam_conv pam_conversation;
57 pam_handle_t *pam_h;
58 const void *authed_user;
59 @@ -97,7 +97,7 @@ main (int argc, char *argv[])
60 openlog ("polkit-agent-helper-1", LOG_CONS | LOG_PID, LOG_AUTHPRIV);
62 /* check for correct invocation */
63 - if (argc != 3)
64 + if (!(argc == 2 || argc == 3))
65 {
66 syslog (LOG_NOTICE, "inappropriate use of helper, wrong number of arguments [uid=%d]", getuid ());
67 fprintf (stderr, "polkit-agent-helper-1: wrong number of arguments. This incident has been logged.\n");
68 @@ -105,7 +105,10 @@ main (int argc, char *argv[])
69 }
71 user_to_auth = argv[1];
72 - cookie = argv[2];
73 +
74 + cookie = read_cookie (argc, argv);
75 + if (!cookie)
76 + goto error;
78 if (getuid () != 0)
79 {
80 @@ -203,6 +206,8 @@ main (int argc, char *argv[])
81 goto error;
82 }
84 + free (cookie);
85 +
86 #ifdef PAH_DEBUG
87 fprintf (stderr, "polkit-agent-helper-1: successfully sent D-Bus message to PolicyKit daemon\n");
88 #endif /* PAH_DEBUG */
89 @@ -212,6 +217,7 @@ main (int argc, char *argv[])
90 return 0;
92 error:
93 + free (cookie);
94 if (pam_h != NULL)
95 pam_end (pam_h, rc);
97 diff --git a/src/polkitagent/polkitagenthelper-shadow.c b/src/polkitagent/polkitagenthelper-shadow.c
98 index a4f73ac..e877915 100644
99 --- a/src/polkitagent/polkitagenthelper-shadow.c
100 +++ b/src/polkitagent/polkitagenthelper-shadow.c
101 @@ -46,7 +46,7 @@ main (int argc, char *argv[])
102 {
103 struct spwd *shadow;
104 const char *user_to_auth;
105 - const char *cookie;
106 + char *cookie = NULL;
107 time_t now;
109 /* clear the entire environment to avoid attacks with
110 @@ -67,7 +67,7 @@ main (int argc, char *argv[])
111 openlog ("polkit-agent-helper-1", LOG_CONS | LOG_PID, LOG_AUTHPRIV);
113 /* check for correct invocation */
114 - if (argc != 3)
115 + if (!(argc == 2 || argc == 3))
116 {
117 syslog (LOG_NOTICE, "inappropriate use of helper, wrong number of arguments [uid=%d]", getuid ());
118 fprintf (stderr, "polkit-agent-helper-1: wrong number of arguments. This incident has been logged.\n");
119 @@ -86,7 +86,10 @@ main (int argc, char *argv[])
120 }
122 user_to_auth = argv[1];
123 - cookie = argv[2];
124 +
125 + cookie = read_cookie (argc, argv);
126 + if (!cookie)
127 + goto error;
129 #ifdef PAH_DEBUG
130 fprintf (stderr, "polkit-agent-helper-1: user to auth is '%s'.\n", user_to_auth);
131 @@ -153,6 +156,8 @@ main (int argc, char *argv[])
132 goto error;
133 }
135 + free (cookie);
136 +
137 #ifdef PAH_DEBUG
138 fprintf (stderr, "polkit-agent-helper-1: successfully sent D-Bus message to PolicyKit daemon\n");
139 #endif /* PAH_DEBUG */
140 @@ -162,6 +167,7 @@ main (int argc, char *argv[])
141 return 0;
143 error:
144 + free (cookie);
145 fprintf (stdout, "FAILURE\n");
146 flush_and_wait ();
147 return 1;
148 diff --git a/src/polkitagent/polkitagenthelperprivate.c b/src/polkitagent/polkitagenthelperprivate.c
149 index cfa77fc..e23f9f5 100644
150 --- a/src/polkitagent/polkitagenthelperprivate.c
151 +++ b/src/polkitagent/polkitagenthelperprivate.c
152 @@ -23,6 +23,7 @@
153 #include "config.h"
154 #include "polkitagenthelperprivate.h"
155 #include <stdio.h>
156 +#include <string.h>
157 #include <stdlib.h>
158 #include <unistd.h>
160 @@ -45,6 +46,38 @@ _polkit_clearenv (void)
161 #endif
164 +char *
165 +read_cookie (int argc, char **argv)
166 +{
167 + /* As part of CVE-2015-4625, we started passing the cookie
168 + * on standard input, to ensure it's not visible to other
169 + * processes. However, to ensure that things continue
170 + * to work if the setuid binary is upgraded while old
171 + * agents are still running (this will be common with
172 + * package managers), we support both modes.
173 + */
174 + if (argc == 3)
175 + return strdup (argv[2]);
176 + else
177 + {
178 + char *ret = NULL;
179 + size_t n = 0;
180 + ssize_t r = getline (&ret, &n, stdin);
181 + if (r == -1)
182 + {
183 + if (!feof (stdin))
184 + perror ("getline");
185 + free (ret);
186 + return NULL;
187 + }
188 + else
189 + {
190 + g_strchomp (ret);
191 + return ret;
192 + }
193 + }
194 +}
195 +
196 gboolean
197 send_dbus_message (const char *cookie, const char *user)
198 {
199 diff --git a/src/polkitagent/polkitagenthelperprivate.h b/src/polkitagent/polkitagenthelperprivate.h
200 index aeca2c7..547fdcc 100644
201 --- a/src/polkitagent/polkitagenthelperprivate.h
202 +++ b/src/polkitagent/polkitagenthelperprivate.h
203 @@ -38,6 +38,8 @@
205 int _polkit_clearenv (void);
207 +char *read_cookie (int argc, char **argv);
208 +
209 gboolean send_dbus_message (const char *cookie, const char *user);
211 void flush_and_wait ();
212 diff --git a/src/polkitagent/polkitagentsession.c b/src/polkitagent/polkitagentsession.c
213 index f014773..8b93ad0 100644
214 --- a/src/polkitagent/polkitagentsession.c
215 +++ b/src/polkitagent/polkitagentsession.c
216 @@ -55,6 +55,7 @@
217 #include <stdio.h>
218 #include <sys/types.h>
219 #include <sys/wait.h>
220 +#include <gio/gunixoutputstream.h>
221 #include <pwd.h>
223 #include "polkitagentmarshal.h"
224 @@ -88,7 +89,7 @@ struct _PolkitAgentSession
225 gchar *cookie;
226 PolkitIdentity *identity;
228 - int child_stdin;
229 + GOutputStream *child_stdin;
230 int child_stdout;
231 GPid child_pid;
233 @@ -129,7 +130,6 @@ G_DEFINE_TYPE (PolkitAgentSession, polkit_agent_session, G_TYPE_OBJECT);
234 static void
235 polkit_agent_session_init (PolkitAgentSession *session)
236 {
237 - session->child_stdin = -1;
238 session->child_stdout = -1;
239 }
241 @@ -395,11 +395,7 @@ kill_helper (PolkitAgentSession *session)
242 session->child_stdout = -1;
243 }
245 - if (session->child_stdin != -1)
246 - {
247 - g_warn_if_fail (close (session->child_stdin) == 0);
248 - session->child_stdin = -1;
249 - }
250 + g_clear_object (&session->child_stdin);
252 session->helper_is_running = FALSE;
254 @@ -545,9 +541,9 @@ polkit_agent_session_response (PolkitAgentSession *session,
256 add_newline = (response[response_len] != '\n');
258 - write (session->child_stdin, response, response_len);
259 + (void) g_output_stream_write_all (session->child_stdin, response, response_len, NULL, NULL, NULL);
260 if (add_newline)
261 - write (session->child_stdin, newline, 1);
262 + (void) g_output_stream_write_all (session->child_stdin, newline, 1, NULL, NULL, NULL);
263 }
265 /**
266 @@ -567,8 +563,9 @@ polkit_agent_session_initiate (PolkitAgentSession *session)
267 {
268 uid_t uid;
269 GError *error;
270 - gchar *helper_argv[4];
271 + gchar *helper_argv[3];
272 struct passwd *passwd;
273 + int stdin_fd = -1;
275 g_return_if_fail (POLKIT_AGENT_IS_SESSION (session));
277 @@ -600,10 +597,8 @@ polkit_agent_session_initiate (PolkitAgentSession *session)
279 helper_argv[0] = PACKAGE_PREFIX "/lib/polkit-1/polkit-agent-helper-1";
280 helper_argv[1] = passwd->pw_name;
281 - helper_argv[2] = session->cookie;
282 - helper_argv[3] = NULL;
283 + helper_argv[2] = NULL;
285 - session->child_stdin = -1;
286 session->child_stdout = -1;
288 error = NULL;
289 @@ -615,7 +610,7 @@ polkit_agent_session_initiate (PolkitAgentSession *session)
290 NULL,
291 NULL,
292 &session->child_pid,
293 - &session->child_stdin,
294 + &stdin_fd,
295 &session->child_stdout,
296 NULL,
297 &error))
298 @@ -628,6 +623,13 @@ polkit_agent_session_initiate (PolkitAgentSession *session)
299 if (G_UNLIKELY (_show_debug ()))
300 g_print ("PolkitAgentSession: spawned helper with pid %d\n", (gint) session->child_pid);
302 + session->child_stdin = (GOutputStream*)g_unix_output_stream_new (stdin_fd, TRUE);
303 +
304 + /* Write the cookie on stdin so it can't be seen by other processes */
305 + (void) g_output_stream_write_all (session->child_stdin, session->cookie, strlen (session->cookie),
306 + NULL, NULL, NULL);
307 + (void) g_output_stream_write_all (session->child_stdin, "\n", 1, NULL, NULL, NULL);
308 +
309 session->child_stdout_channel = g_io_channel_unix_new (session->child_stdout);
310 session->child_stdout_watch_source = g_io_create_watch (session->child_stdout_channel,
311 G_IO_IN | G_IO_ERR | G_IO_HUP);
312 diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c
313 index 3f339e9..15adc6a 100644
314 --- a/src/polkitbackend/polkitbackendinteractiveauthority.c
315 +++ b/src/polkitbackend/polkitbackendinteractiveauthority.c
316 @@ -214,6 +214,8 @@ typedef struct
318 GDBusConnection *system_bus_connection;
319 guint name_owner_changed_signal_id;
320 +
321 + guint64 agent_serial;
322 } PolkitBackendInteractiveAuthorityPrivate;
324 /* ---------------------------------------------------------------------------------------------------- */
325 @@ -439,11 +441,15 @@ struct AuthenticationAgent
326 volatile gint ref_count;
328 PolkitSubject *scope;
329 + guint64 serial;
331 gchar *locale;
332 GVariant *registration_options;
333 gchar *object_path;
334 gchar *unique_system_bus_name;
335 + GRand *cookie_pool;
336 + gchar *cookie_prefix;
337 + guint64 cookie_serial;
339 GDBusProxy *proxy;
341 @@ -1427,9 +1433,54 @@ authentication_session_cancelled_cb (GCancellable *cancellable,
342 authentication_session_cancel (session);
343 }
345 +/* We're not calling this a UUID, but it's basically
346 + * the same thing, just not formatted that way because:
347 + *
348 + * - I'm too lazy to do it
349 + * - If we did, people might think it was actually
350 + * generated from /dev/random, which we're not doing
351 + * because this value doesn't actually need to be
352 + * globally unique.
353 + */
354 +static void
355 +append_rand_u128_str (GString *buf,
356 + GRand *pool)
357 +{
358 + g_string_append_printf (buf, "%08x%08x%08x%08x",
359 + g_rand_int (pool),
360 + g_rand_int (pool),
361 + g_rand_int (pool),
362 + g_rand_int (pool));
363 +}
364 +
365 +/* A value that should be unique to the (AuthenticationAgent, AuthenticationSession)
366 + * pair, and not guessable by other agents.
367 + *
368 + * <agent serial> - <agent uuid> - <session serial> - <session uuid>
369 + *
370 + * See http://lists.freedesktop.org/archives/polkit-devel/2015-June/000425.html
371 + *
372 + */
373 +static gchar *
374 +authentication_agent_generate_cookie (AuthenticationAgent *agent)
375 +{
376 + GString *buf = g_string_new ("");
377 +
378 + g_string_append (buf, agent->cookie_prefix);
379 +
380 + g_string_append_c (buf, '-');
381 + agent->cookie_serial++;
382 + g_string_append_printf (buf, "%" G_GUINT64_FORMAT,
383 + agent->cookie_serial);
384 + g_string_append_c (buf, '-');
385 + append_rand_u128_str (buf, agent->cookie_pool);
386 +
387 + return g_string_free (buf, FALSE);
388 +}
389 +
390 +
391 static AuthenticationSession *
392 authentication_session_new (AuthenticationAgent *agent,
393 - const gchar *cookie,
394 PolkitSubject *subject,
395 PolkitIdentity *user_of_subject,
396 PolkitSubject *caller,
397 @@ -1447,7 +1498,7 @@ authentication_session_new (AuthenticationAgent *agent,
399 session = g_new0 (AuthenticationSession, 1);
400 session->agent = authentication_agent_ref (agent);
401 - session->cookie = g_strdup (cookie);
402 + session->cookie = authentication_agent_generate_cookie (agent);
403 session->subject = g_object_ref (subject);
404 session->user_of_subject = g_object_ref (user_of_subject);
405 session->caller = g_object_ref (caller);
406 @@ -1496,16 +1547,6 @@ authentication_session_free (AuthenticationSession *session)
407 g_free (session);
408 }
410 -static gchar *
411 -authentication_agent_new_cookie (AuthenticationAgent *agent)
412 -{
413 - static gint counter = 0;
414 -
415 - /* TODO: use a more random-looking cookie */
416 -
417 - return g_strdup_printf ("cookie%d", counter++);
418 -}
419 -
420 static PolkitSubject *
421 authentication_agent_get_scope (AuthenticationAgent *agent)
422 {
423 @@ -1553,12 +1594,15 @@ authentication_agent_unref (AuthenticationAgent *agent)
424 g_free (agent->unique_system_bus_name);
425 if (agent->registration_options != NULL)
426 g_variant_unref (agent->registration_options);
427 + g_rand_free (agent->cookie_pool);
428 + g_free (agent->cookie_prefix);
429 g_free (agent);
430 }
431 }
433 static AuthenticationAgent *
434 -authentication_agent_new (PolkitSubject *scope,
435 +authentication_agent_new (guint64 serial,
436 + PolkitSubject *scope,
437 const gchar *unique_system_bus_name,
438 const gchar *locale,
439 const gchar *object_path,
440 @@ -1592,6 +1636,7 @@ authentication_agent_new (PolkitSubject *scope,
442 agent = g_new0 (AuthenticationAgent, 1);
443 agent->ref_count = 1;
444 + agent->serial = serial;
445 agent->scope = g_object_ref (scope);
446 agent->object_path = g_strdup (object_path);
447 agent->unique_system_bus_name = g_strdup (unique_system_bus_name);
448 @@ -1599,6 +1644,25 @@ authentication_agent_new (PolkitSubject *scope,
449 agent->registration_options = registration_options != NULL ? g_variant_ref (registration_options) : NULL;
450 agent->proxy = proxy;
452 + {
453 + GString *cookie_prefix = g_string_new ("");
454 + GRand *agent_private_rand = g_rand_new ();
455 +
456 + g_string_append_printf (cookie_prefix, "%" G_GUINT64_FORMAT "-", agent->serial);
457 +
458 + /* Use a uniquely seeded PRNG to get a prefix cookie for this agent,
459 + * whose sequence will not correlate with the per-authentication session
460 + * cookies.
461 + */
462 + append_rand_u128_str (cookie_prefix, agent_private_rand);
463 + g_rand_free (agent_private_rand);
464 +
465 + agent->cookie_prefix = g_string_free (cookie_prefix, FALSE);
466 +
467 + /* And a newly seeded pool for per-session cookies */
468 + agent->cookie_pool = g_rand_new ();
469 + }
470 +
471 return agent;
472 }
474 @@ -2193,7 +2257,6 @@ authentication_agent_initiate_challenge (AuthenticationAgent *agent,
475 {
476 PolkitBackendInteractiveAuthorityPrivate *priv = POLKIT_BACKEND_INTERACTIVE_AUTHORITY_GET_PRIVATE (authority);
477 AuthenticationSession *session;
478 - gchar *cookie;
479 GList *l;
480 GList *identities;
481 gchar *localized_message;
482 @@ -2215,8 +2278,6 @@ authentication_agent_initiate_challenge (AuthenticationAgent *agent,
483 &localized_icon_name,
484 &localized_details);
486 - cookie = authentication_agent_new_cookie (agent);
487 -
488 identities = NULL;
490 /* select admin user if required by the implicit authorization */
491 @@ -2279,7 +2340,6 @@ authentication_agent_initiate_challenge (AuthenticationAgent *agent,
492 user_identities = g_list_prepend (NULL, polkit_unix_user_new (0));
494 session = authentication_session_new (agent,
495 - cookie,
496 subject,
497 user_of_subject,
498 caller,
499 @@ -2335,7 +2395,6 @@ authentication_agent_initiate_challenge (AuthenticationAgent *agent,
500 g_list_free_full (user_identities, g_object_unref);
501 g_list_foreach (identities, (GFunc) g_object_unref, NULL);
502 g_list_free (identities);
503 - g_free (cookie);
505 g_free (localized_message);
506 g_free (localized_icon_name);
507 @@ -2482,7 +2541,9 @@ polkit_backend_interactive_authority_register_authentication_agent (PolkitBacken
508 goto out;
509 }
511 - agent = authentication_agent_new (subject,
512 + priv->agent_serial++;
513 + agent = authentication_agent_new (priv->agent_serial,
514 + subject,
515 polkit_system_bus_name_get_name (POLKIT_SYSTEM_BUS_NAME (caller)),
516 locale,
517 object_path,
518 --
519 cgit v0.10.2
521 From 493aa5dc1d278ab9097110c1262f5229bbaf1766 Mon Sep 17 00:00:00 2001
522 From: Colin Walters <walters@redhat.com>
523 Date: Wed, 17 Jun 2015 13:07:02 -0400
524 Subject: CVE-2015-4625: Bind use of cookies to specific uids
525 MIME-Version: 1.0
526 Content-Type: text/plain; charset=UTF-8
527 Content-Transfer-Encoding: 8bit
529 http://lists.freedesktop.org/archives/polkit-devel/2015-June/000425.html
531 The "cookie" value that Polkit hands out is global to all polkit
532 users. And when `AuthenticationAgentResponse` is invoked, we
533 previously only received the cookie and *target* identity, and
534 attempted to find an agent from that.
536 The problem is that the current cookie is just an integer
537 counter, and if it overflowed, it would be possible for
538 an successful authorization in one session to trigger a response
539 in another session.
541 The overflow and ability to guess the cookie were fixed by the
542 previous patch.
544 This patch is conceptually further hardening on top of that. Polkit
545 currently treats uids as equivalent from a security domain
546 perspective; there is no support for
547 SELinux/AppArmor/etc. differentiation.
549 We can retrieve the uid from `getuid()` in the setuid helper, which
550 allows us to ensure the uid invoking `AuthenticationAgentResponse2`
551 matches that of the agent.
553 Then the authority only looks at authentication sessions matching the
554 cookie that were created by a matching uid, thus removing the ability
555 for different uids to interfere with each other entirely.
557 Several fixes to this patch were contributed by:
558 Miloslav Trmač <mitr@redhat.com>
560 Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90837
561 CVE: CVE-2015-4625
562 Reported-by: Tavis Ormandy <taviso@google.com>
563 Reviewed-by: Miloslav Trmač <mitr@redhat.com>
564 Signed-off-by: Colin Walters <walters@redhat.com>
566 diff --git a/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml b/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
567 index 3b519c2..5beef7d 100644
568 --- a/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
569 +++ b/data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml
570 @@ -8,7 +8,19 @@
571 <annotation name="org.gtk.EggDBus.DocString" value="<para>This D-Bus interface is used for communication between the system-wide PolicyKit daemon and one or more authentication agents each running in a user session.</para><para>An authentication agent must implement this interface and register (passing the object path of the object implementing the interface) using the org.freedesktop.PolicyKit1.Authority.RegisterAuthenticationAgent() and org.freedesktop.PolicyKit1.Authority.UnregisterAuthenticationAgent() methods on the #org.freedesktop.PolicyKit1.Authority interface of the PolicyKit daemon.</para>"/>
573 <method name="BeginAuthentication">
574 - <annotation name="org.gtk.EggDBus.DocString" value="<para>Called by the PolicyKit daemon when the authentication agent needs the user to authenticate as one of the identities in @identities for the action with the identifier @action_id.</para><para>Upon succesful authentication, the authentication agent must invoke the org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse() method on the #org.freedesktop.PolicyKit1.Authority interface of the PolicyKit daemon before returning.</para><para>If the user dismisses the authentication dialog, the authentication agent should return an error.</para>"/>
575 + <annotation name="org.gtk.EggDBus.DocString" value="<para>Called
576 + by the PolicyKit daemon when the authentication agent needs the
577 + user to authenticate as one of the identities in @identities for
578 + the action with the identifier @action_id.</para><para>This
579 + authentication is normally achieved via the
580 + polkit_agent_session_response() API, which invokes a private
581 + setuid helper process to verify the authentication. When
582 + successful, it calls the
583 + org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2()
584 + method on the #org.freedesktop.PolicyKit1.Authority interface of
585 + the PolicyKit daemon before returning. If the user dismisses the
586 + authentication dialog, the authentication agent should call
587 + polkit_agent_session_cancel().</para>"/>
589 <arg name="action_id" direction="in" type="s">
590 <annotation name="org.gtk.EggDBus.DocString" value="The identifier for the action that the user is authentication for."/>
591 diff --git a/data/org.freedesktop.PolicyKit1.Authority.xml b/data/org.freedesktop.PolicyKit1.Authority.xml
592 index fbfb9cd..f9021ee 100644
593 --- a/data/org.freedesktop.PolicyKit1.Authority.xml
594 +++ b/data/org.freedesktop.PolicyKit1.Authority.xml
595 @@ -313,7 +313,29 @@
596 </method>
598 <method name="AuthenticationAgentResponse">
599 - <annotation name="org.gtk.EggDBus.DocString" value="Method for authentication agents to invoke on successful authentication. This method will fail unless a sufficiently privileged caller invokes it."/>
600 + <annotation name="org.gtk.EggDBus.DocString" value="Method for authentication agents to invoke on successful
601 +authentication, intended only for use by a privileged helper process
602 +internal to polkit."/>
603 +
604 + <arg name="cookie" direction="in" type="s">
605 + <annotation name="org.gtk.EggDBus.DocString" value="The cookie identifying the authentication request that was passed to the authentication agent."/>
606 + </arg>
607 +
608 + <arg name="identity" direction="in" type="(sa{sv})">
609 + <annotation name="org.gtk.EggDBus.Type" value="Identity"/>
610 + <annotation name="org.gtk.EggDBus.DocString" value="A #Identity struct describing what identity was authenticated."/>
611 + </arg>
612 + </method>
613 +
614 + <method name="AuthenticationAgentResponse2">
615 + <annotation name="org.gtk.EggDBus.DocString" value="Method for authentication agents to invoke on successful
616 +authentication, intended only for use by a privileged helper process
617 +internal to polkit. Note this method was added in 0.114, and should be preferred over AuthenticationAgentResponse
618 +as it fixes a security issue."/>
619 +
620 + <arg name="uid" direction="in" type="u">
621 + <annotation name="org.gtk.EggDBus.DocString" value="The real uid of the agent. Normally set by the setuid helper program."/>
622 + </arg>
624 <arg name="cookie" direction="in" type="s">
625 <annotation name="org.gtk.EggDBus.DocString" value="The cookie identifying the authentication request that was passed to the authentication agent."/>
626 diff --git a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
627 index 6525e25..e66bf53 100644
628 --- a/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
629 +++ b/docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml
630 @@ -42,6 +42,8 @@ Structure <link linkend="eggdbus-struct-TemporaryAuthorization">TemporaryAuth
631 IN String object_path)
632 <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse</link> (IN String cookie,
633 IN <link linkend="eggdbus-struct-Identity">Identity</link> identity)
634 +<link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse2</link> (IN uint32 uid, IN String cookie,
635 + IN <link linkend="eggdbus-struct-Identity">Identity</link> identity)
636 <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.EnumerateTemporaryAuthorizations">EnumerateTemporaryAuthorizations</link> (IN <link linkend="eggdbus-struct-Subject">Subject</link> subject,
637 OUT Array<<link linkend="eggdbus-struct-TemporaryAuthorization">TemporaryAuthorization</link>> temporary_authorizations)
638 <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.RevokeTemporaryAuthorizations">RevokeTemporaryAuthorizations</link> (IN <link linkend="eggdbus-struct-Subject">Subject</link> subject)
639 @@ -777,10 +779,52 @@ AuthenticationAgentResponse (IN String cookie,
640 IN <link linkend="eggdbus-struct-Identity">Identity</link> identity)
641 </programlisting>
642 <para>
643 -Method for authentication agents to invoke on successful authentication. This method will fail unless a sufficiently privileged caller invokes it.
644 +Method for authentication agents to invoke on successful
645 +authentication, intended only for use by a privileged helper process
646 +internal to polkit. Deprecated in favor of AuthenticationAgentResponse2.
647 + </para>
648 +<variablelist role="params">
649 + <varlistentry>
650 + <term><literal>IN String <parameter>cookie</parameter></literal>:</term>
651 + <listitem>
652 + <para>
653 +The cookie identifying the authentication request that was passed to the authentication agent.
654 + </para>
655 + </listitem>
656 + </varlistentry>
657 + <varlistentry>
658 + <term><literal>IN <link linkend="eggdbus-struct-Identity">Identity</link> <parameter>identity</parameter></literal>:</term>
659 + <listitem>
660 + <para>
661 +A <link linkend="eggdbus-struct-Identity">Identity</link> struct describing what identity was authenticated.
662 + </para>
663 + </listitem>
664 + </varlistentry>
665 +</variablelist>
666 + </refsect2>
667 + <refsect2 role="function" id="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse2">
668 + <title>AuthenticationAgentResponse2 ()</title>
669 + <programlisting>
670 +AuthenticationAgentResponse2 (IN uint32 uid,
671 + IN String cookie,
672 + IN <link linkend="eggdbus-struct-Identity">Identity</link> identity)
673 + </programlisting>
674 + <para>
675 +Method for authentication agents to invoke on successful
676 +authentication, intended only for use by a privileged helper process
677 +internal to polkit. Note this method was introduced in 0.114 to fix a security issue.
678 </para>
679 <variablelist role="params">
680 <varlistentry>
681 + <term><literal>IN uint32 <parameter>uid</parameter></literal>:</term>
682 + <listitem>
683 + <para>
684 +The user id of the agent; normally this is the owner of the parent pid
685 +of the process that invoked the internal setuid helper.
686 + </para>
687 + </listitem>
688 + </varlistentry>
689 + <varlistentry>
690 <term><literal>IN String <parameter>cookie</parameter></literal>:</term>
691 <listitem>
692 <para>
693 diff --git a/docs/polkit/overview.xml b/docs/polkit/overview.xml
694 index 150a7bc..176d2ea 100644
695 --- a/docs/polkit/overview.xml
696 +++ b/docs/polkit/overview.xml
697 @@ -314,16 +314,18 @@
698 <para>
699 Authentication agents are provided by desktop environments. When
700 an user session starts, the agent registers with the polkit
701 - Authority using
702 - the <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.RegisterAuthenticationAgent">RegisterAuthenticationAgent()</link>
703 + Authority using the <link
704 + linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.RegisterAuthenticationAgent">RegisterAuthenticationAgent()</link>
705 method. When services are needed, the authority will invoke
706 - methods on
707 - the <link linkend="eggdbus-interface-org.freedesktop.PolicyKit1.AuthenticationAgent">org.freedesktop.PolicyKit1.AuthenticationAgent</link>
708 + methods on the <link
709 + linkend="eggdbus-interface-org.freedesktop.PolicyKit1.AuthenticationAgent">org.freedesktop.PolicyKit1.AuthenticationAgent</link>
710 D-Bus interface. Once the user is authenticated, (a privileged
711 - part of) the agent invokes
712 - the <link linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse()</link>
713 - method. Note that the polkit Authority itself does not care
714 - how the agent authenticates the user.
715 + part of) the agent invokes the <link
716 + linkend="eggdbus-method-org.freedesktop.PolicyKit1.Authority.AuthenticationAgentResponse">AuthenticationAgentResponse()</link>
717 + method. This method should be treated as an internal
718 + implementation detail, and callers should use the public shared
719 + library API to invoke it, which currently uses a setuid helper
720 + program.
721 </para>
722 <para>
723 The <link linkend="ref-authentication-agent-api">libpolkit-agent-1</link>
724 diff --git a/src/polkit/polkitauthority.c b/src/polkit/polkitauthority.c
725 index ab6d3cd..6bd684a 100644
726 --- a/src/polkit/polkitauthority.c
727 +++ b/src/polkit/polkitauthority.c
728 @@ -1492,6 +1492,14 @@ polkit_authority_authentication_agent_response (PolkitAuthority *authority,
729 gpointer user_data)
730 {
731 GVariant *identity_value;
732 + /* Note that in reality, this API is only accessible to root, and
733 + * only called from the setuid helper `polkit-agent-helper-1`.
734 + *
735 + * However, because this is currently public API, we avoid
736 + * triggering warnings from ABI diff type programs by just grabbing
737 + * the real uid of the caller here.
738 + */
739 + uid_t uid = getuid ();
741 g_return_if_fail (POLKIT_IS_AUTHORITY (authority));
742 g_return_if_fail (cookie != NULL);
743 @@ -1501,8 +1509,9 @@ polkit_authority_authentication_agent_response (PolkitAuthority *authority,
744 identity_value = polkit_identity_to_gvariant (identity);
745 g_variant_ref_sink (identity_value);
746 g_dbus_proxy_call (authority->proxy,
747 - "AuthenticationAgentResponse",
748 - g_variant_new ("(s@(sa{sv}))",
749 + "AuthenticationAgentResponse2",
750 + g_variant_new ("(us@(sa{sv}))",
751 + (guint32)uid,
752 cookie,
753 identity_value),
754 G_DBUS_CALL_FLAGS_NONE,
755 diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c
756 index 601a974..03a4e84 100644
757 --- a/src/polkitbackend/polkitbackendauthority.c
758 +++ b/src/polkitbackend/polkitbackendauthority.c
759 @@ -355,6 +355,7 @@ polkit_backend_authority_unregister_authentication_agent (PolkitBackendAuthority
760 gboolean
761 polkit_backend_authority_authentication_agent_response (PolkitBackendAuthority *authority,
762 PolkitSubject *caller,
763 + uid_t uid,
764 const gchar *cookie,
765 PolkitIdentity *identity,
766 GError **error)
767 @@ -373,7 +374,7 @@ polkit_backend_authority_authentication_agent_response (PolkitBackendAuthority
768 }
769 else
770 {
771 - return klass->authentication_agent_response (authority, caller, cookie, identity, error);
772 + return klass->authentication_agent_response (authority, caller, uid, cookie, identity, error);
773 }
774 }
776 @@ -587,6 +588,11 @@ static const gchar *server_introspection_data =
777 " <arg type='s' name='cookie' direction='in'/>"
778 " <arg type='(sa{sv})' name='identity' direction='in'/>"
779 " </method>"
780 + " <method name='AuthenticationAgentResponse2'>"
781 + " <arg type='u' name='uid' direction='in'/>"
782 + " <arg type='s' name='cookie' direction='in'/>"
783 + " <arg type='(sa{sv})' name='identity' direction='in'/>"
784 + " </method>"
785 " <method name='EnumerateTemporaryAuthorizations'>"
786 " <arg type='(sa{sv})' name='subject' direction='in'/>"
787 " <arg type='a(ss(sa{sv})tt)' name='temporary_authorizations' direction='out'/>"
788 @@ -1035,6 +1041,57 @@ server_handle_authentication_agent_response (Server *server,
789 error = NULL;
790 if (!polkit_backend_authority_authentication_agent_response (server->authority,
791 caller,
792 + (uid_t)-1,
793 + cookie,
794 + identity,
795 + &error))
796 + {
797 + g_dbus_method_invocation_return_gerror (invocation, error);
798 + g_error_free (error);
799 + goto out;
800 + }
801 +
802 + g_dbus_method_invocation_return_value (invocation, g_variant_new ("()"));
803 +
804 + out:
805 + if (identity != NULL)
806 + g_object_unref (identity);
807 +}
808 +
809 +static void
810 +server_handle_authentication_agent_response2 (Server *server,
811 + GVariant *parameters,
812 + PolkitSubject *caller,
813 + GDBusMethodInvocation *invocation)
814 +{
815 + const gchar *cookie;
816 + GVariant *identity_gvariant;
817 + PolkitIdentity *identity;
818 + GError *error;
819 + guint32 uid;
820 +
821 + identity = NULL;
822 +
823 + g_variant_get (parameters,
824 + "(u&s@(sa{sv}))",
825 + &uid,
826 + &cookie,
827 + &identity_gvariant);
828 +
829 + error = NULL;
830 + identity = polkit_identity_new_for_gvariant (identity_gvariant, &error);
831 + if (identity == NULL)
832 + {
833 + g_prefix_error (&error, "Error getting identity: ");
834 + g_dbus_method_invocation_return_gerror (invocation, error);
835 + g_error_free (error);
836 + goto out;
837 + }
838 +
839 + error = NULL;
840 + if (!polkit_backend_authority_authentication_agent_response (server->authority,
841 + caller,
842 + (uid_t)uid,
843 cookie,
844 identity,
845 &error))
846 @@ -1222,6 +1279,8 @@ server_handle_method_call (GDBusConnection *connection,
847 server_handle_unregister_authentication_agent (server, parameters, caller, invocation);
848 else if (g_strcmp0 (method_name, "AuthenticationAgentResponse") == 0)
849 server_handle_authentication_agent_response (server, parameters, caller, invocation);
850 + else if (g_strcmp0 (method_name, "AuthenticationAgentResponse2") == 0)
851 + server_handle_authentication_agent_response2 (server, parameters, caller, invocation);
852 else if (g_strcmp0 (method_name, "EnumerateTemporaryAuthorizations") == 0)
853 server_handle_enumerate_temporary_authorizations (server, parameters, caller, invocation);
854 else if (g_strcmp0 (method_name, "RevokeTemporaryAuthorizations") == 0)
855 diff --git a/src/polkitbackend/polkitbackendauthority.h b/src/polkitbackend/polkitbackendauthority.h
856 index f9f7385..88df82e 100644
857 --- a/src/polkitbackend/polkitbackendauthority.h
858 +++ b/src/polkitbackend/polkitbackendauthority.h
859 @@ -147,6 +147,7 @@ struct _PolkitBackendAuthorityClass
861 gboolean (*authentication_agent_response) (PolkitBackendAuthority *authority,
862 PolkitSubject *caller,
863 + uid_t uid,
864 const gchar *cookie,
865 PolkitIdentity *identity,
866 GError **error);
867 @@ -249,6 +250,7 @@ gboolean polkit_backend_authority_unregister_authentication_agent (PolkitBackend
869 gboolean polkit_backend_authority_authentication_agent_response (PolkitBackendAuthority *authority,
870 PolkitSubject *caller,
871 + uid_t uid,
872 const gchar *cookie,
873 PolkitIdentity *identity,
874 GError **error);
875 diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c
876 index 15adc6a..96725f7 100644
877 --- a/src/polkitbackend/polkitbackendinteractiveauthority.c
878 +++ b/src/polkitbackend/polkitbackendinteractiveauthority.c
879 @@ -108,8 +108,9 @@ static AuthenticationAgent *get_authentication_agent_for_subject (PolkitBackendI
880 PolkitSubject *subject);
883 -static AuthenticationSession *get_authentication_session_for_cookie (PolkitBackendInteractiveAuthority *authority,
884 - const gchar *cookie);
885 +static AuthenticationSession *get_authentication_session_for_uid_and_cookie (PolkitBackendInteractiveAuthority *authority,
886 + uid_t uid,
887 + const gchar *cookie);
889 static GList *get_authentication_sessions_initiated_by_system_bus_unique_name (PolkitBackendInteractiveAuthority *authority,
890 const gchar *system_bus_unique_name);
891 @@ -169,6 +170,7 @@ static gboolean polkit_backend_interactive_authority_unregister_authentication_a
893 static gboolean polkit_backend_interactive_authority_authentication_agent_response (PolkitBackendAuthority *authority,
894 PolkitSubject *caller,
895 + uid_t uid,
896 const gchar *cookie,
897 PolkitIdentity *identity,
898 GError **error);
899 @@ -440,6 +442,7 @@ struct AuthenticationAgent
900 {
901 volatile gint ref_count;
903 + uid_t creator_uid;
904 PolkitSubject *scope;
905 guint64 serial;
907 @@ -1603,6 +1606,7 @@ authentication_agent_unref (AuthenticationAgent *agent)
908 static AuthenticationAgent *
909 authentication_agent_new (guint64 serial,
910 PolkitSubject *scope,
911 + PolkitIdentity *creator,
912 const gchar *unique_system_bus_name,
913 const gchar *locale,
914 const gchar *object_path,
915 @@ -1611,6 +1615,10 @@ authentication_agent_new (guint64 serial,
916 {
917 AuthenticationAgent *agent;
918 GDBusProxy *proxy;
919 + PolkitUnixUser *creator_user;
920 +
921 + g_assert (POLKIT_IS_UNIX_USER (creator));
922 + creator_user = POLKIT_UNIX_USER (creator);
924 if (!g_variant_is_object_path (object_path))
925 {
926 @@ -1638,6 +1646,7 @@ authentication_agent_new (guint64 serial,
927 agent->ref_count = 1;
928 agent->serial = serial;
929 agent->scope = g_object_ref (scope);
930 + agent->creator_uid = (uid_t)polkit_unix_user_get_uid (creator_user);
931 agent->object_path = g_strdup (object_path);
932 agent->unique_system_bus_name = g_strdup (unique_system_bus_name);
933 agent->locale = g_strdup (locale);
934 @@ -1736,8 +1745,9 @@ get_authentication_agent_for_subject (PolkitBackendInteractiveAuthority *authori
935 }
937 static AuthenticationSession *
938 -get_authentication_session_for_cookie (PolkitBackendInteractiveAuthority *authority,
939 - const gchar *cookie)
940 +get_authentication_session_for_uid_and_cookie (PolkitBackendInteractiveAuthority *authority,
941 + uid_t uid,
942 + const gchar *cookie)
943 {
944 PolkitBackendInteractiveAuthorityPrivate *priv;
945 GHashTableIter hash_iter;
946 @@ -1755,6 +1765,23 @@ get_authentication_session_for_cookie (PolkitBackendInteractiveAuthority *author
947 {
948 GList *l;
950 + /* We need to ensure that if somehow we have duplicate cookies
951 + * due to wrapping, that the cookie used is matched to the user
952 + * who called AuthenticationAgentResponse2. See
953 + * http://lists.freedesktop.org/archives/polkit-devel/2015-June/000425.html
954 + *
955 + * Except if the legacy AuthenticationAgentResponse is invoked,
956 + * we don't know the uid and hence use -1. Continue to support
957 + * the old behavior for backwards compatibility, although everyone
958 + * who is using our own setuid helper will automatically be updated
959 + * to the new API.
960 + */
961 + if (uid != (uid_t)-1)
962 + {
963 + if (agent->creator_uid != uid)
964 + continue;
965 + }
966 +
967 for (l = agent->active_sessions; l != NULL; l = l->next)
968 {
969 AuthenticationSession *session = l->data;
970 @@ -2544,6 +2571,7 @@ polkit_backend_interactive_authority_register_authentication_agent (PolkitBacken
971 priv->agent_serial++;
972 agent = authentication_agent_new (priv->agent_serial,
973 subject,
974 + user_of_caller,
975 polkit_system_bus_name_get_name (POLKIT_SYSTEM_BUS_NAME (caller)),
976 locale,
977 object_path,
978 @@ -2757,6 +2785,7 @@ polkit_backend_interactive_authority_unregister_authentication_agent (PolkitBack
979 static gboolean
980 polkit_backend_interactive_authority_authentication_agent_response (PolkitBackendAuthority *authority,
981 PolkitSubject *caller,
982 + uid_t uid,
983 const gchar *cookie,
984 PolkitIdentity *identity,
985 GError **error)
986 @@ -2799,7 +2828,7 @@ polkit_backend_interactive_authority_authentication_agent_response (PolkitBacken
987 }
989 /* find the authentication session */
990 - session = get_authentication_session_for_cookie (interactive_authority, cookie);
991 + session = get_authentication_session_for_uid_and_cookie (interactive_authority, uid, cookie);
992 if (session == NULL)
993 {
994 g_set_error (error,
995 --
996 cgit v0.10.2
998 --- ./configure.ac.orig
999 +++ ./configure.ac
1000 @@ -122,7 +122,7 @@
1001 changequote([,])dnl
1002 fi
1004 -PKG_CHECK_MODULES(GLIB, [gio-2.0 >= 2.28.0])
1005 +PKG_CHECK_MODULES(GLIB, [gmodule-2.0 gio-unix-2.0 gio-2.0 >= 2.30.0])
1006 AC_SUBST(GLIB_CFLAGS)
1007 AC_SUBST(GLIB_LIBS)