Merge "CallingContext doesn't have a name in it." into stage-aosp-master am: 4172141b88 am: 9994bc2cb5
am: 41dac28ac6
Change-Id: Icf5a1039bbbda665e190ff3c9c44c27e30809982
diff --git a/cmds/servicemanager/Access.cpp b/cmds/servicemanager/Access.cpp
index f4005c4..d936dbe 100644
--- a/cmds/servicemanager/Access.cpp
+++ b/cmds/servicemanager/Access.cpp
@@ -69,7 +69,7 @@
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 @@
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 @@
.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 @@
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 b2c78cc..05a60d3 100644
--- a/cmds/servicemanager/Access.h
+++ b/cmds/servicemanager/Access.h
@@ -36,22 +36,18 @@
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 6cfcf40..c2c71e0 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -32,7 +32,7 @@
}
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 @@
}
// 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 @@
}
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::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 25d2aa8..91485e4 100644
--- a/cmds/servicemanager/test_sm.cpp
+++ b/cmds/servicemanager/test_sm.cpp
@@ -40,18 +40,18 @@
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, 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, 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, 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, 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, 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(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));