From 32228485ffac6ff0b674210be448b121bbd6427c Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Thu, 18 Jan 2018 16:14:25 -0800 Subject: [PATCH] Make vendor_init check SELinux before setting properties Finishing a TODO from vendor_init, check SELinux permissions before setting properties in vendor_init. Bug: 62875318 Test: N/A Change-Id: I3cb6abadd2613ae083705cc6b9c970587b6c6b19 --- init/builtins.cpp | 4 +- init/property_service.cpp | 379 +++++++++++++++++++------------------- init/property_service.h | 8 +- init/subcontext.cpp | 41 ++++- init/subcontext.proto | 6 + 5 files changed, 243 insertions(+), 195 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index f58402166..413d11eb0 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -1039,9 +1039,7 @@ const BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const { {"restorecon_recursive", {1, kMax, {true, do_restorecon_recursive}}}, {"rm", {1, 1, {true, do_rm}}}, {"rmdir", {1, 1, {true, do_rmdir}}}, - // TODO: setprop should be run in the subcontext, but property service needs to be split - // out from init before that is possible. - {"setprop", {2, 2, {false, do_setprop}}}, + {"setprop", {2, 2, {true, do_setprop}}}, {"setrlimit", {3, 3, {false, do_setrlimit}}}, {"start", {1, 1, {false, do_start}}}, {"stop", {1, 1, {false, do_stop}}}, diff --git a/init/property_service.cpp b/init/property_service.cpp index 79f7f25b7..0cb9e63b4 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -84,6 +84,10 @@ static int property_set_fd = -1; static PropertyInfoAreaFile property_info_area; +uint32_t InitPropertySet(const std::string& name, const std::string& value); + +uint32_t (*property_set)(const std::string& name, const std::string& value) = InitPropertySet; + void CreateSerializedPropertyInfo(); void property_init() { @@ -97,7 +101,7 @@ void property_init() { } } static bool CheckMacPerms(const std::string& name, const char* target_context, - const char* source_context, struct ucred* cr) { + const char* source_context, const ucred& cr) { if (!target_context || !source_context) { return false; } @@ -105,7 +109,7 @@ static bool CheckMacPerms(const std::string& name, const char* target_context, property_audit_data audit_data; audit_data.name = name.c_str(); - audit_data.cr = cr; + audit_data.cr = &cr; bool has_access = (selinux_check_access(source_context, target_context, "property_service", "set", &audit_data) == 0); @@ -257,7 +261,7 @@ static int RestoreconRecursiveAsync(const std::string& name, const std::string& return selinux_android_restorecon(value.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE); } -uint32_t property_set(const std::string& name, const std::string& value) { +uint32_t PropertySet(const std::string& name, const std::string& value) { if (name == "selinux.restorecon_recursive") { return PropertySetAsync(name, value, RestoreconRecursiveAsync); } @@ -265,206 +269,208 @@ uint32_t property_set(const std::string& name, const std::string& value) { return PropertySetImpl(name, value); } +uint32_t InitPropertySet(const std::string& name, const std::string& value) { + if (StartsWith(name, "ctl.")) { + LOG(ERROR) << "Do not set ctl. properties from init; call the Service functions directly"; + return PROP_ERROR_INVALID_NAME; + } + + const char* type = nullptr; + property_info_area->GetPropertyInfo(name.c_str(), nullptr, &type); + + if (type == nullptr || !CheckType(type, value)) { + LOG(ERROR) << "property_set: name: '" << name << "' type check failed, type: '" + << (type ?: "(null)") << "' value: '" << value << "'"; + return PROP_ERROR_INVALID_VALUE; + } + + return PropertySet(name, value); +} + class SocketConnection { - public: - SocketConnection(int socket, const struct ucred& cred) - : socket_(socket), cred_(cred) {} + public: + SocketConnection(int socket, const ucred& cred) : socket_(socket), cred_(cred) {} - ~SocketConnection() { - close(socket_); - } + ~SocketConnection() { close(socket_); } - bool RecvUint32(uint32_t* value, uint32_t* timeout_ms) { - return RecvFully(value, sizeof(*value), timeout_ms); - } - - bool RecvChars(char* chars, size_t size, uint32_t* timeout_ms) { - return RecvFully(chars, size, timeout_ms); - } - - bool RecvString(std::string* value, uint32_t* timeout_ms) { - uint32_t len = 0; - if (!RecvUint32(&len, timeout_ms)) { - return false; + bool RecvUint32(uint32_t* value, uint32_t* timeout_ms) { + return RecvFully(value, sizeof(*value), timeout_ms); } - if (len == 0) { - *value = ""; - return true; + bool RecvChars(char* chars, size_t size, uint32_t* timeout_ms) { + return RecvFully(chars, size, timeout_ms); } - // http://b/35166374: don't allow init to make arbitrarily large allocations. - if (len > 0xffff) { - LOG(ERROR) << "sys_prop: RecvString asked to read huge string: " << len; - errno = ENOMEM; - return false; - } - - std::vector chars(len); - if (!RecvChars(&chars[0], len, timeout_ms)) { - return false; - } - - *value = std::string(&chars[0], len); - return true; - } - - bool SendUint32(uint32_t value) { - int result = TEMP_FAILURE_RETRY(send(socket_, &value, sizeof(value), 0)); - return result == sizeof(value); - } - - int socket() { - return socket_; - } - - const struct ucred& cred() { - return cred_; - } - - private: - bool PollIn(uint32_t* timeout_ms) { - struct pollfd ufds[1]; - ufds[0].fd = socket_; - ufds[0].events = POLLIN; - ufds[0].revents = 0; - while (*timeout_ms > 0) { - auto start_time = std::chrono::steady_clock::now(); - int nr = poll(ufds, 1, *timeout_ms); - auto now = std::chrono::steady_clock::now(); - auto time_elapsed = std::chrono::duration_cast(now - start_time); - uint64_t millis = time_elapsed.count(); - *timeout_ms = (millis > *timeout_ms) ? 0 : *timeout_ms - millis; - - if (nr > 0) { - return true; - } - - if (nr == 0) { - // Timeout - break; - } - - if (nr < 0 && errno != EINTR) { - PLOG(ERROR) << "sys_prop: error waiting for uid " << cred_.uid << " to send property message"; - return false; - } else { // errno == EINTR - // Timer rounds milliseconds down in case of EINTR we want it to be rounded up - // to avoid slowing init down by causing EINTR with under millisecond timeout. - if (*timeout_ms > 0) { - --(*timeout_ms); + bool RecvString(std::string* value, uint32_t* timeout_ms) { + uint32_t len = 0; + if (!RecvUint32(&len, timeout_ms)) { + return false; } - } + + if (len == 0) { + *value = ""; + return true; + } + + // http://b/35166374: don't allow init to make arbitrarily large allocations. + if (len > 0xffff) { + LOG(ERROR) << "sys_prop: RecvString asked to read huge string: " << len; + errno = ENOMEM; + return false; + } + + std::vector chars(len); + if (!RecvChars(&chars[0], len, timeout_ms)) { + return false; + } + + *value = std::string(&chars[0], len); + return true; } - LOG(ERROR) << "sys_prop: timeout waiting for uid " << cred_.uid << " to send property message."; - return false; - } - - bool RecvFully(void* data_ptr, size_t size, uint32_t* timeout_ms) { - size_t bytes_left = size; - char* data = static_cast(data_ptr); - while (*timeout_ms > 0 && bytes_left > 0) { - if (!PollIn(timeout_ms)) { - return false; - } - - int result = TEMP_FAILURE_RETRY(recv(socket_, data, bytes_left, MSG_DONTWAIT)); - if (result <= 0) { - return false; - } - - bytes_left -= result; - data += result; + bool SendUint32(uint32_t value) { + int result = TEMP_FAILURE_RETRY(send(socket_, &value, sizeof(value), 0)); + return result == sizeof(value); } - return bytes_left == 0; - } + int socket() { return socket_; } - int socket_; - struct ucred cred_; + const ucred& cred() { return cred_; } - DISALLOW_IMPLICIT_CONSTRUCTORS(SocketConnection); + std::string source_context() const { + char* source_context = nullptr; + getpeercon(socket_, &source_context); + std::string result = source_context; + freecon(source_context); + return result; + } + + private: + bool PollIn(uint32_t* timeout_ms) { + struct pollfd ufds[1]; + ufds[0].fd = socket_; + ufds[0].events = POLLIN; + ufds[0].revents = 0; + while (*timeout_ms > 0) { + auto start_time = std::chrono::steady_clock::now(); + int nr = poll(ufds, 1, *timeout_ms); + auto now = std::chrono::steady_clock::now(); + auto time_elapsed = + std::chrono::duration_cast(now - start_time); + uint64_t millis = time_elapsed.count(); + *timeout_ms = (millis > *timeout_ms) ? 0 : *timeout_ms - millis; + + if (nr > 0) { + return true; + } + + if (nr == 0) { + // Timeout + break; + } + + if (nr < 0 && errno != EINTR) { + PLOG(ERROR) << "sys_prop: error waiting for uid " << cred_.uid + << " to send property message"; + return false; + } else { // errno == EINTR + // Timer rounds milliseconds down in case of EINTR we want it to be rounded up + // to avoid slowing init down by causing EINTR with under millisecond timeout. + if (*timeout_ms > 0) { + --(*timeout_ms); + } + } + } + + LOG(ERROR) << "sys_prop: timeout waiting for uid " << cred_.uid + << " to send property message."; + return false; + } + + bool RecvFully(void* data_ptr, size_t size, uint32_t* timeout_ms) { + size_t bytes_left = size; + char* data = static_cast(data_ptr); + while (*timeout_ms > 0 && bytes_left > 0) { + if (!PollIn(timeout_ms)) { + return false; + } + + int result = TEMP_FAILURE_RETRY(recv(socket_, data, bytes_left, MSG_DONTWAIT)); + if (result <= 0) { + return false; + } + + bytes_left -= result; + data += result; + } + + return bytes_left == 0; + } + + int socket_; + ucred cred_; + + DISALLOW_IMPLICIT_CONSTRUCTORS(SocketConnection); }; -static void handle_property_set(SocketConnection& socket, - const std::string& name, - const std::string& value, - bool legacy_protocol) { - const char* cmd_name = legacy_protocol ? "PROP_MSG_SETPROP" : "PROP_MSG_SETPROP2"; - if (!is_legal_property_name(name)) { - LOG(ERROR) << "sys_prop(" << cmd_name << "): illegal property name \"" << name << "\""; - socket.SendUint32(PROP_ERROR_INVALID_NAME); - return; - } +// 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) { + if (!is_legal_property_name(name)) { + LOG(ERROR) << "PropertySet: illegal property name \"" << name << "\""; + return PROP_ERROR_INVALID_NAME; + } - struct ucred cr = socket.cred(); - char* source_ctx = nullptr; - getpeercon(socket.socket(), &source_ctx); - std::string source_context = source_ctx; - freecon(source_ctx); + 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)) { + LOG(ERROR) << "PropertySet: Unable to " << (name.c_str() + 4) << " service ctl [" + << value << "]" + << " uid:" << cr.uid << " gid:" << cr.gid << " pid:" << cr.pid; + return PROP_ERROR_HANDLE_CONTROL_MESSAGE; + } - 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)) { - LOG(ERROR) << "sys_prop(" << cmd_name << "): Unable to " << (name.c_str() + 4) - << " service ctl [" << value << "]" - << " uid:" << cr.uid << " gid:" << cr.gid << " pid:" << cr.pid; - if (!legacy_protocol) { - socket.SendUint32(PROP_ERROR_HANDLE_CONTROL_MESSAGE); - } - return; - } + handle_control_message(name.c_str() + 4, value.c_str()); + return PROP_SUCCESS; + } - handle_control_message(name.c_str() + 4, value.c_str()); - if (!legacy_protocol) { - socket.SendUint32(PROP_SUCCESS); - } - } else { - const char* target_context = nullptr; - const char* type = nullptr; - property_info_area->GetPropertyInfo(name.c_str(), &target_context, &type); - if (!CheckMacPerms(name, target_context, source_context.c_str(), &cr)) { - LOG(ERROR) << "sys_prop(" << cmd_name << "): permission denied uid:" << cr.uid - << " name:" << name; - if (!legacy_protocol) { - socket.SendUint32(PROP_ERROR_PERMISSION_DENIED); - } - return; - } - if (type == nullptr || !CheckType(type, value)) { - LOG(ERROR) << "sys_prop(" << cmd_name << "): type check failed, type: '" - << (type ?: "(null)") << "' value: '" << value << "'"; - if (!legacy_protocol) { - socket.SendUint32(PROP_ERROR_INVALID_VALUE); - } - return; - } - // sys.powerctl is a special property that is used to make the device reboot. We want to log - // any process that sets this property to be able to accurately blame the cause of a shutdown. - if (name == "sys.powerctl") { - std::string cmdline_path = StringPrintf("proc/%d/cmdline", cr.pid); - std::string process_cmdline; - std::string process_log_string; - if (ReadFileToString(cmdline_path, &process_cmdline)) { - // Since cmdline is null deliminated, .c_str() conveniently gives us just the process path. - process_log_string = StringPrintf(" (%s)", process_cmdline.c_str()); - } - LOG(INFO) << "Received sys.powerctl='" << value << "' from pid: " << cr.pid - << process_log_string; - } + const char* target_context = nullptr; + const char* type = nullptr; + property_info_area->GetPropertyInfo(name.c_str(), &target_context, &type); - uint32_t result = property_set(name, value); - if (!legacy_protocol) { - socket.SendUint32(result); - } - } + if (!CheckMacPerms(name, target_context, source_context.c_str(), cr)) { + LOG(ERROR) << "PropertySet: permission denied uid:" << cr.uid << " name:" << name; + return PROP_ERROR_PERMISSION_DENIED; + } + + if (type == nullptr || !CheckType(type, value)) { + LOG(ERROR) << "PropertySet: name: '" << name << "' type check failed, type: '" + << (type ?: "(null)") << "' value: '" << value << "'"; + return PROP_ERROR_INVALID_VALUE; + } + + // sys.powerctl is a special property that is used to make the device reboot. We want to log + // any process that sets this property to be able to accurately blame the cause of a shutdown. + if (name == "sys.powerctl") { + std::string cmdline_path = StringPrintf("proc/%d/cmdline", cr.pid); + std::string process_cmdline; + std::string process_log_string; + if (ReadFileToString(cmdline_path, &process_cmdline)) { + // Since cmdline is null deliminated, .c_str() conveniently gives us just the process + // path. + process_log_string = StringPrintf(" (%s)", process_cmdline.c_str()); + } + LOG(INFO) << "Received sys.powerctl='" << value << "' from pid: " << cr.pid + << process_log_string; + } + + return PropertySet(name, value); } static void handle_property_set_fd() { @@ -475,7 +481,7 @@ static void handle_property_set_fd() { return; } - struct ucred cr; + ucred cr; socklen_t cr_size = sizeof(cr); if (getsockopt(s, SOL_SOCKET, SO_PEERCRED, &cr, &cr_size) < 0) { close(s); @@ -507,7 +513,7 @@ static void handle_property_set_fd() { prop_name[PROP_NAME_MAX-1] = 0; prop_value[PROP_VALUE_MAX-1] = 0; - handle_property_set(socket, prop_value, prop_value, true); + HandlePropertySet(prop_value, prop_value, socket.source_context(), socket.cred()); break; } @@ -521,7 +527,8 @@ static void handle_property_set_fd() { return; } - handle_property_set(socket, name, value, false); + auto result = HandlePropertySet(name, value, socket.source_context(), socket.cred()); + socket.SendUint32(result); break; } diff --git a/init/property_service.h b/init/property_service.h index a55e79c1d..8161b40e3 100644 --- a/init/property_service.h +++ b/init/property_service.h @@ -25,10 +25,15 @@ namespace android { namespace init { struct property_audit_data { - ucred *cr; + 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, + const std::string& source_context, const ucred& cr); + extern bool PropertyChildReap(pid_t pid); void property_init(void); @@ -36,7 +41,6 @@ void property_load_boot_defaults(void); void load_persist_props(void); void load_system_props(void); void start_property_service(void); -uint32_t property_set(const std::string& name, const std::string& value); bool is_legal_property_name(const std::string& name); } // namespace init diff --git a/init/subcontext.cpp b/init/subcontext.cpp index be754da73..f3b643ad7 100644 --- a/init/subcontext.cpp +++ b/init/subcontext.cpp @@ -27,9 +27,13 @@ #include #include "action.h" +#include "property_service.h" #include "selinux.h" #include "util.h" +#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ +#include + using android::base::GetExecutablePath; using android::base::Join; using android::base::Socketpair; @@ -75,6 +79,13 @@ Result SendMessage(int socket, const T& message) { return Success(); } +std::vector> properties_to_set; + +uint32_t SubcontextPropertySet(const std::string& name, const std::string& value) { + properties_to_set.emplace_back(name, value); + return PROP_SUCCESS; +} + class SubcontextProcess { public: SubcontextProcess(const KeywordFunctionMap* function_map, std::string context, int init_fd) @@ -108,6 +119,14 @@ void SubcontextProcess::RunCommand(const SubcontextCommand::ExecuteCommand& exec result = RunBuiltinFunction(map_result->second, args, context_); } + for (const auto& [name, value] : properties_to_set) { + auto property = reply->add_properties_to_set(); + property->set_name(name); + property->set_value(value); + } + + properties_to_set.clear(); + if (result) { reply->set_success(true); } else { @@ -186,6 +205,9 @@ int SubcontextMain(int argc, char** argv, const KeywordFunctionMap* function_map auto init_fd = std::atoi(argv[3]); SelabelInitialize(); + + property_set = SubcontextPropertySet; + auto subcontext_process = SubcontextProcess(function_map, context, init_fd); subcontext_process.MainLoop(); return 0; @@ -257,10 +279,6 @@ Result Subcontext::TransmitMessage(const SubcontextCommand& sub Restart(); return Error() << "Unable to parse message from subcontext"; } - if (subcontext_reply.reply_case() == SubcontextReply::kFailure) { - auto& failure = subcontext_reply.failure(); - return ResultError(failure.error_string(), failure.error_errno()); - } return subcontext_reply; } @@ -275,6 +293,16 @@ Result Subcontext::Execute(const std::vector& args) { return subcontext_reply.error(); } + for (const auto& property : subcontext_reply->properties_to_set()) { + ucred cr = {.pid = pid_, .uid = 0, .gid = 0}; + HandlePropertySet(property.name(), property.value(), context_, cr); + } + + if (subcontext_reply->reply_case() == SubcontextReply::kFailure) { + auto& failure = subcontext_reply->failure(); + return ResultError(failure.error_string(), failure.error_errno()); + } + if (subcontext_reply->reply_case() != SubcontextReply::kSuccess) { return Error() << "Unexpected message type from subcontext: " << subcontext_reply->reply_case(); @@ -294,6 +322,11 @@ Result> Subcontext::ExpandArgs(const std::vectorreply_case() == SubcontextReply::kFailure) { + auto& failure = subcontext_reply->failure(); + return ResultError(failure.error_string(), failure.error_errno()); + } + if (subcontext_reply->reply_case() != SubcontextReply::kExpandArgsReply) { return Error() << "Unexpected message type from subcontext: " << subcontext_reply->reply_case(); diff --git a/init/subcontext.proto b/init/subcontext.proto index e68115e0e..c31f4fb68 100644 --- a/init/subcontext.proto +++ b/init/subcontext.proto @@ -38,4 +38,10 @@ message SubcontextReply { Failure failure = 2; ExpandArgsReply expand_args_reply = 3; } + + message PropertyToSet { + optional string name = 1; + optional string value = 2; + } + repeated PropertyToSet properties_to_set = 4; } \ No newline at end of file