ACL based on getCallingSid
Bug: 121035042
Test: boot
Change-Id: I07c298c3969c3753ba6af4462661dd9c76125c50
Merged-In: I07c298c3969c3753ba6af4462661dd9c76125c50
(cherry picked from commit d1d10ca12777ec60f4aea77972a220d369612d86)
diff --git a/AccessControl.cpp b/AccessControl.cpp
index fac4dc1..84f84d0 100644
--- a/AccessControl.cpp
+++ b/AccessControl.cpp
@@ -12,11 +12,11 @@
struct audit_data {
const char* interfaceName;
+ const char* sid;
pid_t pid;
};
using android::FQName;
-using Context = AccessControl::Context;
AccessControl::AccessControl() {
mSeHandle = selinux_android_hw_service_context_handle();
@@ -35,7 +35,7 @@
selinux_set_callback(SELINUX_CB_LOG, mSeCallbacks);
}
-bool AccessControl::canAdd(const std::string& fqName, const Context &context, pid_t pid) {
+bool AccessControl::canAdd(const std::string& fqName, const CallingContext& callingContext) {
FQName fqIface(fqName);
if (!fqIface.isValid()) {
@@ -43,10 +43,10 @@
}
const std::string checkName = fqIface.package() + "::" + fqIface.name();
- return checkPermission(context, pid, kPermissionAdd, checkName.c_str());
+ return checkPermission(callingContext, kPermissionAdd, checkName.c_str());
}
-bool AccessControl::canGet(const std::string& fqName, pid_t pid) {
+bool AccessControl::canGet(const std::string& fqName, const CallingContext& callingContext) {
FQName fqIface(fqName);
if (!fqIface.isValid()) {
@@ -54,43 +54,46 @@
}
const std::string checkName = fqIface.package() + "::" + fqIface.name();
- return checkPermission(getContext(pid), pid, kPermissionGet, checkName.c_str());
+ return checkPermission(callingContext, kPermissionGet, checkName.c_str());
}
-bool AccessControl::canList(pid_t pid) {
- return checkPermission(getContext(pid), pid, mSeContext, kPermissionList, nullptr);
+bool AccessControl::canList(const CallingContext& callingContext) {
+ return checkPermission(callingContext, mSeContext, kPermissionList, nullptr);
}
-Context AccessControl::getContext(pid_t sourcePid) {
- char *sourceContext = NULL;
+AccessControl::CallingContext AccessControl::getCallingContext(pid_t sourcePid) {
+ char *sourceContext = nullptr;
if (getpidcon(sourcePid, &sourceContext) < 0) {
ALOGE("SELinux: failed to retrieve process context for pid %d", sourcePid);
- return Context(nullptr, freecon);
+ return { false, "", sourcePid };
}
- return Context(sourceContext, freecon);
+ std::string context = sourceContext;
+ freecon(sourceContext);
+ return { true, context, sourcePid };
}
-bool AccessControl::checkPermission(const Context &context, pid_t sourceAuditPid, const char *targetContext, const char *perm, const char *interface) {
- if (context == nullptr) {
+bool AccessControl::checkPermission(const CallingContext& source, const char *targetContext, const char *perm, const char *interface) {
+ if (!source.sidPresent) {
return false;
}
bool allowed = false;
- struct audit_data ad;
- ad.pid = sourceAuditPid;
+ struct audit_data ad;
+ ad.pid = source.pid;
+ ad.sid = source.sid.c_str();
ad.interfaceName = interface;
- allowed = (selinux_check_access(context.get(), targetContext, "hwservice_manager",
+ allowed = (selinux_check_access(source.sid.c_str(), targetContext, "hwservice_manager",
perm, (void *) &ad) == 0);
return allowed;
}
-bool AccessControl::checkPermission(const Context &context, pid_t sourceAuditPid, const char *perm, const char *interface) {
- char *targetContext = NULL;
+bool AccessControl::checkPermission(const CallingContext& source, const char *perm, const char *interface) {
+ char *targetContext = nullptr;
bool allowed = false;
// Lookup service in hwservice_contexts
@@ -99,7 +102,7 @@
return false;
}
- allowed = checkPermission(context, sourceAuditPid, targetContext, perm, interface);
+ allowed = checkPermission(source, targetContext, perm, interface);
freecon(targetContext);
@@ -114,7 +117,9 @@
return 0;
}
- snprintf(buf, len, "interface=%s pid=%d", ad->interfaceName, ad->pid);
+ const char* sid = ad->sid ? ad->sid : "N/A";
+
+ snprintf(buf, len, "interface=%s sid=%s pid=%d", ad->interfaceName, sid, ad->pid);
return 0;
}
diff --git a/AccessControl.h b/AccessControl.h
index 63a2098..877df99 100644
--- a/AccessControl.h
+++ b/AccessControl.h
@@ -9,17 +9,21 @@
public:
AccessControl();
- using Context = std::unique_ptr<char, decltype(&freecon)>;
- Context getContext(pid_t sourcePid);
+ struct CallingContext {
+ bool sidPresent;
+ std::string sid;
+ pid_t pid;
+ };
+ static CallingContext getCallingContext(pid_t sourcePid);
- bool canAdd(const std::string& fqName, const Context &context, pid_t pid);
- bool canGet(const std::string& fqName, pid_t pid);
- bool canList(pid_t pid);
+ bool canAdd(const std::string& fqName, const CallingContext& callingContext);
+ bool canGet(const std::string& fqName, const CallingContext& callingContext);
+ bool canList(const CallingContext& callingContext);
private:
- bool checkPermission(const Context &context, pid_t sourceAuditPid, const char *targetContext, const char *perm, const char *interface);
- bool checkPermission(const Context &context, pid_t sourcePid, const char *perm, const char *interface);
+ bool checkPermission(const CallingContext& source, const char *targetContext, const char *perm, const char *interface);
+ bool checkPermission(const CallingContext& source, const char *perm, const char *interface);
static int auditCallback(void *data, security_class_t cls, char *buf, size_t len);
diff --git a/HidlService.cpp b/HidlService.cpp
index a0f2cab..dad7bac 100644
--- a/HidlService.cpp
+++ b/HidlService.cpp
@@ -31,7 +31,7 @@
sendRegistrationNotifications();
}
-pid_t HidlService::getPid() const {
+pid_t HidlService::getDebugPid() const {
return mPid;
}
const std::string &HidlService::getInterfaceName() const {
diff --git a/HidlService.h b/HidlService.h
index 2fcb0d6..64fdabb 100644
--- a/HidlService.h
+++ b/HidlService.h
@@ -41,7 +41,7 @@
*/
sp<IBase> getService() const;
void setService(sp<IBase> service, pid_t pid);
- pid_t getPid() const;
+ pid_t getDebugPid() const;
const std::string &getInterfaceName() const;
const std::string &getInstanceName() const;
diff --git a/ServiceManager.cpp b/ServiceManager.cpp
index 0705c71..9c1c788 100644
--- a/ServiceManager.cpp
+++ b/ServiceManager.cpp
@@ -17,6 +17,23 @@
namespace manager {
namespace implementation {
+AccessControl::CallingContext getBinderCallingContext() {
+ const auto& self = IPCThreadState::self();
+
+ pid_t pid = self->getCallingPid();
+ const char* sid = self->getCallingSid();
+
+ if (sid == nullptr) {
+ if (pid != getpid()) {
+ android_errorWriteLog(0x534e4554, "121035042");
+ }
+
+ return AccessControl::getCallingContext(pid);
+ } else {
+ return { true, sid, pid };
+ }
+}
+
static constexpr uint64_t kServiceDiedCookie = 0;
static constexpr uint64_t kPackageListenerDiedCookie = 1;
static constexpr uint64_t kServiceListenerDiedCookie = 2;
@@ -164,8 +181,7 @@
// Methods from ::android::hidl::manager::V1_0::IServiceManager follow.
Return<sp<IBase>> ServiceManager::get(const hidl_string& fqName,
const hidl_string& name) {
- pid_t pid = IPCThreadState::self()->getCallingPid();
- if (!mAcl.canGet(fqName, pid)) {
+ if (!mAcl.canGet(fqName, getBinderCallingContext())) {
return nullptr;
}
@@ -191,10 +207,7 @@
return false;
}
- // TODO(b/34235311): use HIDL way to determine this
- // also, this assumes that the PID that is registering is the pid that is the service
- pid_t pid = IPCThreadState::self()->getCallingPid();
- auto context = mAcl.getContext(pid);
+ auto callingContext = getBinderCallingContext();
auto ret = service->interfaceChain([&](const auto &interfaceChain) {
if (interfaceChain.size() == 0) {
@@ -205,7 +218,7 @@
for(size_t i = 0; i < interfaceChain.size(); i++) {
std::string fqName = interfaceChain[i];
- if (!mAcl.canAdd(fqName, context, pid)) {
+ if (!mAcl.canAdd(fqName, callingContext)) {
return;
}
}
@@ -218,13 +231,13 @@
if (hidlService == nullptr) {
ifaceMap.insertService(
- std::make_unique<HidlService>(fqName, name, service, pid));
+ std::make_unique<HidlService>(fqName, name, service, callingContext.pid));
} else {
if (hidlService->getService() != nullptr) {
auto ret = hidlService->getService()->unlinkToDeath(this);
ret.isOk(); // ignore
}
- hidlService->setService(service, pid);
+ hidlService->setService(service, callingContext.pid);
}
ifaceMap.sendPackageRegistrationNotification(fqName, name);
@@ -248,8 +261,7 @@
const hidl_string& name) {
using ::android::hardware::getTransport;
- pid_t pid = IPCThreadState::self()->getCallingPid();
- if (!mAcl.canGet(fqName, pid)) {
+ if (!mAcl.canGet(fqName, getBinderCallingContext())) {
return Transport::EMPTY;
}
@@ -265,8 +277,7 @@
}
Return<void> ServiceManager::list(list_cb _hidl_cb) {
- pid_t pid = IPCThreadState::self()->getCallingPid();
- if (!mAcl.canList(pid)) {
+ if (!mAcl.canList(getBinderCallingContext())) {
_hidl_cb({});
return Void();
}
@@ -286,8 +297,7 @@
Return<void> ServiceManager::listByInterface(const hidl_string& fqName,
listByInterface_cb _hidl_cb) {
- pid_t pid = IPCThreadState::self()->getCallingPid();
- if (!mAcl.canGet(fqName, pid)) {
+ if (!mAcl.canGet(fqName, getBinderCallingContext())) {
_hidl_cb({});
return Void();
}
@@ -330,8 +340,7 @@
return false;
}
- pid_t pid = IPCThreadState::self()->getCallingPid();
- if (!mAcl.canGet(fqName, pid)) {
+ if (!mAcl.canGet(fqName, getBinderCallingContext())) {
return false;
}
@@ -403,8 +412,7 @@
}
Return<void> ServiceManager::debugDump(debugDump_cb _cb) {
- pid_t pid = IPCThreadState::self()->getCallingPid();
- if (!mAcl.canList(pid)) {
+ if (!mAcl.canList(getBinderCallingContext())) {
_cb({});
return Void();
}
@@ -420,7 +428,7 @@
}
list.push_back({
- .pid = service->getPid(),
+ .pid = service->getDebugPid(),
.interfaceName = service->getInterfaceName(),
.instanceName = service->getInstanceName(),
.clientPids = clientPids,
@@ -435,8 +443,9 @@
Return<void> ServiceManager::registerPassthroughClient(const hidl_string &fqName,
const hidl_string &name) {
- pid_t pid = IPCThreadState::self()->getCallingPid();
- if (!mAcl.canGet(fqName, pid)) {
+ auto callingContext = getBinderCallingContext();
+
+ if (!mAcl.canGet(fqName, callingContext)) {
/* We guard this function with "get", because it's typically used in
* the getService() path, albeit for a passthrough service in this
* case
@@ -456,10 +465,10 @@
if (service == nullptr) {
auto adding = std::make_unique<HidlService>(fqName, name);
- adding->registerPassthroughClient(pid);
+ adding->registerPassthroughClient(callingContext.pid);
ifaceMap.insertService(std::move(adding));
} else {
- service->registerPassthroughClient(pid);
+ service->registerPassthroughClient(callingContext.pid);
}
return Void();
}
diff --git a/service.cpp b/service.cpp
index a7863af..4fdf4df 100644
--- a/service.cpp
+++ b/service.cpp
@@ -10,6 +10,7 @@
#include <android/hidl/token/1.0/ITokenManager.h>
#include <cutils/properties.h>
#include <hidl/Status.h>
+#include <hwbinder/binder_kernel.h>
#include <hwbinder/IPCThreadState.h>
#include <utils/Errors.h>
#include <utils/Looper.h>
@@ -33,6 +34,7 @@
using android::hardware::configureRpcThreadpool;
using android::hardware::hidl_string;
using android::hardware::hidl_vec;
+using android::hardware::setRequestingSid;
// hidl types
using android::hidl::manager::V1_0::BnHwServiceManager;
@@ -59,7 +61,8 @@
int main() {
configureRpcThreadpool(1, true /* callerWillJoin */);
- ServiceManager *manager = new ServiceManager();
+ sp<ServiceManager> manager = new ServiceManager();
+ setRequestingSid(manager, true);
if (!manager->add(serviceName, manager)) {
ALOGE("Failed to register hwservicemanager with itself.");
@@ -95,7 +98,19 @@
sp<BnHwServiceManager> service = new BnHwServiceManager(manager);
IPCThreadState::self()->setTheContextObject(service);
// Then tell binder kernel
- ioctl(binder_fd, BINDER_SET_CONTEXT_MGR, 0);
+ flat_binder_object obj {
+ .flags = FLAT_BINDER_FLAG_TXN_SECURITY_CTX,
+ };
+
+ status_t result = ioctl(binder_fd, BINDER_SET_CONTEXT_MGR_EXT, &obj);
+
+ // fallback to original method
+ if (result != 0) {
+ android_errorWriteLog(0x534e4554, "121035042");
+
+ result = ioctl(binder_fd, BINDER_SET_CONTEXT_MGR, 0);
+ }
+
// Only enable FIFO inheritance for hwbinder
// FIXME: remove define when in the kernel
#define BINDER_SET_INHERIT_FIFO_PRIO _IO('b', 10)
