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
diff --git a/init/action.cpp b/init/action.cpp
index ab51eea..16ecdcd 100644
--- a/init/action.cpp
+++ b/init/action.cpp
@@ -50,12 +50,19 @@
: func_(std::move(f)), execute_in_subcontext_(execute_in_subcontext), args_(args), line_(line) {}
Result<Success> 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 068be6e..be754da 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 @@
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 @@
};
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<std::string>();
for (const auto& string : execute_command.args()) {
@@ -108,11 +109,27 @@
}
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 @@
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 @@
Fork();
}
-Result<Success> Subcontext::Execute(const std::vector<std::string>& args) {
- auto subcontext_command = SubcontextCommand();
- std::copy(
- args.begin(), args.end(),
- RepeatedPtrFieldBackInserter(subcontext_command.mutable_execute_command()->mutable_args()));
-
+Result<SubcontextReply> 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 @@
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<Success> Subcontext::Execute(const std::vector<std::string>& 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<std::vector<std::string>> Subcontext::ExpandArgs(const std::vector<std::string>& 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<std::string>{};
+ for (const auto& string : reply.expanded_args()) {
+ expanded_args.emplace_back(string);
+ }
+ return expanded_args;
}
static std::vector<Subcontext> subcontexts;
diff --git a/init/subcontext.h b/init/subcontext.h
index eadabee..262440d 100644
--- a/init/subcontext.h
+++ b/init/subcontext.h
@@ -25,6 +25,7 @@
#include <android-base/unique_fd.h>
#include "builtins.h"
+#include "system/core/init/subcontext.pb.h"
namespace android {
namespace init {
@@ -39,7 +40,8 @@
Fork();
}
- Result<Success> Execute(const std::vector<std::string>& command);
+ Result<Success> Execute(const std::vector<std::string>& args);
+ Result<std::vector<std::string>> ExpandArgs(const std::vector<std::string>& args);
void Restart();
const std::string& path_prefix() const { return path_prefix_; }
@@ -48,6 +50,7 @@
private:
void Fork();
+ Result<SubcontextReply> TransmitMessage(const SubcontextCommand& subcontext_command);
std::string path_prefix_;
std::string context_;
diff --git a/init/subcontext.proto b/init/subcontext.proto
index 0d89734..e68115e 100644
--- a/init/subcontext.proto
+++ b/init/subcontext.proto
@@ -19,15 +19,23 @@
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 ca45266..230203a 100644
--- a/init/subcontext_test.cpp
+++ b/init/subcontext_test.cpp
@@ -143,6 +143,34 @@
});
}
+TEST(subcontext, ExpandArgs) {
+ RunTest([](auto& subcontext, auto& context_string) {
+ auto args = std::vector<std::string>{
+ "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<std::string>{
+ "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