Merge "Vehicle HAL: return ACCESS_DENIED when appropriate" (a34f6472) · Commits · e / os / android_hardware_interfaces

vehicle/2.0/default/tests/VehicleHalManager_test.cpp

+1 −1

Original line number	Diff line number	Diff line
		@@ -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);

vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.cpp

+12 −11

Original line number	Diff line number	Diff line
		@@ -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) {

vehicle/2.0/default/vehicle_hal_manager/VehicleHalManager.h

+3 −0

Original line number	Diff line number	Diff line
		@@ -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);