src/programs/pkexec.c | 22 22 + 0 - 0 !
1 file changed, 22 insertions(+)

 set xauthority environment variable if is unset

The way it works is that if XAUTHORITY is unset, then its default
value is $HOME/.Xauthority. But since we're changing user identity
this will not work since $HOME will now change. Therefore, if
XAUTHORITY is unset, just set its default value before changing
identity. This bug only affected login managers using X Window
Authorization but not explicitly setting the XAUTHORITY variable.

You can argue that XAUTHORITY is broken since it forces uid-changing
apps like pkexec(1) to do more work - and get involved in intimate
details of how X works and so on - but that doesn't change how things
work.

Based on a patch from Peter Wu <lekensteyn@gmail.com>.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=51623
Signed-off-by: David Zeuthen <zeuthen@gmail.com>

src/programs/pkexec.c | 5 3 + 2 - 0 !
1 file changed, 3 insertions(+), 2 deletions(-)

 fix build on gnu hurd

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=35685

src/programs/pkexec.c | 10 10 + 0 - 0 !
1 file changed, 10 insertions(+)

 pkexec: set process environment from pam_getenvlist()

Various pam modules provide environment variables that are intended to be set
in the environment of the pam session.  pkexec needs to process the output of
pam_getenvlist() to get these.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=62016

src/polkit/polkitidentity.c | 10 6 + 4 - 0 !
src/polkit/polkitsubject.c | 10 6 + 4 - 0 !
src/polkitbackend/polkitbackendactionlookup.c | 10 6 + 4 - 0 !
3 files changed, 18 insertions(+), 12 deletions(-)

 use gonce for interface type registration

Static local variable may not be enough since it doesn't provide locking.

Related to these udisksd warnings:
  GLib-GObject-WARNING **: cannot register existing type `PolkitSubject'

Thanks to Hans de Goede for spotting this!

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=65130

src/polkit/polkitunixprocess.h | 2 2 + 0 - 0 !
1 file changed, 2 insertions(+)

 polkitunixprocess: deprecate racy apis

It's only safe for processes to be created with their owning uid,
(without kernel support, which we don't have).  Anything else is
subject to clients exec()ing setuid binaries after the fact.

data/polkit-gobject-1.pc.in | 3 3 + 0 - 0 !
docs/man/pkcheck.xml | 29 20 + 9 - 0 !
src/programs/pkcheck.c | 7 6 + 1 - 0 !
3 files changed, 29 insertions(+), 10 deletions(-)

 pkcheck: support --process=pid,start-time,uid syntax too

The uid is a new addition; this allows callers such as libvirt to
close a race condition in reading the uid of the process talking to
them.  They can read it via getsockopt(SO_PEERCRED) or equivalent,
rather than having pkcheck look at /proc later after the fact.

Programs which invoke pkcheck but need to know beforehand (i.e.  at
compile time) whether or not it supports passing the uid can
use:

pkcheck_supports_uid=$($PKG_CONFIG --variable pkcheck_supports_uid polkit-gobject-1)
test x$pkcheck_supports_uid = xyes

src/polkit/polkitpermission.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 port internals non-deprecated polkitprocess api where possible

We can't port everything, but in PolkitPermission and these test
cases, we can use _for_owner() with the right information.

[smcv: drop the part that touches
test/polkitbackend/test-polkitbackendjsauthority.c which is not
in this branch]

src/programs/pkexec.c | 33 30 + 3 - 0 !
1 file changed, 30 insertions(+), 3 deletions(-)

 pkexec: work around systemd injecting broken xdg_runtime_dir

This workaround isn't too much code, and it's often better to fix bugs
in two places anyways.

For more information:

See https://bugzilla.redhat.com/show_bug.cgi?id=753882
See http://lists.freedesktop.org/archives/systemd-devel/2013-November/014370.html

src/polkitagent/polkitagentsession.c | 47 11 + 36 - 0 !
1 file changed, 11 insertions(+), 36 deletions(-)

 polkitagentsession: fix race between child and io watches

The helper flushes and fdatasyncs stdout and stderr before terminating
but this doesn't guarantee that our io watch is called before our
child watch. This means that we can end up with a successful return
from the helper which we still report as a failure.

If we add G_IO_HUP and G_IO_ERR to the conditions we look for in the
io watch and the child terminates we still run the io watch handler
which will complete the session.

This means that the child watch is in fact needless and we can remove
it.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=60847

src/polkitd/main.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 polkitd: fix problem with removing non-existent source

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=77167

src/polkit/polkitsystembusname.c | 56 56 + 0 - 0 !
src/polkit/polkitsystembusname.h | 4 4 + 0 - 0 !
src/polkitbackend/polkitbackendsessionmonitor-systemd.c | 20 1 + 19 - 0 !
src/polkitbackend/polkitbackendsessionmonitor.c | 20 1 + 19 - 0 !
4 files changed, 62 insertions(+), 38 deletions(-)

 polkitsystembusname: add public api to retrieve unix user

And change the duplicated code in the backend session monitors to use
it.  This just a code cleanup resulting from review after
CVE-2013-4288.  There's no security impact from this patch, it just
removes duplicated code.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69538

src/polkitbackend/polkitbackendsessionmonitor.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 [patch] fixed compilation problem in the backend

src/polkitbackend/polkitbackendsessionmonitor.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 [patch] don't discard error data returned by
 polkit_system_bus_name_get_user_sync

https://bugs.freedesktop.org/show_bug.cgi?id=71458

src/polkitbackend/polkitbackendsessionmonitor-systemd.c | 63 22 + 41 - 0 !
1 file changed, 22 insertions(+), 41 deletions(-)

 sessionmonitor-systemd: deduplicate code paths

We had the code to go from pid -> session duplicated.  If we have a
PolkitSystemBusName, convert it to a PolkitUnixProcess.
Then we can do PolkitUnixProcess -> pid -> session in one place.

This is just a code cleanup.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69538

configure.ac | 4 4 + 0 - 0 !
src/polkitbackend/polkitbackendsessionmonitor-systemd.c | 29 24 + 5 - 0 !
2 files changed, 28 insertions(+), 5 deletions(-)

 sessionmonitor-systemd: prepare for d-bus "user bus" model

In the D-Bus "user bus" model, all sessions of a user share the same
D-Bus instance, a polkit requesting process might live outside the
login session which registered the user's polkit agent.

In case a polkit requesting process is not part of the user's login
session, we ask systemd-logind for the the user's "display" session
instead.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=78905
Bug-Debian: https://bugs.debian.org/779988

src/programs/pkexec.c | 5 5 + 0 - 0 !
1 file changed, 5 insertions(+)

 refuse duplicate --user arguments to pkexec

This usage is clearly errorneous, so we should tell the users they are
making a mistake.

Besides, this allows an attacker to cause a high number of heap
allocations with attacker-controlled sizes (
http://googleprojectzero.blogspot.cz/2014/08/the-poisoned-nul-byte-2014-edition.html
), making some exploits easier.

(To be clear, this is not a pkexec vulnerability, and we will not
refuse attacker-affected malloc() usage as a matter of policy; but this
commit is both user-friendly and adding some hardening.)

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=83093

src/polkit/polkitauthority.c | 1 0 + 1 - 0 !
1 file changed, 1 deletion(-)

 authority: fix memory leak in enumerateactions call results handler

Policykit-1 doesn't release reference counters of GVariant data for
org.freedesktop.PolicyKit1.Authority.EnumerateActions dbus call.  This
patch fixed reference counting and following memory leak.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88288

src/polkitbackend/polkitbackendinteractiveauthority.c | 53 30 + 23 - 0 !
1 file changed, 30 insertions(+), 23 deletions(-)

 cve-2015-3218: backend: handle invalid object paths in
 RegisterAuthenticationAgent
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Properly propagate the error, otherwise we dereference a `NULL`
pointer.  This is a local, authenticated DoS.

`RegisterAuthenticationAgentWithOptions` and
`UnregisterAuthentication` have been validated to not need changes for
this.

http://lists.freedesktop.org/archives/polkit-devel/2015-May/000420.html

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90829
Bug-Debian: https://bugs.debian.org/787932
Reported-by: Tavis Ormandy <taviso@google.com>

src/polkitbackend/polkitbackendsessionmonitor-systemd.c | 33 32 + 1 - 0 !
1 file changed, 32 insertions(+), 1 deletion(-)

 sessionmonitor-systemd: use sd_uid_get_state() to check session
 activity
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Instead of using sd_pid_get_session() then sd_session_is_active() to
determine whether the user is active, use sd_uid_get_state() directly.
This gets the maximum of the states of all the user’s sessions, rather
than the state of the session containing the subject process. Since the
user is the security boundary, this is fine.

This change is necessary for `systemd --user` sessions, where most user
code will be forked off user@.service, rather than running inside the
logind session (whether that be a foreground/active or background/online
session).

Policy-wise, the change is from checking whether the subject process is
in an active session; to checking whether the subject process is owned
by a user with at least one active session.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=76358

src/polkitbackend/polkitbackendinteractiveauthority.c | 6 5 + 1 - 0 !
1 file changed, 5 insertions(+), 1 deletion(-)

 fix a possible null dereference.
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

polkit_backend_session_monitor_get_user_for_subject() may return NULL
(and because it is using external processes, we can’t really rule it
out).  The code was already anticipating NULL in the cleanup section, so
handle it also when actually using the value.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=80767

src/polkit/polkitsubject.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 fix duplicate gerror use when "uid" is missing

Some GLib versions complain loudly about this.

To reproduce, call e.g. RegisterAuthenticationAgent with the following
parameters:
("unix-process", {"pid": __import__('gi.repository.GLib', globals(),
locals(), ['Variant']).Variant("u", 1), "start-time":
__import__('gi.repository.GLib', globals(), locals(),
['Variant']).Variant("t", 1)}), "cs", "/"

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90877

src/polkitagent/polkitagenttextlistener.c | 2 0 + 2 - 0 !
1 file changed, 2 deletions(-)

 fix a crash when two authentication requests are in flight.

To reproduce:
1. pkttyagent -p $$ # or another suitable PID
2. pkcheck -p $that_pid -a org.freedesktop.policykit.exec -u
3. pkcheck -p $that_pid -a org.freedesktop.policykit.exec -u
4. Then, in the pkttyagent prompt, press Enter.

polkit_agent_text_listener_initiate_authentication was already setting
an appropriate error code, so the g_assert was unnecessary.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90879

configure.ac | 2 1 + 1 - 0 !
src/polkitagent/polkitagenthelper-pam.c | 12 9 + 3 - 0 !
src/polkitagent/polkitagenthelper-shadow.c | 12 9 + 3 - 0 !
src/polkitagent/polkitagenthelperprivate.c | 33 33 + 0 - 0 !
src/polkitagent/polkitagenthelperprivate.h | 2 2 + 0 - 0 !
src/polkitagent/polkitagentsession.c | 30 16 + 14 - 0 !
src/polkitbackend/polkitbackendinteractiveauthority.c | 99 80 + 19 - 0 !
7 files changed, 150 insertions(+), 40 deletions(-)

 cve-2015-4625: use unpredictable cookie values, keep them secret
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Tavis noted that it'd be possible with a 32 bit counter for someone to
cause the cookie to wrap by creating Authentication requests in a
loop.

Something important to note here is that wrapping of signed integers
is undefined behavior in C, so we definitely want to fix that.  All
counter integers used in this patch are unsigned.

See the comment above `authentication_agent_generate_cookie` for
details, but basically we're now using a cookie of the form:

```
        <agent serial> - <agent random id> - <session serial> - <session
random id>
```

Which has multiple 64 bit counters, plus unpredictable random 128 bit
integer ids (effectively UUIDs, but we're not calling them that
because we don't need to be globally unique.

We further ensure that the cookies are not visible to other processes
by changing the setuid helper to accept them over standard input.  This
means that an attacker would have to guess both ids.

In any case, the security hole here is better fixed with the other
change to bind user id (uid) of the agent with cookie lookups, making
cookie guessing worthless.

Nevertheless, I think it's worth doing this change too, for defense in
depth.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90832
CVE: CVE-2015-4625
Reported-by: Tavis Ormandy <taviso@google.com>

data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml | 14 13 + 1 - 0 !
data/org.freedesktop.PolicyKit1.Authority.xml | 24 23 + 1 - 0 !
docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml | 46 45 + 1 - 0 !
docs/polkit/overview.xml | 18 10 + 8 - 0 !
src/polkit/polkitauthority.c | 13 11 + 2 - 0 !
src/polkitbackend/polkitbackendauthority.c | 61 60 + 1 - 0 !
src/polkitbackend/polkitbackendauthority.h | 2 2 + 0 - 0 !
src/polkitbackend/polkitbackendinteractiveauthority.c | 39 34 + 5 - 0 !
8 files changed, 198 insertions(+), 19 deletions(-)

 cve-2015-4625: bind use of cookies to specific uids
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

http://lists.freedesktop.org/archives/polkit-devel/2015-June/000425.html

The "cookie" value that Polkit hands out is global to all polkit
users.  And when `AuthenticationAgentResponse` is invoked, we
previously only received the cookie and *target* identity, and
attempted to find an agent from that.

The problem is that the current cookie is just an integer
counter, and if it overflowed, it would be possible for
an successful authorization in one session to trigger a response
in another session.

The overflow and ability to guess the cookie were fixed by the
previous patch.

This patch is conceptually further hardening on top of that.  Polkit
currently treats uids as equivalent from a security domain
perspective; there is no support for

data/org.freedesktop.PolicyKit1.AuthenticationAgent.xml | 6 3 + 3 - 0 !
data/org.freedesktop.PolicyKit1.Authority.xml | 11 7 + 4 - 0 !
docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.AuthenticationAgent.xml | 7 5 + 2 - 0 !
docs/polkit/docbook-interface-org.freedesktop.PolicyKit1.Authority.xml | 12 8 + 4 - 0 !
docs/polkit/overview.xml | 8 4 + 4 - 0 !
src/polkit/polkitauthority.c | 24 22 + 2 - 0 !
src/polkitagent/polkitagentlistener.c | 5 1 + 4 - 0 !
src/polkitbackend/polkitbackendauthority.c | 1 1 + 0 - 0 !
8 files changed, 51 insertions(+), 23 deletions(-)

 docs: update for changes to uid binding/authenticationagentresponse2

 - Refer to PolkitAgentSession in general instead of to _response only
 - Revert to the original description of authentication cancellation, the
   agent really needs to return an error to the caller (in addition to dealing
   with the session if any).
 - Explicitly document the UID assumption; in the process fixing bug #69980.
 - Keep documenting that we need a sufficiently privileged caller.
 - Refer to the ...Response2 API in more places.
 - Also update docbook documentation.
 - Drop a paragraph suggesting non-PolkitAgentSession implementations are
   expected and commonplace.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90837

src/polkitbackend/polkitbackendauthority.c | 1 1 + 0 - 0 !
src/polkitbackend/polkitbackendinteractiveauthority.c | 5 4 + 1 - 0 !
2 files changed, 5 insertions(+), 1 deletion(-)

 fix a per-authorization memory leak

We were leaking PolkitAuthorizationResult on every request, primarily on
the success path, but also on various error paths as well.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69501

src/polkitbackend/polkitbackendauthority.c | 1 1 + 0 - 0 !
1 file changed, 1 insertion(+)

 fix a memory leak when registering an authentication agent

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69501

src/polkitbackend/polkitbackendactionpool.c | 8 3 + 5 - 0 !
1 file changed, 3 insertions(+), 5 deletions(-)

 cve-2015-3255 fix ghashtable usage.

Don't assume that the hash table with free both the key and the value
at the same time, supply proper deallocation functions for the key
and value separately.

Then drop ParsedAction::action_id which is no longer used for anything.

https://bugs.freedesktop.org/show_bug.cgi?id=69501
and
https://bugs.freedesktop.org/show_bug.cgi?id=83590

CVE: CVE-2015-3255

src/polkitagent/polkitagentsession.c | 3 2 + 1 - 0 !
1 file changed, 2 insertions(+), 1 deletion(-)

 fix use-after-free in polkitagentsession.c

PolkitAgentTextListener's "completed" handler drops the last reference
to the session; in fact this is explicitly recommended in the signal's
documentation.  So we must not access any members of session after
emitting the signal.

Found while dealing with
https://bugs.freedesktop.org/show_bug.cgi?id=69501

README | 18 17 + 1 - 0 !
1 file changed, 17 insertions(+), 1 deletion(-)

 readme: note to send security reports via dbus's mechanism

This avoids duplicating effort.

src/polkitagent/polkitagenthelper-pam.c | 71 26 + 45 - 0 !
1 file changed, 26 insertions(+), 45 deletions(-)

 escape helper output to handle multiline messages
 Some pam modules produce multiline messages which caused errors in
 PolkitAgentSession as the subsequent lines were interpreted as separate
 messages unrecognized by the authenticator. Escaping every message allows
 to avoid such behaviour.

data/Makefile.am | 3 3 + 0 - 0 !
data/polkit.its | 8 8 + 0 - 0 !
data/polkit.loc | 6 6 + 0 - 0 !
3 files changed, 17 insertions(+)

 [patch] add gettext support for .policy files

gettext can extract strings from and merge them back into xml
file formats, with the help of .its files.

https://bugs.freedesktop.org/show_bug.cgi?id=96940

data/polkit-1.in | 10 6 + 4 - 0 !
1 file changed, 6 insertions(+), 4 deletions(-)

 use debian's common-* pam infrastructure, plus pam_env

src/polkitbackend/polkitbackendactionpool.c | 49 49 + 0 - 0 !
1 file changed, 49 insertions(+)

 use gettext for translations in .policy files

Bug: http://bugs.freedesktop.org/show_bug.cgi?id=29639
Bug-Ubuntu: https://launchpad.net/bugs/619632

src/polkit/polkitunixsession-systemd.c | 14 14 + 0 - 0 !
1 file changed, 14 insertions(+)

 add fallback if agent is not running in a logind session
 This fixes polkit with dbus-user-session.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=96977

src/polkitbackend/50-localauthority.conf | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 revert "default to adminidentities=unix-group:wheel for local
 authority"

This reverts commit 763faf434b445c20ae9529100d3ef5290976d0c9.

On Red Hat derivatives, every member of group 'wheel' is necessarily
privileged. On Debian derivatives, there is no wheel group, and gid 0
(root) is not used in this way. Change the default rule to consider
uid 0 to be privileged, instead.

On Red Hat derivatives, 50-default.rules is not preserved by upgrades;
on dpkg-based systems, it is a proper conffile and may be edited
(at the sysadmin's own risk), so the comment about not editing it is
misleading.

[smcv: added longer explanation of why we make this change;
remove unrelated cosmetic change to a man page]

data/org.freedesktop.PolicyKit1.service.in | 1 1 + 0 - 0 !
1 file changed, 1 insertion(+)

 install systemd service file for polkitd.

configure.ac | 4 2 + 2 - 0 !
1 file changed, 2 insertions(+), 2 deletions(-)

 build against libsystemd

Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=779756

src/polkit/polkitunixgroup.c | 15 11 + 4 - 0 !
src/polkit/polkitunixprocess.c | 12 8 + 4 - 0 !
src/polkit/polkitunixuser.c | 13 10 + 3 - 0 !
3 files changed, 29 insertions(+), 11 deletions(-)

 [patch 1/2] allow negative uids/gids in polkitunixuser and group
 objects

(uid_t) -1 is still used as placeholder to mean "unset". This is OK, since
there should be no users with such number, see
https://systemd.io/UIDS-GIDS#special-linux-uids.

(uid_t) -1 is used as the default value in class initialization.

When a user or group above INT32_MAX is created, the numeric uid or
gid wraps around to negative when the value is assigned to gint, and
polkit gets confused. Let's accept such gids, except for -1.

A nicer fix would be to change the underlying type to e.g. uint32 to
not have negative values. But this cannot be done without breaking the
API, so likely new functions will have to be added (a
polkit_unix_user_new variant that takes a unsigned, and the same for
_group_new, _set_uid, _get_uid, _set_gid, _get_gid, etc.). This will
require a bigger patch.

Fixes https://gitlab.freedesktop.org/polkit/polkit/issues/74.