From 20c47b4bae6909f99b523850dc5ea1c3ac2f8084 Mon Sep 17 00:00:00 2001 From: ChengYou Ho Date: Wed, 30 Nov 2022 17:51:17 +0000 Subject: [PATCH 1/8] Add WeaverReadStatus to WeaverReadResponse Bug: 259556049 Change-Id: I83897038eda59ed8cae1bdb0ae68828201615ebc (cherry picked from commit 5c3a2b710ef80c109e9ff95b48ff112a68dd2ee4) Merged-In: I83897038eda59ed8cae1bdb0ae68828201615ebc --- weaver/aidl/Android.bp | 7 +++- .../hardware/weaver/WeaverReadResponse.aidl | 3 +- .../hardware/weaver/WeaverReadStatus.aidl | 41 +++++++++++++++++++ .../hardware/weaver/WeaverReadResponse.aidl | 11 ++++- .../hardware/weaver/WeaverReadStatus.aidl | 26 ++++++++++++ weaver/aidl/vts/Android.bp | 2 +- weaver/aidl/vts/VtsHalWeaverTargetTest.cpp | 28 +++++++++---- 7 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 weaver/aidl/aidl_api/android.hardware.weaver/current/android/hardware/weaver/WeaverReadStatus.aidl create mode 100644 weaver/aidl/android/hardware/weaver/WeaverReadStatus.aidl diff --git a/weaver/aidl/Android.bp b/weaver/aidl/Android.bp index caa92aa9b4..74cec99f7d 100644 --- a/weaver/aidl/Android.bp +++ b/weaver/aidl/Android.bp @@ -17,5 +17,10 @@ aidl_interface { platform_apis: true, }, }, - versions: ["1"], + versions_with_info: [ + { + version: "1", + imports: [], + }, + ], } diff --git a/weaver/aidl/aidl_api/android.hardware.weaver/current/android/hardware/weaver/WeaverReadResponse.aidl b/weaver/aidl/aidl_api/android.hardware.weaver/current/android/hardware/weaver/WeaverReadResponse.aidl index 47ee4c8a13..96e528fa30 100644 --- a/weaver/aidl/aidl_api/android.hardware.weaver/current/android/hardware/weaver/WeaverReadResponse.aidl +++ b/weaver/aidl/aidl_api/android.hardware.weaver/current/android/hardware/weaver/WeaverReadResponse.aidl @@ -1,5 +1,5 @@ /* - * Copyright 2020 The Android Open Source Project + * Copyright 2022 The Android Open Source Project * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,4 +36,5 @@ package android.hardware.weaver; parcelable WeaverReadResponse { long timeout; byte[] value; + android.hardware.weaver.WeaverReadStatus status = android.hardware.weaver.WeaverReadStatus.FAILED; } diff --git a/weaver/aidl/aidl_api/android.hardware.weaver/current/android/hardware/weaver/WeaverReadStatus.aidl b/weaver/aidl/aidl_api/android.hardware.weaver/current/android/hardware/weaver/WeaverReadStatus.aidl new file mode 100644 index 0000000000..fce9758f5c --- /dev/null +++ b/weaver/aidl/aidl_api/android.hardware.weaver/current/android/hardware/weaver/WeaverReadStatus.aidl @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.hardware.weaver; +@Backing(type="int") @VintfStability +enum WeaverReadStatus { + OK = 0, + FAILED = 1, + INCORRECT_KEY = 2, + THROTTLE = 3, +} diff --git a/weaver/aidl/android/hardware/weaver/WeaverReadResponse.aidl b/weaver/aidl/android/hardware/weaver/WeaverReadResponse.aidl index ec006e8c45..17ea718089 100644 --- a/weaver/aidl/android/hardware/weaver/WeaverReadResponse.aidl +++ b/weaver/aidl/android/hardware/weaver/WeaverReadResponse.aidl @@ -1,5 +1,5 @@ /* - * Copyright 2020 The Android Open Source Project + * Copyright 2022 The Android Open Source Project * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,15 +16,22 @@ package android.hardware.weaver; +import android.hardware.weaver.WeaverReadStatus; + @VintfStability parcelable WeaverReadResponse { /** - * The time to wait, in milliseconds, before making the next request. + * The time to wait, in milliseconds, before making the next request, + * must be greater than or equal to zero and less than INT_MAX. */ long timeout; /** * The value read from the slot or empty if the value was not read. */ byte[] value; + /** + * Status from WeaverReadStatus + */ + WeaverReadStatus status = WeaverReadStatus.FAILED; } diff --git a/weaver/aidl/android/hardware/weaver/WeaverReadStatus.aidl b/weaver/aidl/android/hardware/weaver/WeaverReadStatus.aidl new file mode 100644 index 0000000000..36e731fabe --- /dev/null +++ b/weaver/aidl/android/hardware/weaver/WeaverReadStatus.aidl @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.weaver; + +@VintfStability +@Backing(type="int") +enum WeaverReadStatus { + OK, + FAILED, + INCORRECT_KEY, + THROTTLE, +} diff --git a/weaver/aidl/vts/Android.bp b/weaver/aidl/vts/Android.bp index cf1661ce1e..557fe47aa7 100644 --- a/weaver/aidl/vts/Android.bp +++ b/weaver/aidl/vts/Android.bp @@ -34,7 +34,7 @@ cc_test { "libbinder_ndk", "libbase", ], - static_libs: ["android.hardware.weaver-V1-ndk"], + static_libs: ["android.hardware.weaver-V2-ndk"], test_suites: [ "general-tests", "vts", diff --git a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp b/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp index 878c76203f..f016515a6a 100644 --- a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp @@ -25,6 +25,7 @@ using ::aidl::android::hardware::weaver::IWeaver; using ::aidl::android::hardware::weaver::WeaverConfig; using ::aidl::android::hardware::weaver::WeaverReadResponse; +using ::aidl::android::hardware::weaver::WeaverReadStatus; using ::ndk::SpAIBinder; @@ -102,14 +103,17 @@ TEST_P(WeaverAidlTest, WriteFollowedByReadGivesTheSameValue) { WeaverReadResponse response; std::vector readValue; uint32_t timeout; + WeaverReadStatus status; const auto readRet = weaver->read(slotId, KEY, &response); readValue = response.value; timeout = response.timeout; + status = response.status; ASSERT_TRUE(readRet.isOk()); EXPECT_EQ(readValue, VALUE); EXPECT_EQ(timeout, 0u); + EXPECT_EQ(status, WeaverReadStatus::OK); } /* @@ -128,14 +132,17 @@ TEST_P(WeaverAidlTest, OverwritingSlotUpdatesTheValue) { WeaverReadResponse response; std::vector readValue; uint32_t timeout; + WeaverReadStatus status; const auto readRet = weaver->read(slotId, KEY, &response); readValue = response.value; timeout = response.timeout; + status = response.status; ASSERT_TRUE(readRet.isOk()); EXPECT_EQ(readValue, OTHER_VALUE); EXPECT_EQ(timeout, 0u); + EXPECT_EQ(status, WeaverReadStatus::OK); } /* @@ -149,15 +156,16 @@ TEST_P(WeaverAidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { WeaverReadResponse response; std::vector readValue; + WeaverReadStatus status; const auto readRet = weaver->read(slotId, WRONG_KEY, &response); readValue = response.value; + status = response.status; - ASSERT_FALSE(readRet.isOk()); - ASSERT_EQ(EX_SERVICE_SPECIFIC, readRet.getExceptionCode()); - ASSERT_EQ(IWeaver::STATUS_INCORRECT_KEY, readRet.getServiceSpecificError()); + ASSERT_TRUE(readRet.isOk()); EXPECT_TRUE(readValue.empty()); + EXPECT_EQ(status, WeaverReadStatus::INCORRECT_KEY); } /* @@ -193,17 +201,18 @@ TEST_P(WeaverAidlTest, ReadingFromInvalidSlotFails) { WeaverReadResponse response; std::vector readValue; uint32_t timeout; + WeaverReadStatus status; const auto readRet = weaver->read(config.slots, KEY, &response); readValue = response.value; timeout = response.timeout; + status = response.status; - ASSERT_FALSE(readRet.isOk()); - ASSERT_EQ(EX_SERVICE_SPECIFIC, readRet.getExceptionCode()); - ASSERT_EQ(IWeaver::STATUS_FAILED, readRet.getServiceSpecificError()); + ASSERT_TRUE(readRet.isOk()); EXPECT_TRUE(readValue.empty()); EXPECT_EQ(timeout, 0u); + EXPECT_EQ(status, WeaverReadStatus::FAILED); } /* @@ -250,17 +259,18 @@ TEST_P(WeaverAidlTest, ReadWithTooLargeKeyFails) { WeaverReadResponse response; std::vector readValue; uint32_t timeout; + WeaverReadStatus status; const auto readRet = weaver->read(slotId, bigKey, &response); readValue = response.value; timeout = response.timeout; + status = response.status; - ASSERT_FALSE(readRet.isOk()); - ASSERT_EQ(EX_SERVICE_SPECIFIC, readRet.getExceptionCode()); - ASSERT_EQ(IWeaver::STATUS_FAILED, readRet.getServiceSpecificError()); + ASSERT_TRUE(readRet.isOk()); EXPECT_TRUE(readValue.empty()); EXPECT_EQ(timeout, 0u); + EXPECT_EQ(status, WeaverReadStatus::FAILED); } GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WeaverAidlTest); From 316cf58d4905c8a5ec8cc57cacf52e39df9dd156 Mon Sep 17 00:00:00 2001 From: Devin Moore Date: Wed, 7 Dec 2022 00:43:46 +0000 Subject: [PATCH 2/8] Update weaver AIDL default to use V2 Test: VtsHalWeaverTargetTest Bug: 259556049 Change-Id: Id6aa9316a20541bf2c9a7cc53345a5010a188d00 (cherry picked from commit daf12d944042180398acd45e9ccc684d5288c0f5) Merged-In: Id6aa9316a20541bf2c9a7cc53345a5010a188d00 --- compatibility_matrices/compatibility_matrix.9.xml | 2 +- weaver/aidl/default/Android.bp | 2 +- weaver/aidl/default/Weaver.cpp | 11 ++++++----- .../android.hardware.weaver-service.example.xml | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/compatibility_matrices/compatibility_matrix.9.xml b/compatibility_matrices/compatibility_matrix.9.xml index 0dc15bdee1..1a16627de4 100644 --- a/compatibility_matrices/compatibility_matrix.9.xml +++ b/compatibility_matrices/compatibility_matrix.9.xml @@ -742,7 +742,7 @@ android.hardware.weaver - 1 + 2 IWeaver default diff --git a/weaver/aidl/default/Android.bp b/weaver/aidl/default/Android.bp index 70d91718de..494cb1bf4a 100644 --- a/weaver/aidl/default/Android.bp +++ b/weaver/aidl/default/Android.bp @@ -34,7 +34,7 @@ cc_binary { "Weaver.cpp", ], shared_libs: [ - "android.hardware.weaver-V1-ndk", + "android.hardware.weaver-V2-ndk", "libbase", "libbinder_ndk", ], diff --git a/weaver/aidl/default/Weaver.cpp b/weaver/aidl/default/Weaver.cpp index 6b77924be7..c9ffe85594 100644 --- a/weaver/aidl/default/Weaver.cpp +++ b/weaver/aidl/default/Weaver.cpp @@ -37,18 +37,19 @@ std::array slot_array; } ::ndk::ScopedAStatus Weaver::read(int32_t in_slotId, const std::vector& in_key, WeaverReadResponse* out_response) { + using ::aidl::android::hardware::weaver::WeaverReadStatus; if (in_slotId > 15 || in_key.size() > 16) { - *out_response = {0, {}}; - return ndk::ScopedAStatus(AStatus_fromServiceSpecificError(Weaver::STATUS_FAILED)); + *out_response = {0, {}, WeaverReadStatus::FAILED}; + return ndk::ScopedAStatus::ok(); } if (slot_array[in_slotId].key != in_key) { - *out_response = {0, {}}; - return ndk::ScopedAStatus(AStatus_fromServiceSpecificError(Weaver::STATUS_INCORRECT_KEY)); + *out_response = {0, {}, WeaverReadStatus::INCORRECT_KEY}; + return ndk::ScopedAStatus::ok(); } - *out_response = {0, slot_array[in_slotId].value}; + *out_response = {0, slot_array[in_slotId].value, WeaverReadStatus::OK}; return ::ndk::ScopedAStatus::ok(); } diff --git a/weaver/aidl/default/android.hardware.weaver-service.example.xml b/weaver/aidl/default/android.hardware.weaver-service.example.xml index ed291cdf60..bfe43966ab 100644 --- a/weaver/aidl/default/android.hardware.weaver-service.example.xml +++ b/weaver/aidl/default/android.hardware.weaver-service.example.xml @@ -1,7 +1,7 @@ android.hardware.weaver - 1 + 2 IWeaver default From a4742a0291f0726b031275fa07792c2423a0e0f9 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 17 May 2023 17:48:30 +0000 Subject: [PATCH 3/8] Clarify the Weaver documentation - Explicitly mention that Weaver is for *persistent* storage - Explicitly mention secure deletion requirement Change-Id: I3eaf7408570ff20c69a21398e39e16be97d2a917 (cherry picked from commit acd066c61425d6b7f512445c6e2883baa28f381e) Merged-In: I3eaf7408570ff20c69a21398e39e16be97d2a917 --- weaver/aidl/android/hardware/weaver/IWeaver.aidl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/weaver/aidl/android/hardware/weaver/IWeaver.aidl b/weaver/aidl/android/hardware/weaver/IWeaver.aidl index f51034a387..ae816ef3f4 100644 --- a/weaver/aidl/android/hardware/weaver/IWeaver.aidl +++ b/weaver/aidl/android/hardware/weaver/IWeaver.aidl @@ -20,8 +20,8 @@ import android.hardware.weaver.WeaverConfig; import android.hardware.weaver.WeaverReadResponse; /** - * Weaver provides secure storage of secret values that may only be read if the - * corresponding key has been presented. + * Weaver provides secure persistent storage of secret values that may only be + * read if the corresponding key has been presented. * * The storage must be secure as the device's user authentication and encryption * relies on the security of these values. The cardinality of the domains of the @@ -76,7 +76,8 @@ interface IWeaver { WeaverReadResponse read(in int slotId, in byte[] key); /** - * Overwrites the identified slot with the provided key and value. + * Overwrites the identified slot with the provided key and value, rendering + * the previous contents of the slot permanently unrecoverable. * * The new values are written regardless of the current state of the slot in * order to remain idempotent. From b59654f239745583f53fda7d62bb06f1144a347d Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:59 +0000 Subject: [PATCH 4/8] Make VtsHalWeaverTargetTest test both HIDL and AIDL services VtsHalWeaverTargetTest and VtsHalWeaverV1_0TargetTest are identical except for whether they use AIDL or HIDL. Unfortunately, the HIDL test is needed for several more years. For now, we have to make some substantial fixes to both tests. To make continued maintenance of this test easier, update VtsHalWeaverTargetTest to handle both AIDL and HIDL. The test cases are still written in terms of the AIDL API, so it should still be straightforward to remove HIDL support when the time comes. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: I6b760930146ad1b08f17ef810a86c0058601c3bf --- weaver/aidl/vts/Android.bp | 5 +- weaver/aidl/vts/VtsHalWeaverTargetTest.cpp | 200 ++++++++++++++++++--- 2 files changed, 181 insertions(+), 24 deletions(-) diff --git a/weaver/aidl/vts/Android.bp b/weaver/aidl/vts/Android.bp index 557fe47aa7..ee03b28136 100644 --- a/weaver/aidl/vts/Android.bp +++ b/weaver/aidl/vts/Android.bp @@ -34,7 +34,10 @@ cc_test { "libbinder_ndk", "libbase", ], - static_libs: ["android.hardware.weaver-V2-ndk"], + static_libs: [ + "android.hardware.weaver-V2-ndk", + "android.hardware.weaver@1.0", + ], test_suites: [ "general-tests", "vts", diff --git a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp b/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp index f016515a6a..b75a56f039 100644 --- a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp @@ -19,6 +19,9 @@ #include #include #include +#include +#include +#include #include @@ -27,29 +30,162 @@ using ::aidl::android::hardware::weaver::WeaverConfig; using ::aidl::android::hardware::weaver::WeaverReadResponse; using ::aidl::android::hardware::weaver::WeaverReadStatus; -using ::ndk::SpAIBinder; +using HidlIWeaver = ::android::hardware::weaver::V1_0::IWeaver; +using HidlWeaverConfig = ::android::hardware::weaver::V1_0::WeaverConfig; +using HidlWeaverReadStatus = ::android::hardware::weaver::V1_0::WeaverReadStatus; +using HidlWeaverReadResponse = ::android::hardware::weaver::V1_0::WeaverReadResponse; +using HidlWeaverStatus = ::android::hardware::weaver::V1_0::WeaverStatus; const std::vector KEY{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; const std::vector WRONG_KEY{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; const std::vector VALUE{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; const std::vector OTHER_VALUE{0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 255, 255}; -struct WeaverAidlTest : public ::testing::TestWithParam { - virtual void SetUp() override { - weaver = IWeaver::fromBinder( - SpAIBinder(AServiceManager_waitForService(GetParam().c_str()))); - ASSERT_NE(weaver, nullptr); +class WeaverAdapter { + public: + virtual ~WeaverAdapter() {} + virtual bool isReady() = 0; + virtual ::ndk::ScopedAStatus getConfig(WeaverConfig* _aidl_return) = 0; + virtual ::ndk::ScopedAStatus read(int32_t in_slotId, const std::vector& in_key, + WeaverReadResponse* _aidl_return) = 0; + virtual ::ndk::ScopedAStatus write(int32_t in_slotId, const std::vector& in_key, + const std::vector& in_value) = 0; +}; + +class WeaverAidlAdapter : public WeaverAdapter { + public: + WeaverAidlAdapter(const std::string& param) + : aidl_weaver_(IWeaver::fromBinder( + ::ndk::SpAIBinder(AServiceManager_waitForService(param.c_str())))) {} + ~WeaverAidlAdapter() {} + + bool isReady() { return aidl_weaver_ != nullptr; } + + ::ndk::ScopedAStatus getConfig(WeaverConfig* _aidl_return) { + return aidl_weaver_->getConfig(_aidl_return); } - virtual void TearDown() override {} + ::ndk::ScopedAStatus read(int32_t in_slotId, const std::vector& in_key, + WeaverReadResponse* _aidl_return) { + return aidl_weaver_->read(in_slotId, in_key, _aidl_return); + } - std::shared_ptr weaver; + ::ndk::ScopedAStatus write(int32_t in_slotId, const std::vector& in_key, + const std::vector& in_value) { + return aidl_weaver_->write(in_slotId, in_key, in_value); + } + + private: + std::shared_ptr aidl_weaver_; }; +class WeaverHidlAdapter : public WeaverAdapter { + public: + WeaverHidlAdapter(const std::string& param) : hidl_weaver_(HidlIWeaver::getService(param)) {} + ~WeaverHidlAdapter() {} + + bool isReady() { return hidl_weaver_ != nullptr; } + + ::ndk::ScopedAStatus getConfig(WeaverConfig* _aidl_return) { + bool callbackCalled = false; + HidlWeaverStatus status; + HidlWeaverConfig config; + auto ret = hidl_weaver_->getConfig([&](HidlWeaverStatus s, HidlWeaverConfig c) { + callbackCalled = true; + status = s; + config = c; + }); + if (!ret.isOk() || !callbackCalled || status != HidlWeaverStatus::OK) { + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + } + _aidl_return->slots = config.slots; + _aidl_return->keySize = config.keySize; + _aidl_return->valueSize = config.valueSize; + return ::ndk::ScopedAStatus::ok(); + } + + ::ndk::ScopedAStatus read(int32_t in_slotId, const std::vector& in_key, + WeaverReadResponse* _aidl_return) { + bool callbackCalled = false; + HidlWeaverReadStatus status; + std::vector value; + uint32_t timeout; + auto ret = hidl_weaver_->read(in_slotId, in_key, + [&](HidlWeaverReadStatus s, HidlWeaverReadResponse r) { + callbackCalled = true; + status = s; + value = r.value; + timeout = r.timeout; + }); + if (!ret.isOk() || !callbackCalled) { + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + } + switch (status) { + case HidlWeaverReadStatus::OK: + _aidl_return->status = WeaverReadStatus::OK; + break; + case HidlWeaverReadStatus::FAILED: + _aidl_return->status = WeaverReadStatus::FAILED; + break; + case HidlWeaverReadStatus::INCORRECT_KEY: + _aidl_return->status = WeaverReadStatus::INCORRECT_KEY; + break; + case HidlWeaverReadStatus::THROTTLE: + _aidl_return->status = WeaverReadStatus::THROTTLE; + break; + default: + ADD_FAILURE() << "Unknown HIDL read status: " << static_cast(status); + _aidl_return->status = WeaverReadStatus::FAILED; + break; + } + _aidl_return->value = value; + _aidl_return->timeout = timeout; + return ::ndk::ScopedAStatus::ok(); + } + + ::ndk::ScopedAStatus write(int32_t in_slotId, const std::vector& in_key, + const std::vector& in_value) { + auto status = hidl_weaver_->write(in_slotId, in_key, in_value); + switch (status) { + case HidlWeaverStatus::OK: + return ::ndk::ScopedAStatus::ok(); + case HidlWeaverStatus::FAILED: + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + default: + ADD_FAILURE() << "Unknown HIDL write status: " << status.description(); + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + } + } + + private: + android::sp hidl_weaver_; +}; + +class WeaverTest : public ::testing::TestWithParam> { + protected: + void SetUp() override; + void TearDown() override {} + + std::unique_ptr weaver; +}; + +void WeaverTest::SetUp() { + std::string api, instance_name; + std::tie(api, instance_name) = GetParam(); + if (api == "hidl") { + weaver.reset(new WeaverHidlAdapter(instance_name)); + } else if (api == "aidl") { + weaver.reset(new WeaverAidlAdapter(instance_name)); + } else { + FAIL() << "Bad test parameterization"; + } + ASSERT_TRUE(weaver->isReady()); +} + /* * Checks config values are suitably large */ -TEST_P(WeaverAidlTest, GetConfig) { +TEST_P(WeaverTest, GetConfig) { WeaverConfig config; auto ret = weaver->getConfig(&config); @@ -64,7 +200,7 @@ TEST_P(WeaverAidlTest, GetConfig) { /* * Gets the config twice and checks they are the same */ -TEST_P(WeaverAidlTest, GettingConfigMultipleTimesGivesSameResult) { +TEST_P(WeaverTest, GettingConfigMultipleTimesGivesSameResult) { WeaverConfig config1; WeaverConfig config2; @@ -80,7 +216,7 @@ TEST_P(WeaverAidlTest, GettingConfigMultipleTimesGivesSameResult) { /* * Gets the number of slots from the config and writes a key and value to the last one */ -TEST_P(WeaverAidlTest, WriteToLastSlot) { +TEST_P(WeaverTest, WriteToLastSlot) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); @@ -95,7 +231,7 @@ TEST_P(WeaverAidlTest, WriteToLastSlot) { * Writes a key and value to a slot * Reads the slot with the same key and receives the value that was previously written */ -TEST_P(WeaverAidlTest, WriteFollowedByReadGivesTheSameValue) { +TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { constexpr uint32_t slotId = 0; const auto ret = weaver->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); @@ -121,7 +257,7 @@ TEST_P(WeaverAidlTest, WriteFollowedByReadGivesTheSameValue) { * Overwrites the slot with a new key and value * Reads the slot with the new key and receives the new value */ -TEST_P(WeaverAidlTest, OverwritingSlotUpdatesTheValue) { +TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { constexpr uint32_t slotId = 0; const auto initialWriteRet = weaver->write(slotId, WRONG_KEY, VALUE); ASSERT_TRUE(initialWriteRet.isOk()); @@ -149,7 +285,7 @@ TEST_P(WeaverAidlTest, OverwritingSlotUpdatesTheValue) { * Writes a key and value to a slot * Reads the slot with a different key so does not receive the value */ -TEST_P(WeaverAidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { +TEST_P(WeaverTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { constexpr uint32_t slotId = 0; const auto ret = weaver->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); @@ -171,7 +307,7 @@ TEST_P(WeaverAidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { /* * Writing to an invalid slot fails */ -TEST_P(WeaverAidlTest, WritingToInvalidSlotFails) { +TEST_P(WeaverTest, WritingToInvalidSlotFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -188,7 +324,7 @@ TEST_P(WeaverAidlTest, WritingToInvalidSlotFails) { /* * Reading from an invalid slot fails rather than incorrect key */ -TEST_P(WeaverAidlTest, ReadingFromInvalidSlotFails) { +TEST_P(WeaverTest, ReadingFromInvalidSlotFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -218,7 +354,7 @@ TEST_P(WeaverAidlTest, ReadingFromInvalidSlotFails) { /* * Writing a key that is too large fails */ -TEST_P(WeaverAidlTest, WriteWithTooLargeKeyFails) { +TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -233,7 +369,7 @@ TEST_P(WeaverAidlTest, WriteWithTooLargeKeyFails) { /* * Writing a value that is too large fails */ -TEST_P(WeaverAidlTest, WriteWithTooLargeValueFails) { +TEST_P(WeaverTest, WriteWithTooLargeValueFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -248,7 +384,7 @@ TEST_P(WeaverAidlTest, WriteWithTooLargeValueFails) { /* * Reading with a key that is loo large fails */ -TEST_P(WeaverAidlTest, ReadWithTooLargeKeyFails) { +TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -273,11 +409,29 @@ TEST_P(WeaverAidlTest, ReadWithTooLargeKeyFails) { EXPECT_EQ(status, WeaverReadStatus::FAILED); } -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WeaverAidlTest); +// Instantiate the test for each HIDL Weaver service. INSTANTIATE_TEST_SUITE_P( - PerInstance, WeaverAidlTest, - testing::ValuesIn(android::getAidlHalInstanceNames(IWeaver::descriptor)), - android::PrintInstanceNameToString); + PerHidlInstance, WeaverTest, + testing::Combine(testing::Values("hidl"), + testing::ValuesIn(android::hardware::getAllHalInstanceNames( + HidlIWeaver::descriptor))), + [](const testing::TestParamInfo>& info) { + return android::hardware::PrintInstanceNameToString( + testing::TestParamInfo{std::get<1>(info.param), info.index}); + }); + +// Instantiate the test for each AIDL Weaver service. +INSTANTIATE_TEST_SUITE_P( + PerAidlInstance, WeaverTest, + testing::Combine(testing::Values("aidl"), + testing::ValuesIn(android::getAidlHalInstanceNames(IWeaver::descriptor))), + [](const testing::TestParamInfo>& info) { + // This name_generator makes the instance name be included in the test case names, e.g. + // "PerAidlInstance/WeaverTest#GetConfig/0_android_hardware_weaver_IWeaver_default" + // instead of "PerAidlInstance/WeaverTest#GetConfig/0". + return android::PrintInstanceNameToString( + testing::TestParamInfo{std::get<1>(info.param), info.index}); + }); int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); From f0d6907d20a72391762e3e479e4a5833e4091b3b Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:59 +0000 Subject: [PATCH 5/8] Move VtsHalWeaverTargetTest to common directory Since VtsHalWeaverTargetTest now handles both AIDL and HIDL, move it from weaver/aidl/vts/ to weaver/vts/. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: Icfa0ff3b22b036110df327674fda44820057aabd --- weaver/{aidl => }/vts/Android.bp | 0 weaver/{aidl => }/vts/VtsHalWeaverTargetTest.cpp | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename weaver/{aidl => }/vts/Android.bp (100%) rename weaver/{aidl => }/vts/VtsHalWeaverTargetTest.cpp (100%) diff --git a/weaver/aidl/vts/Android.bp b/weaver/vts/Android.bp similarity index 100% rename from weaver/aidl/vts/Android.bp rename to weaver/vts/Android.bp diff --git a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp similarity index 100% rename from weaver/aidl/vts/VtsHalWeaverTargetTest.cpp rename to weaver/vts/VtsHalWeaverTargetTest.cpp From e2e40d69a6185494855e691e37cea2e40c7edd2f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:59 +0000 Subject: [PATCH 6/8] Remove redundant HIDL Weaver VTS test Now that there is a single Weaver VTS test that covers both the HIDL and AIDL services (weaver/vts/), the HIDL-specific test can be deleted. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: Ie942825c154e6792e6ffdbf0c59248de9de10d92 --- weaver/1.0/vts/functional/Android.bp | 32 -- .../functional/VtsHalWeaverV1_0TargetTest.cpp | 343 ------------------ 2 files changed, 375 deletions(-) delete mode 100644 weaver/1.0/vts/functional/Android.bp delete mode 100644 weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp diff --git a/weaver/1.0/vts/functional/Android.bp b/weaver/1.0/vts/functional/Android.bp deleted file mode 100644 index cc1d28465d..0000000000 --- a/weaver/1.0/vts/functional/Android.bp +++ /dev/null @@ -1,32 +0,0 @@ -// -// Copyright (C) 2017 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -package { - // See: http://go/android-license-faq - // A large-scale-change added 'default_applicable_licenses' to import - // all of the 'license_kinds' from "hardware_interfaces_license" - // to get the below license kinds: - // SPDX-license-identifier-Apache-2.0 - default_applicable_licenses: ["hardware_interfaces_license"], -} - -cc_test { - name: "VtsHalWeaverV1_0TargetTest", - defaults: ["VtsHalTargetTestDefaults"], - srcs: ["VtsHalWeaverV1_0TargetTest.cpp"], - static_libs: ["android.hardware.weaver@1.0"], - test_suites: ["general-tests", "vts"], -} diff --git a/weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp b/weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp deleted file mode 100644 index 66465a9798..0000000000 --- a/weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp +++ /dev/null @@ -1,343 +0,0 @@ -/* - * Copyright (C) 2017 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include -#include -#include - -#include - -using ::android::hardware::weaver::V1_0::IWeaver; -using ::android::hardware::weaver::V1_0::WeaverConfig; -using ::android::hardware::weaver::V1_0::WeaverReadStatus; -using ::android::hardware::weaver::V1_0::WeaverReadResponse; -using ::android::hardware::weaver::V1_0::WeaverStatus; -using ::android::hardware::Return; -using ::android::sp; - -const std::vector KEY{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; -const std::vector WRONG_KEY{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; -const std::vector VALUE{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; -const std::vector OTHER_VALUE{0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 255, 255}; - -struct WeaverHidlTest : public ::testing::TestWithParam { - virtual void SetUp() override { - weaver = IWeaver::getService(GetParam()); - ASSERT_NE(weaver, nullptr); - } - - virtual void TearDown() override {} - - sp weaver; -}; - -/* - * Checks config values are suitably large - */ -TEST_P(WeaverHidlTest, GetConfig) { - WeaverStatus status; - WeaverConfig config; - - bool callbackCalled = false; - auto ret = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - callbackCalled = true; - status = s; - config = c; - }); - ASSERT_TRUE(ret.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverStatus::OK); - - EXPECT_GE(config.slots, 16u); - EXPECT_GE(config.keySize, 16u); - EXPECT_GE(config.valueSize, 16u); -} - -/* - * Gets the config twice and checks they are the same - */ -TEST_P(WeaverHidlTest, GettingConfigMultipleTimesGivesSameResult) { - WeaverConfig config1; - WeaverConfig config2; - - WeaverStatus status; - bool callbackCalled = false; - auto ret = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - callbackCalled = true; - status = s; - config1 = c; - }); - ASSERT_TRUE(ret.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverStatus::OK); - - callbackCalled = false; - ret = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - callbackCalled = true; - status = s; - config2 = c; - }); - ASSERT_TRUE(ret.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverStatus::OK); - - EXPECT_EQ(config1, config2); -} - -/* - * Gets the number of slots from the config and writes a key and value to the last one - */ -TEST_P(WeaverHidlTest, WriteToLastSlot) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - const uint32_t lastSlot = config.slots - 1; - const auto writeRet = weaver->write(lastSlot, KEY, VALUE); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::OK); -} - -/* - * Writes a key and value to a slot - * Reads the slot with the same key and receives the value that was previously written - */ -TEST_P(WeaverHidlTest, WriteFollowedByReadGivesTheSameValue) { - constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); - ASSERT_TRUE(ret.isOk()); - ASSERT_EQ(ret, WeaverStatus::OK); - - bool callbackCalled = false; - WeaverReadStatus status; - std::vector readValue; - uint32_t timeout; - const auto readRet = weaver->read(slotId, KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - status = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(readRet.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverReadStatus::OK); - EXPECT_EQ(readValue, VALUE); - EXPECT_EQ(timeout, 0u); -} - -/* - * Writes a key and value to a slot - * Overwrites the slot with a new key and value - * Reads the slot with the new key and receives the new value - */ -TEST_P(WeaverHidlTest, OverwritingSlotUpdatesTheValue) { - constexpr uint32_t slotId = 0; - const auto initialWriteRet = weaver->write(slotId, WRONG_KEY, VALUE); - ASSERT_TRUE(initialWriteRet.isOk()); - ASSERT_EQ(initialWriteRet, WeaverStatus::OK); - - const auto overwriteRet = weaver->write(slotId, KEY, OTHER_VALUE); - ASSERT_TRUE(overwriteRet.isOk()); - ASSERT_EQ(overwriteRet, WeaverStatus::OK); - - bool callbackCalled = false; - WeaverReadStatus status; - std::vector readValue; - uint32_t timeout; - const auto readRet = weaver->read(slotId, KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - status = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(readRet.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverReadStatus::OK); - EXPECT_EQ(readValue, OTHER_VALUE); - EXPECT_EQ(timeout, 0u); -} - -/* - * Writes a key and value to a slot - * Reads the slot with a different key so does not receive the value - */ -TEST_P(WeaverHidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { - constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); - ASSERT_TRUE(ret.isOk()); - ASSERT_EQ(ret, WeaverStatus::OK); - - bool callbackCalled = false; - WeaverReadStatus status; - std::vector readValue; - const auto readRet = - weaver->read(slotId, WRONG_KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - status = s; - readValue = r.value; - }); - ASSERT_TRUE(callbackCalled); - ASSERT_TRUE(readRet.isOk()); - ASSERT_EQ(status, WeaverReadStatus::INCORRECT_KEY); - EXPECT_TRUE(readValue.empty()); -} - -/* - * Writing to an invalid slot fails - */ -TEST_P(WeaverHidlTest, WritingToInvalidSlotFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - if (config.slots == std::numeric_limits::max()) { - // If there are no invalid slots then pass - return; - } - - const auto writeRet = weaver->write(config.slots, KEY, VALUE); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::FAILED); -} - -/* - * Reading from an invalid slot fails rather than incorrect key - */ -TEST_P(WeaverHidlTest, ReadingFromInvalidSlotFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - if (config.slots == std::numeric_limits::max()) { - // If there are no invalid slots then pass - return; - } - - bool callbackCalled = false; - WeaverReadStatus readStatus; - std::vector readValue; - uint32_t timeout; - const auto readRet = - weaver->read(config.slots, KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - readStatus = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(callbackCalled); - ASSERT_TRUE(readRet.isOk()); - ASSERT_EQ(readStatus, WeaverReadStatus::FAILED); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); -} - -/* - * Writing a key that is too large fails - */ -TEST_P(WeaverHidlTest, WriteWithTooLargeKeyFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - std::vector bigKey(config.keySize + 1); - - constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, bigKey, VALUE); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::FAILED); -} - -/* - * Writing a value that is too large fails - */ -TEST_P(WeaverHidlTest, WriteWithTooLargeValueFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - std::vector bigValue(config.valueSize + 1); - - constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, KEY, bigValue); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::FAILED); -} - -/* - * Reading with a key that is loo large fails - */ -TEST_P(WeaverHidlTest, ReadWithTooLargeKeyFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - std::vector bigKey(config.keySize + 1); - - constexpr uint32_t slotId = 0; - bool callbackCalled = false; - WeaverReadStatus readStatus; - std::vector readValue; - uint32_t timeout; - const auto readRet = - weaver->read(slotId, bigKey, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - readStatus = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(callbackCalled); - ASSERT_TRUE(readRet.isOk()); - ASSERT_EQ(readStatus, WeaverReadStatus::FAILED); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); -} - -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WeaverHidlTest); -INSTANTIATE_TEST_SUITE_P( - PerInstance, WeaverHidlTest, - testing::ValuesIn(android::hardware::getAllHalInstanceNames(IWeaver::descriptor)), - android::hardware::PrintInstanceNameToString); From 961a138e47542b46b574f1f9ba94177e89e06c11 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:34:00 +0000 Subject: [PATCH 7/8] Simplify Weaver VTS test - Get the config in SetUp() so that it's easily available to test cases. - Rename "weaver" class member to "weaver_" to match coding style. - Eliminate unnecessary variables when checking WeaverReadResponse. - Fix a typo. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: Ia6dca996103057bfdc9002bc9ab2c039e2333ed9 --- weaver/vts/VtsHalWeaverTargetTest.cpp | 171 ++++++++------------------ 1 file changed, 51 insertions(+), 120 deletions(-) diff --git a/weaver/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp index b75a56f039..047481e123 100644 --- a/weaver/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/vts/VtsHalWeaverTargetTest.cpp @@ -166,64 +166,56 @@ class WeaverTest : public ::testing::TestWithParam weaver; + std::unique_ptr weaver_; + WeaverConfig config_; }; void WeaverTest::SetUp() { std::string api, instance_name; std::tie(api, instance_name) = GetParam(); if (api == "hidl") { - weaver.reset(new WeaverHidlAdapter(instance_name)); + weaver_.reset(new WeaverHidlAdapter(instance_name)); } else if (api == "aidl") { - weaver.reset(new WeaverAidlAdapter(instance_name)); + weaver_.reset(new WeaverAidlAdapter(instance_name)); } else { FAIL() << "Bad test parameterization"; } - ASSERT_TRUE(weaver->isReady()); + ASSERT_TRUE(weaver_->isReady()); + + auto ret = weaver_->getConfig(&config_); + ASSERT_TRUE(ret.isOk()); + ASSERT_GT(config_.slots, 0); + GTEST_LOG_(INFO) << "WeaverConfig: slots=" << config_.slots << ", keySize=" << config_.keySize + << ", valueSize=" << config_.valueSize; } /* * Checks config values are suitably large */ TEST_P(WeaverTest, GetConfig) { - WeaverConfig config; - - auto ret = weaver->getConfig(&config); - - ASSERT_TRUE(ret.isOk()); - - EXPECT_GE(config.slots, 16u); - EXPECT_GE(config.keySize, 16u); - EXPECT_GE(config.valueSize, 16u); + EXPECT_GE(config_.slots, 16u); + EXPECT_GE(config_.keySize, 16u); + EXPECT_GE(config_.valueSize, 16u); } /* * Gets the config twice and checks they are the same */ TEST_P(WeaverTest, GettingConfigMultipleTimesGivesSameResult) { - WeaverConfig config1; WeaverConfig config2; - auto ret = weaver->getConfig(&config1); + auto ret = weaver_->getConfig(&config2); ASSERT_TRUE(ret.isOk()); - ret = weaver->getConfig(&config2); - ASSERT_TRUE(ret.isOk()); - - EXPECT_EQ(config1, config2); + EXPECT_EQ(config_, config2); } /* * Gets the number of slots from the config and writes a key and value to the last one */ TEST_P(WeaverTest, WriteToLastSlot) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - - ASSERT_TRUE(configRet.isOk()); - - const uint32_t lastSlot = config.slots - 1; - const auto writeRet = weaver->write(lastSlot, KEY, VALUE); + const uint32_t lastSlot = config_.slots - 1; + const auto writeRet = weaver_->write(lastSlot, KEY, VALUE); ASSERT_TRUE(writeRet.isOk()); } @@ -233,23 +225,15 @@ TEST_P(WeaverTest, WriteToLastSlot) { */ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); + const auto ret = weaver_->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = weaver->read(slotId, KEY, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(slotId, KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_EQ(readValue, VALUE); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::OK); + EXPECT_EQ(response.value, VALUE); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::OK); } /* @@ -259,26 +243,18 @@ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { */ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { constexpr uint32_t slotId = 0; - const auto initialWriteRet = weaver->write(slotId, WRONG_KEY, VALUE); + const auto initialWriteRet = weaver_->write(slotId, WRONG_KEY, VALUE); ASSERT_TRUE(initialWriteRet.isOk()); - const auto overwriteRet = weaver->write(slotId, KEY, OTHER_VALUE); + const auto overwriteRet = weaver_->write(slotId, KEY, OTHER_VALUE); ASSERT_TRUE(overwriteRet.isOk()); WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = weaver->read(slotId, KEY, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(slotId, KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_EQ(readValue, OTHER_VALUE); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::OK); + EXPECT_EQ(response.value, OTHER_VALUE); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::OK); } /* @@ -287,37 +263,26 @@ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { */ TEST_P(WeaverTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); - ASSERT_TRUE(ret.isOk()); + const auto writeRet = weaver_->write(slotId, KEY, VALUE); + ASSERT_TRUE(writeRet.isOk()); WeaverReadResponse response; - std::vector readValue; - WeaverReadStatus status; - const auto readRet = - weaver->read(slotId, WRONG_KEY, &response); - - readValue = response.value; - status = response.status; - + const auto readRet = weaver_->read(slotId, WRONG_KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(status, WeaverReadStatus::INCORRECT_KEY); + EXPECT_TRUE(response.value.empty()); + EXPECT_EQ(response.status, WeaverReadStatus::INCORRECT_KEY); } /* * Writing to an invalid slot fails */ TEST_P(WeaverTest, WritingToInvalidSlotFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - if (config.slots == std::numeric_limits::max()) { + if (config_.slots == std::numeric_limits::max()) { // If there are no invalid slots then pass return; } - const auto writeRet = weaver->write(config.slots, KEY, VALUE); + const auto writeRet = weaver_->write(config_.slots, KEY, VALUE); ASSERT_FALSE(writeRet.isOk()); } @@ -325,44 +290,27 @@ TEST_P(WeaverTest, WritingToInvalidSlotFails) { * Reading from an invalid slot fails rather than incorrect key */ TEST_P(WeaverTest, ReadingFromInvalidSlotFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - if (config.slots == std::numeric_limits::max()) { + if (config_.slots == std::numeric_limits::max()) { // If there are no invalid slots then pass return; } WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = - weaver->read(config.slots, KEY, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(config_.slots, KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::FAILED); + EXPECT_TRUE(response.value.empty()); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::FAILED); } /* * Writing a key that is too large fails */ TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - std::vector bigKey(config.keySize + 1); + std::vector bigKey(config_.keySize + 1); constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, bigKey, VALUE); + const auto writeRet = weaver_->write(slotId, bigKey, VALUE); ASSERT_FALSE(writeRet.isOk()); } @@ -370,43 +318,26 @@ TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { * Writing a value that is too large fails */ TEST_P(WeaverTest, WriteWithTooLargeValueFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - std::vector bigValue(config.valueSize + 1); + std::vector bigValue(config_.valueSize + 1); constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, KEY, bigValue); + const auto writeRet = weaver_->write(slotId, KEY, bigValue); ASSERT_FALSE(writeRet.isOk()); } /* - * Reading with a key that is loo large fails + * Reading with a key that is too large fails */ TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - std::vector bigKey(config.keySize + 1); + std::vector bigKey(config_.keySize + 1); constexpr uint32_t slotId = 0; WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = - weaver->read(slotId, bigKey, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(slotId, bigKey, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::FAILED); + EXPECT_TRUE(response.value.empty()); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::FAILED); } // Instantiate the test for each HIDL Weaver service. From 31380e7bc9122147e7a5d65cc1739ab78f853b24 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:34:00 +0000 Subject: [PATCH 8/8] Don't overwrite in-use Weaver slots during VTS VtsHalWeaverTargetTest always overwrote the first and last Weaver slots. Before Android 14, apparently this didn't cause problems because Android didn't use Weaver for users that never had an LSKF set. However, now users get a Weaver slot right away. That typically means that the first Weaver slot will be used by user 0. Fix the test to read /metadata/password_slots/slot_map to determine which slots are in use by the system, and only write to unused slots. Bug: 291284381 Test: 'atest -v VtsHalWeaverTargetTest'. Checked for INFO messages showing that slots 1 and 63 were used by the test. Then rebooted and verified that the device can still be unlocked. Change-Id: Id2cce4240d68999471e7d1e8fc7a8449587eed97 --- weaver/vts/VtsHalWeaverTargetTest.cpp | 77 ++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 12 deletions(-) diff --git a/weaver/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp index 047481e123..9ee6d93cf0 100644 --- a/weaver/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/vts/VtsHalWeaverTargetTest.cpp @@ -17,6 +17,9 @@ #include #include +#include +#include +#include #include #include #include @@ -36,6 +39,7 @@ using HidlWeaverReadStatus = ::android::hardware::weaver::V1_0::WeaverReadStatus using HidlWeaverReadResponse = ::android::hardware::weaver::V1_0::WeaverReadResponse; using HidlWeaverStatus = ::android::hardware::weaver::V1_0::WeaverStatus; +const std::string kSlotMapFile = "/metadata/password_slots/slot_map"; const std::vector KEY{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; const std::vector WRONG_KEY{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; const std::vector VALUE{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; @@ -165,9 +169,12 @@ class WeaverTest : public ::testing::TestWithParam weaver_; WeaverConfig config_; + uint32_t first_free_slot_; + uint32_t last_free_slot_; }; void WeaverTest::SetUp() { @@ -187,6 +194,56 @@ void WeaverTest::SetUp() { ASSERT_GT(config_.slots, 0); GTEST_LOG_(INFO) << "WeaverConfig: slots=" << config_.slots << ", keySize=" << config_.keySize << ", valueSize=" << config_.valueSize; + + FindFreeSlots(); + GTEST_LOG_(INFO) << "First free slot is " << first_free_slot_ << ", last free slot is " + << last_free_slot_; +} + +void WeaverTest::FindFreeSlots() { + // Determine which Weaver slots are in use by the system. These slots can't be used by the test. + std::set used_slots; + if (access(kSlotMapFile.c_str(), F_OK) == 0) { + std::string contents; + ASSERT_TRUE(android::base::ReadFileToString(kSlotMapFile, &contents)) + << "Failed to read " << kSlotMapFile; + for (const auto& line : android::base::Split(contents, "\n")) { + auto trimmed_line = android::base::Trim(line); + if (trimmed_line[0] == '#' || trimmed_line[0] == '\0') continue; + auto slot_and_user = android::base::Split(trimmed_line, "="); + uint32_t slot; + ASSERT_TRUE(slot_and_user.size() == 2 && + android::base::ParseUint(slot_and_user[0], &slot)) + << "Error parsing " << kSlotMapFile << " at \"" << line << "\""; + GTEST_LOG_(INFO) << "Slot " << slot << " is in use by " << slot_and_user[1]; + ASSERT_LT(slot, config_.slots); + used_slots.insert(slot); + } + } + // Starting in Android 14, the system will always use at least one Weaver slot if Weaver is + // supported at all. Make sure we saw at least one. + // TODO: uncomment after Android 14 is merged into AOSP + // ASSERT_FALSE(used_slots.empty()) + //<< "Could not determine which Weaver slots are in use by the system"; + + // Find the first free slot. + int found = 0; + for (uint32_t i = 0; i < config_.slots; i++) { + if (used_slots.find(i) == used_slots.end()) { + first_free_slot_ = i; + found++; + break; + } + } + // Find the last free slot. + for (uint32_t i = config_.slots; i > 0; i--) { + if (used_slots.find(i - 1) == used_slots.end()) { + last_free_slot_ = i - 1; + found++; + break; + } + } + ASSERT_EQ(found, 2) << "All Weaver slots are already in use by the system"; } /* @@ -211,11 +268,10 @@ TEST_P(WeaverTest, GettingConfigMultipleTimesGivesSameResult) { } /* - * Gets the number of slots from the config and writes a key and value to the last one + * Writes a key and value to the last free slot */ TEST_P(WeaverTest, WriteToLastSlot) { - const uint32_t lastSlot = config_.slots - 1; - const auto writeRet = weaver_->write(lastSlot, KEY, VALUE); + const auto writeRet = weaver_->write(last_free_slot_, KEY, VALUE); ASSERT_TRUE(writeRet.isOk()); } @@ -224,7 +280,7 @@ TEST_P(WeaverTest, WriteToLastSlot) { * Reads the slot with the same key and receives the value that was previously written */ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { - constexpr uint32_t slotId = 0; + const uint32_t slotId = first_free_slot_; const auto ret = weaver_->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); @@ -242,7 +298,7 @@ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { * Reads the slot with the new key and receives the new value */ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { - constexpr uint32_t slotId = 0; + const uint32_t slotId = first_free_slot_; const auto initialWriteRet = weaver_->write(slotId, WRONG_KEY, VALUE); ASSERT_TRUE(initialWriteRet.isOk()); @@ -262,7 +318,7 @@ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { * Reads the slot with a different key so does not receive the value */ TEST_P(WeaverTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { - constexpr uint32_t slotId = 0; + const uint32_t slotId = first_free_slot_; const auto writeRet = weaver_->write(slotId, KEY, VALUE); ASSERT_TRUE(writeRet.isOk()); @@ -309,8 +365,7 @@ TEST_P(WeaverTest, ReadingFromInvalidSlotFails) { TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { std::vector bigKey(config_.keySize + 1); - constexpr uint32_t slotId = 0; - const auto writeRet = weaver_->write(slotId, bigKey, VALUE); + const auto writeRet = weaver_->write(first_free_slot_, bigKey, VALUE); ASSERT_FALSE(writeRet.isOk()); } @@ -320,8 +375,7 @@ TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { TEST_P(WeaverTest, WriteWithTooLargeValueFails) { std::vector bigValue(config_.valueSize + 1); - constexpr uint32_t slotId = 0; - const auto writeRet = weaver_->write(slotId, KEY, bigValue); + const auto writeRet = weaver_->write(first_free_slot_, KEY, bigValue); ASSERT_FALSE(writeRet.isOk()); } @@ -331,9 +385,8 @@ TEST_P(WeaverTest, WriteWithTooLargeValueFails) { TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { std::vector bigKey(config_.keySize + 1); - constexpr uint32_t slotId = 0; WeaverReadResponse response; - const auto readRet = weaver_->read(slotId, bigKey, &response); + const auto readRet = weaver_->read(first_free_slot_, bigKey, &response); ASSERT_TRUE(readRet.isOk()); EXPECT_TRUE(response.value.empty()); EXPECT_EQ(response.timeout, 0u);