Skip to content

Commit 7f358de

Browse files
Treehugger RobotGerrit Code Review
authored andcommitted
Merge "Fix fs-verity API for secondary users" into main
2 parents 86e2512 + c72e4d7 commit 7f358de

4 files changed

Lines changed: 21 additions & 25 deletions

File tree

cmds/installd/InstalldNativeService.cpp

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -234,12 +234,12 @@ binder::Status checkArgumentFileName(const std::string& path) {
234234
return ok();
235235
}
236236

237-
binder::Status checkUidInAppRange(int32_t appUid) {
238-
if (FIRST_APPLICATION_UID <= appUid && appUid <= LAST_APPLICATION_UID) {
237+
binder::Status checkArgumentAppId(int32_t appId) {
238+
if (FIRST_APPLICATION_UID <= appId && appId <= LAST_APPLICATION_UID) {
239239
return ok();
240240
}
241241
return exception(binder::Status::EX_ILLEGAL_ARGUMENT,
242-
StringPrintf("UID %d is outside of the range", appUid));
242+
StringPrintf("appId %d is outside of the range", appId));
243243
}
244244

245245
#define ENFORCE_UID(uid) { \
@@ -302,12 +302,12 @@ binder::Status checkUidInAppRange(int32_t appUid) {
302302
} \
303303
}
304304

305-
#define CHECK_ARGUMENT_UID_IN_APP_RANGE(uid) \
306-
{ \
307-
binder::Status status = checkUidInAppRange((uid)); \
308-
if (!status.isOk()) { \
309-
return status; \
310-
} \
305+
#define CHECK_ARGUMENT_APP_ID(appId) \
306+
{ \
307+
binder::Status status = checkArgumentAppId((appId)); \
308+
if (!status.isOk()) { \
309+
return status; \
310+
} \
311311
}
312312

313313
#ifdef GRANULAR_LOCKS
@@ -411,7 +411,7 @@ using PackageLockGuard = std::lock_guard<PackageLock>;
411411
} // namespace
412412

413413
binder::Status InstalldNativeService::FsveritySetupAuthToken::authenticate(
414-
const ParcelFileDescriptor& authFd, int32_t appUid, int32_t userId) {
414+
const ParcelFileDescriptor& authFd, int32_t uid) {
415415
int open_flags = fcntl(authFd.get(), F_GETFL);
416416
if (open_flags < 0) {
417417
return exception(binder::Status::EX_SERVICE_SPECIFIC, "fcntl failed");
@@ -426,9 +426,8 @@ binder::Status InstalldNativeService::FsveritySetupAuthToken::authenticate(
426426
return exception(binder::Status::EX_SECURITY, "Not a regular file");
427427
}
428428
// Don't accept a file owned by a different app.
429-
uid_t uid = multiuser_get_uid(userId, appUid);
430-
if (this->mStatFromAuthFd.st_uid != uid) {
431-
return exception(binder::Status::EX_SERVICE_SPECIFIC, "File not owned by appUid");
429+
if (this->mStatFromAuthFd.st_uid != (uid_t)uid) {
430+
return exception(binder::Status::EX_SERVICE_SPECIFIC, "File not owned by uid");
432431
}
433432
return ok();
434433
}
@@ -3986,21 +3985,21 @@ binder::Status InstalldNativeService::getOdexVisibility(
39863985
// attacker-in-the-middle cannot enable fs-verity on arbitrary app files. If the FD is not writable,
39873986
// return null.
39883987
//
3989-
// appUid and userId are passed for additional ownership check, such that one app can not be
3988+
// app process uid is passed for additional ownership check, such that one app can not be
39903989
// authenticated for another app's file. These parameters are assumed trusted for this purpose of
39913990
// consistency check.
39923991
//
39933992
// Notably, creating the token allows us to manage the writable FD easily during enableFsverity.
39943993
// Since enabling fs-verity to a file requires no outstanding writable FD, passing the authFd to the
39953994
// server allows the server to hold the only reference (as long as the client app doesn't).
39963995
binder::Status InstalldNativeService::createFsveritySetupAuthToken(
3997-
const ParcelFileDescriptor& authFd, int32_t appUid, int32_t userId,
3996+
const ParcelFileDescriptor& authFd, int32_t uid,
39983997
sp<IFsveritySetupAuthToken>* _aidl_return) {
3999-
CHECK_ARGUMENT_UID_IN_APP_RANGE(appUid);
4000-
ENFORCE_VALID_USER(userId);
3998+
CHECK_ARGUMENT_APP_ID(multiuser_get_app_id(uid));
3999+
ENFORCE_VALID_USER(multiuser_get_user_id(uid));
40014000

40024001
auto token = sp<FsveritySetupAuthToken>::make();
4003-
binder::Status status = token->authenticate(authFd, appUid, userId);
4002+
binder::Status status = token->authenticate(authFd, uid);
40044003
if (!status.isOk()) {
40054004
return status;
40064005
}

cmds/installd/InstalldNativeService.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@ class InstalldNativeService : public BinderService<InstalldNativeService>, publi
4444
public:
4545
FsveritySetupAuthToken() : mStatFromAuthFd() {}
4646

47-
binder::Status authenticate(const android::os::ParcelFileDescriptor& authFd, int32_t appUid,
48-
int32_t userId);
47+
binder::Status authenticate(const android::os::ParcelFileDescriptor& authFd, int32_t uid);
4948
bool isSameStat(const struct stat& st) const;
5049

5150
private:
@@ -213,7 +212,7 @@ class InstalldNativeService : public BinderService<InstalldNativeService>, publi
213212
int32_t* _aidl_return);
214213

215214
binder::Status createFsveritySetupAuthToken(const android::os::ParcelFileDescriptor& authFd,
216-
int32_t appUid, int32_t userId,
215+
int32_t uid,
217216
android::sp<IFsveritySetupAuthToken>* _aidl_return);
218217
binder::Status enableFsverity(const android::sp<IFsveritySetupAuthToken>& authToken,
219218
const std::string& filePath, const std::string& packageName,

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ interface IInstalld {
145145
//
146146
// We don't necessarily need a method here, so it's left blank intentionally.
147147
}
148-
IFsveritySetupAuthToken createFsveritySetupAuthToken(in ParcelFileDescriptor authFd, int appUid,
149-
int userId);
148+
IFsveritySetupAuthToken createFsveritySetupAuthToken(in ParcelFileDescriptor authFd, int uid);
150149
int enableFsverity(in IFsveritySetupAuthToken authToken, @utf8InCpp String filePath,
151150
@utf8InCpp String packageName);
152151

cmds/installd/tests/installd_service_test.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -548,8 +548,7 @@ class FsverityTest : public ServiceTest {
548548
unique_fd ufd(open(path.c_str(), open_mode));
549549
EXPECT_GE(ufd.get(), 0) << "open failed: " << strerror(errno);
550550
ParcelFileDescriptor rfd(std::move(ufd));
551-
return service->createFsveritySetupAuthToken(std::move(rfd), kTestAppId, kTestUserId,
552-
_aidl_return);
551+
return service->createFsveritySetupAuthToken(std::move(rfd), kTestAppId, _aidl_return);
553552
}
554553
};
555554

0 commit comments

Comments
 (0)