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

Commit 1d89216e authored by Alex Buynytskyy's avatar Alex Buynytskyy
Browse files

Appops permission monitoring for GET_USAGE_STATS. 

This makes sure DataLoader won't be able to obtain read logs once user
denies access.

Bug: b/152633648
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Test: adb shell appops set 1000 GET_USAGE_STATS deny
Change-Id: Ibbb74933b4ef0dd8f5fe27732743e5820b8ee4dc
parent 3fc58ee5

PREUPLOAD.cfg

+1 −0
Original line number Diff line number Diff line
@@ -9,6 +9,7 @@ clang_format = --commit ${PREUPLOAD_COMMIT} --style file --extensions c,h,cc,cpp 
               core/jni/

               libs/input/

               services/core/jni/

               services/incremental/



[Hook Scripts]

checkstyle_hook = ${REPO_ROOT}/prebuilts/checkstyle/checkstyle.py --sha ${PREUPLOAD_COMMIT}

services/incremental/Android.bp

+0 −1
Original line number Diff line number Diff line
@@ -50,7 +50,6 @@ cc_defaults { 
        "libbinder",

        "libcrypto",

        "libcutils",

        "libdataloader",

        "libincfs",

        "liblog",

        "libz",

services/incremental/IncrementalService.cpp

+47 −35
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ 
#define LOG_TAG "IncrementalService"



#include "IncrementalService.h"

#include "IncrementalServiceValidation.h"



#include <android-base/file.h>

#include <android-base/logging.h>
@@ -582,25 +581,29 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog 
        return -EINVAL;

    }



    ifs->dataLoaderFilesystemParams.readLogsEnabled = enableReadLogs;

    if (enableReadLogs) {

        // We never unregister the callbacks, but given a restricted number of data loaders and even fewer asking for read log access, should be ok.

        registerAppOpsCallback(ifs->dataLoaderParams.packageName);

        if (auto status =

                    mAppOpsManager->checkPermission(kDataUsageStats, kOpUsage,

                                                    ifs->dataLoaderParams.packageName.c_str());

            !status.isOk()) {

            LOG(ERROR) << "checkPermission failed: " << status.toString8();

            return fromBinderStatus(status);

        }

    }



    return applyStorageParams(*ifs);

    if (auto status = applyStorageParams(*ifs, enableReadLogs); !status.isOk()) {

        LOG(ERROR) << "applyStorageParams failed: " << status.toString8();

        return fromBinderStatus(status);

    }



int IncrementalService::applyStorageParams(IncFsMount& ifs) {

    const bool enableReadLogs = ifs.dataLoaderFilesystemParams.readLogsEnabled;

    if (enableReadLogs) {

        if (auto status = CheckPermissionForDataDelivery(kDataUsageStats, kOpUsage);

            !status.isOk()) {

            LOG(ERROR) << "CheckPermissionForDataDelivery failed: " << status.toString8();

            return fromBinderStatus(status);

        registerAppOpsCallback(ifs->dataLoaderParams.packageName);

    }



    return 0;

}



binder::Status IncrementalService::applyStorageParams(IncFsMount& ifs, bool enableReadLogs) {

    using unique_fd = ::android::base::unique_fd;

    ::android::os::incremental::IncrementalFileSystemControlParcel control;

    control.cmd.reset(unique_fd(dup(ifs.control.cmd())));
@@ -611,13 +614,7 @@ int IncrementalService::applyStorageParams(IncFsMount& ifs) { 
    }



    std::lock_guard l(mMountOperationLock);

    const auto status = mVold->setIncFsMountOptions(control, enableReadLogs);

    if (!status.isOk()) {

        LOG(ERROR) << "Calling Vold::setIncFsMountOptions() failed: " << status.toString8();

        return fromBinderStatus(status);

    }



    return 0;

    return mVold->setIncFsMountOptions(control, enableReadLogs);

}



void IncrementalService::deleteStorage(StorageId storageId) {
@@ -1280,39 +1277,54 @@ bool IncrementalService::configureNativeBinaries(StorageId storage, std::string_ 
}



void IncrementalService::registerAppOpsCallback(const std::string& packageName) {

    if (packageName.empty()) {

    sp<IAppOpsCallback> listener;

    {

        std::unique_lock lock{mCallbacksLock};

        auto& cb = mCallbackRegistered[packageName];

        if (cb) {

            return;

        }

        cb = new AppOpsListener(*this, packageName);

        listener = cb;

    }



    mAppOpsManager->startWatchingMode(AppOpsManager::OP_GET_USAGE_STATS, String16(packageName.c_str()), listener);

}



bool IncrementalService::unregisterAppOpsCallback(const std::string& packageName) {

    sp<IAppOpsCallback> listener;

    {

        std::unique_lock lock{mCallbacksLock};

        if (!mCallbackRegistered.insert(packageName).second) {

            return;

        auto found = mCallbackRegistered.find(packageName);

        if (found == mCallbackRegistered.end()) {

            return false;

        }

        listener = found->second;

        mCallbackRegistered.erase(found);

    }



    /* TODO(b/152633648): restore callback after it's not crashing Binder anymore.

    sp<AppOpsListener> listener = new AppOpsListener(*this, packageName);

    mAppOpsManager->startWatchingMode(AppOpsManager::OP_GET_USAGE_STATS, String16(packageName.c_str()), listener);

    */

    mAppOpsManager->stopWatchingMode(listener);

    return true;

}



void IncrementalService::onAppOpChanged(const std::string& packageName) {

    if (!unregisterAppOpsCallback(packageName)) {

        return;

    }



void IncrementalService::onAppOppChanged(const std::string& packageName) {

    std::vector<IfsMountPtr> affected;

    {

        std::lock_guard l(mLock);

        affected.reserve(mMounts.size());

        for (auto&& [id, ifs] : mMounts) {

            if (ifs->dataLoaderFilesystemParams.readLogsEnabled && ifs->dataLoaderParams.packageName == packageName) {

            if (ifs->mountId == id && ifs->dataLoaderParams.packageName == packageName) {

                affected.push_back(ifs);

            }

        }

    }

    /* TODO(b/152633648): restore callback after it's not crashing Kernel anymore.

    for (auto&& ifs : affected) {

        applyStorageParams(*ifs);

        applyStorageParams(*ifs, false);

    }

    */

}



binder::Status IncrementalService::IncrementalDataLoaderListener::onStatusChanged(MountId mountId,
@@ -1378,8 +1390,8 @@ binder::Status IncrementalService::IncrementalDataLoaderListener::onStatusChange 
    return binder::Status::ok();

}



void IncrementalService::AppOpsListener::opChanged(int32_t op, const String16&) {

    incrementalService.onAppOppChanged(packageName);

void IncrementalService::AppOpsListener::opChanged(int32_t, const String16&) {

    incrementalService.onAppOpChanged(packageName);

}



} // namespace android::incremental

services/incremental/IncrementalService.h

+5 −8
Original line number Diff line number Diff line
@@ -40,7 +40,6 @@ 
#include "ServiceWrappers.h"

#include "android/content/pm/BnDataLoaderStatusListener.h"

#include "incfs.h"

#include "dataloader_ndk.h"

#include "path.h"



using namespace android::os::incremental;
@@ -182,7 +181,6 @@ private: 
        StorageMap storages;

        BindMap bindPoints;

        DataLoaderParamsParcel dataLoaderParams;

        DataLoaderFilesystemParams dataLoaderFilesystemParams;

        std::atomic<int> nextStorageDirNo{0};

        std::atomic<int> dataLoaderStatus = -1;

        bool dataLoaderStartRequested = false;
@@ -193,9 +191,7 @@ private: 
              : root(std::move(root)),

                control(std::move(control)),

                mountId(mountId),

                incrementalService(incrementalService) {

            dataLoaderFilesystemParams.readLogsEnabled = false;

        }

                incrementalService(incrementalService) {}

        IncFsMount(IncFsMount&&) = delete;

        IncFsMount& operator=(IncFsMount&&) = delete;

        ~IncFsMount();
@@ -234,10 +230,11 @@ private: 
    std::string normalizePathToStorage(const IfsMountPtr incfs, StorageId storage,

                                       std::string_view path);



    int applyStorageParams(IncFsMount& ifs);

    binder::Status applyStorageParams(IncFsMount& ifs, bool enableReadLogs);



    void registerAppOpsCallback(const std::string& packageName);

    void onAppOppChanged(const std::string& packageName);

    bool unregisterAppOpsCallback(const std::string& packageName);

    void onAppOpChanged(const std::string& packageName);



    // Member variables

    std::unique_ptr<VoldServiceWrapper> const mVold;
@@ -252,7 +249,7 @@ private: 
    BindPathMap mBindsByPath;



    std::mutex mCallbacksLock;

    std::set<std::string> mCallbackRegistered;

    std::map<std::string, sp<AppOpsListener>> mCallbackRegistered;



    std::atomic_bool mSystemReady = false;

    StorageId mNextId = 0;

services/incremental/IncrementalServiceValidation.h

+10 −9
Original line number Diff line number Diff line
@@ -41,7 +41,8 @@ inline int fromBinderStatus(const binder::Status& status) { 
            : -EIO;

}



inline binder::Status CheckPermissionForDataDelivery(const char* permission, const char* operation) {

inline binder::Status CheckPermissionForDataDelivery(const char* permission, const char* operation,

                                                     const char* package) {

    using android::base::StringPrintf;



    int32_t pid;
@@ -52,23 +53,23 @@ inline binder::Status CheckPermissionForDataDelivery(const char* permission, con 
                         StringPrintf("UID %d / PID %d lacks permission %s", uid, pid, permission));

    }



    String16 packageName{package};



    // Caller must also have op granted.

    PermissionController pc;

    // Package is a required parameter. Need to obtain one.

    Vector<String16> packages;

    pc.getPackagesForUid(uid, packages);

    if (packages.empty()) {

    if (auto packageUid = pc.getPackageUid(packageName, 0); packageUid != uid) {

        return Exception(binder::Status::EX_SECURITY,

                         StringPrintf("UID %d / PID %d has no packages", uid, pid));

                         StringPrintf("UID %d / PID %d does not own package %s", uid, pid,

                                      package));

    }

    switch (auto result = pc.noteOp(String16(operation), uid, packages[0]); result) {

    switch (auto result = pc.noteOp(String16(operation), uid, packageName); result) {

        case PermissionController::MODE_ALLOWED:

        case PermissionController::MODE_DEFAULT:

            return binder::Status::ok();

        default:

            return Exception(binder::Status::EX_SECURITY,

                             StringPrintf("UID %d / PID %d lacks app-op %s, error %d", uid, pid,

                                          operation, result));

                             StringPrintf("UID %d / PID %d / package %s lacks app-op %s, error %d",

                                          uid, pid, package, operation, result));

    }

}