Updated DefaultVehicleHal to check VehicleAreaConfig.access (18b3db5a) · Commits · e / os / android_hardware_interfaces

automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h

+4 −0

Original line number	Diff line number	Diff line
		@@ -191,6 +191,10 @@ class DefaultVehicleHal final : public aidl::android::hardware::automotive::vehi
		const std::vector<aidl::android::hardware::automotive::vehicle::SubscribeOptions>&
		options);

		VhalResult<void> checkPermissionHelper(
		const aidl::android::hardware::automotive::vehicle::VehiclePropValue& value,
		aidl::android::hardware::automotive::vehicle::VehiclePropertyAccess accessToTest) const;

		VhalResult<void> checkReadPermission(
		const aidl::android::hardware::automotive::vehicle::VehiclePropValue& value) const;

automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp

+63 −33

Original line number	Diff line number	Diff line
		@@ -605,6 +605,23 @@ ScopedAStatus DefaultVehicleHal::getPropConfigs(const std::vector<int32_t>& prop
		return vectorToStableLargeParcelable(std::move(configs), output);
		}

		bool hasRequiredAccess(VehiclePropertyAccess access, VehiclePropertyAccess requiredAccess) {
		return access == requiredAccess \|\| access == VehiclePropertyAccess::READ_WRITE;
		}

		bool areaConfigsHaveRequiredAccess(const std::vector<VehicleAreaConfig>& areaConfigs,
		VehiclePropertyAccess requiredAccess) {
		if (areaConfigs.empty()) {
		return false;
		}
		for (VehicleAreaConfig areaConfig : areaConfigs) {
		if (!hasRequiredAccess(areaConfig.access, requiredAccess)) {
		return false;
		}
		}
		return true;
		}

		VhalResult<void> DefaultVehicleHal::checkSubscribeOptions(
		const std::vector<SubscribeOptions>& options) {
		for (const auto& option : options) {
		@@ -614,6 +631,26 @@ VhalResult<void> DefaultVehicleHal::checkSubscribeOptions(
		<< StringPrintf("no config for property, ID: %" PRId32, propId);
		}
		const VehiclePropConfig& config = mConfigsByPropId[propId];
		std::vector<VehicleAreaConfig> areaConfigs;
		if (option.areaIds.empty()) {
		areaConfigs = config.areaConfigs;
		} else {
		std::unordered_map<int, VehicleAreaConfig> areaConfigByAreaId;
		for (const VehicleAreaConfig& areaConfig : config.areaConfigs) {
		areaConfigByAreaId.emplace(areaConfig.areaId, areaConfig);
		}
		for (int areaId : option.areaIds) {
		auto it = areaConfigByAreaId.find(areaId);
		if (it != areaConfigByAreaId.end()) {
		areaConfigs.push_back(it->second);
		} else if (areaId != 0 \|\| !areaConfigByAreaId.empty()) {
		return StatusError(StatusCode::INVALID_ARG)
		<< StringPrintf("invalid area ID: %" PRId32 " for prop ID: %" PRId32
		", not listed in config",
		areaId, propId);
		}
		}
		}

		if (config.changeMode != VehiclePropertyChangeMode::ON_CHANGE &&
		config.changeMode != VehiclePropertyChangeMode::CONTINUOUS) {
		@@ -621,8 +658,9 @@ VhalResult<void> DefaultVehicleHal::checkSubscribeOptions(
		<< "only support subscribing to ON_CHANGE or CONTINUOUS property";
		}

		if (config.access != VehiclePropertyAccess::READ &&
		config.access != VehiclePropertyAccess::READ_WRITE) {
		// Either VehiclePropConfig.access or VehicleAreaConfig.access will be specified
		if (!hasRequiredAccess(config.access, VehiclePropertyAccess::READ) &&
		!areaConfigsHaveRequiredAccess(areaConfigs, VehiclePropertyAccess::READ)) {
		return StatusError(StatusCode::ACCESS_DENIED)
		<< StringPrintf("Property %" PRId32 " has no read access", propId);
		}
		@@ -644,20 +682,6 @@ VhalResult<void> DefaultVehicleHal::checkSubscribeOptions(
		<< "invalid sample rate: " << sampleRateHz << " HZ";
		}
		}

		if (isGlobalProp(propId)) {
		continue;
		}

		// Non-global property.
		for (int32_t areaId : option.areaIds) {
		if (auto areaConfig = getAreaConfig(propId, areaId, config); areaConfig == nullptr) {
		return StatusError(StatusCode::INVALID_ARG)
		<< StringPrintf("invalid area ID: %" PRId32 " for prop ID: %" PRId32
		", not listed in config",
		areaId, propId);
		}
		}
		}
		return {};
		}
		@@ -776,36 +800,42 @@ IVehicleHardware* DefaultVehicleHal::getHardware() {
		return mVehicleHardware.get();
		}

		VhalResult<void> DefaultVehicleHal::checkWritePermission(const VehiclePropValue& value) const {
		VhalResult<void> DefaultVehicleHal::checkPermissionHelper(
		const VehiclePropValue& value, VehiclePropertyAccess accessToTest) const {
		static const std::unordered_set<VehiclePropertyAccess> validAccesses = {
		VehiclePropertyAccess::WRITE, VehiclePropertyAccess::READ,
		VehiclePropertyAccess::READ_WRITE};
		if (validAccesses.find(accessToTest) == validAccesses.end()) {
		return StatusError(StatusCode::INVALID_ARG)
		<< "checkPermissionHelper parameter is an invalid access type";
		}

		int32_t propId = value.prop;
		auto result = getConfig(propId);
		if (!result.ok()) {
		return StatusError(StatusCode::INVALID_ARG) << getErrorMsg(result);
		}
		const VehiclePropConfig* config = result.value();
		const VehicleAreaConfig* areaConfig = getAreaConfig(value, *config);

		if (config->access != VehiclePropertyAccess::WRITE &&
		config->access != VehiclePropertyAccess::READ_WRITE) {
		if (areaConfig == nullptr && !isGlobalProp(propId)) {
		return StatusError(StatusCode::INVALID_ARG) << "no config for area ID: " << value.areaId;
		}
		if (!hasRequiredAccess(config->access, accessToTest) &&
		(areaConfig == nullptr \|\| !hasRequiredAccess(areaConfig->access, accessToTest))) {
		return StatusError(StatusCode::ACCESS_DENIED)
		<< StringPrintf("Property %" PRId32 " has no write access", propId);
		<< StringPrintf("Property %" PRId32 " does not have the following access: %" PRId32,
		propId, accessToTest);
		}
		return {};
		}

		VhalResult<void> DefaultVehicleHal::checkReadPermission(const VehiclePropValue& value) const {
		int32_t propId = value.prop;
		auto result = getConfig(propId);
		if (!result.ok()) {
		return StatusError(StatusCode::INVALID_ARG) << getErrorMsg(result);
		VhalResult<void> DefaultVehicleHal::checkWritePermission(const VehiclePropValue& value) const {
		return checkPermissionHelper(value, VehiclePropertyAccess::WRITE);
		}
		const VehiclePropConfig* config = result.value();

		if (config->access != VehiclePropertyAccess::READ &&
		config->access != VehiclePropertyAccess::READ_WRITE) {
		return StatusError(StatusCode::ACCESS_DENIED)
		<< StringPrintf("Property %" PRId32 " has no read access", propId);
		}
		return {};
		VhalResult<void> DefaultVehicleHal::checkReadPermission(const VehiclePropValue& value) const {
		return checkPermissionHelper(value, VehiclePropertyAccess::READ);
		}

		void DefaultVehicleHal::checkHealth(IVehicleHardware* vehicleHardware,

automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp

+110 −10

Original line number	Diff line number	Diff line
		@@ -100,6 +100,10 @@ constexpr int32_t READ_ONLY_PROP = 10006 + 0x10000000 + 0x01000000 + 0x00400000;
		constexpr int32_t WRITE_ONLY_PROP = 10007 + 0x10000000 + 0x01000000 + 0x00400000;
		// VehiclePropertyGroup:SYSTEM,VehicleArea:GLOBAL,VehiclePropertyType:INT32
		constexpr int32_t GLOBAL_CONTINUOUS_PROP_NO_VUR = 10008 + 0x10000000 + 0x01000000 + 0x00400000;
		// VehiclePropertyGroup:SYSTEM,VehicleArea:GLOBAL,VehiclePropertyType:INT32
		constexpr int32_t GLOBAL_NONE_ACCESS_PROP = 10009 + 0x10000000 + 0x01000000 + 0x00400000;
		// VehiclePropertyGroup:SYSTEM,VehicleArea:WINDOW,VehiclePropertyType:INT32
		constexpr int32_t AREA_NONE_ACCESS_PROP = 10010 + 0x10000000 + 0x03000000 + 0x00400000;

		int32_t testInt32VecProp(size_t i) {
		// VehiclePropertyGroup:SYSTEM,VehicleArea:GLOBAL,VehiclePropertyType:INT32_VEC
		@@ -175,6 +179,26 @@ std::vector<SetValuesInvalidRequestTestCase> getSetValuesInvalidRequestTestCases
		.value.int32Values = {0},
		},
		.expectedStatus = StatusCode::ACCESS_DENIED,
		},
		{
		.name = "none_access",
		.request =
		{
		.prop = GLOBAL_NONE_ACCESS_PROP,
		.value.int32Values = {0},
		},
		.expectedStatus = StatusCode::ACCESS_DENIED,
		},
		{
		.name = "none_area_access",
		.request =
		{
		.prop = AREA_NONE_ACCESS_PROP,
		.value.int32Values = {0},
		// Only ROW_1_LEFT is allowed.
		.areaId = toInt(VehicleAreaWindow::ROW_1_RIGHT),
		},
		.expectedStatus = StatusCode::ACCESS_DENIED,
		}};
		}

		@@ -228,11 +252,11 @@ class DefaultVehicleHalTest : public testing::Test {
		for (size_t i = 0; i < 10000; i++) {
		testConfigs.push_back(VehiclePropConfig{
		.prop = testInt32VecProp(i),
		.access = VehiclePropertyAccess::READ_WRITE,
		.areaConfigs =
		{
		{
		.areaId = 0,
		.access = VehiclePropertyAccess::READ_WRITE,
		.minInt32Value = 0,
		.maxInt32Value = 100,
		},
		@@ -242,9 +266,9 @@ class DefaultVehicleHalTest : public testing::Test {
		// A property with area config.
		testConfigs.push_back(
		VehiclePropConfig{.prop = INT32_WINDOW_PROP,
		.access = VehiclePropertyAccess::READ_WRITE,
		.areaConfigs = {{
		.areaId = toInt(VehicleAreaWindow::ROW_1_LEFT),
		.access = VehiclePropertyAccess::READ_WRITE,
		.minInt32Value = 0,
		.maxInt32Value = 100,
		}}});
		@@ -275,18 +299,19 @@ class DefaultVehicleHalTest : public testing::Test {
		// A per-area on-change property.
		testConfigs.push_back(VehiclePropConfig{
		.prop = AREA_ON_CHANGE_PROP,
		.access = VehiclePropertyAccess::READ_WRITE,
		.changeMode = VehiclePropertyChangeMode::ON_CHANGE,
		.areaConfigs =
		{
		{

		.areaId = toInt(VehicleAreaWindow::ROW_1_LEFT),
		.access = VehiclePropertyAccess::READ_WRITE,
		.minInt32Value = 0,
		.maxInt32Value = 100,
		},
		{
		.areaId = toInt(VehicleAreaWindow::ROW_1_RIGHT),
		.access = VehiclePropertyAccess::READ,
		.minInt32Value = 0,
		.maxInt32Value = 100,
		},
		@@ -295,7 +320,6 @@ class DefaultVehicleHalTest : public testing::Test {
		// A per-area continuous property.
		testConfigs.push_back(VehiclePropConfig{
		.prop = AREA_CONTINUOUS_PROP,
		.access = VehiclePropertyAccess::READ_WRITE,
		.changeMode = VehiclePropertyChangeMode::CONTINUOUS,
		.minSampleRate = 0.0,
		.maxSampleRate = 1000.0,
		@@ -304,12 +328,14 @@ class DefaultVehicleHalTest : public testing::Test {
		{

		.areaId = toInt(VehicleAreaWindow::ROW_1_LEFT),
		.access = VehiclePropertyAccess::READ_WRITE,
		.minInt32Value = 0,
		.maxInt32Value = 100,
		.supportVariableUpdateRate = true,
		},
		{
		.areaId = toInt(VehicleAreaWindow::ROW_1_RIGHT),
		.access = VehiclePropertyAccess::READ_WRITE,
		.minInt32Value = 0,
		.maxInt32Value = 100,
		.supportVariableUpdateRate = false,
		@@ -332,6 +358,37 @@ class DefaultVehicleHalTest : public testing::Test {
		.minSampleRate = 0.0,
		.maxSampleRate = 1000.0,
		});
		// Global access set to NONE
		testConfigs.push_back(VehiclePropConfig{
		.prop = GLOBAL_NONE_ACCESS_PROP,
		.access = VehiclePropertyAccess::NONE,
		.changeMode = VehiclePropertyChangeMode::CONTINUOUS,
		.minSampleRate = 0.0,
		.maxSampleRate = 100.0,
		});
		// Area access set to NONE
		testConfigs.push_back(VehiclePropConfig{
		.prop = AREA_NONE_ACCESS_PROP,
		.changeMode = VehiclePropertyChangeMode::CONTINUOUS,
		.minSampleRate = 0.0,
		.maxSampleRate = 1000.0,
		.areaConfigs =
		{
		{

		.areaId = toInt(VehicleAreaWindow::ROW_1_LEFT),
		.access = VehiclePropertyAccess::NONE,
		.minInt32Value = 0,
		.maxInt32Value = 100,
		},
		{
		.areaId = toInt(VehicleAreaWindow::ROW_1_RIGHT),
		.access = VehiclePropertyAccess::NONE,
		.minInt32Value = 0,
		.maxInt32Value = 100,
		},
		},
		});
		// Register the heartbeat event property.
		testConfigs.push_back(VehiclePropConfig{
		.prop = toInt(VehicleProperty::VHAL_HEARTBEAT),
		@@ -673,6 +730,21 @@ TEST_F(DefaultVehicleHalTest, testGetValuesNoReadPermission) {
		.prop = WRITE_ONLY_PROP,
		},
		},
		{
		.requestId = 1,
		.prop =
		{
		.prop = GLOBAL_NONE_ACCESS_PROP,
		},
		},
		{
		.requestId = 2,
		.prop =
		{
		.prop = AREA_NONE_ACCESS_PROP,
		.areaId = toInt(VehicleAreaWindow::ROW_1_LEFT),
		},
		},
		},
		};

		@@ -690,6 +762,14 @@ TEST_F(DefaultVehicleHalTest, testGetValuesNoReadPermission) {
		.requestId = 0,
		.status = StatusCode::ACCESS_DENIED,
		},
		{
		.requestId = 1,
		.status = StatusCode::ACCESS_DENIED,
		},
		{
		.requestId = 2,
		.status = StatusCode::ACCESS_DENIED,
		},
		}))
		<< "expect to get ACCESS_DENIED status if no read permission";
		}
		@@ -1316,8 +1396,8 @@ TEST_F(DefaultVehicleHalTest, testSubscribeAreaOnChangeAllAreas) {

		auto maybeResults = getCallback()->nextOnPropertyEventResults();
		ASSERT_TRUE(maybeResults.has_value()) << "no results in callback";
		ASSERT_THAT(maybeResults.value().payloads, UnorderedElementsAre(testValue1, testValue2))
		<< "results mismatch, expect two on-change events for all updated areas";
		ASSERT_THAT(maybeResults.value().payloads, UnorderedElementsAre(testValue1))
		<< "results mismatch, expect one on-change events for all updated areas";
		ASSERT_FALSE(getCallback()->nextOnPropertyEventResults().has_value())
		<< "more results than expected";
		}
		@@ -1627,6 +1707,27 @@ TEST_F(DefaultVehicleHalTest, testSubscribeNoReadPermission) {
		ASSERT_EQ(status.getServiceSpecificError(), toInt(StatusCode::ACCESS_DENIED));
		}

		TEST_F(DefaultVehicleHalTest, testSubscribeGlobalNoneAccess) {
		std::vector<SubscribeOptions> options = {{
		.propId = GLOBAL_NONE_ACCESS_PROP,
		}};

		auto status = getClient()->subscribe(getCallbackClient(), options, 0);

		ASSERT_FALSE(status.isOk()) << "subscribe to a property with NONE global access must fail";
		ASSERT_EQ(status.getServiceSpecificError(), toInt(StatusCode::ACCESS_DENIED));
		}

		TEST_F(DefaultVehicleHalTest, testSubscribeAreaNoneAccess) {
		std::vector<SubscribeOptions> options = {
		{.propId = AREA_NONE_ACCESS_PROP, .areaIds = {toInt(VehicleAreaWindow::ROW_1_LEFT)}}};

		auto status = getClient()->subscribe(getCallbackClient(), options, 0);

		ASSERT_FALSE(status.isOk()) << "subscribe to a property with NONE area access must fail";
		ASSERT_EQ(status.getServiceSpecificError(), toInt(StatusCode::ACCESS_DENIED));
		}

		TEST_F(DefaultVehicleHalTest, testUnsubscribeFailure) {
		auto status = getClient()->unsubscribe(getCallbackClient(),
		std::vector<int32_t>({GLOBAL_ON_CHANGE_PROP}));
		@@ -1881,7 +1982,7 @@ TEST_F(DefaultVehicleHalTest, testBatchOnPropertyChangeEvents) {
		};
		SetValueResult result3 = {
		.requestId = 3,
		.status = StatusCode::OK,
		.status = StatusCode::ACCESS_DENIED,
		};
		// Prepare the responses
		for (int i = 0; i < 2; i++) {
		@@ -1907,9 +2008,8 @@ TEST_F(DefaultVehicleHalTest, testBatchOnPropertyChangeEvents) {

		auto maybeResults = getCallback()->nextOnPropertyEventResults();
		ASSERT_TRUE(maybeResults.has_value()) << "no results in callback";
		ASSERT_THAT(maybeResults.value().payloads,
		UnorderedElementsAre(testValue1, testValue2, testValue3))
		<< "results mismatch, expect 3 batched on change events";
		ASSERT_THAT(maybeResults.value().payloads, UnorderedElementsAre(testValue1, testValue2))
		<< "results mismatch, expect 2 batched on change events";
		ASSERT_FALSE(getCallback()->nextOnPropertyEventResults().has_value())
		<< "more results than expected";
		}