Author: Sebastian Reichel Description: add apparmor support Forwarded: https://github.com/bus1/dbus-broker/pull/286 --- a/src/bus/policy.c +++ b/src/bus/policy.c @@ -1064,7 +1064,7 @@ size_t i; int r; - r = bus_apparmor_check_xmit(snapshot->apparmor, true, snapshot->seclabel, subject_seclabel, + r = bus_apparmor_check_send(snapshot->apparmor, snapshot->seclabel, subject_seclabel, subject, subject_id, path, interface, method); if (r) { if (r == BUS_APPARMOR_E_DENIED) @@ -1111,16 +1111,6 @@ size_t n_fds) { PolicyVerdict verdict = POLICY_VERDICT_INIT; size_t i; - int r; - - r = bus_apparmor_check_xmit(snapshot->apparmor, false, subject_seclabel, snapshot->seclabel, - subject, subject_id, path, interface, method); - if (r) { - if (r == BUS_APPARMOR_E_DENIED) - return POLICY_E_APPARMOR_ACCESS_DENIED; - - return error_fold(r); - } for (i = 0; i < snapshot->n_batches; ++i) policy_snapshot_check_xmit(snapshot->batches[i], --- a/src/util/apparmor-fallback.c +++ b/src/util/apparmor-fallback.c @@ -62,7 +62,7 @@ return 0; } -int bus_apparmor_check_xmit(BusAppArmorRegistry *registry, bool check_send, +int bus_apparmor_check_send(BusAppArmorRegistry *registry, const char *sender_context, const char *receiver_context, NameSet *subject, uint64_t subject_id, const char *path, const char *interface, const char *method) { --- a/src/util/apparmor.c +++ b/src/util/apparmor.c @@ -473,26 +473,25 @@ if (r) return error_origin(-c_errno()); + if (string_equal(security_mode, "complain")) + allow = true; + if (audit) bus_apparmor_log( registry, "apparmor=\"%s\" operation=\"dbus_bind\" " - "bus=\"%s\" name=\"%s\"", + "bus=\"%s\" name=\"%s\" mask=\"bind\"", allow ? "ALLOWED" : "DENIED", registry->bustype, name ); - if (string_equal(security_mode, "complain")) - allow = true; - return allow ? 0 : BUS_APPARMOR_E_DENIED; } /** - * bus_apparmor_check_xmit() - check if the given transaction is allowed + * bus_apparmor_check_send() - check if the given transaction is allowed * @registry: AppArmor registry to operate on - * @check_send: true if sending should be checked, false if receiving * @sender_context: security context of the sender * @receiver_context: security context of the receiver, or NULL * @subject: List of names @@ -511,8 +510,7 @@ * Return: 0 if the transaction is allowed, BUS_APPARMOR_E_DENIED if it is not, * or a negative error code on failure. */ -int bus_apparmor_check_xmit(BusAppArmorRegistry *registry, - bool check_send, +int bus_apparmor_check_send(BusAppArmorRegistry *registry, const char *sender_context, const char *receiver_context, NameSet *subject, @@ -524,7 +522,7 @@ _c_cleanup_(c_freep) char *receiver_context_dup = NULL; char *sender_security_label, *sender_security_mode; char *receiver_security_label, *receiver_security_mode; - int r, allow, audit; + int r, src_allow = false, src_audit = true, dst_allow = false, dst_audit = true; if (!registry->bustype) return 0; @@ -539,46 +537,71 @@ sender_security_label = aa_splitcon(sender_context_dup, &sender_security_mode); receiver_security_label = aa_splitcon(receiver_context_dup, &receiver_security_mode); - if (is_unconfined(sender_security_label, sender_security_mode)) - return 0; - if (is_unconfined(receiver_security_label, receiver_security_mode)) - return 0; - - if (check_send) + if (is_unconfined(sender_security_label, sender_security_mode)) { + src_allow = true; + src_audit = false; + } else { r = apparmor_message_query(true, sender_security_label, registry->bustype, receiver_security_label, subject, subject_id, path, - interface, method, &allow, &audit); - else + interface, method, &src_allow, &src_audit); + + if (r) + return error_fold(r); + } + + if (is_unconfined(receiver_security_label, receiver_security_mode)) { + dst_allow = true; + dst_audit = false; + } else { r = apparmor_message_query(false, receiver_security_label, registry->bustype, sender_security_label, subject, subject_id, path, - interface, method, &allow, &audit); - if (r) - return error_fold(r); - - if (audit) - bus_apparmor_log( - registry, - "apparmor=\"%s\" operation=\"dbus_%s\" bus=\"%s\" " - "path=\"%s\" interface=\"%s\" method=\"%s\"", - allow ? "ALLOWED" : "DENIED", - check_send ? "send" : "receive", + interface, method, &dst_allow, &dst_audit); + if (r) + return error_fold(r); + } + + if (string_equal(sender_security_mode, "complain")) + src_allow = 1; + if (string_equal(receiver_security_mode, "complain")) + dst_allow = 1; + + if (src_audit) { + bus_apparmor_log(registry, + "apparmor=\"%s\" operation=\"dbus_method_call\" " + "bus=\"%s\" path=\"%s\" interface=\"%s\" method=\"%s\" " + "mask=\"send\" label=\"%s\" peer_label=\"%s\"", + src_allow ? "ALLOWED" : "DENIED", registry->bustype, path, interface, - method + method, + sender_security_label, + receiver_security_label ); + } - if ((check_send && string_equal(sender_security_mode, "complain")) || - (!check_send && string_equal(receiver_security_mode, "complain"))) - allow = 1; + if (dst_audit) { + bus_apparmor_log(registry, + "apparmor=\"%s\" operation=\"dbus_method_call\" " + "bus=\"%s\" path=\"%s\" interface=\"%s\" method=\"%s\" " + "mask=\"receive\" label=\"%s\" peer_label=\"%s\"", + dst_allow ? "ALLOWED" : "DENIED", + registry->bustype, + path, + interface, + method, + receiver_security_label, + sender_security_label + ); + } - return allow ? 0 : BUS_APPARMOR_E_DENIED; + return (src_allow && dst_allow) ? 0 : BUS_APPARMOR_E_DENIED; } /** @@ -624,6 +647,9 @@ if (r) return error_origin(-c_errno()); + if (string_equal(security_mode, "complain")) + allow = 1; + if (audit) bus_apparmor_log( registry, @@ -634,8 +660,5 @@ context ); - if (string_equal(security_mode, "complain")) - allow = 1; - return allow ? 0 : BUS_APPARMOR_E_DENIED; } --- a/src/util/apparmor.h +++ b/src/util/apparmor.h @@ -29,7 +29,7 @@ int bus_apparmor_check_own(struct BusAppArmorRegistry *registry, const char *context, const char *name); -int bus_apparmor_check_xmit(BusAppArmorRegistry *registry, bool check_send, +int bus_apparmor_check_send(BusAppArmorRegistry *registry, const char *sender_context, const char *receiver_context, NameSet *subject, uint64_t subject_id, const char *path, const char *interface, const char *method); --- a/src/launch/launcher.c +++ b/src/launch/launcher.c @@ -1156,6 +1156,55 @@ return 0; } +/** + * launcher_apparmor_check() - checks if AppArmor should be used + * @apparmor_mode: bi-directional argument telling if AppArmor should be enabled + * + * This should be called with the requested apparmor mode. The + * function modifies the mode to either CONFIG_APPARMOR_DISABLED + * or CONFIG_APPARMOR_ENABLED based on platform support. If the + * function keeps the mode as CONFIG_APPARMOR_REQUIRED, AppArmor + * has been configured as mandatory but is not supported. + * + * Returns: 0 if check succeeded, or negative error code on failure. + */ +int launcher_apparmor_check(unsigned int *apparmor_mode) { + bool enabled, supported; + int r; + + if (*apparmor_mode == CONFIG_APPARMOR_DISABLED) + return CONFIG_APPARMOR_DISABLED; + + r = bus_apparmor_is_enabled(&enabled); + if (r) + return error_fold(r); + + r = bus_apparmor_dbus_supported(&supported); + if (r) + return error_fold(r); + + if (*apparmor_mode == CONFIG_APPARMOR_ENABLED) { + if (enabled && !supported) { + fprintf(stderr, "Kernel is missing AppArmor DBus support.\n"); + *apparmor_mode = CONFIG_APPARMOR_DISABLED; + } else if (enabled) { + *apparmor_mode = CONFIG_APPARMOR_ENABLED; + } else { + *apparmor_mode = CONFIG_APPARMOR_DISABLED; + } + } else if (*apparmor_mode == CONFIG_APPARMOR_REQUIRED) { + if (!enabled || !supported) { + fprintf(stderr, "AppArmor required, but not supported. Exiting.\n"); + *apparmor_mode = CONFIG_APPARMOR_REQUIRED; + } else { + *apparmor_mode = CONFIG_APPARMOR_ENABLED; + } + } + + return 0; +} + + static int launcher_reload_config(Launcher *launcher) { _c_cleanup_(config_root_freep) ConfigRoot *root = NULL; _c_cleanup_(policy_deinit) Policy policy = POLICY_INIT(policy); @@ -1196,29 +1245,13 @@ if (r) goto out; - switch (policy.apparmor_mode) { - case CONFIG_APPARMOR_ENABLED: { - bool enabled; - - /* XXX: See comments in launcher_run() */ - - r = bus_apparmor_is_enabled(&enabled); - if (r) - return error_fold(r); - - if (enabled) - fprintf(stderr, "AppArmor enabled, but not supported. Ignoring.\n"); - - policy.apparmor_mode = CONFIG_APPARMOR_DISABLED; - break; - } - case CONFIG_APPARMOR_REQUIRED: - fprintf(stderr, "AppArmor required, but not supported. Exiting.\n"); - + r = launcher_apparmor_check(&policy.apparmor_mode); + if (r < 0) + return error_fold(r); + if (policy.apparmor_mode == CONFIG_APPARMOR_REQUIRED) { r = sd_event_exit(launcher->event, 0); if (r < 0) return error_fold(r); - return 0; } @@ -1348,35 +1381,11 @@ if (r) return error_trace(r); - switch (policy.apparmor_mode) { - case CONFIG_APPARMOR_ENABLED: { - bool enabled, supported; - - r = bus_apparmor_is_enabled(&enabled); - if (r) - return error_fold(r); - - r = bus_apparmor_dbus_supported(&supported); - if (r) - return error_fold(r); - - if (enabled && !supported) - fprintf(stderr, "Kernel is missing AppArmor DBus support. Ignoring.\n"); - else if (enabled) - fprintf(stderr, "AppArmor enabled, but not supported. Ignoring.\n"); - - /* XXX: once the broker supports AppArmor, set this to DISABLED if and only if - * it is disabled in the kernel. */ - policy.apparmor_mode = CONFIG_APPARMOR_DISABLED; - break; - } - case CONFIG_APPARMOR_REQUIRED: - fprintf(stderr, "AppArmor required, but not supported. Exiting.\n"); - - /* XXX: once the broker supports AppArmor, set this to enabled if and only - * if it is enabled in the kernel, and exit the launcher otherwise. */ + r = launcher_apparmor_check(&policy.apparmor_mode); + if (r < 0) + return error_fold(r); + if (policy.apparmor_mode == CONFIG_APPARMOR_REQUIRED) return 0; - } c_assert(launcher->fd_listen >= 0);