Vehicle HAL: return ACCESS_DENIED when appropriate

Fix: b/34359485

Test: unit tests updated
Change-Id: I48bd7b5eb54109e333fffd2ed562597f89cc8a27
This commit is contained in:
Pavel Maltsev 2017-01-17 17:45:00 -08:00
parent 387f9e775f
commit 85db07a681
3 changed files with 16 additions and 12 deletions

View file

@ -353,7 +353,7 @@ TEST_F(VehicleHalManagerTest, get_StaticString) {
TEST_F(VehicleHalManagerTest, get_NegativeCases) {
// Write-only property must fail.
invokeGet(VehicleProperty::HVAC_SEAT_TEMPERATURE, 0);
ASSERT_EQ(StatusCode::INVALID_ARG, actualStatusCode);
ASSERT_EQ(StatusCode::ACCESS_DENIED, actualStatusCode);
// Unknown property must fail.
invokeGet(VehicleProperty::MIRROR_Z_MOVE, 0);

View file

@ -87,7 +87,7 @@ Return<void> VehicleHalManager::get(
}
if (!checkReadPermission(*config, getCaller())) {
_hidl_cb(StatusCode::INVALID_ARG, kEmptyValue);
_hidl_cb(StatusCode::ACCESS_DENIED, kEmptyValue);
return Void();
}
@ -108,7 +108,7 @@ Return<StatusCode> VehicleHalManager::set(const VehiclePropValue &value) {
}
if (!checkWritePermission(*config, getCaller())) {
return StatusCode::INVALID_ARG;
return StatusCode::ACCESS_DENIED;
}
handlePropertySetEvent(value);
@ -122,6 +122,7 @@ Return<StatusCode> VehicleHalManager::subscribe(
const sp<IVehicleCallback> &callback,
const hidl_vec<SubscribeOptions> &options) {
hidl_vec<SubscribeOptions> verifiedOptions(options);
auto caller = getCaller();
for (size_t i = 0; i < verifiedOptions.size(); i++) {
SubscribeOptions& ops = verifiedOptions[i];
VehicleProperty prop = ops.propId;
@ -133,6 +134,10 @@ Return<StatusCode> VehicleHalManager::subscribe(
return StatusCode::INVALID_ARG;
}
if (!checkAcl(caller.uid, config->prop, VehiclePropertyAccess::READ)) {
return StatusCode::ACCESS_DENIED;
}
if (!isSubscribable(*config, ops.flags)) {
ALOGE("Failed to subscribe: property 0x%x is not subscribable",
prop);
@ -304,15 +309,13 @@ bool VehicleHalManager::isSubscribable(const VehiclePropConfig& config,
return true;
}
bool checkAcl(const PropertyAclMap& aclMap,
uid_t callerUid,
VehicleProperty propertyId,
VehiclePropertyAccess requiredAccess) {
bool VehicleHalManager::checkAcl(uid_t callerUid, VehicleProperty propertyId,
VehiclePropertyAccess requiredAccess) const {
if (callerUid == AID_SYSTEM && isSystemProperty(propertyId)) {
return true;
}
auto range = aclMap.equal_range(propertyId);
auto range = mPropertyAclMap.equal_range(propertyId);
for (auto it = range.first; it != range.second; ++it) {
auto& acl = it->second;
if (acl.uid == callerUid && (acl.access & requiredAccess)) {
@ -328,8 +331,7 @@ bool VehicleHalManager::checkWritePermission(const VehiclePropConfig &config,
ALOGW("Property 0%x has no write access", config.prop);
return false;
}
return checkAcl(mPropertyAclMap, caller.uid, config.prop,
VehiclePropertyAccess::WRITE);
return checkAcl(caller.uid, config.prop, VehiclePropertyAccess::WRITE);
}
bool VehicleHalManager::checkReadPermission(const VehiclePropConfig &config,
@ -339,8 +341,7 @@ bool VehicleHalManager::checkReadPermission(const VehiclePropConfig &config,
return false;
}
return checkAcl(mPropertyAclMap, caller.uid, config.prop,
VehiclePropertyAccess::READ);
return checkAcl(caller.uid, config.prop, VehiclePropertyAccess::READ);
}
void VehicleHalManager::handlePropertySetEvent(const VehiclePropValue& value) {

View file

@ -100,6 +100,9 @@ private:
const Caller& callee) const;
bool checkReadPermission(const VehiclePropConfig &config,
const Caller& caller) const;
bool checkAcl(uid_t callerUid,
VehicleProperty propertyId,
VehiclePropertyAccess requiredAccess) const;
static bool isSubscribable(const VehiclePropConfig& config,
SubscribeFlags flags);