diff options
| -rw-r--r-- | services/incremental/IncrementalService.cpp | 19 | ||||
| -rw-r--r-- | services/incremental/IncrementalServiceValidation.cpp | 7 | ||||
| -rw-r--r-- | services/incremental/test/IncrementalServiceTest.cpp | 26 |
3 files changed, 48 insertions, 4 deletions
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp index 45c9ad9b344b..dde70caa797f 100644 --- a/services/incremental/IncrementalService.cpp +++ b/services/incremental/IncrementalService.cpp @@ -38,9 +38,11 @@ using namespace std::literals; -constexpr const char* kDataUsageStats = "android.permission.LOADER_USAGE_STATS"; +constexpr const char* kLoaderUsageStats = "android.permission.LOADER_USAGE_STATS"; constexpr const char* kOpUsage = "android:loader_usage_stats"; +constexpr const char* kInteractAcrossUsers = "android.permission.INTERACT_ACROSS_USERS"; + namespace android::incremental { using content::pm::DataLoaderParamsParcel; @@ -684,10 +686,21 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog return -EPERM; } - if (auto status = mAppOpsManager->checkPermission(kDataUsageStats, kOpUsage, + // Check loader usage stats permission and apop. + if (auto status = mAppOpsManager->checkPermission(kLoaderUsageStats, kOpUsage, + params.packageName.c_str()); + !status.isOk()) { + LOG(ERROR) << " Permission: " << kLoaderUsageStats + << " check failed: " << status.toString8(); + return fromBinderStatus(status); + } + + // Check multiuser permission. + if (auto status = mAppOpsManager->checkPermission(kInteractAcrossUsers, nullptr, params.packageName.c_str()); !status.isOk()) { - LOG(ERROR) << "checkPermission failed: " << status.toString8(); + LOG(ERROR) << " Permission: " << kInteractAcrossUsers + << " check failed: " << status.toString8(); return fromBinderStatus(status); } } diff --git a/services/incremental/IncrementalServiceValidation.cpp b/services/incremental/IncrementalServiceValidation.cpp index abadbbf10742..9f2639a81666 100644 --- a/services/incremental/IncrementalServiceValidation.cpp +++ b/services/incremental/IncrementalServiceValidation.cpp @@ -56,13 +56,18 @@ binder::Status CheckPermissionForDataDelivery(const char* permission, const char String16 packageName{package}; - // Caller must also have op granted. PermissionController pc; if (auto packageUid = pc.getPackageUid(packageName, 0); packageUid != uid) { return Exception(binder::Status::EX_SECURITY, StringPrintf("UID %d / PID %d does not own package %s", uid, pid, package)); } + + if (!operation) { + return binder::Status::ok(); + } + + // Caller must also have op granted. switch (auto result = pc.noteOp(String16(operation), uid, packageName); result) { case PermissionController::MODE_ALLOWED: case PermissionController::MODE_DEFAULT: diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp index 02eaa2fe07b1..47b9051970e4 100644 --- a/services/incremental/test/IncrementalServiceTest.cpp +++ b/services/incremental/test/IncrementalServiceTest.cpp @@ -397,6 +397,15 @@ public: void checkPermissionSuccess() { ON_CALL(*this, checkPermission(_, _, _)).WillByDefault(Return(android::incremental::Ok())); } + void checkPermissionNoCrossUsers() { + ON_CALL(*this, + checkPermission("android.permission.LOADER_USAGE_STATS", + "android:loader_usage_stats", _)) + .WillByDefault(Return(android::incremental::Ok())); + ON_CALL(*this, checkPermission("android.permission.INTERACT_ACROSS_USERS", nullptr, _)) + .WillByDefault( + Return(android::incremental::Exception(binder::Status::EX_SECURITY, {}))); + } void checkPermissionFails() { ON_CALL(*this, checkPermission(_, _, _)) .WillByDefault( @@ -1047,6 +1056,23 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsCheckPermissionFails) { ASSERT_LT(mDataLoader->setStorageParams(true), 0); } +TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsCheckPermissionNoCrossUsers) { + mAppOpsManager->checkPermissionNoCrossUsers(); + + EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)); + EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); + // checkPermission fails, no calls to set opitions, start or stop WatchingMode. + EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(0); + EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(0); + EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0); + TemporaryDir tempDir; + int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), + IncrementalService::CreateOptions::CreateNew, + {}, {}, {}, {}); + ASSERT_GE(storageId, 0); + ASSERT_LT(mDataLoader->setStorageParams(true), 0); +} + TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsFails) { mVold->setIncFsMountOptionsFails(); mAppOpsManager->checkPermissionSuccess(); |