From 1c005f3a78619381bb23ff0c59188ef71d9d81ed Mon Sep 17 00:00:00 2001
From: Tom Cherry <tomcherry@google.com>
Date: Wed, 20 Nov 2019 15:51:36 -0800
Subject: [PATCH] init: fix subcontext tests running as non-root.

A recently added subcontext test was failing beause it was running as
non-root, but GTEST_SKIP() didn't work as I expected it to.

In retrospect, all of these tests except for the property one, can
easily run as root, so this changes allows all of these tests to run
as root, while fixing the original issue.

Bug: 144707143
Test: root and nonroot subcontext unit tests
Change-Id: Ia835597701698f6be2101f92d6f4c9450bd3c7dd
---
 init/subcontext.cpp      |  8 ++++++--
 init/subcontext.h        |  1 +
 init/subcontext_test.cpp | 39 ++++++++++++++++-----------------------
 3 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/init/subcontext.cpp b/init/subcontext.cpp
index e55265b0a..bebcc7733 100644
--- a/init/subcontext.cpp
+++ b/init/subcontext.cpp
@@ -209,8 +209,12 @@ void Subcontext::Fork() {
             PLOG(FATAL) << "Could not dup child_fd";
         }
 
-        if (setexeccon(context_.c_str()) < 0) {
-            PLOG(FATAL) << "Could not set execcon for '" << context_ << "'";
+        // We don't switch contexts if we're running the unit tests.  We don't use std::optional,
+        // since we still need a real context string to pass to the builtin functions.
+        if (context_ != kTestContext) {
+            if (setexeccon(context_.c_str()) < 0) {
+                PLOG(FATAL) << "Could not set execcon for '" << context_ << "'";
+            }
         }
 
         auto init_path = GetExecutablePath();
diff --git a/init/subcontext.h b/init/subcontext.h
index bcaad2980..5e1d8a80b 100644
--- a/init/subcontext.h
+++ b/init/subcontext.h
@@ -32,6 +32,7 @@ namespace init {
 
 static constexpr const char kInitContext[] = "u:r:init:s0";
 static constexpr const char kVendorContext[] = "u:r:vendor_init:s0";
+static constexpr const char kTestContext[] = "test-test-test";
 
 class Subcontext {
   public:
diff --git a/init/subcontext_test.cpp b/init/subcontext_test.cpp
index 9c1a78818..2e5a25688 100644
--- a/init/subcontext_test.cpp
+++ b/init/subcontext_test.cpp
@@ -39,24 +39,12 @@ using android::base::WaitForProperty;
 namespace android {
 namespace init {
 
-// I would use test fixtures, but I cannot skip the test if not root with them, so instead we have
-// this test runner.
 template <typename F>
 void RunTest(F&& test_function) {
-    if (getuid() != 0) {
-        GTEST_SKIP() << "Skipping test, must be run as root.";
-        return;
-    }
-
-    char* context;
-    ASSERT_EQ(0, getcon(&context));
-    auto context_string = std::string(context);
-    free(context);
-
-    auto subcontext = Subcontext({"dummy_path"}, context_string);
+    auto subcontext = Subcontext({"dummy_path"}, kTestContext);
     ASSERT_NE(0, subcontext.pid());
 
-    test_function(subcontext, context_string);
+    test_function(subcontext);
 
     if (subcontext.pid() > 0) {
         kill(subcontext.pid(), SIGTERM);
@@ -65,7 +53,7 @@ void RunTest(F&& test_function) {
 }
 
 TEST(subcontext, CheckDifferentPid) {
-    RunTest([](auto& subcontext, auto& context_string) {
+    RunTest([](auto& subcontext) {
         auto result = subcontext.Execute(std::vector<std::string>{"return_pids_as_error"});
         ASSERT_FALSE(result);
 
@@ -78,7 +66,12 @@ TEST(subcontext, CheckDifferentPid) {
 }
 
 TEST(subcontext, SetProp) {
-    RunTest([](auto& subcontext, auto& context_string) {
+    if (getuid() != 0) {
+        GTEST_SKIP() << "Skipping test, must be run as root.";
+        return;
+    }
+
+    RunTest([](auto& subcontext) {
         SetProperty("init.test.subcontext", "fail");
         WaitForProperty("init.test.subcontext", "fail");
 
@@ -95,7 +88,7 @@ TEST(subcontext, SetProp) {
 }
 
 TEST(subcontext, MultipleCommands) {
-    RunTest([](auto& subcontext, auto& context_string) {
+    RunTest([](auto& subcontext) {
         auto first_pid = subcontext.pid();
 
         auto expected_words = std::vector<std::string>{
@@ -122,7 +115,7 @@ TEST(subcontext, MultipleCommands) {
 }
 
 TEST(subcontext, RecoverAfterAbort) {
-    RunTest([](auto& subcontext, auto& context_string) {
+    RunTest([](auto& subcontext) {
         auto first_pid = subcontext.pid();
 
         auto result = subcontext.Execute(std::vector<std::string>{"cause_log_fatal"});
@@ -136,10 +129,10 @@ TEST(subcontext, RecoverAfterAbort) {
 }
 
 TEST(subcontext, ContextString) {
-    RunTest([](auto& subcontext, auto& context_string) {
+    RunTest([](auto& subcontext) {
         auto result = subcontext.Execute(std::vector<std::string>{"return_context_as_error"});
         ASSERT_FALSE(result);
-        ASSERT_EQ(context_string, result.error().message());
+        ASSERT_EQ(kTestContext, result.error().message());
     });
 }
 
@@ -147,7 +140,7 @@ TEST(subcontext, TriggerShutdown) {
     static constexpr const char kTestShutdownCommand[] = "reboot,test-shutdown-command";
     static std::string trigger_shutdown_command;
     trigger_shutdown = [](const std::string& command) { trigger_shutdown_command = command; };
-    RunTest([](auto& subcontext, auto& context_string) {
+    RunTest([](auto& subcontext) {
         auto result = subcontext.Execute(
                 std::vector<std::string>{"trigger_shutdown", kTestShutdownCommand});
         ASSERT_TRUE(result);
@@ -156,7 +149,7 @@ TEST(subcontext, TriggerShutdown) {
 }
 
 TEST(subcontext, ExpandArgs) {
-    RunTest([](auto& subcontext, auto& context_string) {
+    RunTest([](auto& subcontext) {
         auto args = std::vector<std::string>{
             "first",
             "${ro.hardware}",
@@ -172,7 +165,7 @@ TEST(subcontext, ExpandArgs) {
 }
 
 TEST(subcontext, ExpandArgsFailure) {
-    RunTest([](auto& subcontext, auto& context_string) {
+    RunTest([](auto& subcontext) {
         auto args = std::vector<std::string>{
             "first",
             "${",