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

4 files changed, 39 insertions, 46 deletions

diff --git a/cmds/servicemanager/Access.cpp b/cmds/servicemanager/Access.cpp
index f4005c4dee..d936dbe3a2 100644
--- a/cmds/servicemanager/Access.cpp
+++ b/cmds/servicemanager/Access.cpp
@@ -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;
     }
 
diff --git a/cmds/servicemanager/Access.h b/cmds/servicemanager/Access.h
index b2c78cc34d..05a60d33f1 100644
--- a/cmds/servicemanager/Access.h
+++ b/cmds/servicemanager/Access.h
@@ -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;
 };
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index 6cfcf40970..c2c71e0770 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -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);
     }
 
diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp
index 25d2aa8bfa..91485e456b 100644
--- a/cmds/servicemanager/test_sm.cpp
+++ b/cmds/servicemanager/test_sm.cpp
@@ -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));