From: Lennart Poettering Date: Fri, 15 Aug 2014 18:08:51 +0000 (+0200) Subject: sd-bus: add API to check if a client has privileges X-Git-Tag: v216~66 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=def9a7aa0182e5ecca3ac61b26f75136a5c4f103 sd-bus: add API to check if a client has privileges This is a generalization of the vtable privilege check we already have, but exported, and hence useful when preparing for a polkit change. This will deal with the complexity that on dbus1 one cannot trust the capability field we retrieve via the bus, since it is read via /proc/$$/stat (and thus might be out-of-date) rather than directly from the message (like on kdbus) or bus connection (as for uid creds on dbus1). Also, port over all code to this new API. --- diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 4acdb72b1..dae73b390 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "util.h" #include "strv.h" @@ -425,7 +426,7 @@ static int method_set_hostname(sd_bus *bus, sd_bus_message *m, void *userdata, s if (streq_ptr(name, c->data[PROP_HOSTNAME])) return sd_bus_reply_method_return(m, NULL); - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, "org.freedesktop.hostname1.set-hostname", interactive, error, method_set_hostname, c); + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_ADMIN, "org.freedesktop.hostname1.set-hostname", interactive, error, method_set_hostname, c); if (r < 0) return r; if (r == 0) @@ -467,7 +468,7 @@ static int method_set_static_hostname(sd_bus *bus, sd_bus_message *m, void *user if (streq_ptr(name, c->data[PROP_STATIC_HOSTNAME])) return sd_bus_reply_method_return(m, NULL); - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, "org.freedesktop.hostname1.set-static-hostname", interactive, error, method_set_static_hostname, c); + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_ADMIN, "org.freedesktop.hostname1.set-static-hostname", interactive, error, method_set_static_hostname, c); if (r < 0) return r; if (r == 0) @@ -532,9 +533,10 @@ static int set_machine_info(Context *c, sd_bus *bus, sd_bus_message *m, int prop * same time as the static one, use the same policy action for * both... */ - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, prop == PROP_PRETTY_HOSTNAME ? - "org.freedesktop.hostname1.set-static-hostname" : - "org.freedesktop.hostname1.set-machine-info", interactive, error, cb, c); + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_ADMIN, + prop == PROP_PRETTY_HOSTNAME ? + "org.freedesktop.hostname1.set-static-hostname" : + "org.freedesktop.hostname1.set-machine-info", interactive, error, cb, c); if (r < 0) return r; if (r == 0) diff --git a/src/libsystemd/sd-bus/bus-convenience.c b/src/libsystemd/sd-bus/bus-convenience.c index c5b9cd4ca..f88836b88 100644 --- a/src/libsystemd/sd-bus/bus-convenience.c +++ b/src/libsystemd/sd-bus/bus-convenience.c @@ -472,3 +472,56 @@ _public_ int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_b return bus_creds_extend_by_pid(c, mask, creds); } + +_public_ int sd_bus_query_sender_privilege(sd_bus_message *call, int capability) { + _cleanup_bus_creds_unref_ sd_bus_creds *creds = NULL; + uid_t our_uid; + int r; + + assert_return(call, -EINVAL); + assert_return(call->sealed, -EPERM); + assert_return(call->bus, -EINVAL); + assert_return(!bus_pid_changed(call->bus), -ECHILD); + + if (!BUS_IS_OPEN(call->bus->state)) + return -ENOTCONN; + + /* We only trust the effective capability set if this is + * kdbus. On classic dbus1 we cannot retrieve the value + * without races. Since this function is supposed to be useful + * for authentication decision we hence avoid requesting and + * using that information. */ + if (call->bus->is_kernel && capability >= 0) { + r = sd_bus_query_sender_creds(call, SD_BUS_CREDS_UID|SD_BUS_CREDS_EFFECTIVE_CAPS, &creds); + if (r < 0) + return r; + + r = sd_bus_creds_has_effective_cap(creds, capability); + if (r > 0) + return 1; + } else { + r = sd_bus_query_sender_creds(call, SD_BUS_CREDS_UID, &creds); + if (r < 0) + return r; + } + + /* Now, check the UID, but only if the capability check wasn't + * sufficient */ + our_uid = getuid(); + if (our_uid != 0 || !call->bus->is_kernel || capability < 0) { + uid_t sender_uid; + + r = sd_bus_creds_get_uid(creds, &sender_uid); + if (r >= 0) { + /* Sender has same UID as us, then let's grant access */ + if (sender_uid == our_uid) + return 1; + + /* Sender is root, we are not root. */ + if (our_uid != 0 && sender_uid == 0) + return 1; + } + } + + return 0; +} diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index dbb04e5ec..78dab8048 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -289,7 +289,6 @@ static int node_callbacks_run( static int check_access(sd_bus *bus, sd_bus_message *m, struct vtable_member *c, sd_bus_error *error) { _cleanup_bus_creds_unref_ sd_bus_creds *creds = NULL; uint64_t cap; - uid_t uid; int r; assert(bus); @@ -304,17 +303,6 @@ static int check_access(sd_bus *bus, sd_bus_message *m, struct vtable_member *c, if (c->vtable->flags & SD_BUS_VTABLE_UNPRIVILEGED) return 0; - /* If we are not connected to kdbus we cannot retrieve the - * effective capability set without race. Since we need this - * for a security decision we cannot use racy data, hence - * don't request it. */ - if (bus->is_kernel) - r = sd_bus_query_sender_creds(m, SD_BUS_CREDS_UID|SD_BUS_CREDS_EFFECTIVE_CAPS, &creds); - else - r = sd_bus_query_sender_creds(m, SD_BUS_CREDS_UID, &creds); - if (r < 0) - return r; - /* Check have the caller has the requested capability * set. Note that the flags value contains the capability * number plus one, which we need to subtract here. We do this @@ -328,16 +316,11 @@ static int check_access(sd_bus *bus, sd_bus_message *m, struct vtable_member *c, else cap --; - r = sd_bus_creds_has_effective_cap(creds, cap); + r = sd_bus_query_sender_privilege(m, cap); + if (r < 0) + return r; if (r > 0) - return 1; - - /* Caller has same UID as us, then let's grant access */ - r = sd_bus_creds_get_uid(creds, &uid); - if (r >= 0) { - if (uid == getuid()) - return 1; - } + return 0; return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Access to %s.%s() not permitted.", c->interface, c->member); } diff --git a/src/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c index 32c536813..0ab11c390 100644 --- a/src/libsystemd/sd-bus/bus-util.c +++ b/src/libsystemd/sd-bus/bus-util.c @@ -186,28 +186,22 @@ int bus_name_has_owner(sd_bus *c, const char *name, sd_bus_error *error) { int bus_verify_polkit( sd_bus *bus, sd_bus_message *m, + int capability, const char *action, bool interactive, bool *_challenge, sd_bus_error *e) { - _cleanup_bus_creds_unref_ sd_bus_creds *creds = NULL; - uid_t uid; int r; assert(bus); assert(m); assert(action); - r = sd_bus_query_sender_creds(m, SD_BUS_CREDS_UID, &creds); + r = sd_bus_query_sender_privilege(m, capability); if (r < 0) return r; - - r = sd_bus_creds_get_uid(creds, &uid); - if (r < 0) - return r; - - if (uid == 0) + if (r > 0) return 1; #ifdef ENABLE_POLKIT @@ -325,6 +319,7 @@ int bus_verify_polkit_async( sd_bus *bus, Hashmap **registry, sd_bus_message *m, + int capability, const char *action, bool interactive, sd_bus_error *error, @@ -336,8 +331,6 @@ int bus_verify_polkit_async( AsyncPolkitQuery *q; const char *sender; #endif - _cleanup_bus_creds_unref_ sd_bus_creds *creds = NULL; - uid_t uid; int r; assert(bus); @@ -383,15 +376,10 @@ int bus_verify_polkit_async( } #endif - r = sd_bus_query_sender_creds(m, SD_BUS_CREDS_UID, &creds); + r = sd_bus_query_sender_privilege(m, capability); if (r < 0) return r; - - r = sd_bus_creds_get_uid(creds, &uid); - if (r < 0) - return r; - - if (uid == 0) + if (r > 0) return 1; #ifdef ENABLE_POLKIT diff --git a/src/libsystemd/sd-bus/bus-util.h b/src/libsystemd/sd-bus/bus-util.h index af5055392..f760a1e80 100644 --- a/src/libsystemd/sd-bus/bus-util.h +++ b/src/libsystemd/sd-bus/bus-util.h @@ -62,9 +62,9 @@ int bus_name_has_owner(sd_bus *c, const char *name, sd_bus_error *error); int bus_check_peercred(sd_bus *c); -int bus_verify_polkit(sd_bus *bus, sd_bus_message *m, const char *action, bool interactive, bool *_challenge, sd_bus_error *e); +int bus_verify_polkit(sd_bus *bus, sd_bus_message *m, int capability, const char *action, bool interactive, bool *_challenge, sd_bus_error *e); -int bus_verify_polkit_async(sd_bus *bus, Hashmap **registry, sd_bus_message *m, const char *action, bool interactive, sd_bus_error *error, sd_bus_message_handler_t callback, void *userdata); +int bus_verify_polkit_async(sd_bus *bus, Hashmap **registry, sd_bus_message *m, int capability, const char *action, bool interactive, sd_bus_error *error, sd_bus_message_handler_t callback, void *userdata); void bus_verify_polkit_async_registry_free(sd_bus *bus, Hashmap *registry); int bus_open_system_systemd(sd_bus **_bus); diff --git a/src/locale/localed.c b/src/locale/localed.c index 3cfc2ea4d..087066786 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "sd-bus.h" @@ -876,7 +877,7 @@ static int method_set_locale(sd_bus *bus, sd_bus_message *m, void *userdata, sd_ } if (modified) { - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_ADMIN, "org.freedesktop.locale1.set-locale", interactive, error, method_set_locale, c); if (r < 0) @@ -954,7 +955,7 @@ static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata (keymap_toggle && (!filename_is_safe(keymap_toggle) || !string_is_safe(keymap_toggle)))) return sd_bus_error_set_errnof(error, -EINVAL, "Received invalid keymap data"); - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_ADMIN, "org.freedesktop.locale1.set-keyboard", interactive, error, method_set_vc_keyboard, c); if (r < 0) @@ -1026,7 +1027,7 @@ static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdat (options && !string_is_safe(options))) return sd_bus_error_set_errnof(error, -EINVAL, "Received invalid keyboard data"); - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_ADMIN, "org.freedesktop.locale1.set-keyboard", interactive, error, method_set_x11_keyboard, c); if (r < 0) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 1a363c2c5..bcfcba2d0 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -1032,6 +1032,7 @@ static int method_set_user_linger(sd_bus *bus, sd_bus_message *message, void *us r = bus_verify_polkit_async(bus, &m->polkit_registry, message, + CAP_SYS_ADMIN, "org.freedesktop.login1.set-user-linger", interactive, error, @@ -1204,6 +1205,7 @@ static int method_attach_device(sd_bus *bus, sd_bus_message *message, void *user r = bus_verify_polkit_async(bus, &m->polkit_registry, message, + CAP_SYS_ADMIN, "org.freedesktop.login1.attach-device", interactive, error, @@ -1235,6 +1237,7 @@ static int method_flush_devices(sd_bus *bus, sd_bus_message *message, void *user r = bus_verify_polkit_async(bus, &m->polkit_registry, message, + CAP_SYS_ADMIN, "org.freedesktop.login1.flush-devices", interactive, error, @@ -1532,7 +1535,7 @@ static int method_do_shutdown_or_sleep( blocked = manager_is_inhibited(m, w, INHIBIT_BLOCK, NULL, false, true, uid, NULL); if (multiple_sessions) { - r = bus_verify_polkit_async(m->bus, &m->polkit_registry, message, + r = bus_verify_polkit_async(m->bus, &m->polkit_registry, message, CAP_SYS_BOOT, action_multiple_sessions, interactive, error, method, m); if (r < 0) return r; @@ -1541,7 +1544,7 @@ static int method_do_shutdown_or_sleep( } if (blocked) { - r = bus_verify_polkit_async(m->bus, &m->polkit_registry, message, + r = bus_verify_polkit_async(m->bus, &m->polkit_registry, message, CAP_SYS_BOOT, action_ignore_inhibit, interactive, error, method, m); if (r < 0) return r; @@ -1550,7 +1553,7 @@ static int method_do_shutdown_or_sleep( } if (!multiple_sessions && !blocked) { - r = bus_verify_polkit_async(m->bus, &m->polkit_registry, message, + r = bus_verify_polkit_async(m->bus, &m->polkit_registry, message, CAP_SYS_BOOT, action, interactive, error, method, m); if (r < 0) return r; @@ -1688,7 +1691,7 @@ static int method_can_shutdown_or_sleep( blocked = manager_is_inhibited(m, w, INHIBIT_BLOCK, NULL, false, true, uid, NULL); if (multiple_sessions) { - r = bus_verify_polkit(m->bus, message, action_multiple_sessions, false, &challenge, error); + r = bus_verify_polkit(m->bus, message, CAP_SYS_BOOT, action_multiple_sessions, false, &challenge, error); if (r < 0) return r; @@ -1701,7 +1704,7 @@ static int method_can_shutdown_or_sleep( } if (blocked) { - r = bus_verify_polkit(m->bus, message, action_ignore_inhibit, false, &challenge, error); + r = bus_verify_polkit(m->bus, message, CAP_SYS_BOOT, action_ignore_inhibit, false, &challenge, error); if (r < 0) return r; @@ -1717,7 +1720,7 @@ static int method_can_shutdown_or_sleep( /* If neither inhibit nor multiple sessions * apply then just check the normal policy */ - r = bus_verify_polkit(m->bus, message, action, false, &challenge, error); + r = bus_verify_polkit(m->bus, message, CAP_SYS_BOOT, action, false, &challenge, error); if (r < 0) return r; @@ -1837,7 +1840,7 @@ static int method_inhibit(sd_bus *bus, sd_bus_message *message, void *userdata, if (m->action_what & w) return sd_bus_error_setf(error, BUS_ERROR_OPERATION_IN_PROGRESS, "The operation inhibition has been requested for is already running"); - r = bus_verify_polkit_async(bus, &m->polkit_registry, message, + r = bus_verify_polkit_async(bus, &m->polkit_registry, message, CAP_SYS_BOOT, w == INHIBIT_SHUTDOWN ? (mm == INHIBIT_BLOCK ? "org.freedesktop.login1.inhibit-block-shutdown" : "org.freedesktop.login1.inhibit-delay-shutdown") : w == INHIBIT_SLEEP ? (mm == INHIBIT_BLOCK ? "org.freedesktop.login1.inhibit-block-sleep" : "org.freedesktop.login1.inhibit-delay-sleep") : w == INHIBIT_IDLE ? "org.freedesktop.login1.inhibit-block-idle" : diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 79566d210..3eedb4450 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -282,6 +282,7 @@ int sd_bus_emit_interfaces_removed_strv(sd_bus *bus, const char *path, char **in int sd_bus_emit_interfaces_removed(sd_bus *bus, const char *path, const char *interface, ...) _sd_sentinel_; int sd_bus_query_sender_creds(sd_bus_message *call, uint64_t mask, sd_bus_creds **creds); +int sd_bus_query_sender_privilege(sd_bus_message *call, int capability); /* Credential handling */ diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 791e2b436..f0371759a 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -395,7 +395,7 @@ static int method_set_timezone(sd_bus *bus, sd_bus_message *m, void *userdata, s if (streq_ptr(z, c->zone)) return sd_bus_reply_method_return(m, NULL); - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, "org.freedesktop.timedate1.set-timezone", interactive, error, method_set_timezone, c); + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_TIME, "org.freedesktop.timedate1.set-timezone", interactive, error, method_set_timezone, c); if (r < 0) return r; if (r == 0) @@ -456,7 +456,7 @@ static int method_set_local_rtc(sd_bus *bus, sd_bus_message *m, void *userdata, if (lrtc == c->local_rtc) return sd_bus_reply_method_return(m, NULL); - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, "org.freedesktop.timedate1.set-local-rtc", interactive, error, method_set_local_rtc, c); + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_TIME, "org.freedesktop.timedate1.set-local-rtc", interactive, error, method_set_local_rtc, c); if (r < 0) return r; if (r == 0) @@ -561,7 +561,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bu } else timespec_store(&ts, (usec_t) utc); - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, "org.freedesktop.timedate1.set-time", interactive, error, method_set_time, c); + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_TIME, "org.freedesktop.timedate1.set-time", interactive, error, method_set_time, c); if (r < 0) return r; if (r == 0) @@ -601,7 +601,7 @@ static int method_set_ntp(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bus if ((bool)ntp == c->use_ntp) return sd_bus_reply_method_return(m, NULL); - r = bus_verify_polkit_async(bus, &c->polkit_registry, m, "org.freedesktop.timedate1.set-ntp", interactive, error, method_set_ntp, c); + r = bus_verify_polkit_async(bus, &c->polkit_registry, m, CAP_SYS_TIME, "org.freedesktop.timedate1.set-ntp", interactive, error, method_set_ntp, c); if (r < 0) return r; if (r == 0)