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
diff --git a/cmds/idmap2/idmap2/Create.cpp b/cmds/idmap2/idmap2/Create.cpp
index c455ac0..0c581f3 100644
--- a/cmds/idmap2/idmap2/Create.cpp
+++ b/cmds/idmap2/idmap2/Create.cpp
@@ -39,6 +39,7 @@
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 @@
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 a3c7527..f30ce9b 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::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 @@
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 @@
}
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 5c41c49..3f03236 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(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 0255727..a9b68cd 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 @@
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 d9d9a7f..45f84fe 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 @@
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 0c8f164..a60886d 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 @@
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, Dump) {
+ SKIP_TEST_IF_CANT_EXEC_IDMAP2;
+
// clang-format off
auto result = ExecuteBinary({"idmap2",
"create",
@@ -144,6 +161,8 @@
}
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, Lookup) {
+ SKIP_TEST_IF_CANT_EXEC_IDMAP2;
+
// clang-format off
auto result = ExecuteBinary({"idmap2",
"create",
@@ -285,6 +306,8 @@
}
TEST_F(Idmap2BinaryTests, InvalidCommandLineOptions) {
+ SKIP_TEST_IF_CANT_EXEC_IDMAP2;
+
const std::string invalid_target_apk_path = GetTestDataPath() + "/DOES-NOT-EXIST";
// missing mandatory options