From c49719fc5d2cf3817f6997ce40fc2dac7d411efa Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 10 Jan 2018 11:04:34 -0800 Subject: [PATCH] init: always expand args in subcontext Currently init expands properties in arguments only when those commands are run in a subcontext. This creates a hole where properties that should not be accessible from a given subcontext of init can be accessed when running a command in the main init executable (for example `start`). This change creates a callback in subcontext init that simply expands and returns arguments back to the main init process, to ensure that only those properties that a subcontext can access get expanded. Bug: 62875318 Test: boot bullhead, new unit tests Change-Id: I2850009e70da877c08e4cc83350c727b0ea98796 --- init/action.cpp | 17 +++++-- init/subcontext.cpp | 106 ++++++++++++++++++++++++++++----------- init/subcontext.h | 5 +- init/subcontext.proto | 20 +++++--- init/subcontext_test.cpp | 28 +++++++++++ 5 files changed, 136 insertions(+), 40 deletions(-) diff --git a/init/action.cpp b/init/action.cpp index ab51eeae7..16ecdcda4 100644 --- a/init/action.cpp +++ b/init/action.cpp @@ -50,12 +50,19 @@ Command::Command(BuiltinFunction f, bool execute_in_subcontext, : func_(std::move(f)), execute_in_subcontext_(execute_in_subcontext), args_(args), line_(line) {} Result Command::InvokeFunc(Subcontext* subcontext) const { - if (execute_in_subcontext_ && subcontext) { - return subcontext->Execute(args_); - } else { - const std::string& context = subcontext ? subcontext->context() : kInitContext; - return RunBuiltinFunction(func_, args_, context); + if (subcontext) { + if (execute_in_subcontext_) { + return subcontext->Execute(args_); + } + + auto expanded_args = subcontext->ExpandArgs(args_); + if (!expanded_args) { + return expanded_args.error(); + } + return RunBuiltinFunction(func_, *expanded_args, subcontext->context()); } + + return RunBuiltinFunction(func_, args_, kInitContext); } std::string Command::BuildCommandString() const { diff --git a/init/subcontext.cpp b/init/subcontext.cpp index 068be6e6f..be754da73 100644 --- a/init/subcontext.cpp +++ b/init/subcontext.cpp @@ -28,7 +28,6 @@ #include "action.h" #include "selinux.h" -#include "system/core/init/subcontext.pb.h" #include "util.h" using android::base::GetExecutablePath; @@ -84,7 +83,9 @@ class SubcontextProcess { private: void RunCommand(const SubcontextCommand::ExecuteCommand& execute_command, - SubcontextReply::ResultMessage* result_message) const; + SubcontextReply* reply) const; + void ExpandArgs(const SubcontextCommand::ExpandArgsCommand& expand_args_command, + SubcontextReply* reply) const; const KeywordFunctionMap* function_map_; const std::string context_; @@ -92,7 +93,7 @@ class SubcontextProcess { }; void SubcontextProcess::RunCommand(const SubcontextCommand::ExecuteCommand& execute_command, - SubcontextReply::ResultMessage* result_message) const { + SubcontextReply* reply) const { // Need to use ArraySplice instead of this code. auto args = std::vector(); for (const auto& string : execute_command.args()) { @@ -108,11 +109,27 @@ void SubcontextProcess::RunCommand(const SubcontextCommand::ExecuteCommand& exec } if (result) { - result_message->set_success(true); + reply->set_success(true); } else { - result_message->set_success(false); - result_message->set_error_string(result.error_string()); - result_message->set_error_errno(result.error_errno()); + auto* failure = reply->mutable_failure(); + failure->set_error_string(result.error_string()); + failure->set_error_errno(result.error_errno()); + } +} + +void SubcontextProcess::ExpandArgs(const SubcontextCommand::ExpandArgsCommand& expand_args_command, + SubcontextReply* reply) const { + for (const auto& arg : expand_args_command.args()) { + auto expanded_prop = std::string{}; + if (!expand_props(arg, &expanded_prop)) { + auto* failure = reply->mutable_failure(); + failure->set_error_string("Failed to expand '" + arg + "'"); + failure->set_error_errno(0); + return; + } else { + auto* expand_args_reply = reply->mutable_expand_args_reply(); + expand_args_reply->add_expanded_args(expanded_prop); + } } } @@ -142,7 +159,11 @@ void SubcontextProcess::MainLoop() { auto reply = SubcontextReply(); switch (subcontext_command.command_case()) { case SubcontextCommand::kExecuteCommand: { - RunCommand(subcontext_command.execute_command(), reply.mutable_result()); + RunCommand(subcontext_command.execute_command(), &reply); + break; + } + case SubcontextCommand::kExpandArgsCommand: { + ExpandArgs(subcontext_command.expand_args_command(), &reply); break; } default: @@ -219,12 +240,7 @@ void Subcontext::Restart() { Fork(); } -Result Subcontext::Execute(const std::vector& args) { - auto subcontext_command = SubcontextCommand(); - std::copy( - args.begin(), args.end(), - RepeatedPtrFieldBackInserter(subcontext_command.mutable_execute_command()->mutable_args())); - +Result Subcontext::TransmitMessage(const SubcontextCommand& subcontext_command) { if (auto result = SendMessage(socket_, subcontext_command); !result) { Restart(); return ErrnoError() << "Failed to send message to subcontext"; @@ -236,25 +252,59 @@ Result Subcontext::Execute(const std::vector& args) { return Error() << "Failed to receive result from subcontext: " << subcontext_message.error(); } - auto subcontext_reply = SubcontextReply(); + auto subcontext_reply = SubcontextReply{}; if (!subcontext_reply.ParseFromString(*subcontext_message)) { Restart(); return Error() << "Unable to parse message from subcontext"; } - - switch (subcontext_reply.reply_case()) { - case SubcontextReply::kResult: { - auto result = subcontext_reply.result(); - if (result.success()) { - return Success(); - } else { - return ResultError(result.error_string(), result.error_errno()); - } - } - default: - return Error() << "Unknown message type from subcontext: " - << subcontext_reply.reply_case(); + if (subcontext_reply.reply_case() == SubcontextReply::kFailure) { + auto& failure = subcontext_reply.failure(); + return ResultError(failure.error_string(), failure.error_errno()); } + return subcontext_reply; +} + +Result Subcontext::Execute(const std::vector& args) { + auto subcontext_command = SubcontextCommand(); + std::copy( + args.begin(), args.end(), + RepeatedPtrFieldBackInserter(subcontext_command.mutable_execute_command()->mutable_args())); + + auto subcontext_reply = TransmitMessage(subcontext_command); + if (!subcontext_reply) { + return subcontext_reply.error(); + } + + if (subcontext_reply->reply_case() != SubcontextReply::kSuccess) { + return Error() << "Unexpected message type from subcontext: " + << subcontext_reply->reply_case(); + } + + return Success(); +} + +Result> Subcontext::ExpandArgs(const std::vector& args) { + auto subcontext_command = SubcontextCommand{}; + std::copy(args.begin(), args.end(), + RepeatedPtrFieldBackInserter( + subcontext_command.mutable_expand_args_command()->mutable_args())); + + auto subcontext_reply = TransmitMessage(subcontext_command); + if (!subcontext_reply) { + return subcontext_reply.error(); + } + + if (subcontext_reply->reply_case() != SubcontextReply::kExpandArgsReply) { + return Error() << "Unexpected message type from subcontext: " + << subcontext_reply->reply_case(); + } + + auto& reply = subcontext_reply->expand_args_reply(); + auto expanded_args = std::vector{}; + for (const auto& string : reply.expanded_args()) { + expanded_args.emplace_back(string); + } + return expanded_args; } static std::vector subcontexts; diff --git a/init/subcontext.h b/init/subcontext.h index eadabee8d..262440df2 100644 --- a/init/subcontext.h +++ b/init/subcontext.h @@ -25,6 +25,7 @@ #include #include "builtins.h" +#include "system/core/init/subcontext.pb.h" namespace android { namespace init { @@ -39,7 +40,8 @@ class Subcontext { Fork(); } - Result Execute(const std::vector& command); + Result Execute(const std::vector& args); + Result> ExpandArgs(const std::vector& args); void Restart(); const std::string& path_prefix() const { return path_prefix_; } @@ -48,6 +50,7 @@ class Subcontext { private: void Fork(); + Result TransmitMessage(const SubcontextCommand& subcontext_command); std::string path_prefix_; std::string context_; diff --git a/init/subcontext.proto b/init/subcontext.proto index 0d8973457..e68115e0e 100644 --- a/init/subcontext.proto +++ b/init/subcontext.proto @@ -19,15 +19,23 @@ option optimize_for = LITE_RUNTIME; message SubcontextCommand { message ExecuteCommand { repeated string args = 1; } - oneof command { ExecuteCommand execute_command = 1; } + message ExpandArgsCommand { repeated string args = 1; } + oneof command { + ExecuteCommand execute_command = 1; + ExpandArgsCommand expand_args_command = 2; + } } message SubcontextReply { - message ResultMessage { - optional bool success = 1; - optional string error_string = 2; - optional int32 error_errno = 3; + message Failure { + optional string error_string = 1; + optional int32 error_errno = 2; } + message ExpandArgsReply { repeated string expanded_args = 1; } - oneof reply { ResultMessage result = 1; } + oneof reply { + bool success = 1; + Failure failure = 2; + ExpandArgsReply expand_args_reply = 3; + } } \ No newline at end of file diff --git a/init/subcontext_test.cpp b/init/subcontext_test.cpp index ca45266cc..230203ae0 100644 --- a/init/subcontext_test.cpp +++ b/init/subcontext_test.cpp @@ -143,6 +143,34 @@ TEST(subcontext, ContextString) { }); } +TEST(subcontext, ExpandArgs) { + RunTest([](auto& subcontext, auto& context_string) { + auto args = std::vector{ + "first", + "${ro.hardware}", + "$$third", + }; + auto result = subcontext.ExpandArgs(args); + ASSERT_TRUE(result) << result.error(); + ASSERT_EQ(3U, result->size()); + EXPECT_EQ(args[0], result->at(0)); + EXPECT_EQ(GetProperty("ro.hardware", ""), result->at(1)); + EXPECT_EQ("$third", result->at(2)); + }); +} + +TEST(subcontext, ExpandArgsFailure) { + RunTest([](auto& subcontext, auto& context_string) { + auto args = std::vector{ + "first", + "${", + }; + auto result = subcontext.ExpandArgs(args); + ASSERT_FALSE(result); + EXPECT_EQ("Failed to expand '" + args[1] + "'", result.error_string()); + }); +} + TestFunctionMap BuildTestFunctionMap() { TestFunctionMap test_function_map; // For CheckDifferentPid