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

Commit a9fe4743 authored by Steven Moreland's avatar Steven Moreland
Browse files

CallingContext doesn't have a name in it. 

This makes checking multiple things in the policy painful (which we may
need to do in the future). Also, it makes the APIs around listing
services somewhat bad.

Bug: 135768100
Test: servicemanager_test
Change-Id: I29cac39f6b63934740aa07a192e06b74ce8580ed
parent 8f4c72d3

cmds/servicemanager/Access.cpp

+9 −12
Original line number Diff line number Diff line
@@ -69,7 +69,7 @@ static int auditCallback(void *data, security_class_t /*cls*/, char *buf, size_t 
        return 0;

    }



    snprintf(buf, len, "service=%s pid=%d uid=%d", ad->name.c_str(), ad->debugPid, ad->uid);

    snprintf(buf, len, "pid=%d uid=%d", ad->debugPid, ad->uid);

    return 0;

}
@@ -91,7 +91,7 @@ Access::~Access() { 
    freecon(mThisProcessContext);

}



Access::CallingContext Access::getCallingContext(const std::string& name) {

Access::CallingContext Access::getCallingContext() {

    IPCThreadState* ipc = IPCThreadState::self();



    const char* callingSid = ipc->getCallingSid();
@@ -101,21 +101,18 @@ Access::CallingContext Access::getCallingContext(const std::string& name) { 
        .debugPid = callingPid,

        .uid = ipc->getCallingUid(),

        .sid = callingSid ? std::string(callingSid) : getPidcon(callingPid),

        .name = name,

    };

}



bool Access::canFind(const CallingContext& ctx) {

    return actionAllowedFromLookup(ctx, "find");

bool Access::canFind(const CallingContext& ctx,const std::string& name) {

    return actionAllowedFromLookup(ctx, name, "find");

}



bool Access::canAdd(const CallingContext& ctx) {

    return actionAllowedFromLookup(ctx, "add");

bool Access::canAdd(const CallingContext& ctx, const std::string& name) {

    return actionAllowedFromLookup(ctx, name, "add");

}



bool Access::canList(const CallingContext& ctx) {

    CHECK(ctx.name == "");



    return actionAllowed(ctx, mThisProcessContext, "list");

}
@@ -125,10 +122,10 @@ bool Access::actionAllowed(const CallingContext& sctx, const char* tctx, const c 
    return 0 == selinux_check_access(sctx.sid.c_str(), tctx, tclass, perm, reinterpret_cast<void*>(const_cast<CallingContext*>((&sctx))));

}



bool Access::actionAllowedFromLookup(const CallingContext& sctx, const char *perm) {

bool Access::actionAllowedFromLookup(const CallingContext& sctx, const std::string& name, const char *perm) {

    char *tctx = nullptr;

    if (selabel_lookup(getSehandle(), &tctx, sctx.name.c_str(), 0) != 0) {

        LOG(ERROR) << "SELinux: No match for " << sctx.name << " in service_contexts.\n";

    if (selabel_lookup(getSehandle(), &tctx, name.c_str(), 0) != 0) {

        LOG(ERROR) << "SELinux: No match for " << name << " in service_contexts.\n";

        return false;

    }

cmds/servicemanager/Access.h

+5 −9
Original line number Diff line number Diff line
@@ -36,22 +36,18 @@ public: 
        pid_t debugPid;

        uid_t uid;

        std::string sid;



        // name of the service

        //

        // empty if call is unrelated to service (e.g. list)

        std::string name;

    };



    virtual CallingContext getCallingContext(const std::string& name);

    virtual CallingContext getCallingContext();



    virtual bool canFind(const CallingContext& ctx);

    virtual bool canAdd(const CallingContext& ctx);

    virtual bool canFind(const CallingContext& ctx, const std::string& name);

    virtual bool canAdd(const CallingContext& ctx, const std::string& name);

    virtual bool canList(const CallingContext& ctx);



private:

    bool actionAllowed(const CallingContext& sctx, const char* tctx, const char* perm);

    bool actionAllowedFromLookup(const CallingContext& sctx, const char *perm);

    bool actionAllowedFromLookup(const CallingContext& sctx, const std::string& name,

            const char *perm);



    char* mThisProcessContext = nullptr;

};

cmds/servicemanager/ServiceManager.cpp

+5 −5
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ Status ServiceManager::getService(const std::string& name, sp<IBinder>* outBinde 
}



Status ServiceManager::checkService(const std::string& name, sp<IBinder>* outBinder) {

    auto ctx = mAccess->getCallingContext(name);

    auto ctx = mAccess->getCallingContext();



    auto it = mNameToService.find(name);

    if (it == mNameToService.end()) {
@@ -53,7 +53,7 @@ Status ServiceManager::checkService(const std::string& name, sp<IBinder>* outBin 
    }



    // TODO(b/136023468): move this check to be first

    if (!mAccess->canFind(ctx)) {

    if (!mAccess->canFind(ctx, name)) {

        // returns ok and null for legacy reasons

        *outBinder = nullptr;

        return Status::ok();
@@ -79,14 +79,14 @@ bool isValidServiceName(const std::string& name) { 
}



Status ServiceManager::addService(const std::string& name, const sp<IBinder>& binder, bool allowIsolated, int32_t dumpPriority) {

    auto ctx = mAccess->getCallingContext(name);

    auto ctx = mAccess->getCallingContext();



    // apps cannot add services

    if (multiuser_get_app_id(ctx.uid) >= AID_APP) {

        return Status::fromExceptionCode(Status::EX_SECURITY);

    }



    if (!mAccess->canAdd(ctx)) {

    if (!mAccess->canAdd(ctx, name)) {

        return Status::fromExceptionCode(Status::EX_SECURITY);

    }
@@ -121,7 +121,7 @@ Status ServiceManager::addService(const std::string& name, const sp<IBinder>& bi 
}



Status ServiceManager::listServices(int32_t dumpPriority, std::vector<std::string>* outList) {

    if (!mAccess->canList(mAccess->getCallingContext(""))) {

    if (!mAccess->canList(mAccess->getCallingContext())) {

        return Status::fromExceptionCode(Status::EX_SECURITY);

    }

cmds/servicemanager/test_sm.cpp

+20 −20
Original line number Diff line number Diff line
@@ -40,18 +40,18 @@ static sp<IBinder> getBinder() { 


class MockAccess : public Access {

public:

    MOCK_METHOD1(getCallingContext, CallingContext(const std::string& name));

    MOCK_METHOD1(canAdd, bool(const CallingContext&));

    MOCK_METHOD1(canFind, bool(const CallingContext&));

    MOCK_METHOD0(getCallingContext, CallingContext());

    MOCK_METHOD2(canAdd, bool(const CallingContext&, const std::string& name));

    MOCK_METHOD2(canFind, bool(const CallingContext&, const std::string& name));

    MOCK_METHOD1(canList, bool(const CallingContext&));

};



static sp<ServiceManager> getPermissiveServiceManager() {

    std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();



    ON_CALL(*access, getCallingContext(_)).WillByDefault(Return(Access::CallingContext{}));

    ON_CALL(*access, canAdd(_)).WillByDefault(Return(true));

    ON_CALL(*access, canFind(_)).WillByDefault(Return(true));

    ON_CALL(*access, getCallingContext()).WillByDefault(Return(Access::CallingContext{}));

    ON_CALL(*access, canAdd(_, _)).WillByDefault(Return(true));

    ON_CALL(*access, canFind(_, _)).WillByDefault(Return(true));

    ON_CALL(*access, canList(_)).WillByDefault(Return(true));



    sp<ServiceManager> sm = new ServiceManager(std::move(access));
@@ -97,11 +97,11 @@ TEST(AddService, AddNullServiceDisallowed) { 
TEST(AddService, AddDisallowedFromApp) {

    for (uid_t uid : { AID_APP_START, AID_APP_START + 1, AID_APP_END }) {

        std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();

        EXPECT_CALL(*access, getCallingContext(_)).WillOnce(Return(Access::CallingContext{

        EXPECT_CALL(*access, getCallingContext()).WillOnce(Return(Access::CallingContext{

            .debugPid = 1337,

            .uid = uid,

        }));

        EXPECT_CALL(*access, canAdd(_)).Times(0);

        EXPECT_CALL(*access, canAdd(_, _)).Times(0);

        sp<ServiceManager> sm = new ServiceManager(std::move(access));



        EXPECT_FALSE(sm->addService("foo", getBinder(), false /*allowIsolated*/,
@@ -121,8 +121,8 @@ TEST(AddService, HappyOverExistingService) { 
TEST(AddService, NoPermissions) {

    std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();



    EXPECT_CALL(*access, getCallingContext(_)).WillOnce(Return(Access::CallingContext{}));

    EXPECT_CALL(*access, canAdd(_)).WillOnce(Return(false));

    EXPECT_CALL(*access, getCallingContext()).WillOnce(Return(Access::CallingContext{}));

    EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(false));



    sp<ServiceManager> sm = new ServiceManager(std::move(access));
@@ -151,9 +151,9 @@ TEST(GetService, NonExistant) { 
TEST(GetService, NoPermissionsForGettingService) {

    std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();



    EXPECT_CALL(*access, getCallingContext(_)).WillRepeatedly(Return(Access::CallingContext{}));

    EXPECT_CALL(*access, canAdd(_)).WillOnce(Return(true));

    EXPECT_CALL(*access, canFind(_)).WillOnce(Return(false));

    EXPECT_CALL(*access, getCallingContext()).WillRepeatedly(Return(Access::CallingContext{}));

    EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(true));

    EXPECT_CALL(*access, canFind(_, _)).WillOnce(Return(false));



    sp<ServiceManager> sm = new ServiceManager(std::move(access));
@@ -169,15 +169,15 @@ TEST(GetService, NoPermissionsForGettingService) { 
TEST(GetService, AllowedFromIsolated) {

    std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();



    EXPECT_CALL(*access, getCallingContext(_))

    EXPECT_CALL(*access, getCallingContext())

        // something adds it

        .WillOnce(Return(Access::CallingContext{}))

        // next call is from isolated app

        .WillOnce(Return(Access::CallingContext{

            .uid = AID_ISOLATED_START,

        }));

    EXPECT_CALL(*access, canAdd(_)).WillOnce(Return(true));

    EXPECT_CALL(*access, canFind(_)).WillOnce(Return(true));

    EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(true));

    EXPECT_CALL(*access, canFind(_, _)).WillOnce(Return(true));



    sp<ServiceManager> sm = new ServiceManager(std::move(access));
@@ -192,17 +192,17 @@ TEST(GetService, AllowedFromIsolated) { 
TEST(GetService, NotAllowedFromIsolated) {

    std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();



    EXPECT_CALL(*access, getCallingContext(_))

    EXPECT_CALL(*access, getCallingContext())

        // something adds it

        .WillOnce(Return(Access::CallingContext{}))

        // next call is from isolated app

        .WillOnce(Return(Access::CallingContext{

            .uid = AID_ISOLATED_START,

        }));

    EXPECT_CALL(*access, canAdd(_)).WillOnce(Return(true));

    EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(true));



    // TODO(b/136023468): when security check is first, this should be called first

    // EXPECT_CALL(*access, canFind(_)).WillOnce(Return(true));

    // EXPECT_CALL(*access, canFind(_, _)).WillOnce(Return(true));



    sp<ServiceManager> sm = new ServiceManager(std::move(access));
@@ -218,7 +218,7 @@ TEST(GetService, NotAllowedFromIsolated) { 
TEST(ListServices, NoPermissions) {

    std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();



    EXPECT_CALL(*access, getCallingContext(_)).WillOnce(Return(Access::CallingContext{}));

    EXPECT_CALL(*access, getCallingContext()).WillOnce(Return(Access::CallingContext{}));

    EXPECT_CALL(*access, canList(_)).WillOnce(Return(false));



    sp<ServiceManager> sm = new ServiceManager(std::move(access));