Fix fs-verity API for secondary users
createFsveritySetupAuthToken previously takes a userId in order to
handle the app uid, ended up did it incorrectly. Since the
authentication in question is really to compare the calling uid with
the file owner's uid, userId doesn't really matter. Drop it and fix the
range check accordingly.
Ignore-AOSP-First: cross-repo API change
Bug: 319280249
Test: atest FsVerityTest
Test: atest FsVerityTest --user-type secondary_user
Test: atest installd_service_test
Change-Id: I4090792afaf05c3dff5cb34731ef7030243196c2
Merged-In: I4090792afaf05c3dff5cb34731ef7030243196c2
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index cad7787..71a8740 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -234,12 +234,12 @@
return ok();
}
-binder::Status checkUidInAppRange(int32_t appUid) {
- if (FIRST_APPLICATION_UID <= appUid && appUid <= LAST_APPLICATION_UID) {
+binder::Status checkArgumentAppId(int32_t appId) {
+ if (FIRST_APPLICATION_UID <= appId && appId <= LAST_APPLICATION_UID) {
return ok();
}
return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
- StringPrintf("UID %d is outside of the range", appUid));
+ StringPrintf("appId %d is outside of the range", appId));
}
#define ENFORCE_UID(uid) { \
@@ -302,12 +302,12 @@
} \
}
-#define CHECK_ARGUMENT_UID_IN_APP_RANGE(uid) \
- { \
- binder::Status status = checkUidInAppRange((uid)); \
- if (!status.isOk()) { \
- return status; \
- } \
+#define CHECK_ARGUMENT_APP_ID(appId) \
+ { \
+ binder::Status status = checkArgumentAppId((appId)); \
+ if (!status.isOk()) { \
+ return status; \
+ } \
}
#ifdef GRANULAR_LOCKS
@@ -411,7 +411,7 @@
} // namespace
binder::Status InstalldNativeService::FsveritySetupAuthToken::authenticate(
- const ParcelFileDescriptor& authFd, int32_t appUid, int32_t userId) {
+ const ParcelFileDescriptor& authFd, int32_t uid) {
int open_flags = fcntl(authFd.get(), F_GETFL);
if (open_flags < 0) {
return exception(binder::Status::EX_SERVICE_SPECIFIC, "fcntl failed");
@@ -426,9 +426,8 @@
return exception(binder::Status::EX_SECURITY, "Not a regular file");
}
// Don't accept a file owned by a different app.
- uid_t uid = multiuser_get_uid(userId, appUid);
- if (this->mStatFromAuthFd.st_uid != uid) {
- return exception(binder::Status::EX_SERVICE_SPECIFIC, "File not owned by appUid");
+ if (this->mStatFromAuthFd.st_uid != (uid_t)uid) {
+ return exception(binder::Status::EX_SERVICE_SPECIFIC, "File not owned by uid");
}
return ok();
}
@@ -3986,7 +3985,7 @@
// attacker-in-the-middle cannot enable fs-verity on arbitrary app files. If the FD is not writable,
// return null.
//
-// appUid and userId are passed for additional ownership check, such that one app can not be
+// app process uid is passed for additional ownership check, such that one app can not be
// authenticated for another app's file. These parameters are assumed trusted for this purpose of
// consistency check.
//
@@ -3994,13 +3993,13 @@
// Since enabling fs-verity to a file requires no outstanding writable FD, passing the authFd to the
// server allows the server to hold the only reference (as long as the client app doesn't).
binder::Status InstalldNativeService::createFsveritySetupAuthToken(
- const ParcelFileDescriptor& authFd, int32_t appUid, int32_t userId,
+ const ParcelFileDescriptor& authFd, int32_t uid,
sp<IFsveritySetupAuthToken>* _aidl_return) {
- CHECK_ARGUMENT_UID_IN_APP_RANGE(appUid);
- ENFORCE_VALID_USER(userId);
+ CHECK_ARGUMENT_APP_ID(multiuser_get_app_id(uid));
+ ENFORCE_VALID_USER(multiuser_get_user_id(uid));
auto token = sp<FsveritySetupAuthToken>::make();
- binder::Status status = token->authenticate(authFd, appUid, userId);
+ binder::Status status = token->authenticate(authFd, uid);
if (!status.isOk()) {
return status;
}
diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h
index 1ec092d..88caba7 100644
--- a/cmds/installd/InstalldNativeService.h
+++ b/cmds/installd/InstalldNativeService.h
@@ -44,8 +44,7 @@
public:
FsveritySetupAuthToken() : mStatFromAuthFd() {}
- binder::Status authenticate(const android::os::ParcelFileDescriptor& authFd, int32_t appUid,
- int32_t userId);
+ binder::Status authenticate(const android::os::ParcelFileDescriptor& authFd, int32_t uid);
bool isSameStat(const struct stat& st) const;
private:
@@ -213,7 +212,7 @@
int32_t* _aidl_return);
binder::Status createFsveritySetupAuthToken(const android::os::ParcelFileDescriptor& authFd,
- int32_t appUid, int32_t userId,
+ int32_t uid,
android::sp<IFsveritySetupAuthToken>* _aidl_return);
binder::Status enableFsverity(const android::sp<IFsveritySetupAuthToken>& authToken,
const std::string& filePath, const std::string& packageName,
diff --git a/cmds/installd/binder/android/os/IInstalld.aidl b/cmds/installd/binder/android/os/IInstalld.aidl
index 8893e38..120d61d 100644
--- a/cmds/installd/binder/android/os/IInstalld.aidl
+++ b/cmds/installd/binder/android/os/IInstalld.aidl
@@ -145,8 +145,7 @@
//
// We don't necessarily need a method here, so it's left blank intentionally.
}
- IFsveritySetupAuthToken createFsveritySetupAuthToken(in ParcelFileDescriptor authFd, int appUid,
- int userId);
+ IFsveritySetupAuthToken createFsveritySetupAuthToken(in ParcelFileDescriptor authFd, int uid);
int enableFsverity(in IFsveritySetupAuthToken authToken, @utf8InCpp String filePath,
@utf8InCpp String packageName);
diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp
index 4bc92af..f2b578a 100644
--- a/cmds/installd/tests/installd_service_test.cpp
+++ b/cmds/installd/tests/installd_service_test.cpp
@@ -548,8 +548,7 @@
unique_fd ufd(open(path.c_str(), open_mode));
EXPECT_GE(ufd.get(), 0) << "open failed: " << strerror(errno);
ParcelFileDescriptor rfd(std::move(ufd));
- return service->createFsveritySetupAuthToken(std::move(rfd), kTestAppId, kTestUserId,
- _aidl_return);
+ return service->createFsveritySetupAuthToken(std::move(rfd), kTestAppId, _aidl_return);
}
};