Merge "idmap2: lock down write access to /data/resouce-cache" (6302322f) · Commits · e / os / android_frameworks_base

cmds/idmap2/idmap2/Create.cpp

+8 −0

Original line number	Diff line number	Diff line
		@@ -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;

cmds/idmap2/idmap2d/Idmap2Service.cpp

+17 −3

Original line number	Diff line number	Diff line
		@@ -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);

cmds/idmap2/include/idmap2/FileUtils.h

+5 −0

Original line number	Diff line number	Diff line
		@@ -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_

cmds/idmap2/libidmap2/FileUtils.cpp

+30 −0

Original line number	Diff line number	Diff line
		@@ -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

cmds/idmap2/tests/FileUtilsTests.cpp

+23 −0

Original line number	Diff line number	Diff line
		@@ -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