From d03d42b5478f9b65f8f0a7807626c4ec9c65ad8a Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Sun, 19 Jan 2020 17:27:41 -0800 Subject: [PATCH] flexibility for having extra policy files add an interface to accept a list of additional policy files, instead of just a single file. Also add TEST_MAPPING for presubmit. Bug: 147914640 Test: boot with empty, 1, and 2 element lists, atest Change-Id: I97e8e0ec7b68699838595c09a4a8e7c7eef657c3 Merged-In: I97e8e0ec7b68699838595c09a4a8e7c7eef657c3 (cherry picked from commit 11c1a68778c39564ed37e653d7af6881a2d11ea6) --- services/minijail/Android.bp | 1 + services/minijail/TEST_MAPPING | 5 +++ .../av_services_minijail_unittest.cpp | 33 +++++++++++++++- services/minijail/minijail.cpp | 39 ++++++++++++------- services/minijail/minijail.h | 8 +++- 5 files changed, 69 insertions(+), 17 deletions(-) create mode 100644 services/minijail/TEST_MAPPING diff --git a/services/minijail/Android.bp b/services/minijail/Android.bp index 0713a87595..5ea6d1e737 100644 --- a/services/minijail/Android.bp +++ b/services/minijail/Android.bp @@ -39,4 +39,5 @@ cc_test { srcs: [ "av_services_minijail_unittest.cpp", ], + test_suites: ["device-tests"], } diff --git a/services/minijail/TEST_MAPPING b/services/minijail/TEST_MAPPING new file mode 100644 index 0000000000..0d89760edb --- /dev/null +++ b/services/minijail/TEST_MAPPING @@ -0,0 +1,5 @@ +{ + "presubmit": [ + { "name": "libavservices_minijail_unittest" } + ] +} diff --git a/services/minijail/av_services_minijail_unittest.cpp b/services/minijail/av_services_minijail_unittest.cpp index 31313f8ae8..896a764a59 100644 --- a/services/minijail/av_services_minijail_unittest.cpp +++ b/services/minijail/av_services_minijail_unittest.cpp @@ -34,13 +34,32 @@ class WritePolicyTest : public ::testing::Test "mmap: 1\n" "munmap: 1\n"; + const std::string third_policy_ = + "open: 1\n" + "close: 1\n"; + const std::string full_policy_ = base_policy_ + std::string("\n") + additional_policy_; + const std::string triple_policy_ = base_policy_ + + std::string("\n") + additional_policy_ + + std::string("\n") + third_policy_; }; TEST_F(WritePolicyTest, OneFile) { std::string final_string; - android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, std::string())); + // vector with an empty pathname + android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, {std::string()})); + EXPECT_LE(0, fd.get()); + bool success = android::base::ReadFdToString(fd.get(), &final_string); + EXPECT_TRUE(success); + EXPECT_EQ(final_string, base_policy_); +} + +TEST_F(WritePolicyTest, OneFileAlternate) +{ + std::string final_string; + // empty vector + android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, {})); EXPECT_LE(0, fd.get()); bool success = android::base::ReadFdToString(fd.get(), &final_string); EXPECT_TRUE(success); @@ -50,9 +69,19 @@ TEST_F(WritePolicyTest, OneFile) TEST_F(WritePolicyTest, TwoFiles) { std::string final_string; - android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, additional_policy_)); + android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, {additional_policy_})); EXPECT_LE(0, fd.get()); bool success = android::base::ReadFdToString(fd.get(), &final_string); EXPECT_TRUE(success); EXPECT_EQ(final_string, full_policy_); } + +TEST_F(WritePolicyTest, ThreeFiles) +{ + std::string final_string; + android::base::unique_fd fd(android::WritePolicyToPipe(base_policy_, {additional_policy_, third_policy_})); + EXPECT_LE(0, fd.get()); + bool success = android::base::ReadFdToString(fd.get(), &final_string); + EXPECT_TRUE(success); + EXPECT_EQ(final_string, triple_policy_); +} diff --git a/services/minijail/minijail.cpp b/services/minijail/minijail.cpp index f213287955..f40f0c5145 100644 --- a/services/minijail/minijail.cpp +++ b/services/minijail/minijail.cpp @@ -29,7 +29,7 @@ namespace android { int WritePolicyToPipe(const std::string& base_policy_content, - const std::string& additional_policy_content) + const std::vector& additional_policy_contents) { int pipefd[2]; if (pipe(pipefd) == -1) { @@ -40,9 +40,11 @@ int WritePolicyToPipe(const std::string& base_policy_content, base::unique_fd write_end(pipefd[1]); std::string content = base_policy_content; - if (additional_policy_content.length() > 0) { - content += "\n"; - content += additional_policy_content; + for (auto one_content : additional_policy_contents) { + if (one_content.length() > 0) { + content += "\n"; + content += one_content; + } } if (!base::WriteStringToFd(content, write_end.get())) { @@ -53,29 +55,40 @@ int WritePolicyToPipe(const std::string& base_policy_content, return pipefd[0]; } -void SetUpMinijail(const std::string& base_policy_path, const std::string& additional_policy_path) +void SetUpMinijail(const std::string& base_policy_path, + const std::string& additional_policy_path) +{ + SetUpMinijailList(base_policy_path, {additional_policy_path}); +} + +void SetUpMinijailList(const std::string& base_policy_path, + const std::vector& additional_policy_paths) { // No seccomp policy defined for this architecture. if (access(base_policy_path.c_str(), R_OK) == -1) { - LOG(WARNING) << "No seccomp policy defined for this architecture."; + // LOG(WARNING) << "No seccomp policy defined for this architecture."; + LOG(WARNING) << "missing base seccomp_policy file '" << base_policy_path << "'"; return; } std::string base_policy_content; - std::string additional_policy_content; + std::vector additional_policy_contents; if (!base::ReadFileToString(base_policy_path, &base_policy_content, false /* follow_symlinks */)) { LOG(FATAL) << "Could not read base policy file '" << base_policy_path << "'"; } - if (additional_policy_path.length() > 0 && - !base::ReadFileToString(additional_policy_path, &additional_policy_content, - false /* follow_symlinks */)) { - LOG(WARNING) << "Could not read additional policy file '" << additional_policy_path << "'"; - additional_policy_content = std::string(); + for (auto one_policy_path : additional_policy_paths) { + std::string one_policy_content; + if (one_policy_path.length() > 0 && + !base::ReadFileToString(one_policy_path, &one_policy_content, + false /* follow_symlinks */)) { + LOG(WARNING) << "Could not read additional policy file '" << one_policy_path << "'"; + } + additional_policy_contents.push_back(one_policy_content); } - base::unique_fd policy_fd(WritePolicyToPipe(base_policy_content, additional_policy_content)); + base::unique_fd policy_fd(WritePolicyToPipe(base_policy_content, additional_policy_contents)); if (policy_fd.get() == -1) { LOG(FATAL) << "Could not write seccomp policy to fd"; } diff --git a/services/minijail/minijail.h b/services/minijail/minijail.h index c8a2149a4f..298af86b01 100644 --- a/services/minijail/minijail.h +++ b/services/minijail/minijail.h @@ -16,11 +16,15 @@ #define AV_SERVICES_MINIJAIL_MINIJAIL #include +#include namespace android { int WritePolicyToPipe(const std::string& base_policy_content, - const std::string& additional_policy_content); -void SetUpMinijail(const std::string& base_policy_path, const std::string& additional_policy_path); + const std::vector& additional_policy_contents); +void SetUpMinijail(const std::string& base_policy_path, + const std::string& additional_policy_path); +void SetUpMinijailList(const std::string& base_policy_path, + const std::vector& additional_policy_paths); } #endif // AV_SERVICES_MINIJAIL_MINIJAIL