idmap2: lock down write access to /data/resouce-cache

Deny write access to /data/resource-cache for UIDs other than root and
system. While this is already handled by SELinux rules, add an
additional layer of security to explicitly prevent malicious apps from
messing with the system's idmap files.

Test: make idmap2_tests
Change-Id: Id986633558d5d02452276f05f64337a8700f148a

6 files changed, 106 insertions, 3 deletions

diff --git a/cmds/idmap2/idmap2/Create.cpp b/cmds/idmap2/idmap2/Create.cpp
index c455ac0f83af..0c581f3b1a98 100644
--- a/cmds/idmap2/idmap2/Create.cpp
+++ b/cmds/idmap2/idmap2/Create.cpp
@@ -39,6 +39,7 @@ using android::idmap2::PolicyBitmask;
 using android::idmap2::PolicyFlags;
 using android::idmap2::Result;
 using android::idmap2::utils::kIdmapFilePermissionMask;
+using android::idmap2::utils::UidHasWriteAccessToPath;
 
 bool Create(const std::vector<std::string>& args, std::ostream& out_error) {
   std::string target_apk_path;
@@ -66,6 +67,13 @@ bool Create(const std::vector<std::string>& args, std::ostream& out_error) {
     return false;
   }
 
+  const uid_t uid = getuid();
+  if (!UidHasWriteAccessToPath(uid, idmap_path)) {
+    out_error << "error: uid " << uid << " does not have write access to " << idmap_path
+              << std::endl;
+    return false;
+  }
+
   PolicyBitmask fulfilled_policies = 0;
   if (auto result = PoliciesToBitmask(policies, out_error)) {
     fulfilled_policies |= *result;
diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp
index a3c752718ee2..f30ce9b08d6e 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.cpp
+++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp
@@ -27,6 +27,7 @@
 
 #include "android-base/macros.h"
 #include "android-base/stringprintf.h"
+#include "binder/IPCThreadState.h"
 #include "utils/String8.h"
 #include "utils/Trace.h"
 
@@ -38,18 +39,19 @@
 
 #include "idmap2d/Idmap2Service.h"
 
+using android::IPCThreadState;
 using android::binder::Status;
 using android::idmap2::BinaryStreamVisitor;
 using android::idmap2::Idmap;
 using android::idmap2::IdmapHeader;
 using android::idmap2::PolicyBitmask;
 using android::idmap2::Result;
+using android::idmap2::utils::kIdmapCacheDir;
 using android::idmap2::utils::kIdmapFilePermissionMask;
+using android::idmap2::utils::UidHasWriteAccessToPath;
 
 namespace {
 
-constexpr const char* kIdmapCacheDir = "/data/resource-cache";
-
 Status ok() {
   return Status::ok();
 }
@@ -77,7 +79,13 @@ Status Idmap2Service::getIdmapPath(const std::string& overlay_apk_path,
 Status Idmap2Service::removeIdmap(const std::string& overlay_apk_path,
                                   int32_t user_id ATTRIBUTE_UNUSED, bool* _aidl_return) {
   assert(_aidl_return);
+  const uid_t uid = IPCThreadState::self()->getCallingUid();
   const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
+  if (!UidHasWriteAccessToPath(uid, idmap_path)) {
+    *_aidl_return = false;
+    return error(base::StringPrintf("failed to unlink %s: calling uid %d lacks write access",
+                                    idmap_path.c_str(), uid));
+  }
   if (unlink(idmap_path.c_str()) != 0) {
     *_aidl_return = false;
     return error("failed to unlink " + idmap_path + ": " + strerror(errno));
@@ -118,6 +126,13 @@ Status Idmap2Service::createIdmap(const std::string& target_apk_path,
 
   const PolicyBitmask policy_bitmask = ConvertAidlArgToPolicyBitmask(fulfilled_policies);
 
+  const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
+  const uid_t uid = IPCThreadState::self()->getCallingUid();
+  if (!UidHasWriteAccessToPath(uid, idmap_path)) {
+    return error(base::StringPrintf("will not write to %s: calling uid %d lacks write accesss",
+                                    idmap_path.c_str(), uid));
+  }
+
   const std::unique_ptr<const ApkAssets> target_apk = ApkAssets::Load(target_apk_path);
   if (!target_apk) {
     return error("failed to load apk " + target_apk_path);
@@ -137,7 +152,6 @@ Status Idmap2Service::createIdmap(const std::string& target_apk_path,
   }
 
   umask(kIdmapFilePermissionMask);
-  const std::string idmap_path = Idmap::CanonicalIdmapPathFor(kIdmapCacheDir, overlay_apk_path);
   std::ofstream fout(idmap_path);
   if (fout.fail()) {
     return error("failed to open idmap path " + idmap_path);
diff --git a/cmds/idmap2/include/idmap2/FileUtils.h b/cmds/idmap2/include/idmap2/FileUtils.h
index 5c41c49906cd..3f03236d5e1a 100644
--- a/cmds/idmap2/include/idmap2/FileUtils.h
+++ b/cmds/idmap2/include/idmap2/FileUtils.h
@@ -17,6 +17,8 @@
 #ifndef IDMAP2_INCLUDE_IDMAP2_FILEUTILS_H_
 #define IDMAP2_INCLUDE_IDMAP2_FILEUTILS_H_
 
+#include <sys/types.h>
+
 #include <functional>
 #include <memory>
 #include <string>
@@ -24,6 +26,7 @@
 
 namespace android::idmap2::utils {
 
+constexpr const char* kIdmapCacheDir = "/data/resource-cache";
 constexpr const mode_t kIdmapFilePermissionMask = 0133;  // u=rw,g=r,o=r
 
 typedef std::function<bool(unsigned char type /* DT_* from dirent.h */, const std::string& path)>
@@ -35,6 +38,8 @@ std::unique_ptr<std::string> ReadFile(int fd);
 
 std::unique_ptr<std::string> ReadFile(const std::string& path);
 
+bool UidHasWriteAccessToPath(uid_t uid, const std::string& path);
+
 }  // namespace android::idmap2::utils
 
 #endif  // IDMAP2_INCLUDE_IDMAP2_FILEUTILS_H_
diff --git a/cmds/idmap2/libidmap2/FileUtils.cpp b/cmds/idmap2/libidmap2/FileUtils.cpp
index 0255727fc8c6..a9b68cd6d5d5 100644
--- a/cmds/idmap2/libidmap2/FileUtils.cpp
+++ b/cmds/idmap2/libidmap2/FileUtils.cpp
@@ -19,12 +19,20 @@
 #include <unistd.h>
 
 #include <cerrno>
+#include <climits>
+#include <cstdlib>
+#include <cstring>
 #include <fstream>
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
 
+#include "android-base/file.h"
+#include "android-base/macros.h"
+#include "android-base/stringprintf.h"
+#include "private/android_filesystem_config.h"
+
 #include "idmap2/FileUtils.h"
 
 namespace android::idmap2::utils {
@@ -77,4 +85,26 @@ std::unique_ptr<std::string> ReadFile(int fd) {
   return r == 0 ? std::move(str) : nullptr;
 }
 
+#ifdef __ANDROID__
+bool UidHasWriteAccessToPath(uid_t uid, const std::string& path) {
+  // resolve symlinks and relative paths; the directories must exist
+  std::string canonical_path;
+  if (!base::Realpath(base::Dirname(path), &canonical_path)) {
+    return false;
+  }
+
+  const std::string cache_subdir = base::StringPrintf("%s/", kIdmapCacheDir);
+  if (canonical_path == kIdmapCacheDir ||
+      canonical_path.compare(0, cache_subdir.size(), cache_subdir) == 0) {
+    // limit access to /data/resource-cache to root and system
+    return uid == AID_ROOT || uid == AID_SYSTEM;
+  }
+  return true;
+}
+#else
+bool UidHasWriteAccessToPath(uid_t uid ATTRIBUTE_UNUSED, const std::string& path ATTRIBUTE_UNUSED) {
+  return true;
+}
+#endif
+
 }  // namespace android::idmap2::utils
diff --git a/cmds/idmap2/tests/FileUtilsTests.cpp b/cmds/idmap2/tests/FileUtilsTests.cpp
index d9d9a7f829cf..45f84fe341cc 100644
--- a/cmds/idmap2/tests/FileUtilsTests.cpp
+++ b/cmds/idmap2/tests/FileUtilsTests.cpp
@@ -22,6 +22,8 @@
 #include "gtest/gtest.h"
 
 #include "android-base/macros.h"
+#include "android-base/stringprintf.h"
+#include "private/android_filesystem_config.h"
 
 #include "idmap2/FileUtils.h"
 
@@ -71,4 +73,25 @@ TEST(FileUtilsTests, ReadFile) {
   close(pipefd[0]);
 }
 
+#ifdef __ANDROID__
+TEST(FileUtilsTests, UidHasWriteAccessToPath) {
+  constexpr const char* tmp_path = "/data/local/tmp/test@idmap";
+  const std::string cache_path(base::StringPrintf("%s/test@idmap", kIdmapCacheDir));
+  const std::string sneaky_cache_path(base::StringPrintf("/data/../%s/test@idmap", kIdmapCacheDir));
+
+  ASSERT_TRUE(UidHasWriteAccessToPath(AID_ROOT, tmp_path));
+  ASSERT_TRUE(UidHasWriteAccessToPath(AID_ROOT, cache_path));
+  ASSERT_TRUE(UidHasWriteAccessToPath(AID_ROOT, sneaky_cache_path));
+
+  ASSERT_TRUE(UidHasWriteAccessToPath(AID_SYSTEM, tmp_path));
+  ASSERT_TRUE(UidHasWriteAccessToPath(AID_SYSTEM, cache_path));
+  ASSERT_TRUE(UidHasWriteAccessToPath(AID_SYSTEM, sneaky_cache_path));
+
+  constexpr const uid_t AID_SOME_APP = AID_SYSTEM + 1;
+  ASSERT_TRUE(UidHasWriteAccessToPath(AID_SOME_APP, tmp_path));
+  ASSERT_FALSE(UidHasWriteAccessToPath(AID_SOME_APP, cache_path));
+  ASSERT_FALSE(UidHasWriteAccessToPath(AID_SOME_APP, sneaky_cache_path));
+}
+#endif
+
 }  // namespace android::idmap2::utils
diff --git a/cmds/idmap2/tests/Idmap2BinaryTests.cpp b/cmds/idmap2/tests/Idmap2BinaryTests.cpp
index 0c8f164bf096..a60886d2a352 100644
--- a/cmds/idmap2/tests/Idmap2BinaryTests.cpp
+++ b/cmds/idmap2/tests/Idmap2BinaryTests.cpp
@@ -38,6 +38,7 @@
 #include "gtest/gtest.h"
 
 #include "androidfw/PosixUtils.h"
+#include "private/android_filesystem_config.h"
 
 #include "idmap2/FileUtils.h"
 #include "idmap2/Idmap.h"
@@ -69,9 +70,23 @@ void AssertIdmap(const Idmap& idmap, const std::string& target_apk_path,
     ASSERT_NO_FATAL_FAILURE(AssertIdmap(idmap_ref, target_apk_path, overlay_apk_path)); \
   } while (0)
 
+#ifdef __ANDROID__
+#define SKIP_TEST_IF_CANT_EXEC_IDMAP2           \
+  do {                                          \
+    const uid_t uid = getuid();                 \
+    if (uid != AID_ROOT && uid != AID_SYSTEM) { \
+      GTEST_SKIP();                             \
+    }                                           \
+  } while (0)
+#else
+#define SKIP_TEST_IF_CANT_EXEC_IDMAP2
+#endif
+
 }  // namespace
 
 TEST_F(Idmap2BinaryTests, Create) {
+  SKIP_TEST_IF_CANT_EXEC_IDMAP2;
+
   // clang-format off
   auto result = ExecuteBinary({"idmap2",
                                "create",
@@ -97,6 +112,8 @@ TEST_F(Idmap2BinaryTests, Create) {
 }
 
 TEST_F(Idmap2BinaryTests, Dump) {
+  SKIP_TEST_IF_CANT_EXEC_IDMAP2;
+
   // clang-format off
   auto result = ExecuteBinary({"idmap2",
                                "create",
@@ -144,6 +161,8 @@ TEST_F(Idmap2BinaryTests, Dump) {
 }
 
 TEST_F(Idmap2BinaryTests, Scan) {
+  SKIP_TEST_IF_CANT_EXEC_IDMAP2;
+
   const std::string overlay_static_1_apk_path = GetTestDataPath() + "/overlay/overlay-static-1.apk";
   const std::string overlay_static_2_apk_path = GetTestDataPath() + "/overlay/overlay-static-2.apk";
   const std::string idmap_static_1_path =
@@ -236,6 +255,8 @@ TEST_F(Idmap2BinaryTests, Scan) {
 }
 
 TEST_F(Idmap2BinaryTests, Lookup) {
+  SKIP_TEST_IF_CANT_EXEC_IDMAP2;
+
   // clang-format off
   auto result = ExecuteBinary({"idmap2",
                                "create",
@@ -285,6 +306,8 @@ TEST_F(Idmap2BinaryTests, Lookup) {
 }
 
 TEST_F(Idmap2BinaryTests, InvalidCommandLineOptions) {
+  SKIP_TEST_IF_CANT_EXEC_IDMAP2;
+
   const std::string invalid_target_apk_path = GetTestDataPath() + "/DOES-NOT-EXIST";
 
   // missing mandatory options