Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9336614a authored by Victor Hsieh's avatar Victor Hsieh
Browse files

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
parent da774518

cmds/installd/InstalldNativeService.cpp

+17 −18
Original line number Diff line number Diff line
@@ -233,12 +233,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) {                                  \
@@ -301,9 +301,9 @@ binder::Status checkUidInAppRange(int32_t appUid) { 
        }                                                      \

    }



#define CHECK_ARGUMENT_UID_IN_APP_RANGE(uid)               \

#define CHECK_ARGUMENT_APP_ID(appId)                         \

    {                                                        \

        binder::Status status = checkUidInAppRange((uid)); \

        binder::Status status = checkArgumentAppId((appId)); \

        if (!status.isOk()) {                                \

            return status;                                   \

        }                                                    \
@@ -410,7 +410,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");
@@ -425,9 +425,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();

}
@@ -3974,7 +3973,7 @@ 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.

//
@@ -3982,13 +3981,13 @@ binder::Status InstalldNativeService::getOdexVisibility( 
// 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

+2 −3
Original line number Diff line number Diff line
@@ -44,8 +44,7 @@ public: 
    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:
@@ -210,7 +209,7 @@ public: 
                                     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

+1 −2
Original line number Diff line number Diff line
@@ -143,8 +143,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

+1 −2
Original line number Diff line number Diff line
@@ -548,8 +548,7 @@ protected: 
        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);

    }

};