Merge "init: finer grained permissions for ctl. properties" into pi-dev

2 files changed, 39 insertions, 16 deletions

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.<action> 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.<service> 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.<action> 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.<service> 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<property_audit_data*>(data);
+    auto* d = reinterpret_cast<PropertyAuditData*>(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,