diff --git a/init/property_service.cpp b/init/property_service.cpp index c3100a5f1..4172ba754 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -95,6 +95,11 @@ uint32_t (*property_set)(const std::string& name, const std::string& value) = In void CreateSerializedPropertyInfo(); +struct PropertyAuditData { + const ucred* cr; + const char* name; +}; + void property_init() { mkdir("/dev/__properties__", S_IRWXU | S_IXGRP | S_IXOTH); CreateSerializedPropertyInfo(); @@ -111,7 +116,7 @@ static bool CheckMacPerms(const std::string& name, const char* target_context, return false; } - property_audit_data audit_data; + PropertyAuditData audit_data; audit_data.name = name.c_str(); audit_data.cr = &cr; @@ -388,6 +393,35 @@ class SocketConnection { DISALLOW_IMPLICIT_CONSTRUCTORS(SocketConnection); }; +bool CheckControlPropertyPerms(const std::string& name, const std::string& value, + const std::string& source_context, const ucred& cr) { + // We check the legacy method first but these properties are dontaudit, so we only log an audit + // if the newer method fails as well. We only do this with the legacy ctl. properties. + if (name == "ctl.start" || name == "ctl.stop" || name == "ctl.restart") { + // The legacy permissions model is that ctl. properties have their name ctl. and + // their value is the name of the service to apply that action to. Permissions for these + // actions are based on the service, so we must create a fake name of ctl. to + // check permissions. + auto control_string_legacy = "ctl." + value; + const char* target_context_legacy = nullptr; + const char* type_legacy = nullptr; + property_info_area->GetPropertyInfo(control_string_legacy.c_str(), &target_context_legacy, + &type_legacy); + + if (CheckMacPerms(control_string_legacy, target_context_legacy, source_context.c_str(), cr)) { + return true; + } + } + + auto control_string_full = name + "$" + value; + const char* target_context_full = nullptr; + const char* type_full = nullptr; + property_info_area->GetPropertyInfo(control_string_full.c_str(), &target_context_full, + &type_full); + + return CheckMacPerms(control_string_full, target_context_full, source_context.c_str(), cr); +} + // This returns one of the enum of PROP_SUCCESS or PROP_ERROR*. uint32_t HandlePropertySet(const std::string& name, const std::string& value, const std::string& source_context, const ucred& cr, std::string* error) { @@ -397,15 +431,9 @@ uint32_t HandlePropertySet(const std::string& name, const std::string& value, } if (StartsWith(name, "ctl.")) { - // ctl. properties have their name ctl. and their value is the name of the service - // to apply that action to. Permissions for these actions are based on the service, so we - // must create a fake name of ctl. to check permissions. - auto control_string = "ctl." + value; - const char* target_context = nullptr; - const char* type = nullptr; - property_info_area->GetPropertyInfo(control_string.c_str(), &target_context, &type); - if (!CheckMacPerms(control_string, target_context, source_context.c_str(), cr)) { - *error = StringPrintf("Unable to '%s' service %s", name.c_str() + 4, value.c_str()); + if (!CheckControlPropertyPerms(name, value, source_context, cr)) { + *error = StringPrintf("Invalid permissions to perform '%s' on '%s'", name.c_str() + 4, + value.c_str()); return PROP_ERROR_HANDLE_CONTROL_MESSAGE; } @@ -737,7 +765,7 @@ void load_system_props() { } static int SelinuxAuditCallback(void* data, security_class_t /*cls*/, char* buf, size_t len) { - property_audit_data* d = reinterpret_cast(data); + auto* d = reinterpret_cast(data); if (!d || !d->name || !d->cr) { LOG(ERROR) << "AuditCallback invoked with null data arguments!"; diff --git a/init/property_service.h b/init/property_service.h index 29eaaa901..897ac1519 100644 --- a/init/property_service.h +++ b/init/property_service.h @@ -24,11 +24,6 @@ namespace android { namespace init { -struct property_audit_data { - const ucred* cr; - const char* name; -}; - extern uint32_t (*property_set)(const std::string& name, const std::string& value); uint32_t HandlePropertySet(const std::string& name, const std::string& value,