PolicyKit: Branch 'master' - 3 commits
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Thu Dec 6 14:50:05 UTC 2018
src/polkit/polkitunixgroup.c | 15 +++-
src/polkit/polkitunixprocess.c | 12 ++-
src/polkit/polkitunixuser.c | 13 ++-
test/data/etc/group | 1
test/data/etc/passwd | 2
test/data/etc/polkit-1/rules.d/10-testing.rules | 21 ++++++
test/polkitbackend/test-polkitbackendjsauthority.c | 72 +++++++++++++++++++++
7 files changed, 125 insertions(+), 11 deletions(-)
New commits:
commit 5230646dc6876ef6e27f57926b1bad348f636147
Merge: 23fd211 b534a10
Author: Jan Rybar <jrybar at redhat.com>
Date: Thu Dec 6 14:50:03 2018 +0000
Merge branch 'negative-uids' into 'master'
High uid fixup
CVE-2018-19788
Closes #74
See merge request polkit/polkit!14
commit b534a10727455409acd54018a9c91000e7626126
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date: Mon Dec 3 11:20:34 2018 +0100
tests: add tests for high uids
diff --git a/test/data/etc/group b/test/data/etc/group
index 12ef328..b9acab9 100644
--- a/test/data/etc/group
+++ b/test/data/etc/group
@@ -5,3 +5,4 @@ john:x:500:
jane:x:501:
sally:x:502:
henry:x:503:
+highuid2:x:4000000000:
diff --git a/test/data/etc/passwd b/test/data/etc/passwd
index 8544feb..5cf14a5 100644
--- a/test/data/etc/passwd
+++ b/test/data/etc/passwd
@@ -3,3 +3,5 @@ john:x:500:500:John Done:/home/john:/bin/bash
jane:x:501:501:Jane Smith:/home/jane:/bin/bash
sally:x:502:502:Sally Derp:/home/sally:/bin/bash
henry:x:503:503:Henry Herp:/home/henry:/bin/bash
+highuid1:x:2147483648:2147483648:The first high uid:/home/highuid1:/sbin/nologin
+highuid2:x:4000000000:4000000000:An example high uid:/home/example:/sbin/nologin
diff --git a/test/data/etc/polkit-1/rules.d/10-testing.rules b/test/data/etc/polkit-1/rules.d/10-testing.rules
index 446e622..98bf062 100644
--- a/test/data/etc/polkit-1/rules.d/10-testing.rules
+++ b/test/data/etc/polkit-1/rules.d/10-testing.rules
@@ -53,6 +53,27 @@ polkit.addRule(function(action, subject) {
}
});
+polkit.addRule(function(action, subject) {
+ if (action.id == "net.company.john_action") {
+ if (subject.user == "john") {
+ return polkit.Result.YES;
+ } else {
+ return polkit.Result.NO;
+ }
+ }
+});
+
+polkit.addRule(function(action, subject) {
+ if (action.id == "net.company.highuid2_action") {
+ if (subject.user == "highuid2") {
+ return polkit.Result.YES;
+ } else {
+ return polkit.Result.NO;
+ }
+ }
+});
+
+
// ---------------------------------------------------------------------
// variables
diff --git a/test/polkitbackend/test-polkitbackendjsauthority.c b/test/polkitbackend/test-polkitbackendjsauthority.c
index b484a26..71aad23 100644
--- a/test/polkitbackend/test-polkitbackendjsauthority.c
+++ b/test/polkitbackend/test-polkitbackendjsauthority.c
@@ -330,6 +330,78 @@ static const RulesTestCase rules_test_cases[] = {
NULL,
POLKIT_IMPLICIT_AUTHORIZATION_AUTHORIZED,
},
+
+ {
+ /* highuid1 is not a member of group 'users', see test/data/etc/group */
+ "group_membership_with_non_member(highuid22)",
+ "net.company.group.only_group_users",
+ "unix-user:highuid2",
+ NULL,
+ POLKIT_IMPLICIT_AUTHORIZATION_NOT_AUTHORIZED,
+ },
+
+ {
+ /* highuid2 is not a member of group 'users', see test/data/etc/group */
+ "group_membership_with_non_member(highuid21)",
+ "net.company.group.only_group_users",
+ "unix-user:highuid2",
+ NULL,
+ POLKIT_IMPLICIT_AUTHORIZATION_NOT_AUTHORIZED,
+ },
+
+ {
+ /* highuid1 is not a member of group 'users', see test/data/etc/group */
+ "group_membership_with_non_member(highuid24)",
+ "net.company.group.only_group_users",
+ "unix-user:2147483648",
+ NULL,
+ POLKIT_IMPLICIT_AUTHORIZATION_NOT_AUTHORIZED,
+ },
+
+ {
+ /* highuid2 is not a member of group 'users', see test/data/etc/group */
+ "group_membership_with_non_member(highuid23)",
+ "net.company.group.only_group_users",
+ "unix-user:4000000000",
+ NULL,
+ POLKIT_IMPLICIT_AUTHORIZATION_NOT_AUTHORIZED,
+ },
+
+ {
+ /* john is authorized to do this, see 10-testing.rules */
+ "john_action",
+ "net.company.john_action",
+ "unix-user:john",
+ NULL,
+ POLKIT_IMPLICIT_AUTHORIZATION_AUTHORIZED,
+ },
+
+ {
+ /* only john is authorized to do this, see 10-testing.rules */
+ "jane_action",
+ "net.company.john_action",
+ "unix-user:jane",
+ NULL,
+ POLKIT_IMPLICIT_AUTHORIZATION_NOT_AUTHORIZED,
+ },
+
+ {
+ /* highuid2 is authorized to do this, see 10-testing.rules */
+ "highuid2_action",
+ "net.company.highuid2_action",
+ "unix-user:highuid2",
+ NULL,
+ POLKIT_IMPLICIT_AUTHORIZATION_AUTHORIZED,
+ },
+
+ {
+ /* only highuid2 is authorized to do this, see 10-testing.rules */
+ "highuid1_action",
+ "net.company.highuid2_action",
+ "unix-user:highuid1",
+ NULL,
+ POLKIT_IMPLICIT_AUTHORIZATION_NOT_AUTHORIZED,
+ },
};
/* ---------------------------------------------------------------------------------------------------- */
commit 2cb40c4d5feeaa09325522bd7d97910f1b59e379
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date: Mon Dec 3 10:28:58 2018 +0100
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.
diff --git a/src/polkit/polkitunixgroup.c b/src/polkit/polkitunixgroup.c
index c57a1aa..309f689 100644
--- a/src/polkit/polkitunixgroup.c
+++ b/src/polkit/polkitunixgroup.c
@@ -71,6 +71,7 @@ G_DEFINE_TYPE_WITH_CODE (PolkitUnixGroup, polkit_unix_group, G_TYPE_OBJECT,
static void
polkit_unix_group_init (PolkitUnixGroup *unix_group)
{
+ unix_group->gid = -1; /* (git_t) -1 is not a valid GID under Linux */
}
static void
@@ -100,11 +101,14 @@ polkit_unix_group_set_property (GObject *object,
GParamSpec *pspec)
{
PolkitUnixGroup *unix_group = POLKIT_UNIX_GROUP (object);
+ gint val;
switch (prop_id)
{
case PROP_GID:
- unix_group->gid = g_value_get_int (value);
+ val = g_value_get_int (value);
+ g_return_if_fail (val != -1);
+ unix_group->gid = val;
break;
default:
@@ -131,9 +135,9 @@ polkit_unix_group_class_init (PolkitUnixGroupClass *klass)
g_param_spec_int ("gid",
"Group ID",
"The UNIX group ID",
- 0,
+ G_MININT,
G_MAXINT,
- 0,
+ -1,
G_PARAM_CONSTRUCT |
G_PARAM_READWRITE |
G_PARAM_STATIC_NAME |
@@ -166,9 +170,10 @@ polkit_unix_group_get_gid (PolkitUnixGroup *group)
*/
void
polkit_unix_group_set_gid (PolkitUnixGroup *group,
- gint gid)
+ gint gid)
{
g_return_if_fail (POLKIT_IS_UNIX_GROUP (group));
+ g_return_if_fail (gid != -1);
group->gid = gid;
}
@@ -183,6 +188,8 @@ polkit_unix_group_set_gid (PolkitUnixGroup *group,
PolkitIdentity *
polkit_unix_group_new (gint gid)
{
+ g_return_val_if_fail (gid != -1, NULL);
+
return POLKIT_IDENTITY (g_object_new (POLKIT_TYPE_UNIX_GROUP,
"gid", gid,
NULL));
diff --git a/src/polkit/polkitunixprocess.c b/src/polkit/polkitunixprocess.c
index 972b777..b02b258 100644
--- a/src/polkit/polkitunixprocess.c
+++ b/src/polkit/polkitunixprocess.c
@@ -159,9 +159,14 @@ polkit_unix_process_set_property (GObject *object,
polkit_unix_process_set_pid (unix_process, g_value_get_int (value));
break;
- case PROP_UID:
- polkit_unix_process_set_uid (unix_process, g_value_get_int (value));
+ case PROP_UID: {
+ gint val;
+
+ val = g_value_get_int (value);
+ g_return_if_fail (val != -1);
+ polkit_unix_process_set_uid (unix_process, val);
break;
+ }
case PROP_START_TIME:
polkit_unix_process_set_start_time (unix_process, g_value_get_uint64 (value));
@@ -239,7 +244,7 @@ polkit_unix_process_class_init (PolkitUnixProcessClass *klass)
g_param_spec_int ("uid",
"User ID",
"The UNIX user ID",
- -1,
+ G_MININT,
G_MAXINT,
-1,
G_PARAM_CONSTRUCT |
@@ -303,7 +308,6 @@ polkit_unix_process_set_uid (PolkitUnixProcess *process,
gint uid)
{
g_return_if_fail (POLKIT_IS_UNIX_PROCESS (process));
- g_return_if_fail (uid >= -1);
process->uid = uid;
}
diff --git a/src/polkit/polkitunixuser.c b/src/polkit/polkitunixuser.c
index 8bfd3a1..234a697 100644
--- a/src/polkit/polkitunixuser.c
+++ b/src/polkit/polkitunixuser.c
@@ -72,6 +72,7 @@ G_DEFINE_TYPE_WITH_CODE (PolkitUnixUser, polkit_unix_user, G_TYPE_OBJECT,
static void
polkit_unix_user_init (PolkitUnixUser *unix_user)
{
+ unix_user->uid = -1; /* (uid_t) -1 is not a valid UID under Linux */
unix_user->name = NULL;
}
@@ -112,11 +113,14 @@ polkit_unix_user_set_property (GObject *object,
GParamSpec *pspec)
{
PolkitUnixUser *unix_user = POLKIT_UNIX_USER (object);
+ gint val;
switch (prop_id)
{
case PROP_UID:
- unix_user->uid = g_value_get_int (value);
+ val = g_value_get_int (value);
+ g_return_if_fail (val != -1);
+ unix_user->uid = val;
break;
default:
@@ -144,9 +148,9 @@ polkit_unix_user_class_init (PolkitUnixUserClass *klass)
g_param_spec_int ("uid",
"User ID",
"The UNIX user ID",
- 0,
+ G_MININT,
G_MAXINT,
- 0,
+ -1,
G_PARAM_CONSTRUCT |
G_PARAM_READWRITE |
G_PARAM_STATIC_NAME |
@@ -182,6 +186,7 @@ polkit_unix_user_set_uid (PolkitUnixUser *user,
gint uid)
{
g_return_if_fail (POLKIT_IS_UNIX_USER (user));
+ g_return_if_fail (uid != -1);
user->uid = uid;
}
@@ -196,6 +201,8 @@ polkit_unix_user_set_uid (PolkitUnixUser *user,
PolkitIdentity *
polkit_unix_user_new (gint uid)
{
+ g_return_val_if_fail (uid != -1, NULL);
+
return POLKIT_IDENTITY (g_object_new (POLKIT_TYPE_UNIX_USER,
"uid", uid,
NULL));
More information about the hal-commit
mailing list