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

Author: victorhsieh

cmds/installd/InstalldNativeService.cpp

@@ -234,12 +234,12 @@ binder::Status checkArgumentFileName(const std::string& path) {
     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 @@ binder::Status checkUidInAppRange(int32_t appUid) {
         }                                                      \
     }
 
-#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 @@ using PackageLockGuard = std::lock_guard<PackageLock>;
 }  // 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 @@ binder::Status InstalldNativeService::FsveritySetupAuthToken::authenticate(
         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,21 +3985,21 @@ binder::Status InstalldNativeService::getOdexVisibility(
 // 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.
 //
 // Notably, creating the token allows us to manage the writable FD easily during enableFsverity.
 // 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;
     }

cmds/installd/InstalldNativeService.h

@@ -44,8 +44,7 @@ class InstalldNativeService : public BinderService<InstalldNativeService>, publi
     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 @@ class InstalldNativeService : public BinderService<InstalldNativeService>, publi
                                      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,

cmds/installd/binder/android/os/IInstalld.aidl

@@ -145,8 +145,7 @@ interface IInstalld {
         //
         // 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);
 

cmds/installd/tests/installd_service_test.cpp

@@ -548,8 +548,7 @@ class FsverityTest : public ServiceTest {
         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);
     }
 };