Vehicle HAL: return ACCESS_DENIED when appropriate
Fix: b/34359485
Test: unit tests updated
Change-Id: I48bd7b5eb54109e333fffd2ed562597f89cc8a27
diff --git a/vehicle/2.0/default/tests/VehicleHalManager_test.cpp b/vehicle/2.0/default/tests/VehicleHalManager_test.cpp
index dffcfbb..4a20ea5 100644
--- a/vehicle/2.0/default/tests/VehicleHalManager_test.cpp
+++ b/vehicle/2.0/default/tests/VehicleHalManager_test.cpp
@@ -353,7 +353,7 @@
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);
diff --git a/vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.cpp b/vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.cpp
index b4eb484..5d2e6fe 100644
--- a/vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.cpp
+++ b/vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.cpp
@@ -87,7 +87,7 @@
}
if (!checkReadPermission(*config, getCaller())) {
- _hidl_cb(StatusCode::INVALID_ARG, kEmptyValue);
+ _hidl_cb(StatusCode::ACCESS_DENIED, kEmptyValue);
return Void();
}
@@ -108,7 +108,7 @@
}
if (!checkWritePermission(*config, getCaller())) {
- return StatusCode::INVALID_ARG;
+ return StatusCode::ACCESS_DENIED;
}
handlePropertySetEvent(value);
@@ -122,6 +122,7 @@
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::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 @@
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 @@
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 @@
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) {
diff --git a/vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.h b/vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.h
index 519b09b..6768741 100644
--- a/vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.h
+++ b/vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.h
@@ -100,6 +100,9 @@
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);