From d468101f149e30bc4ec5105555973d4ed8b4e009 Mon Sep 17 00:00:00 2001 From: Edwin Wong Date: Tue, 2 Feb 2021 10:04:02 -0800 Subject: [PATCH 01/61] [RESTRICT AUTOMERGE] Fix potential decrypt destPtr overflow. There is a potential integer overflow to bypass the destination base size check in decrypt. The destPtr can then point to the outside of the destination buffer. Test: sts-tradefed sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Bug_176444622#testPocBug_176444622 Test: push to device with target_hwasan-userdebug build adb shell /data/local/tmp/Bug-17644462264 Bug: 176444622 Bug: 176496353 Change-Id: I75ae057793a4ff4e4f52a8577bef189ad842fb0e --- drm/1.0/default/CryptoPlugin.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drm/1.0/default/CryptoPlugin.cpp b/drm/1.0/default/CryptoPlugin.cpp index 2a85f0e3d3..4ab9029434 100644 --- a/drm/1.0/default/CryptoPlugin.cpp +++ b/drm/1.0/default/CryptoPlugin.cpp @@ -80,7 +80,7 @@ namespace implementation { } } - android::CryptoPlugin::Mode legacyMode; + android::CryptoPlugin::Mode legacyMode = android::CryptoPlugin::kMode_Unencrypted; switch(mode) { case Mode::UNENCRYPTED: legacyMode = android::CryptoPlugin::kMode_Unencrypted; @@ -149,7 +149,10 @@ namespace implementation { return Void(); } - if (destBuffer.offset + destBuffer.size > destBase->getSize()) { + size_t totalDstSize = 0; + if (__builtin_add_overflow(destBuffer.offset, destBuffer.size, &totalDstSize) || + totalDstSize > destBase->getSize()) { + android_errorWriteLog(0x534e4554, "176496353"); _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "invalid buffer size"); return Void(); } From ff537c8516e1f8be96264d3c5db3ac084e307566 Mon Sep 17 00:00:00 2001 From: Edwin Wong Date: Tue, 2 Feb 2021 10:42:38 -0800 Subject: [PATCH 02/61] [RESTRICT AUTOMERGE] Fix potential decrypt destPtr overflow. There is a potential integer overflow to bypass the destination base size check in decrypt. The destPtr can then point to the outside of the destination buffer. Test: sts-tradefed sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Bug_176444622#testPocBug_176444622 Test: push to device with target_hwasan-userdebug build adb shell /data/local/tmp/Bug-17644462264 Bug: 176444622 Bug: 176496353 Change-Id: I71b390846a17aecbb9180865e1f9538b4b464abf --- drm/1.0/default/CryptoPlugin.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drm/1.0/default/CryptoPlugin.cpp b/drm/1.0/default/CryptoPlugin.cpp index e60ba8f54d..e71385da5d 100644 --- a/drm/1.0/default/CryptoPlugin.cpp +++ b/drm/1.0/default/CryptoPlugin.cpp @@ -80,7 +80,7 @@ namespace implementation { } } - android::CryptoPlugin::Mode legacyMode; + android::CryptoPlugin::Mode legacyMode = android::CryptoPlugin::kMode_Unencrypted; switch(mode) { case Mode::UNENCRYPTED: legacyMode = android::CryptoPlugin::kMode_Unencrypted; @@ -147,7 +147,10 @@ namespace implementation { return Void(); } - if (destBuffer.offset + destBuffer.size > destBase->getSize()) { + size_t totalDstSize = 0; + if (__builtin_add_overflow(destBuffer.offset, destBuffer.size, &totalDstSize) || + totalDstSize > destBase->getSize()) { + android_errorWriteLog(0x534e4554, "176496353"); _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "invalid buffer size"); return Void(); } @@ -158,7 +161,7 @@ namespace implementation { } base = static_cast(static_cast(destBase->getPointer())); - destPtr = static_cast(base + destination.nonsecureMemory.offset); + destPtr = static_cast(base + destination.nonsecureMemory.offset); } else if (destination.type == BufferType::NATIVE_HANDLE) { if (!secure) { _hidl_cb(Status::BAD_VALUE, 0, "native handle destination must be secure"); From 7e4c587ae32aca644254fa206de5131553975f4b Mon Sep 17 00:00:00 2001 From: Edwin Wong Date: Fri, 5 Feb 2021 12:47:20 -0800 Subject: [PATCH 03/61] [RESTRICT AUTOMERGE] Fix CryptoPlugin use after free vulnerability. The shared memory buffer used by srcPtr can be freed by another thread because it is not protected by a mutex. Subsequently, a use after free AIGABRT can occur in a race condition. SafetyNet logging is not added to avoid log spamming. The mutex lock is called to setup for decryption, which is called frequently. Test is run on rvc-dev branch, using target_hwasan-userdebug build. Test: sts sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Bug_176495665#testPocBug_176495665 Test: push to device with target_hwasan-userdebug build adb shell /data/local/tmp/Bug-176495665_sts64 Bug: 176495665 Bug: 176444161 Change-Id: If40e792cf78445a4b2dcce6a7d7905b5342c1724 --- drm/1.0/default/Android.bp | 3 ++- drm/1.0/default/CryptoPlugin.cpp | 9 +++++++-- drm/1.0/default/CryptoPlugin.h | 21 +++++++++++++-------- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/drm/1.0/default/Android.bp b/drm/1.0/default/Android.bp index ed6bcdeee6..1122e46a05 100644 --- a/drm/1.0/default/Android.bp +++ b/drm/1.0/default/Android.bp @@ -9,6 +9,7 @@ cc_library_static { "-Werror", "-Wextra", "-Wall", + "-Wthread-safety", ], shared_libs: [ "liblog", @@ -19,5 +20,5 @@ cc_library_static { export_header_lib_headers: [ "libutils_headers", ], - export_include_dirs : ["include"] + export_include_dirs: ["include"], } diff --git a/drm/1.0/default/CryptoPlugin.cpp b/drm/1.0/default/CryptoPlugin.cpp index 2a85f0e3d3..74047ff9e9 100644 --- a/drm/1.0/default/CryptoPlugin.cpp +++ b/drm/1.0/default/CryptoPlugin.cpp @@ -54,6 +54,8 @@ namespace implementation { sp hidlMemory = mapMemory(base); ALOGE_IF(hidlMemory == nullptr, "mapMemory returns nullptr"); + std::unique_lock lock(mSharedBufferLock); + // allow mapMemory to return nullptr mSharedBufferMap[bufferId] = hidlMemory; return Void(); @@ -66,7 +68,7 @@ namespace implementation { const SharedBuffer& source, uint64_t offset, const DestinationBuffer& destination, decrypt_cb _hidl_cb) { - + std::unique_lock lock(mSharedBufferLock); if (mSharedBufferMap.find(source.bufferId) == mSharedBufferMap.end()) { _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "source decrypt buffer base not set"); return Void(); @@ -80,7 +82,7 @@ namespace implementation { } } - android::CryptoPlugin::Mode legacyMode; + android::CryptoPlugin::Mode legacyMode = android::CryptoPlugin::kMode_Unencrypted; switch(mode) { case Mode::UNENCRYPTED: legacyMode = android::CryptoPlugin::kMode_Unencrypted; @@ -176,6 +178,9 @@ namespace implementation { _hidl_cb(Status::BAD_VALUE, 0, "invalid destination type"); return Void(); } + + // release mSharedBufferLock + lock.unlock(); ssize_t result = mLegacyPlugin->decrypt(secure, keyId.data(), iv.data(), legacyMode, legacyPattern, srcPtr, legacySubSamples, subSamples.size(), destPtr, &detailMessage); diff --git a/drm/1.0/default/CryptoPlugin.h b/drm/1.0/default/CryptoPlugin.h index 11cc2aae47..13fe898cb6 100644 --- a/drm/1.0/default/CryptoPlugin.h +++ b/drm/1.0/default/CryptoPlugin.h @@ -17,11 +17,14 @@ #ifndef ANDROID_HARDWARE_DRM_V1_0__CRYPTOPLUGIN_H #define ANDROID_HARDWARE_DRM_V1_0__CRYPTOPLUGIN_H -#include +#include #include +#include #include #include +#include + namespace android { namespace hardware { namespace drm { @@ -60,19 +63,21 @@ struct CryptoPlugin : public ICryptoPlugin { Return setSharedBufferBase(const ::android::hardware::hidl_memory& base, uint32_t bufferId) override; - Return decrypt(bool secure, const hidl_array& keyId, - const hidl_array& iv, Mode mode, const Pattern& pattern, - const hidl_vec& subSamples, const SharedBuffer& source, - uint64_t offset, const DestinationBuffer& destination, - decrypt_cb _hidl_cb) override; + Return decrypt( + bool secure, const hidl_array& keyId, const hidl_array& iv, + Mode mode, const Pattern& pattern, const hidl_vec& subSamples, + const SharedBuffer& source, uint64_t offset, const DestinationBuffer& destination, + decrypt_cb _hidl_cb) override NO_THREAD_SAFETY_ANALYSIS; // use unique_lock -private: + private: android::CryptoPlugin *mLegacyPlugin; - std::map > mSharedBufferMap; + std::map> mSharedBufferMap GUARDED_BY(mSharedBufferLock); CryptoPlugin() = delete; CryptoPlugin(const CryptoPlugin &) = delete; void operator=(const CryptoPlugin &) = delete; + + std::mutex mSharedBufferLock; }; } // namespace implementation From ae1c624ba44ccc43dc8371328a4b3caa017c0ff8 Mon Sep 17 00:00:00 2001 From: Edwin Wong Date: Sat, 23 Jan 2021 17:25:14 -0800 Subject: [PATCH 04/61] [RESTRICT AUTOMERGE] Fix CryptoPlugin use after free vulnerability. The shared memory buffer used by srcPtr can be freed by another thread because it is not protected by a mutex. Subsequently, a use after free AIGABRT can occur in a race condition. SafetyNet logging is not added to avoid log spamming. The mutex lock is called to setup for decryption, which is called frequently. Test is run on rvc-dev branch, using target_hwasan-userdebug build. Test: sts sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Bug_176495665#testPocBug_176495665 Test: push to device with target_hwasan-userdebug build adb shell /data/local/tmp/Bug-176495665_sts64 Bug: 176495665 Bug: 176444161 Change-Id: I3ec33cd444183f40ee76bec4c88dec0dac859cd3 --- drm/1.0/default/Android.bp | 3 ++- drm/1.0/default/CryptoPlugin.cpp | 7 ++++++- drm/1.0/default/CryptoPlugin.h | 21 +++++++++++++-------- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/drm/1.0/default/Android.bp b/drm/1.0/default/Android.bp index ed6bcdeee6..1122e46a05 100644 --- a/drm/1.0/default/Android.bp +++ b/drm/1.0/default/Android.bp @@ -9,6 +9,7 @@ cc_library_static { "-Werror", "-Wextra", "-Wall", + "-Wthread-safety", ], shared_libs: [ "liblog", @@ -19,5 +20,5 @@ cc_library_static { export_header_lib_headers: [ "libutils_headers", ], - export_include_dirs : ["include"] + export_include_dirs: ["include"], } diff --git a/drm/1.0/default/CryptoPlugin.cpp b/drm/1.0/default/CryptoPlugin.cpp index e71385da5d..9ba7ae7504 100644 --- a/drm/1.0/default/CryptoPlugin.cpp +++ b/drm/1.0/default/CryptoPlugin.cpp @@ -54,6 +54,8 @@ namespace implementation { sp hidlMemory = mapMemory(base); ALOGE_IF(hidlMemory == nullptr, "mapMemory returns nullptr"); + std::lock_guard shared_buffer_lock(mSharedBufferLock); + // allow mapMemory to return nullptr mSharedBufferMap[bufferId] = hidlMemory; return Void(); @@ -66,7 +68,7 @@ namespace implementation { const SharedBuffer& source, uint64_t offset, const DestinationBuffer& destination, decrypt_cb _hidl_cb) { - + std::unique_lock lock(mSharedBufferLock); if (mSharedBufferMap.find(source.bufferId) == mSharedBufferMap.end()) { _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "source decrypt buffer base not set"); return Void(); @@ -174,6 +176,9 @@ namespace implementation { _hidl_cb(Status::BAD_VALUE, 0, "invalid destination type"); return Void(); } + + // release mSharedBufferLock + lock.unlock(); ssize_t result = mLegacyPlugin->decrypt(secure, keyId.data(), iv.data(), legacyMode, legacyPattern, srcPtr, legacySubSamples.get(), subSamples.size(), destPtr, &detailMessage); diff --git a/drm/1.0/default/CryptoPlugin.h b/drm/1.0/default/CryptoPlugin.h index 11cc2aae47..13fe898cb6 100644 --- a/drm/1.0/default/CryptoPlugin.h +++ b/drm/1.0/default/CryptoPlugin.h @@ -17,11 +17,14 @@ #ifndef ANDROID_HARDWARE_DRM_V1_0__CRYPTOPLUGIN_H #define ANDROID_HARDWARE_DRM_V1_0__CRYPTOPLUGIN_H -#include +#include #include +#include #include #include +#include + namespace android { namespace hardware { namespace drm { @@ -60,19 +63,21 @@ struct CryptoPlugin : public ICryptoPlugin { Return setSharedBufferBase(const ::android::hardware::hidl_memory& base, uint32_t bufferId) override; - Return decrypt(bool secure, const hidl_array& keyId, - const hidl_array& iv, Mode mode, const Pattern& pattern, - const hidl_vec& subSamples, const SharedBuffer& source, - uint64_t offset, const DestinationBuffer& destination, - decrypt_cb _hidl_cb) override; + Return decrypt( + bool secure, const hidl_array& keyId, const hidl_array& iv, + Mode mode, const Pattern& pattern, const hidl_vec& subSamples, + const SharedBuffer& source, uint64_t offset, const DestinationBuffer& destination, + decrypt_cb _hidl_cb) override NO_THREAD_SAFETY_ANALYSIS; // use unique_lock -private: + private: android::CryptoPlugin *mLegacyPlugin; - std::map > mSharedBufferMap; + std::map> mSharedBufferMap GUARDED_BY(mSharedBufferLock); CryptoPlugin() = delete; CryptoPlugin(const CryptoPlugin &) = delete; void operator=(const CryptoPlugin &) = delete; + + std::mutex mSharedBufferLock; }; } // namespace implementation From bd78085f08d5e342a1e0b02dde7a25832c2dd62e Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Fri, 16 Apr 2021 18:54:28 +0000 Subject: [PATCH 05/61] audio HAL - fix UAFs Bug: 185259758 Test: N/A Change-Id: I5ec70b098a00746108e10ab39e966607d78c84ae Merged-In: I5ec70b098a00746108e10ab39e966607d78c84ae (cherry picked from commit a8ac7cf706be7a77589070ea7c62f8e1b94ce316) --- audio/2.0/default/StreamIn.cpp | 8 ++++---- audio/2.0/default/StreamOut.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/audio/2.0/default/StreamIn.cpp b/audio/2.0/default/StreamIn.cpp index 9c933a9201..fea9156f62 100644 --- a/audio/2.0/default/StreamIn.cpp +++ b/audio/2.0/default/StreamIn.cpp @@ -378,9 +378,9 @@ Return StreamIn::prepareForReading(uint32_t frameSize, } // Create and launch the thread. - auto tempReadThread = std::make_unique( - &mStopReadThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), - tempStatusMQ.get(), tempElfGroup.get()); + sp tempReadThread = + new ReadThread(&mStopReadThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), + tempStatusMQ.get(), tempElfGroup.get()); if (!tempReadThread->init()) { ALOGW("failed to start reader thread: %s", strerror(-status)); sendError(Result::INVALID_ARGUMENTS); @@ -396,7 +396,7 @@ Return StreamIn::prepareForReading(uint32_t frameSize, mCommandMQ = std::move(tempCommandMQ); mDataMQ = std::move(tempDataMQ); mStatusMQ = std::move(tempStatusMQ); - mReadThread = tempReadThread.release(); + mReadThread = tempReadThread; mEfGroup = tempElfGroup.release(); threadInfo.pid = getpid(); threadInfo.tid = mReadThread->getTid(); diff --git a/audio/2.0/default/StreamOut.cpp b/audio/2.0/default/StreamOut.cpp index 22dcd0c994..b9eaea8268 100644 --- a/audio/2.0/default/StreamOut.cpp +++ b/audio/2.0/default/StreamOut.cpp @@ -353,9 +353,9 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, } // Create and launch the thread. - auto tempWriteThread = std::make_unique( - &mStopWriteThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), - tempStatusMQ.get(), tempElfGroup.get()); + sp tempWriteThread = + new WriteThread(&mStopWriteThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), + tempStatusMQ.get(), tempElfGroup.get()); if (!tempWriteThread->init()) { ALOGW("failed to start writer thread: %s", strerror(-status)); sendError(Result::INVALID_ARGUMENTS); @@ -371,7 +371,7 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, mCommandMQ = std::move(tempCommandMQ); mDataMQ = std::move(tempDataMQ); mStatusMQ = std::move(tempStatusMQ); - mWriteThread = tempWriteThread.release(); + mWriteThread = tempWriteThread; mEfGroup = tempElfGroup.release(); threadInfo.pid = getpid(); threadInfo.tid = mWriteThread->getTid(); From 9f6d6ae26c1f0a6d03fb8c035565a25104aa3c6d Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 5 May 2021 18:07:59 -0700 Subject: [PATCH 06/61] audio HAL - fix UAFs resolve merge conflicts of bd78085f08d5e342a1e0b02dde7a25832c2dd62e to pi-dev Bug: 185259758 Test: N/A Change-Id: Ia85fb88e85e94b4d63fc155d89063bba6c61e875 Merged-In: I5ec70b098a00746108e10ab39e966607d78c84ae --- .../include/core/all-versions/default/StreamIn.impl.h | 8 ++++---- .../include/core/all-versions/default/StreamOut.impl.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/audio/core/all-versions/default/include/core/all-versions/default/StreamIn.impl.h b/audio/core/all-versions/default/include/core/all-versions/default/StreamIn.impl.h index 64c85ab5fc..adad331de1 100644 --- a/audio/core/all-versions/default/include/core/all-versions/default/StreamIn.impl.h +++ b/audio/core/all-versions/default/include/core/all-versions/default/StreamIn.impl.h @@ -387,9 +387,9 @@ Return StreamIn::prepareForReading(uint32_t frameSize, uint32_t framesCoun } // Create and launch the thread. - auto tempReadThread = - std::make_unique(&mStopReadThread, mStream, tempCommandMQ.get(), - tempDataMQ.get(), tempStatusMQ.get(), tempElfGroup.get()); + sp tempReadThread = + new ReadThread(&mStopReadThread, mStream, tempCommandMQ.get(), + tempDataMQ.get(), tempStatusMQ.get(), tempElfGroup.get()); if (!tempReadThread->init()) { ALOGW("failed to start reader thread: %s", strerror(-status)); sendError(Result::INVALID_ARGUMENTS); @@ -405,7 +405,7 @@ Return StreamIn::prepareForReading(uint32_t frameSize, uint32_t framesCoun mCommandMQ = std::move(tempCommandMQ); mDataMQ = std::move(tempDataMQ); mStatusMQ = std::move(tempStatusMQ); - mReadThread = tempReadThread.release(); + mReadThread = tempReadThread; mEfGroup = tempElfGroup.release(); threadInfo.pid = getpid(); threadInfo.tid = mReadThread->getTid(); diff --git a/audio/core/all-versions/default/include/core/all-versions/default/StreamOut.impl.h b/audio/core/all-versions/default/include/core/all-versions/default/StreamOut.impl.h index 6fb157f7de..a49059ccba 100644 --- a/audio/core/all-versions/default/include/core/all-versions/default/StreamOut.impl.h +++ b/audio/core/all-versions/default/include/core/all-versions/default/StreamOut.impl.h @@ -370,9 +370,9 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, uint32_t framesCou } // Create and launch the thread. - auto tempWriteThread = - std::make_unique(&mStopWriteThread, mStream, tempCommandMQ.get(), - tempDataMQ.get(), tempStatusMQ.get(), tempElfGroup.get()); + sp tempWriteThread = + new WriteThread(&mStopWriteThread, mStream, tempCommandMQ.get(), + tempDataMQ.get(), tempStatusMQ.get(), tempElfGroup.get()); if (!tempWriteThread->init()) { ALOGW("failed to start writer thread: %s", strerror(-status)); sendError(Result::INVALID_ARGUMENTS); @@ -388,7 +388,7 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, uint32_t framesCou mCommandMQ = std::move(tempCommandMQ); mDataMQ = std::move(tempDataMQ); mStatusMQ = std::move(tempStatusMQ); - mWriteThread = tempWriteThread.release(); + mWriteThread = tempWriteThread; mEfGroup = tempElfGroup.release(); threadInfo.pid = getpid(); threadInfo.tid = mWriteThread->getTid(); From 7a55bb5cf8efa7570cd672dad08cff6a78fb8f88 Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Tue, 22 Jun 2021 16:47:48 -0700 Subject: [PATCH 07/61] Add a unit test for remote_prov_utils This functionality will be used for the factory tooling, so we should test it. Additionally, some new functionality will soon be added, and it also needs to be tested. Ignore-AOSP-First: No merge path to aosp, will manually merge Test: libkeymint_remote_prov_support_test Bug: 191301285 Change-Id: I6a8798fc4b09fff1e829185a4b9e471921e5d2a9 --- security/keymint/support/Android.bp | 16 ++++++ .../keymint/support/remote_prov_utils.cpp | 4 ++ .../support/remote_prov_utils_test.cpp | 55 +++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 security/keymint/support/remote_prov_utils_test.cpp diff --git a/security/keymint/support/Android.bp b/security/keymint/support/Android.bp index 718133aa41..c2dba044bf 100644 --- a/security/keymint/support/Android.bp +++ b/security/keymint/support/Android.bp @@ -62,3 +62,19 @@ cc_library { "libcrypto", ], } + +cc_test { + name: "libkeymint_remote_prov_support_test", + srcs: ["remote_prov_utils_test.cpp"], + static_libs: [ + "libgmock", + "libgtest_main", + ], + shared_libs: [ + "libcppbor_external", + "libcppcose_rkp", + "libcrypto", + "libkeymaster_portable", + "libkeymint_remote_prov_support", + ], +} diff --git a/security/keymint/support/remote_prov_utils.cpp b/security/keymint/support/remote_prov_utils.cpp index 33f1ed3353..ac7cb6219f 100644 --- a/security/keymint/support/remote_prov_utils.cpp +++ b/security/keymint/support/remote_prov_utils.cpp @@ -31,6 +31,10 @@ bytevec randomBytes(size_t numBytes) { } ErrMsgOr generateEekChain(size_t length, const bytevec& eekId) { + if (length < 2) { + return "EEK chain must contain at least 2 certs."; + } + auto eekChain = cppbor::Array(); bytevec prev_priv_key; diff --git a/security/keymint/support/remote_prov_utils_test.cpp b/security/keymint/support/remote_prov_utils_test.cpp new file mode 100644 index 0000000000..fbf5b95897 --- /dev/null +++ b/security/keymint/support/remote_prov_utils_test.cpp @@ -0,0 +1,55 @@ +/* + * Copyright 2021 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 +#include +#include + +namespace aidl::android::hardware::security::keymint::remote_prov { +namespace { + +using ::keymaster::KeymasterBlob; +using ::keymaster::validateAndExtractEekPubAndId; +using ::testing::ElementsAreArray; + +TEST(RemoteProvUtilsTest, GenerateEekChainInvalidLength) { + ASSERT_FALSE(generateEekChain(1, /*eekId=*/{})); +} + +TEST(RemoteProvUtilsTest, GenerateEekChain) { + bytevec kTestEekId = {'t', 'e', 's', 't', 'I', 'd', 0}; + for (size_t length : {2, 3, 31}) { + auto get_eek_result = generateEekChain(length, kTestEekId); + ASSERT_TRUE(get_eek_result) << get_eek_result.message(); + + auto& [chain, pubkey, privkey] = *get_eek_result; + + auto validation_result = validateAndExtractEekPubAndId( + /*testMode=*/true, KeymasterBlob(chain.data(), chain.size())); + ASSERT_TRUE(validation_result.isOk()); + + auto& [eekPub, eekId] = *validation_result; + EXPECT_THAT(eekId, ElementsAreArray(kTestEekId)); + EXPECT_THAT(eekPub, ElementsAreArray(pubkey)); + } +} + +} // namespace +} // namespace aidl::android::hardware::security::keymint::remote_prov From 415f0ce4fe05f37ddd47a23dafc4489c3aea7ebb Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Tue, 22 Jun 2021 17:13:05 -0700 Subject: [PATCH 08/61] Add real GEEK for RKP factory enrollment Include a unit test to verify the GEEK cert chain is valid. Test: libkeymint_remote_prov_support_test Ignore-AOSP-First: No merge path to aosp, will manually merge Bug: 191301285 Change-Id: Icf9cfa165fbccb24b36b03ff3ce729a7e9c44cfd --- .../include/remote_prov/remote_prov_utils.h | 30 +++++++++++++++++++ .../keymint/support/remote_prov_utils.cpp | 14 +++++++++ .../support/remote_prov_utils_test.cpp | 29 ++++++++++++++++++ 3 files changed, 73 insertions(+) diff --git a/security/keymint/support/include/remote_prov/remote_prov_utils.h b/security/keymint/support/include/remote_prov/remote_prov_utils.h index e4261f31bc..b02d273fde 100644 --- a/security/keymint/support/include/remote_prov/remote_prov_utils.h +++ b/security/keymint/support/include/remote_prov/remote_prov_utils.h @@ -27,6 +27,31 @@ using namespace cppcose; extern bytevec kTestMacKey; +// The Google root key for the Endpoint Encryption Key chain, encoded as COSE_Sign1 +inline constexpr uint8_t kCoseEncodedRootCert[] = { + 0x84, 0x43, 0xa1, 0x01, 0x27, 0xa0, 0x58, 0x2a, 0xa4, 0x01, 0x01, 0x03, 0x27, 0x20, 0x06, + 0x21, 0x58, 0x20, 0x99, 0xb9, 0xee, 0xdd, 0x5e, 0xe4, 0x52, 0xf6, 0x85, 0xc6, 0x4c, 0x62, + 0xdc, 0x3e, 0x61, 0xab, 0x57, 0x48, 0x7d, 0x75, 0x37, 0x29, 0xad, 0x76, 0x80, 0x32, 0xd2, + 0xb3, 0xcb, 0x63, 0x58, 0xd9, 0x58, 0x40, 0x1e, 0x22, 0x08, 0x4b, 0xa4, 0xb7, 0xa4, 0xc8, + 0xd7, 0x4e, 0x03, 0x0e, 0xfe, 0xb8, 0xaf, 0x14, 0x4c, 0xa7, 0x3b, 0x6f, 0xa5, 0xcd, 0xdc, + 0xda, 0x79, 0xc6, 0x2b, 0x64, 0xfe, 0x99, 0x39, 0xaf, 0x76, 0xe7, 0x80, 0xfa, 0x66, 0x00, + 0x85, 0x0d, 0x07, 0x98, 0x2a, 0xac, 0x91, 0x5c, 0xa7, 0x25, 0x14, 0x49, 0x06, 0x34, 0x75, + 0xca, 0x8a, 0x27, 0x7a, 0xd9, 0xe3, 0x5a, 0x49, 0xeb, 0x02, 0x03}; + +// The Google Endpoint Encryption Key certificate, encoded as COSE_Sign1 +inline constexpr uint8_t kCoseEncodedGeekCert[] = { + 0x84, 0x43, 0xa1, 0x01, 0x27, 0xa0, 0x58, 0x4e, 0xa5, 0x01, 0x01, 0x02, 0x58, 0x20, + 0xd0, 0xae, 0xc1, 0x15, 0xca, 0x2a, 0xcf, 0x73, 0xae, 0x6b, 0xcc, 0xcb, 0xd1, 0x96, + 0x1d, 0x65, 0xe8, 0xb1, 0xdd, 0xd7, 0x4a, 0x1a, 0x37, 0xb9, 0x43, 0x3a, 0x97, 0xd5, + 0x99, 0xdf, 0x98, 0x08, 0x03, 0x38, 0x18, 0x20, 0x04, 0x21, 0x58, 0x20, 0xbe, 0x85, + 0xe7, 0x46, 0xc4, 0xa3, 0x42, 0x5a, 0x40, 0xd9, 0x36, 0x3a, 0xa6, 0x15, 0xd0, 0x2c, + 0x58, 0x7e, 0x3d, 0xdc, 0x33, 0x02, 0x32, 0xd2, 0xfc, 0x5e, 0x1e, 0x87, 0x25, 0x5f, + 0x72, 0x60, 0x58, 0x40, 0x9b, 0xcf, 0x90, 0xe2, 0x2e, 0x4b, 0xab, 0xd1, 0x18, 0xb1, + 0x0e, 0x8e, 0x5d, 0x20, 0x27, 0x4b, 0x84, 0x58, 0xfe, 0xfc, 0x32, 0x90, 0x7e, 0x72, + 0x05, 0x83, 0xbc, 0xd7, 0x82, 0xbe, 0xfa, 0x64, 0x78, 0x2d, 0x54, 0x10, 0x4b, 0xc0, + 0x31, 0xbf, 0x6b, 0xe8, 0x1e, 0x35, 0xe2, 0xf0, 0x2d, 0xce, 0x6c, 0x2f, 0x4f, 0xf2, + 0xf5, 0x4f, 0xa5, 0xd4, 0x83, 0xad, 0x96, 0xa2, 0xf1, 0x87, 0x58, 0x04}; + /** * Generates random bytes. */ @@ -44,6 +69,11 @@ struct EekChain { */ ErrMsgOr generateEekChain(size_t length, const bytevec& eekId); +/** + * Returns the CBOR-encoded, production Google Endpoint Encryption Key chain. + */ +bytevec getProdEekChain(); + struct BccEntryData { bytevec pubKey; }; diff --git a/security/keymint/support/remote_prov_utils.cpp b/security/keymint/support/remote_prov_utils.cpp index ac7cb6219f..982a1eb7b1 100644 --- a/security/keymint/support/remote_prov_utils.cpp +++ b/security/keymint/support/remote_prov_utils.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include + #include #include @@ -82,6 +84,18 @@ ErrMsgOr generateEekChain(size_t length, const bytevec& eekId) { return EekChain{eekChain.encode(), pub_key, priv_key}; } +bytevec getProdEekChain() { + bytevec prodEek; + prodEek.reserve(1 + sizeof(kCoseEncodedRootCert) + sizeof(kCoseEncodedGeekCert)); + + // In CBOR encoding, 0x82 indicates an array of two items + prodEek.push_back(0x82); + prodEek.insert(prodEek.end(), std::begin(kCoseEncodedRootCert), std::end(kCoseEncodedRootCert)); + prodEek.insert(prodEek.end(), std::begin(kCoseEncodedGeekCert), std::end(kCoseEncodedGeekCert)); + + return prodEek; +} + ErrMsgOr verifyAndParseCoseSign1Cwt(const cppbor::Array* coseSign1, const bytevec& signingCoseKey, const bytevec& aad) { if (!coseSign1 || coseSign1->size() != kCoseSign1EntryCount) { diff --git a/security/keymint/support/remote_prov_utils_test.cpp b/security/keymint/support/remote_prov_utils_test.cpp index fbf5b95897..c360c06506 100644 --- a/security/keymint/support/remote_prov_utils_test.cpp +++ b/security/keymint/support/remote_prov_utils_test.cpp @@ -14,13 +14,16 @@ * limitations under the License. */ +#include #include #include #include +#include #include #include #include #include +#include "keymaster/cppcose/cppcose.h" namespace aidl::android::hardware::security::keymint::remote_prov { namespace { @@ -51,5 +54,31 @@ TEST(RemoteProvUtilsTest, GenerateEekChain) { } } +TEST(RemoteProvUtilsTest, GetProdEekChain) { + auto chain = getProdEekChain(); + + auto validation_result = validateAndExtractEekPubAndId( + /*testMode=*/false, KeymasterBlob(chain.data(), chain.size())); + ASSERT_TRUE(validation_result.isOk()) << "Error: " << validation_result.moveError(); + + auto& [eekPub, eekId] = *validation_result; + + auto [geekCert, ignoredNewPos, error] = + cppbor::parse(kCoseEncodedGeekCert, sizeof(kCoseEncodedGeekCert)); + ASSERT_NE(geekCert, nullptr) << "Error: " << error; + ASSERT_NE(geekCert->asArray(), nullptr); + + auto& encodedGeekCoseKey = geekCert->asArray()->get(kCoseSign1Payload); + ASSERT_NE(encodedGeekCoseKey, nullptr); + ASSERT_NE(encodedGeekCoseKey->asBstr(), nullptr); + + auto geek = CoseKey::parse(encodedGeekCoseKey->asBstr()->value()); + ASSERT_TRUE(geek) << "Error: " << geek.message(); + + const std::vector empty; + EXPECT_THAT(eekId, ElementsAreArray(geek->getBstrValue(CoseKey::KEY_ID).value_or(empty))); + EXPECT_THAT(eekPub, ElementsAreArray(geek->getBstrValue(CoseKey::PUBKEY_X).value_or(empty))); +} + } // namespace } // namespace aidl::android::hardware::security::keymint::remote_prov From 3fd4ec47060c3e55de54bcb6678160fb5bdd01e4 Mon Sep 17 00:00:00 2001 From: Lev Proleev Date: Mon, 28 Jun 2021 13:10:54 +0100 Subject: [PATCH 09/61] Fix ordering of cache files requirements from device Data and model numbers were switched in the AIDL implementation of canonical Device. Bug: 190757709 Test: neuralnetworks_utils_hal_aidl_test Change-Id: I0d95b2d436994ffc877a4e02eb31f449b983e61e --- neuralnetworks/aidl/utils/src/Device.cpp | 2 +- neuralnetworks/aidl/utils/test/DeviceTest.cpp | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/neuralnetworks/aidl/utils/src/Device.cpp b/neuralnetworks/aidl/utils/src/Device.cpp index 0fd453b3c7..e80de0be76 100644 --- a/neuralnetworks/aidl/utils/src/Device.cpp +++ b/neuralnetworks/aidl/utils/src/Device.cpp @@ -119,7 +119,7 @@ nn::GeneralResult> getNumberOfCacheFilesNeededFrom << numberOfCacheFiles.numDataCache << " vs " << nn::kMaxNumberOfCacheFiles << ")"; } - return std::make_pair(numberOfCacheFiles.numDataCache, numberOfCacheFiles.numModelCache); + return std::make_pair(numberOfCacheFiles.numModelCache, numberOfCacheFiles.numDataCache); } } // namespace diff --git a/neuralnetworks/aidl/utils/test/DeviceTest.cpp b/neuralnetworks/aidl/utils/test/DeviceTest.cpp index e53b0a8df9..f121acaf7b 100644 --- a/neuralnetworks/aidl/utils/test/DeviceTest.cpp +++ b/neuralnetworks/aidl/utils/test/DeviceTest.cpp @@ -58,7 +58,7 @@ const std::string kInvalidName = ""; const std::shared_ptr kInvalidDevice; constexpr PerformanceInfo kNoPerformanceInfo = {.execTime = std::numeric_limits::max(), .powerUsage = std::numeric_limits::max()}; -constexpr NumberOfCacheFiles kNumberOfCacheFiles = {.numModelCache = nn::kMaxNumberOfCacheFiles, +constexpr NumberOfCacheFiles kNumberOfCacheFiles = {.numModelCache = nn::kMaxNumberOfCacheFiles - 1, .numDataCache = nn::kMaxNumberOfCacheFiles}; constexpr auto makeStatusOk = [] { return ndk::ScopedAStatus::ok(); }; @@ -300,6 +300,21 @@ TEST(DeviceTest, getSupportedExtensionsDeadObject) { EXPECT_EQ(result.error().code, nn::ErrorStatus::DEAD_OBJECT); } +TEST(DeviceTest, getNumberOfCacheFilesNeeded) { + // setup call + const auto mockDevice = createMockDevice(); + EXPECT_CALL(*mockDevice, getNumberOfCacheFilesNeeded(_)).Times(1); + + // run test + const auto result = Device::create(kName, mockDevice); + + // verify result + ASSERT_TRUE(result.has_value()); + constexpr auto kNumberOfCacheFilesPair = std::make_pair( + kNumberOfCacheFiles.numModelCache, kNumberOfCacheFiles.numDataCache); + EXPECT_EQ(result.value()->getNumberOfCacheFilesNeeded(), kNumberOfCacheFilesPair); +} + TEST(DeviceTest, getNumberOfCacheFilesNeededError) { // setup call const auto mockDevice = createMockDevice(); From 62272fc2f5fb8ed60eb42aaa2e7e417a7a4223b6 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Mon, 28 Jun 2021 13:18:23 +0100 Subject: [PATCH 10/61] KeyMint VTS: allow for stricter SharedSecret impls Bug: 192223752 Test: VtsAidlSharedSecretTargetTest Merged-Ind: Iccf2d0fe2a2d10ad12269dfecf78ea1d831c3ad4 Change-Id: Iccf2d0fe2a2d10ad12269dfecf78ea1d831c3ad4 Ignore-AOSP-First: already merged in aosp/master --- .../vts/functional/SharedSecretAidlTest.cpp | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/security/sharedsecret/aidl/vts/functional/SharedSecretAidlTest.cpp b/security/sharedsecret/aidl/vts/functional/SharedSecretAidlTest.cpp index 919f882631..51938baa82 100644 --- a/security/sharedsecret/aidl/vts/functional/SharedSecretAidlTest.cpp +++ b/security/sharedsecret/aidl/vts/functional/SharedSecretAidlTest.cpp @@ -268,10 +268,16 @@ TEST_F(SharedSecretAidlTest, ComputeSharedSecretShortNonce) { << "Shared secret service that provided tweaked param should fail to compute " "shared secret"; } else { - EXPECT_EQ(ErrorCode::OK, responses[i].error) << "Others should succeed"; - EXPECT_NE(correct_response, responses[i].sharing_check) - << "Others should calculate a different shared secret, due to the tweaked " - "nonce."; + // Other services *may* succeed, or may notice the invalid size for the nonce. + // However, if another service completes the computation, it should get the 'wrong' + // answer. + if (responses[i].error == ErrorCode::OK) { + EXPECT_NE(correct_response, responses[i].sharing_check) + << "Others should calculate a different shared secret, due to the tweaked " + "nonce."; + } else { + EXPECT_EQ(ErrorCode::INVALID_ARGUMENT, responses[i].error); + } } } } @@ -348,10 +354,16 @@ TEST_F(SharedSecretAidlTest, ComputeSharedSecretShortSeed) { << "Shared secret service that provided tweaked param should fail to compute " "shared secret"; } else { - EXPECT_EQ(ErrorCode::OK, responses[i].error) << "Others should succeed"; - EXPECT_NE(correct_response, responses[i].sharing_check) - << "Others should calculate a different shared secret, due to the tweaked " - "nonce."; + // Other services *may* succeed, or may notice the invalid size for the seed. + // However, if another service completes the computation, it should get the 'wrong' + // answer. + if (responses[i].error == ErrorCode::OK) { + EXPECT_NE(correct_response, responses[i].sharing_check) + << "Others should calculate a different shared secret, due to the tweaked " + "seed."; + } else { + EXPECT_EQ(ErrorCode::INVALID_ARGUMENT, responses[i].error); + } } } } From 382e34835d5c73447728378f244c99e16f91461a Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Mon, 28 Jun 2021 09:36:22 +0100 Subject: [PATCH 11/61] KeyMint HAL: clarify spec text - Make clear that CERTIFICATE_NOT_{BEFORE,AFTER} must be specified for generating/importing asymmetric keys. - Fix enforcement level of Tag::UNLOCKED_DEVICE_REQUIRED. - Fix reference to exportKey() for Tag::STORAGE_KEY to mention convertStorageKeyToEphemeral instead. - Mark Tag::CONFIRMATION_TOKEN as deprecated. Test: none, comment change Bug: 188672564 Merged-In: I68727b024f6b6743403941763aefca64e3eb091a Change-Id: I68727b024f6b6743403941763aefca64e3eb091a Ignore-AOSP-First: already merged in aosp/master --- .../security/keymint/IKeyMintDevice.aidl | 8 ++++++ .../hardware/security/keymint/Tag.aidl | 26 ++++++++++--------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl b/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl index 9cc795d582..4cecff7f7a 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl @@ -277,6 +277,10 @@ interface IKeyMintDevice { * must return ErrorCode::INVALID_ARGUMENT. The values 3 and 65537 must be supported. It is * recommended to support all prime values up to 2^64. * + * o Tag::CERTIFICATE_NOT_BEFORE and Tag::CERTIFICATE_NOT_AFTER specify the valid date range for + * the returned X.509 certificate holding the public key. If omitted, generateKey must return + * ErrorCode::MISSING_NOT_BEFORE or ErrorCode::MISSING_NOT_AFTER. + * * The following parameters are not necessary to generate a usable RSA key, but generateKey must * not return an error if they are omitted: * @@ -297,6 +301,10 @@ interface IKeyMintDevice { * Tag::EC_CURVE must be provided to generate an ECDSA key. If it is not provided, generateKey * must return ErrorCode::UNSUPPORTED_KEY_SIZE. TEE IKeyMintDevice implementations must support * all curves. StrongBox implementations must support P_256. + + * Tag::CERTIFICATE_NOT_BEFORE and Tag::CERTIFICATE_NOT_AFTER must be provided to specify the + * valid date range for the returned X.509 certificate holding the public key. If omitted, + * generateKey must return ErrorCode::MISSING_NOT_BEFORE or ErrorCode::MISSING_NOT_AFTER. * * == AES Keys == * diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl index 58e02b35b2..270574bbfb 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl @@ -483,12 +483,12 @@ enum Tag { /** * Tag::TRUSTED_CONFIRMATION_REQUIRED is only applicable to keys with KeyPurpose SIGN, and - * specifies that this key must not be usable unless the user provides confirmation of the data - * to be signed. Confirmation is proven to keyMint via an approval token. See - * CONFIRMATION_TOKEN, as well as the ConfirmationUI HAL. + * specifies that this key must not be usable unless the user provides confirmation of the data + * to be signed. Confirmation is proven to keyMint via an approval token. See the authToken + * parameter of begin(), as well as the ConfirmationUI HAL. * * If an attempt to use a key with this tag does not have a cryptographically valid - * CONFIRMATION_TOKEN provided to finish() or if the data provided to update()/finish() does not + * token provided to finish() or if the data provided to update()/finish() does not * match the data described in the token, keyMint must return NO_USER_CONFIRMATION. * * Must be hardware-enforced. @@ -497,9 +497,11 @@ enum Tag { /** * Tag::UNLOCKED_DEVICE_REQUIRED specifies that the key may only be used when the device is - * unlocked. + * unlocked, as reported to KeyMint via authToken operation parameter and the + * IKeyMintDevice::deviceLocked() method * - * Must be software-enforced. + * Must be hardware-enforced (but is also keystore-enforced on a per-user basis: see the + * deviceLocked() documentation). */ UNLOCKED_DEVICE_REQUIRED = (7 << 28) /* TagType:BOOL */ | 509, @@ -870,8 +872,9 @@ enum Tag { * * STORAGE_KEY is used to denote that a key generated or imported is a key used for storage * encryption. Keys of this type can either be generated or imported or secure imported using - * keyMint. exportKey() can be used to re-wrap storage key with a per-boot ephemeral key - * wrapped key once the key characteristics are enforced. + * keyMint. The convertStorageKeyToEphemeral() method of IKeyMintDevice can be used to re-wrap + * storage key with a per-boot ephemeral key wrapped key once the key characteristics are + * enforced. * * Keys with this tag cannot be used for any operation within keyMint. * ErrorCode::INVALID_OPERATION is returned when a key with Tag::STORAGE_KEY is provided to @@ -919,11 +922,10 @@ enum Tag { RESET_SINCE_ID_ROTATION = (7 << 28) /* TagType:BOOL */ | 1004, /** - * Tag::CONFIRMATION_TOKEN is used to deliver a cryptographic token proving that the user - * confirmed a signing request. The content is a full-length HMAC-SHA256 value. See the - * ConfirmationUI HAL for details of token computation. + * OBSOLETE: Do not use. See the authToken parameter for IKeyMintDevice::begin and for + * IKeyMintOperation methods instead. * - * Must never appear in KeyCharacteristics. + * TODO(b/191738660): Delete when keystore1 is deleted. */ CONFIRMATION_TOKEN = (9 << 28) /* TagType:BYTES */ | 1005, From ba831331399e310d53f766920f16a8da062b0f2a Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Tue, 29 Jun 2021 21:42:43 +0000 Subject: [PATCH 12/61] IFace: Add more comments Bug: 152412683 Test: N/A Change-Id: I0d444804acdd7e063c411cc2dec05efb26c6facf --- .../biometrics/face/EnrollmentType.aidl | 9 ++++++++ .../biometrics/face/FaceSensorType.aidl | 21 ++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/biometrics/face/aidl/android/hardware/biometrics/face/EnrollmentType.aidl b/biometrics/face/aidl/android/hardware/biometrics/face/EnrollmentType.aidl index d7f31756bc..c960933996 100644 --- a/biometrics/face/aidl/android/hardware/biometrics/face/EnrollmentType.aidl +++ b/biometrics/face/aidl/android/hardware/biometrics/face/EnrollmentType.aidl @@ -19,6 +19,15 @@ package android.hardware.biometrics.face; @VintfStability @Backing(type="byte") enum EnrollmentType { + /** + * Default enrollment type. + */ DEFAULT, + + /** + * Enrollment type for people with limited vision or mobility. For example, + * enrollment of this type will not ask the user to move their head or + * look directly at the device. + */ ACCESSIBILITY, } diff --git a/biometrics/face/aidl/android/hardware/biometrics/face/FaceSensorType.aidl b/biometrics/face/aidl/android/hardware/biometrics/face/FaceSensorType.aidl index 57f39d4f51..a5ed2e84e7 100644 --- a/biometrics/face/aidl/android/hardware/biometrics/face/FaceSensorType.aidl +++ b/biometrics/face/aidl/android/hardware/biometrics/face/FaceSensorType.aidl @@ -16,4 +16,23 @@ package android.hardware.biometrics.face; -@VintfStability @Backing(type="byte") enum FaceSensorType { UNKNOWN, RGB, IR } +@VintfStability +@Backing(type="byte") +enum FaceSensorType { + /** + * Placeholder value used for default initialization of FaceSensorType. + * This value means FaceSensorType wasn't explicitly initialized and must + * be discarded by the recipient. + */ + UNKNOWN, + + /** + * The face sensor is an RGB camera. + */ + RGB, + + /** + * The face sensor is an infrared camera. + */ + IR, +} From b22f307ccfd1072e3d61deb4e03659db7a168b7d Mon Sep 17 00:00:00 2001 From: Carter Hsu Date: Wed, 30 Jun 2021 14:33:09 +0800 Subject: [PATCH 13/61] audio: exclude the echo reference device in capture position test Bug: 192307382 Test: VtsHalAudioV7_0TargetTest --gtest_filter=*PcmOnlyConfigInputStreamTest* Signed-off-by: Carter Hsu Change-Id: I4e38e093bc3be7ee54c0c7cce4d1181e260a23bb --- .../android_audio_policy_configuration_V7_0-enums.h | 8 ++++++++ .../vts/functional/7.0/AudioPrimaryHidlHalTest.cpp | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/audio/common/7.0/enums/include/android_audio_policy_configuration_V7_0-enums.h b/audio/common/7.0/enums/include/android_audio_policy_configuration_V7_0-enums.h index a92a277471..79243b6e05 100644 --- a/audio/common/7.0/enums/include/android_audio_policy_configuration_V7_0-enums.h +++ b/audio/common/7.0/enums/include/android_audio_policy_configuration_V7_0-enums.h @@ -225,6 +225,14 @@ static inline bool isTelephonyDevice(const std::string& device) { return isTelephonyDevice(stringToAudioDevice(device)); } +static inline bool isEchoReferenceDevice(AudioDevice device) { + return device == AudioDevice::AUDIO_DEVICE_IN_ECHO_REFERENCE; +} + +static inline bool isEchoReferenceDevice(const std::string& device) { + return isEchoReferenceDevice(stringToAudioDevice(device)); +} + static inline bool maybeVendorExtension(const std::string& s) { // Only checks whether the string starts with the "vendor prefix". static const std::string vendorPrefix = "VX_"; diff --git a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp index 0b3098b872..79ac295151 100644 --- a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp @@ -710,7 +710,8 @@ class PcmOnlyConfigInputStreamTest : public InputStreamTest { // Returning 'true' when no source is found so the test can fail later with a more clear // problem description. return !maybeSourceAddress.has_value() || - !xsd::isTelephonyDevice(maybeSourceAddress.value().deviceType); + !(xsd::isTelephonyDevice(maybeSourceAddress.value().deviceType) || + xsd::isEchoReferenceDevice(maybeSourceAddress.value().deviceType)); } void createPatchIfNeeded() { From 23f624599f4f72b9e07a051104ccbfcaaaeda3d9 Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Fri, 25 Jun 2021 14:20:15 -0700 Subject: [PATCH 14/61] Add a utility to JSON-format a CSR with build info We need both the build fingerprint as well as the CSR when uploading data to the APFE provisioning server. Add a utility function to format the output as a JSON blob so that it may be easily collected in the factory in a serialized data format, then later uploaded. Test: libkeymint_remote_prov_support_test Test: VtsAidlKeyMintTargetTest Test: VtsHalRemotelyProvisionedComponentTargetTest Bug: 191301285 Change-Id: I751c5461876d83251869539f1a395ba13cb5cf84 --- .../keymint/aidl/vts/functional/Android.bp | 54 +++++++------------ security/keymint/support/Android.bp | 4 ++ .../include/remote_prov/remote_prov_utils.h | 22 ++++++++ .../keymint/support/remote_prov_utils.cpp | 42 +++++++++++++-- .../support/remote_prov_utils_test.cpp | 17 ++++++ 5 files changed, 101 insertions(+), 38 deletions(-) diff --git a/security/keymint/aidl/vts/functional/Android.bp b/security/keymint/aidl/vts/functional/Android.bp index ff08ce626e..386029f306 100644 --- a/security/keymint/aidl/vts/functional/Android.bp +++ b/security/keymint/aidl/vts/functional/Android.bp @@ -23,16 +23,11 @@ package { default_applicable_licenses: ["hardware_interfaces_license"], } -cc_test { - name: "VtsAidlKeyMintTargetTest", +cc_defaults { + name: "keymint_vts_defaults", defaults: [ - "VtsHalTargetTestDefaults", "use_libaidlvintf_gtest_helper_static", - ], - srcs: [ - "AttestKeyTest.cpp", - "DeviceUniqueAttestationTest.cpp", - "KeyMintTest.cpp", + "VtsHalTargetTestDefaults", ], shared_libs: [ "libbinder_ndk", @@ -43,9 +38,24 @@ cc_test { "android.hardware.security.secureclock-V1-ndk_platform", "libcppbor_external", "libcppcose_rkp", + "libjsoncpp", "libkeymint", "libkeymint_remote_prov_support", "libkeymint_support", + ], +} + +cc_test { + name: "VtsAidlKeyMintTargetTest", + defaults: [ + "keymint_vts_defaults", + ], + srcs: [ + "AttestKeyTest.cpp", + "DeviceUniqueAttestationTest.cpp", + "KeyMintTest.cpp", + ], + static_libs: [ "libkeymint_vts_test_utils", ], test_suites: [ @@ -57,8 +67,7 @@ cc_test { cc_test_library { name: "libkeymint_vts_test_utils", defaults: [ - "VtsHalTargetTestDefaults", - "use_libaidlvintf_gtest_helper_static", + "keymint_vts_defaults", ], srcs: [ "KeyMintAidlTestBase.cpp", @@ -66,45 +75,22 @@ cc_test_library { export_include_dirs: [ ".", ], - shared_libs: [ - "libbinder_ndk", - "libcrypto", - ], static_libs: [ - "android.hardware.security.keymint-V1-ndk_platform", - "android.hardware.security.secureclock-V1-ndk_platform", - "libcppbor_external", - "libcppcose_rkp", "libgmock_ndk", - "libkeymint", - "libkeymint_remote_prov_support", - "libkeymint_support", ], } cc_test { name: "VtsHalRemotelyProvisionedComponentTargetTest", defaults: [ - "VtsHalTargetTestDefaults", - "use_libaidlvintf_gtest_helper_static", + "keymint_vts_defaults", ], srcs: [ "VtsRemotelyProvisionedComponentTests.cpp", ], - shared_libs: [ - "libbinder_ndk", - "libcrypto", - ], static_libs: [ - "android.hardware.security.keymint-V1-ndk_platform", - "android.hardware.security.secureclock-V1-ndk_platform", - "libcppbor_external", - "libcppcose_rkp", "libgmock_ndk", "libkeymaster_portable", - "libkeymint", - "libkeymint_support", - "libkeymint_remote_prov_support", "libkeymint_vts_test_utils", "libpuresoftkeymasterdevice", ], diff --git a/security/keymint/support/Android.bp b/security/keymint/support/Android.bp index c2dba044bf..9e218b6a3d 100644 --- a/security/keymint/support/Android.bp +++ b/security/keymint/support/Android.bp @@ -57,9 +57,11 @@ cc_library { "include", ], shared_libs: [ + "libbase", "libcppbor_external", "libcppcose_rkp", "libcrypto", + "libjsoncpp", ], } @@ -71,9 +73,11 @@ cc_test { "libgtest_main", ], shared_libs: [ + "libbase", "libcppbor_external", "libcppcose_rkp", "libcrypto", + "libjsoncpp", "libkeymaster_portable", "libkeymint_remote_prov_support", ], diff --git a/security/keymint/support/include/remote_prov/remote_prov_utils.h b/security/keymint/support/include/remote_prov/remote_prov_utils.h index b02d273fde..406b7a9b79 100644 --- a/security/keymint/support/include/remote_prov/remote_prov_utils.h +++ b/security/keymint/support/include/remote_prov/remote_prov_utils.h @@ -87,4 +87,26 @@ struct BccEntryData { */ ErrMsgOr> validateBcc(const cppbor::Array* bcc); +struct JsonOutput { + static JsonOutput Ok(std::string json) { return {std::move(json), ""}; } + static JsonOutput Error(std::string error) { return {"", std::move(error)}; } + + std::string output; + std::string error; // if non-empty, this describes what went wrong +}; + +/** + * Take a given certificate request and output a JSON blob containing both the + * build fingerprint and certificate request. This data may be serialized, then + * later uploaded to the remote provisioning service. The input csr is not + * validated, only encoded. + * + * Output format: + * { + * "build_fingerprint": + * "csr": + * } + */ +JsonOutput jsonEncodeCsrWithBuild(const cppbor::Array& csr); + } // namespace aidl::android::hardware::security::keymint::remote_prov diff --git a/security/keymint/support/remote_prov_utils.cpp b/security/keymint/support/remote_prov_utils.cpp index 982a1eb7b1..0cbee51044 100644 --- a/security/keymint/support/remote_prov_utils.cpp +++ b/security/keymint/support/remote_prov_utils.cpp @@ -14,13 +14,15 @@ * limitations under the License. */ +#include #include -#include - -#include - +#include #include +#include +#include +#include +#include namespace aidl::android::hardware::security::keymint::remote_prov { @@ -180,4 +182,36 @@ ErrMsgOr> validateBcc(const cppbor::Array* bcc) { return result; } +JsonOutput jsonEncodeCsrWithBuild(const cppbor::Array& csr) { + const std::string kFingerprintProp = "ro.build.fingerprint"; + + if (!::android::base::WaitForPropertyCreation(kFingerprintProp)) { + return JsonOutput::Error("Unable to read build fingerprint"); + } + + bytevec csrCbor = csr.encode(); + size_t base64Length; + int rc = EVP_EncodedLength(&base64Length, csrCbor.size()); + if (!rc) { + return JsonOutput::Error("Error getting base64 length. Size overflow?"); + } + + std::vector base64(base64Length); + rc = EVP_EncodeBlock(reinterpret_cast(base64.data()), csrCbor.data(), csrCbor.size()); + ++rc; // Account for NUL, which BoringSSL does not for some reason. + if (rc != base64Length) { + return JsonOutput::Error("Error writing base64. Expected " + std::to_string(base64Length) + + " bytes to be written, but " + std::to_string(rc) + + " bytes were actually written."); + } + + Json::Value json(Json::objectValue); + json["build_fingerprint"] = ::android::base::GetProperty(kFingerprintProp, /*default=*/""); + json["csr"] = base64.data(); // Boring writes a NUL-terminated c-string + + Json::StreamWriterBuilder factory; + factory["indentation"] = ""; // disable pretty formatting + return JsonOutput::Ok(Json::writeString(factory, json)); +} + } // namespace aidl::android::hardware::security::keymint::remote_prov diff --git a/security/keymint/support/remote_prov_utils_test.cpp b/security/keymint/support/remote_prov_utils_test.cpp index c360c06506..8697c5190f 100644 --- a/security/keymint/support/remote_prov_utils_test.cpp +++ b/security/keymint/support/remote_prov_utils_test.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include #include @@ -23,6 +24,7 @@ #include #include #include +#include "cppbor.h" #include "keymaster/cppcose/cppcose.h" namespace aidl::android::hardware::security::keymint::remote_prov { @@ -80,5 +82,20 @@ TEST(RemoteProvUtilsTest, GetProdEekChain) { EXPECT_THAT(eekPub, ElementsAreArray(geek->getBstrValue(CoseKey::PUBKEY_X).value_or(empty))); } +TEST(RemoteProvUtilsTest, JsonEncodeCsr) { + cppbor::Array array; + array.add(1); + + auto [json, error] = jsonEncodeCsrWithBuild(array); + + ASSERT_TRUE(error.empty()) << error; + + std::string expected = R"({"build_fingerprint":")" + + ::android::base::GetProperty("ro.build.fingerprint", /*default=*/"") + + R"(","csr":"gQE="})"; + + ASSERT_EQ(json, expected); +} + } // namespace } // namespace aidl::android::hardware::security::keymint::remote_prov From 8b372b6734a9e787a94dd7a5c541ba30c11df060 Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Wed, 30 Jun 2021 18:26:35 +0000 Subject: [PATCH 15/61] IFace: update comments Bug: 152412683 Test: N/A Change-Id: I4bae4ba9eaa5631d72d5665b54dd5c2b23f7f4d5 --- .../hardware/biometrics/face/IFace.aidl | 22 +- .../hardware/biometrics/face/ISession.aidl | 192 +++++++++++------- .../biometrics/face/ISessionCallback.aidl | 26 +-- 3 files changed, 147 insertions(+), 93 deletions(-) diff --git a/biometrics/face/aidl/android/hardware/biometrics/face/IFace.aidl b/biometrics/face/aidl/android/hardware/biometrics/face/IFace.aidl index 11cdf7753e..4d7e59ebb7 100644 --- a/biometrics/face/aidl/android/hardware/biometrics/face/IFace.aidl +++ b/biometrics/face/aidl/android/hardware/biometrics/face/IFace.aidl @@ -25,28 +25,30 @@ interface IFace { /** * getSensorProps: * - * @return A list of properties for all face sensors available to the HAL. + * @return A list of properties for all of the face sensors supported by the HAL. */ SensorProps[] getSensorProps(); /** * createSession: * - * Creates a session that can be used by the framework to perform operations such as - * enroll, authenticate, etc. for the given sensorId and userId. + * Creates an instance of ISession that can be used by the framework to perform operations such + * as ISession#enroll, ISession#authenticate, etc. for the given sensorId and userId. * - * Calling this method while there is an active session is considered an error. If the - * framework is in a bad state and for some reason cannot close its session, it should use - * the reset method below. + * Calling this method while there is an active session is considered an error. If the framework + * wants to create a new session when it already has an active session, it must first cancel the + * current operation if it's cancellable or wait until it completes. Then, the framework must + * explicitly close the session with ISession#close. Once the framework receives + * ISessionCallback#onSessionClosed, a new session can be created. * * Implementations must store user-specific state or metadata in /data/vendor_de//facedata * as specified by the SELinux policy. The directory /data/vendor_de is managed by vold (see * vold_prepare_subdirs.cpp). Implementations may store additional user-specific data, such as - * embeddings or templates in StrongBox. + * embeddings or templates, in StrongBox. * - * @param sensorId The sensorId with which this session is being created. - * @param userId The userId with which this session is being created. - * @param cb A callback to notify the framework about the session's results and events. + * @param sensorId The sensorId for which this session is being created. + * @param userId The userId for which this session is being created. + * @param cb A callback to notify the framework about the session's events. * @return A new session. */ ISession createSession(in int sensorId, in int userId, in ISessionCallback cb); diff --git a/biometrics/face/aidl/android/hardware/biometrics/face/ISession.aidl b/biometrics/face/aidl/android/hardware/biometrics/face/ISession.aidl index a9a8c16cf2..2a57e3aa46 100644 --- a/biometrics/face/aidl/android/hardware/biometrics/face/ISession.aidl +++ b/biometrics/face/aidl/android/hardware/biometrics/face/ISession.aidl @@ -24,13 +24,12 @@ import android.hardware.common.NativeHandle; import android.hardware.keymaster.HardwareAuthToken; /** - * Operations that can be performed for unique sessions retrieved via IFace#createSession. * Operations defined within this interface can be divided into the following categories: * 1) Cancellable operations. These are usually the operations that can execute for several - * minutes. To allow for cancellation, they return an instance of ICancellationSignal that - * lets the framework cancel them by calling ICancellationSignal#cancel. If such an operation - * is cancelled, it must notify the framework by calling ISessionCallback#onError with - * Error::CANCELED. + * minutes. To allow for cancellation, they return an instance of ICancellationSignal that + * lets the framework cancel them by calling ICancellationSignal#cancel. If such an operation + * is cancelled, it must notify the framework by calling ISessionCallback#onError with + * Error::CANCELED. * 2) Non-cancellable operations. Such operations cannot be cancelled once started. * * The lifecycle of an operation ends when one of its terminal callbacks is called. For example, @@ -83,15 +82,20 @@ interface ISession { * | 0 | 10 | | | * ---------------------------------------------- * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onChallengeGenerated + * */ void generateChallenge(); /** * revokeChallenge: * - * Revokes a challenge that was previously generated. Note that if an invalid combination of - * parameters is requested, the implementation must still notify the framework using the - * provided callback. + * Revokes a challenge that was previously generated. Note that if a non-existent challenge is + * provided, the HAL must still notify the framework using ISessionCallback#onChallengeRevoked. + * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onChallengeRevoked * * @param challenge Challenge that should be revoked. */ @@ -100,9 +104,9 @@ interface ISession { /** * getEnrollmentConfig: * - * Returns the enrollment configuration depending on the provided enrollment type. Enrollment - * configuration determines how many stages the enrollment will have and the requirements for - * each of the stages. + * Returns the enrollment configuration for the provided enrollment type. Enrollment + * configuration determines how many stages the enrollment will have and the requirements + * for each of the stages. * * @param enrollmentType See the EnrollmentType enum. * @return An EnrollmentStageConfig array that describes each enrollment stage. @@ -117,22 +121,28 @@ interface ISession { * At any point during enrollment, if a non-recoverable error occurs, the HAL must notify the * framework via ISessionCallback#onError with the applicable enrollment-specific error. * - * Before capturing face data, the implementation must first verify the authenticity and - * integrity of the provided HardwareAuthToken. In addition, it must check that the challenge - * within the provided HardwareAuthToken is valid. See ISession#generateChallenge. If any of - * the above checks fail, the framework must be notified using ISessionCallback#onError with - * Error::UNABLE_TO_PROCESS. + * Before capturing face data, the HAL must first verify the authenticity and integrity of the + * provided HardwareAuthToken. In addition, it must check that the challenge within the provided + * HardwareAuthToken is valid. See ISession#generateChallenge. If any of the above checks fail, + * the framework must be notified using ISessionCallback#onError with Error::UNABLE_TO_PROCESS. * - * During enrollment, the implementation may notify the framework via - * ISessionCallback#onAcquired with messages that may be used to guide the user. This callback - * can be invoked multiple times if necessary. Similarly, the framework may be notified of - * enrollment progress changes via ISessionCallback#onEnrollmentProgress. Once the framework is - * notified that there are 0 "remaining" steps, the framework may cache the "enrollmentId". See + * During enrollment, the HAL may notify the framework via ISessionCallback#onAcquired with + * messages that may be used to guide the user. This callback can be invoked multiple times if + * necessary. Similarly, the framework may be notified of enrollment progress changes via + * ISessionCallback#onEnrollmentProgress. Once the framework is notified that there are 0 + * "remaining" steps, the framework may cache the "enrollmentId". See * ISessionCallback#onEnrollmentProgress for more info. * * When a face is successfully added and before the framework is notified of remaining=0, the - * implementation MUST update and associate this (sensorId, userId) pair with a new - * entropy-encoded random identifier. See ISession#getAuthenticatorId for more information. + * HAL must update and associate this (sensorId, userId) pair with a new entropy-encoded random + * identifier. See ISession#getAuthenticatorId for more information. + * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onError + * - ISessionCallback#onEnrollmentProgress(enrollmentId, remaining=0) + * + * Other applicable callbacks: + * - ISessionCallback#onAcquired * * @param hat See above documentation. * @param enrollmentType See the EnrollmentType enum. @@ -154,15 +164,18 @@ interface ISession { * At any point during authentication, if a non-recoverable error occurs, the HAL must notify * the framework via ISessionCallback#onError with the applicable authentication-specific error. * - * During authentication, the implementation may notify the framework via - * ISessionCallback#onAcquired with messages that may be used to guide the user. This callback - * can be invoked multiple times if necessary. + * During authentication, the HAL may notify the framework via ISessionCallback#onAcquired with + * messages that may be used to guide the user. This callback can be invoked multiple times if + * necessary. * - * The HAL must notify the framework of accepts/rejects via ISessionCallback#onAuthentication*. + * The HAL must notify the framework of accepts/rejects via + * ISessionCallback#onAuthenticationSucceeded and ISessionCallback#onAuthenticationFailed, + * correspondingly. * - * The authentication lifecycle ends when either - * 1) A face is accepted, and ISessionCallback#onAuthenticationSucceeded is invoked, or - * 2) Any non-recoverable error occurs (such as lockout). See the full list of + * The authentication lifecycle ends when any of the following happens: + * 1) A face is accepted, and ISessionCallback#onAuthenticationSucceeded is invoked. + * 2) A face is rejected, and ISessionCallback#onAuthenticationFailed is invoked. + * 3) Any non-recoverable error occurs (such as lockout). See the full list of * authentication-specific errors in the Error enum. * * Note that upon successful authentication, the lockout counter for this (sensorId, userId) @@ -174,16 +187,26 @@ interface ISession { * must be set with the operationId passed in during #authenticate. If the sensor is NOT * SensorStrength::STRONG, the HardwareAuthToken MUST be null. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onError + * - ISessionCallback#onAuthenticationSucceeded + * - ISessionCallback#onAuthenticationFailed + * + * Other applicable callbacks: + * - ISessionCallback#onAcquired + * - ISessionCallback#onLockoutTimed + * - ISessionCallback#onLockoutPermanent + * * @param operationId For sensors configured as SensorStrength::STRONG, this must be used ONLY * upon successful authentication and wrapped in the HardwareAuthToken's * "challenge" field and sent to the framework via - * ISessionCallback#onAuthenticated. The operationId is an opaque identifier - * created from a separate secure subsystem such as, but not limited to - * KeyStore/KeyMaster. The HardwareAuthToken can then be used as an - * attestation for the provided operation. For example, this is used - * to unlock biometric-bound auth-per-use keys (see + * ISessionCallback#onAuthenticationSucceeded. The operationId is an opaque + * identifier created from a separate secure subsystem such as, but not + * limited to KeyStore/KeyMaster. The HardwareAuthToken can then be used as + * an attestation for the provided operation. For example, this is used to + * unlock biometric-bound auth-per-use keys (see * setUserAuthenticationParameters in KeyGenParameterSpec.Builder and - * KeyProtection.Builder. + * KeyProtection.Builder). * @return ICancellationSignal An object that can be used by the framework to cancel this * operation. */ @@ -193,32 +216,36 @@ interface ISession { * detectInteraction: * * A request to start looking for faces without performing matching. Must only be called if - * SensorProps#supportsDetectInteraction is true. If invoked on implementations that do not - * support this functionality, the HAL must respond with ISession#onError(UNABLE_TO_PROCESS, 0). + * SensorProps#supportsDetectInteraction is true. If invoked on HALs that do not support this + * functionality, the HAL must respond with ISession#onError(UNABLE_TO_PROCESS, 0). * - * The framework will use this method in cases where determing user presence is required, but - * identifying/authentication is not. For example, when the device is encrypted (first boot) or - * in lockdown mode. + * The framework will use this operation in cases where determining user presence is required, + * but identifying/authenticating is not. For example, when the device is encrypted (first boot) + * or in lockdown mode. * * At any point during detectInteraction, if a non-recoverable error occurs, the HAL must notify * the framework via ISessionCallback#onError with the applicable error. * - * The implementation must only check for a face-like image was detected (e.g. to - * minimize interactions due to non-face objects), and the lockout counter must not - * be modified. + * The HAL must only check whether a face-like image was detected (e.g. to minimize interactions + * due to non-face objects), and the lockout counter must not be modified. * - * Upon detecting any face, the implementation must invoke - * ISessionCallback#onInteractionDetected. + * Upon detecting any face, the HAL must invoke ISessionCallback#onInteractionDetected. * - * The lifecycle of this operation ends when either + * The lifecycle of this operation ends when either: * 1) Any face is detected and the framework is notified via - * ISessionCallback#onInteractiondetected - * 2) The operation was cancelled by the framework (see ICancellationSignal) - * 3) An error occurred, for example ERROR::TIMEOUT + * ISessionCallback#onInteractionDetected. + * 2) An error occurrs, for example Error::TIMEOUT. * - * Note that if the operation is canceled, the implementation must notify the framework via + * Note that if the operation is canceled, the HAL must notify the framework via * ISessionCallback#onError with Error::CANCELED. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onError + * - ISessionCallback#onInteractionDetected + * + * Other applicable callbacks: + * - ISessionCallback#onAcquired + * * @return ICancellationSignal An object that can be used by the framework to cancel this * operation. */ @@ -227,12 +254,14 @@ interface ISession { /* * enumerateEnrollments: * - * A request to enumerate (list) the enrollments for this (sensorId, userId) pair. The - * framework typically uses this to ensure that its cache is in sync with the HAL. + * A request to enumerate (list) the enrollments for this (sensorId, userId) pair. The framework + * typically uses this to ensure that its cache is in sync with the HAL. * - * The implementation must then notify the framework with a list of enrollments applicable - * for the current session via ISessionCallback#onEnrollmentsEnumerated. + * The HAL must then notify the framework with a list of enrollments applicable for the current + * session via ISessionCallback#onEnrollmentsEnumerated. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onEnrollmentsEnumerated */ void enumerateEnrollments(); @@ -242,8 +271,12 @@ interface ISession { * A request to remove the enrollments for this (sensorId, userId) pair. * * After removing the enrollmentIds from everywhere necessary (filesystem, secure subsystems, - * etc), the implementation must notify the framework via ISessionCallback#onEnrollmentsRemoved. + * etc), the HAL must notify the framework via ISessionCallback#onEnrollmentsRemoved. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onEnrollmentsRemoved + * + * @param enrollmentIds a list of enrollments that should be removed. */ void removeEnrollments(in int[] enrollmentIds); @@ -257,6 +290,10 @@ interface ISession { * * The HAL must notify the framework about the result by calling * ISessionCallback#onFeaturesRetrieved. + * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onError + * - ISessionCallback#onFeaturesRetrieved */ void getFeatures(); @@ -264,15 +301,19 @@ interface ISession { * setFeature: * * Enables or disables a feature for this (sensorId, userId) pair. Because certain features may - * decrease security, the user must enter their password before this method is invoked - * (see @param hat). The HAL must verify the hat before changing any feature state. + * decrease security, the user must enter their password before this operation is invoked + * (see @param hat). The HAL must verify the HAT before changing any feature state. * - * If the hat is invalid or if the user is not enrolled, the HAL must invoke + * If the HAT is invalid or if the user is not enrolled, the HAL must invoke * ISessionCallback#onError with Error::UNABLE_TO_PROCESS. * * After the feature is successfully set, the HAL must notify the framework by calling * ISessionCallback#onFeatureSet. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onError + * - ISessionCallback#onFeatureSet + * * @param hat HardwareAuthToken See above documentation. * @param feature The feature to be enabled or disabled. * @param enabled Whether the provided features should be enabled or disabled. @@ -295,8 +336,8 @@ interface ISession { * KeyProtection.Builder.setInvalidatedByBiometricEnrollment. * * In addition, upon successful face authentication, the signed HAT that is returned to - * the framework via ISessionCallback#onAuthenticated must contain this identifier in the - * authenticatorId field. + * the framework via ISessionCallback#onAuthenticationSucceeded must contain this identifier in + * the authenticatorId field. * * Returns an entropy-encoded random identifier associated with the current set of enrollments * via ISessionCallback#onAuthenticatorIdRetrieved. The authenticatorId @@ -305,20 +346,21 @@ interface ISession { * 3) MUST not change if a face is deleted. * 4) MUST be an entropy-encoded random number * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onAuthenticatorIdRetrieved */ void getAuthenticatorId(); /** * invalidateAuthenticatorId: * - * This method only applies to sensors that are configured as SensorStrength::STRONG. If invoked - * by the framework for sensor of other strengths, the HAL should immediately invoke + * This operation only applies to sensors that are configured as SensorStrength::STRONG. If + * invoked by the framework for sensors of other strengths, the HAL should immediately invoke * ISessionCallback#onAuthenticatorIdInvalidated. * * The following only applies to sensors that are configured as SensorStrength::STRONG. * - * When invoked by the framework, the implementation must perform the following sequence of - * events: + * When invoked by the framework, the HAL must perform the following sequence of events: * 1) Update the authenticatorId with a new entropy-encoded random number * 2) Persist the new authenticatorId to non-ephemeral storage * 3) Notify the framework that the above is completed, via @@ -326,18 +368,20 @@ interface ISession { * * A practical use case of invalidation would be when the user adds a new enrollment to a sensor * managed by a different HAL instance. The public android.security.keystore APIs bind keys to - * "all biometrics" rather than "face-only" or "face-only" (see #getAuthenticatorId - * for more details). As such, the framework would coordinate invalidation across multiple - * biometric HALs as necessary. + * "all biometrics" rather than "fingerprint-only" or "face-only" (see #getAuthenticatorId for + * more details). As such, the framework would coordinate invalidation across multiple biometric + * HALs as necessary. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onAuthenticatorIdInvalidated */ void invalidateAuthenticatorId(); /** * resetLockout: * - * Requests the implementation to clear the lockout counter. Upon receiving this request, the - * implementation must perform the following: + * Requests the HAL to clear the lockout counter. Upon receiving this request, the HAL must + * perform the following: * 1) Verify the authenticity and integrity of the provided HAT * 2) Verify that the timestamp provided within the HAT is relatively recent (e.g. on the * order of minutes, not hours). @@ -373,6 +417,9 @@ interface ISession { * See the Android CDD section 7.3.10 for the full set of lockout and rate-limiting * requirements. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onLockoutCleared + * * @param hat HardwareAuthToken See above documentation. */ void resetLockout(in HardwareAuthToken hat); @@ -384,9 +431,14 @@ interface ISession { * If the HAL is busy performing a cancellable operation, the operation must be explicitly * cancelled with a call to ICancellationSignal#cancel before the session can be closed. * + * After a session is closed, the HAL must notify the framework by calling + * ISessionCallback#onSessionClosed. + * * All sessions must be explicitly closed. Calling IFace#createSession while there is an active * session is considered an error. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onSessionClosed */ void close(); } diff --git a/biometrics/face/aidl/android/hardware/biometrics/face/ISessionCallback.aidl b/biometrics/face/aidl/android/hardware/biometrics/face/ISessionCallback.aidl index 23570bd753..b3c348d521 100644 --- a/biometrics/face/aidl/android/hardware/biometrics/face/ISessionCallback.aidl +++ b/biometrics/face/aidl/android/hardware/biometrics/face/ISessionCallback.aidl @@ -37,11 +37,11 @@ interface ISessionCallback { /** * This method must only be used to notify the framework during the following operations: - * 1) ISession#authenticate - * 2) ISession#detectInteraction + * - ISession#authenticate + * - ISession#detectInteraction * - * These messages may be used to provide user guidance multiple times if necessary per - * operation. + * These messages may be used to provide user guidance multiple times per operation if + * necessary. * * @param frame See the AuthenticationFrame enum. */ @@ -51,8 +51,8 @@ interface ISessionCallback { * This method must only be used to notify the framework during the ISession#enroll * operation. * - * These messages may be used to provide user guidance multiple times if necessary per - * operation. + * These messages may be used to provide user guidance multiple times per operation if + * necessary. * * @param frame See the EnrollmentFrame enum. */ @@ -60,18 +60,18 @@ interface ISessionCallback { /** * This method must only be used to notify the framework during the following operations: - * 1) ISession#enroll - * 2) ISession#authenticate - * 3) ISession#detectInteraction - * 4) ISession#invalidateAuthenticatorId - * 5) ISession#resetLockout + * - ISession#enroll + * - ISession#authenticate + * - ISession#detectInteraction + * - ISession#invalidateAuthenticatorId + * - ISession#resetLockout * * These messages may be used to notify the framework or user that a non-recoverable error * has occurred. The operation is finished, and the HAL must proceed with the next operation * or return to the idling state. * - * Note that cancellation (see common::ICancellationSignal) and preemption must be followed with - * an Error::CANCELED message. + * Note that cancellation (see common::ICancellationSignal) must be followed with an + * Error::CANCELED message. * * @param error See the Error enum. * @param vendorCode Only valid if error == Error::VENDOR. The vendorCode must be used to index From ea8115d85d4530ae204abffb613059337c62c2ce Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Wed, 30 Jun 2021 18:27:27 +0000 Subject: [PATCH 16/61] IFingerprint: update comments Bug: 166824758 Test: N/A Change-Id: Ie77034b88bea1c1317e8d057f19f57167d48d2c3 --- .../biometrics/fingerprint/IFingerprint.aidl | 20 +- .../biometrics/fingerprint/ISession.aidl | 226 +++++++++++------- .../fingerprint/ISessionCallback.aidl | 28 +-- 3 files changed, 162 insertions(+), 112 deletions(-) diff --git a/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/IFingerprint.aidl b/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/IFingerprint.aidl index 271a9bf1cf..75f90a12ae 100644 --- a/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/IFingerprint.aidl +++ b/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/IFingerprint.aidl @@ -25,31 +25,31 @@ interface IFingerprint { /** * getSensorProps: * - * @return A list of properties for all sensors that an instance of the HAL supports. + * @return A list of properties for all of the fingerprint sensors supported by the HAL. */ SensorProps[] getSensorProps(); /** * createSession: * - * Creates a instance of ISession which can be used by the framework to perform operations - * such as ISession#enroll, ISession#authenticate, etc. for the given sensorId and userId. + * Creates an instance of ISession that can be used by the framework to perform operations such + * as ISession#enroll, ISession#authenticate, etc. for the given sensorId and userId. * * Calling this method while there is an active session is considered an error. If the framework * wants to create a new session when it already has an active session, it must first cancel the - * current operation if it's cancellable, or wait until it completes. Then, the framework must + * current operation if it's cancellable or wait until it completes. Then, the framework must * explicitly close the session with ISession#close. Once the framework receives * ISessionCallback#onSessionClosed, a new session can be created. * * Implementations must store user-specific state or metadata in /data/vendor_de//fpdata - * as specified by the SeLinux policy. This directory is created/removed by vold (see + * as specified by the SELinux policy. The directory /data/vendor_de is managed by vold (see * vold_prepare_subdirs.cpp). Implementations may store additional user-specific data, such as - * embeddings or templates in StrongBox. + * embeddings or templates, in StrongBox. * - * @param sensorId The sensor with which this session is being created. - * @param userId The userId with which this session is being created. - * @param cb Used to notify the framework. - * @return A new session + * @param sensorId The sensorId for which this session is being created. + * @param userId The userId for which this session is being created. + * @param cb A callback to notify the framework about the session's events. + * @return A new session. */ ISession createSession(in int sensorId, in int userId, in ISessionCallback cb); } diff --git a/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/ISession.aidl b/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/ISession.aidl index 02ef138427..f1d96d3039 100644 --- a/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/ISession.aidl +++ b/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/ISession.aidl @@ -20,30 +20,29 @@ import android.hardware.biometrics.common.ICancellationSignal; import android.hardware.keymaster.HardwareAuthToken; /** - * Operations that can be performed for unique sessions retrieved via IFingerprint#createSession. - * Methods defined within this interface can be split into the following categories: - * 1) Non-interrupting operations. These operations are handled by the HAL in FIFO order. - * 1a) Cancellable operations. These are usually the operations that can execute for several - * minutes. To allow for cancellation, they return an instance of ICancellationSignal that - * lets the framework cancel them by calling ICancellationSignal#cancel. If such an operation - * is cancelled, it must notify the framework by calling ISessionCallback#onError with - * Error::CANCELED. - * 1b) Non-cancellable operations. Such operations cannot be cancelled once started. + * Operations defined within this interface can be split into the following categories: + * 1) Non-interrupting operations. These operations are handled by the HAL in a FIFO order. + * 1a) Cancellable operations. These operations can usually run for several minutes. To allow + * for cancellation, they return an instance of ICancellationSignal that allows the + * framework to cancel them by calling ICancellationSignal#cancel. If such an operation is + * cancelled, it must notify the framework by calling ISessionCallback#onError with + * Error::CANCELED. + * 1b) Non-cancellable operations. Such operations cannot be cancelled once started. * 2) Interrupting operations. These operations may be invoked by the framework immediately, * regardless of whether another operation is executing. For example, on devices with sensors - * of FingerprintSensorType::UNDER_DISPLAY_*, ISession#onFingerDown may be invoked while the + * of FingerprintSensorType::UNDER_DISPLAY_*, ISession#onPointerDown may be invoked while the * HAL is executing ISession#enroll, ISession#authenticate or ISession#detectInteraction. * - * The lifecycle of a non-interrupting operation ends when one of its terminal callbacks is called. - * For example, ISession#authenticate is considered completed when either of the following callbacks - * is called: ISessionCallback#onError or ISessionCallback#onAuthenticationSucceeded. + * The lifecycle of a non-interrupting operation ends when one of its final callbacks is called. + * For example, ISession#authenticate is considered completed when either ISessionCallback#onError + * or ISessionCallback#onAuthenticationSucceeded is called. * * The lifecycle of an interrupting operation ends when it returns. Interrupting operations do not * have callbacks. * * ISession only supports execution of one non-interrupting operation at a time, regardless of - * whether it's cancellable. The framework must wait for a corresponding callback indicating the end - * of the current non-interrupting operation before a new non-interrupting operation can be started. + * whether it's cancellable. The framework must wait for a callback indicating the end of the + * current non-interrupting operation before a new non-interrupting operation can be started. */ @VintfStability interface ISession { @@ -89,15 +88,19 @@ interface ISession { * | 0 | 10 | | | * ---------------------------------------------- * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onChallengeGenerated */ void generateChallenge(); /** * revokeChallenge: * - * Revokes a challenge that was previously generated. Note that if an invalid combination of - * parameters is requested, the implementation must still notify the framework using the - * provided callback. + * Revokes a challenge that was previously generated. Note that if a non-existent challenge is + * provided, the HAL must still notify the framework using ISessionCallback#onChallengeRevoked. + * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onChallengeRevoked * * @param challenge Challenge that should be revoked. */ @@ -111,26 +114,33 @@ interface ISession { * At any point during enrollment, if a non-recoverable error occurs, the HAL must notify the * framework via ISessionCallback#onError with the applicable enrollment-specific error. * - * Before capturing fingerprint data, the implementation must first verify the authenticity and - * integrity of the provided HardwareAuthToken. In addition, it must check that the challenge - * within the provided HardwareAuthToken is valid. See ISession#generateChallenge. If any of - * the above checks fail, the framework must be notified via ISessionCallback#onError and the - * HAL must notify the framework when it returns to the idle state. See + * Before capturing fingerprint data, the HAL must first verify the authenticity and integrity + * of the provided HardwareAuthToken. In addition, it must check that the challenge within the + * provided HardwareAuthToken is valid. See ISession#generateChallenge. If any of the above + * checks fail, the framework must be notified using ISessionCallback#onError with * Error::UNABLE_TO_PROCESS. * - * During enrollment, the implementation may notify the framework via - * ISessionCallback#onAcquired with messages that may be used to guide the user. This callback - * can be invoked multiple times if necessary. Similarly, the framework may be notified of - * enrollment progress changes via ISessionCallback#onEnrollmentProgress. Once the framework is - * notified that there are 0 "remaining" steps, the framework may cache the "enrollmentId". See - * ISessionCallback#onEnrollmentProgress for more info. The HAL must notify the framework once - * it returns to the idle state. + * During enrollment, the HAL may notify the framework via ISessionCallback#onAcquired with + * messages that may be used to guide the user. This callback can be invoked multiple times if + * necessary. Similarly, the framework may be notified of enrollment progress changes via + * ISessionCallback#onEnrollmentProgress. Once the framework is notified that there are 0 + * "remaining" steps, the framework may cache the "enrollmentId". See + * ISessionCallback#onEnrollmentProgress for more info. * * When a finger is successfully added and before the framework is notified of remaining=0, the - * implementation MUST update and associate this (sensorId, userId) pair with a new new - * entropy-encoded random identifier. See ISession#getAuthenticatorId for more information. + * HAL must update and associate this (sensorId, userId) pair with a new entropy-encoded random + * identifier. See ISession#getAuthenticatorId for more information. + * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onError + * - ISessionCallback#onEnrollmentProgress(enrollmentId, remaining=0) + * + * Other applicable callbacks: + * - ISessionCallback#onAcquired * * @param hat See above documentation. + * @return ICancellationSignal An object that can be used by the framework to cancel this + * operation. */ ICancellationSignal enroll(in HardwareAuthToken hat); @@ -142,14 +152,16 @@ interface ISession { * At any point during authentication, if a non-recoverable error occurs, the HAL must notify * the framework via ISessionCallback#onError with the applicable authentication-specific error. * - * During authentication, the implementation may notify the framework via - * ISessionCallback#onAcquired with messages that may be used to guide the user. This callback - * can be invoked multiple times if necessary. + * During authentication, the HAL may notify the framework via ISessionCallback#onAcquired with + * messages that may be used to guide the user. This callback can be invoked multiple times if + * necessary. * - * The HAL must notify the framework of accepts/rejects via ISessionCallback#onAuthentication*. + * The HAL must notify the framework of accepts and rejects via + * ISessionCallback#onAuthenticationSucceeded and ISessionCallback#onAuthenticationFailed, + * correspondingly. * - * The authentication lifecycle ends when either - * 1) A fingerprint is accepted, and ISessionCallback#onAuthenticationSucceeded is invoked, or + * The authentication lifecycle ends when either: + * 1) A fingerprint is accepted, and ISessionCallback#onAuthenticationSucceeded is invoked. * 2) Any non-recoverable error occurs (such as lockout). See the full list of * authentication-specific errors in the Error enum. * @@ -162,16 +174,28 @@ interface ISession { * must be set with the operationId passed in during #authenticate. If the sensor is NOT * SensorStrength::STRONG, the HardwareAuthToken MUST be null. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onError + * - ISessionCallback#onAuthenticationSucceeded + * + * Other applicable callbacks: + * - ISessionCallback#onAcquired + * - ISessionCallback#onAuthenticationFailed + * - ISessionCallback#onLockoutTimed + * - ISessionCallback#onLockoutPermanent + * * @param operationId For sensors configured as SensorStrength::STRONG, this must be used ONLY * upon successful authentication and wrapped in the HardwareAuthToken's * "challenge" field and sent to the framework via - * ISessionCallback#onAuthenticated. The operationId is an opaque identifier - * created from a separate secure subsystem such as, but not limited to - * KeyStore/KeyMaster. The HardwareAuthToken can then be used as an - * attestation for the provided operation. For example, this is used - * to unlock biometric-bound auth-per-use keys (see + * ISessionCallback#onAuthenticationSucceeded. The operationId is an opaque + * identifier created from a separate secure subsystem such as, but not + * limited to KeyStore/KeyMaster. The HardwareAuthToken can then be used as + * an attestation for the provided operation. For example, this is used to + * unlock biometric-bound auth-per-use keys (see * setUserAuthenticationParameters in KeyGenParameterSpec.Builder and - * KeyProtection.Builder. + * KeyProtection.Builder). + * @return ICancellationSignal An object that can be used by the framework to cancel this + * operation. */ ICancellationSignal authenticate(in long operationId); @@ -179,44 +203,52 @@ interface ISession { * detectInteraction: * * A request to start looking for fingerprints without performing matching. Must only be called - * if SensorProps#supportsDetectInteraction is true. If invoked on implementations that do not - * support this functionality, the HAL must respond with ISession#onError(UNABLE_TO_PROCESS, 0). + * if SensorProps#supportsDetectInteraction is true. If invoked on HALs that do not support this + * functionality, the HAL must respond with ISession#onError(UNABLE_TO_PROCESS, 0). * - * The framework will use this method in cases where determing user presence is required, but - * identifying/authentication is not. For example, when the device is encrypted (first boot) or - * in lockdown mode. + * The framework will use this operation in cases where determining user presence is required, + * but identifying/authenticating is not. For example, when the device is encrypted (first boot) + * or in lockdown mode. * * At any point during detectInteraction, if a non-recoverable error occurs, the HAL must notify * the framework via ISessionCallback#onError with the applicable error. * - * The implementation must only check for a fingerprint-like image was detected (e.g. to - * minimize interactions due to non-fingerprint objects), and the lockout counter must not - * be modified. + * The HAL must only check whether a fingerprint-like image was detected (e.g. to minimize + * interactions due to non-fingerprint objects), and the lockout counter must not be modified. * - * Upon detecting any fingerprint, the implementation must invoke - * ISessionCallback#onInteractionDetected. + * Upon detecting any fingerprint, the HAL must invoke ISessionCallback#onInteractionDetected. * - * The lifecycle of this operation ends when either + * The lifecycle of this operation ends when either: * 1) Any fingerprint is detected and the framework is notified via - * ISessionCallback#onInteractiondetected - * 2) The operation was cancelled by the framework (see ICancellationSignal) - * 3) The HAL ends the operation, for example when a subsequent operation pre-empts this one. + * ISessionCallback#onInteractionDetected. + * 2) An error occurs, for example Error::TIMEOUT. * - * Note that if the operation is canceled, the implementation must notify the framework via + * Note that if the operation is canceled, the HAL must notify the framework via * ISessionCallback#onError with Error::CANCELED. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onError + * - ISessionCallback#onInteractionDetected + * + * Other applicable callbacks: + * - ISessionCallback#onAcquired + * + * @return ICancellationSignal An object that can be used by the framework to cancel this + * operation. */ ICancellationSignal detectInteraction(); /* * enumerateEnrollments: * - * A request to enumerate (list) the enrollments for this (sensorId, userId) pair. The - * framework typically uses this to ensure that its cache is in sync with the HAL. + * A request to enumerate (list) the enrollments for this (sensorId, userId) pair. The framework + * typically uses this to ensure that its cache is in sync with the HAL. * - * The implementation must then notify the framework with a list of enrollments applicable - * for the current session via ISessionCallback#onEnrollmentsEnumerated. + * The HAL must then notify the framework with a list of enrollments applicable for the current + * session via ISessionCallback#onEnrollmentsEnumerated. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onEnrollmentsEnumerated */ void enumerateEnrollments(); @@ -226,8 +258,12 @@ interface ISession { * A request to remove the enrollments for this (sensorId, userId) pair. * * After removing the enrollmentIds from everywhere necessary (filesystem, secure subsystems, - * etc), the implementation must notify the framework via ISessionCallback#onEnrollmentsRemoved. + * etc), the HAL must notify the framework via ISessionCallback#onEnrollmentsRemoved. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onEnrollmentsRemoved + * + * @param enrollmentIds a list of enrollments that should be removed. */ void removeEnrollments(in int[] enrollmentIds); @@ -240,15 +276,15 @@ interface ISession { * The following only applies to sensors that are configured as SensorStrength::STRONG. * * The authenticatorId is a (sensorId, user)-specific identifier which can be used during key - * generation and key import to to associate a key (in KeyStore / KeyMaster) with the current - * set of enrolled fingerprints. For example, the following public Android APIs allow for keys - * to be invalidated when the user adds a new enrollment after the key was created: + * generation and import to associate the key (in KeyStore / KeyMaster) with the current set of + * enrolled fingerprints. For example, the following public Android APIs allow for keys to be + * invalidated when the user adds a new enrollment after the key was created: * KeyGenParameterSpec.Builder.setInvalidatedByBiometricEnrollment and * KeyProtection.Builder.setInvalidatedByBiometricEnrollment. * * In addition, upon successful fingerprint authentication, the signed HAT that is returned to - * the framework via ISessionCallback#onAuthenticated must contain this identifier in the - * authenticatorId field. + * the framework via ISessionCallback#onAuthenticationSucceeded must contain this identifier in + * the authenticatorId field. * * Returns an entropy-encoded random identifier associated with the current set of enrollments * via ISessionCallback#onAuthenticatorIdRetrieved. The authenticatorId @@ -257,20 +293,21 @@ interface ISession { * 3) MUST not change if a fingerprint is deleted. * 4) MUST be an entropy-encoded random number * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onAuthenticatorIdRetrieved */ void getAuthenticatorId(); /** * invalidateAuthenticatorId: * - * This method only applies to sensors that are configured as SensorStrength::STRONG. If invoked - * by the framework for sensor of other strengths, the HAL should immediately invoke + * This operation only applies to sensors that are configured as SensorStrength::STRONG. If + * invoked by the framework for sensors of other strengths, the HAL should immediately invoke * ISessionCallback#onAuthenticatorIdInvalidated. * * The following only applies to sensors that are configured as SensorStrength::STRONG. * - * When invoked by the framework, the implementation must perform the following sequence of - * events: + * When invoked by the framework, the HAL must perform the following sequence of events: * 1) Update the authenticatorId with a new entropy-encoded random number * 2) Persist the new authenticatorId to non-ephemeral storage * 3) Notify the framework that the above is completed, via @@ -278,23 +315,25 @@ interface ISession { * * A practical use case of invalidation would be when the user adds a new enrollment to a sensor * managed by a different HAL instance. The public android.security.keystore APIs bind keys to - * "all biometrics" rather than "fingerprint-only" or "face-only" (see #getAuthenticatorId - * for more details). As such, the framework would coordinate invalidation across multiple - * biometric HALs as necessary. + * "all biometrics" rather than "fingerprint-only" or "face-only" (see #getAuthenticatorId for + * more details). As such, the framework would coordinate invalidation across multiple biometric + * HALs as necessary. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onAuthenticatorIdInvalidated */ void invalidateAuthenticatorId(); /** * resetLockout: * - * Requests the implementation to clear the lockout counter. Upon receiving this request, the - * implementation must perform the following: + * Requests the HAL to clear the lockout counter. Upon receiving this request, the HAL must + * perform the following: * 1) Verify the authenticity and integrity of the provided HAT * 2) Verify that the timestamp provided within the HAT is relatively recent (e.g. on the * order of minutes, not hours). * If either of the checks fail, the HAL must invoke ISessionCallback#onError with - * Error::UNABLE_TO_PROCESS and return to the idling state. + * Error::UNABLE_TO_PROCESS. * * Upon successful verification, the HAL must clear the lockout counter and notify the framework * via ISessionCallback#onLockoutCleared. @@ -325,6 +364,9 @@ interface ISession { * See the Android CDD section 7.3.10 for the full set of lockout and rate-limiting * requirements. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onLockoutCleared + * * @param hat HardwareAuthToken See above documentation. */ void resetLockout(in HardwareAuthToken hat); @@ -343,6 +385,8 @@ interface ISession { * All sessions must be explicitly closed. Calling IFingerprint#createSession while there is an * active session is considered an error. * + * Callbacks that signify the end of this operation's lifecycle: + * - ISessionCallback#onSessionClosed */ void close(); @@ -353,16 +397,16 @@ interface ISession { /** * onPointerDown: * - * This method only applies to sensors that are configured as + * This operation only applies to sensors that are configured as * FingerprintSensorType::UNDER_DISPLAY_*. If invoked erroneously by the framework for sensors * of other types, the HAL must treat this as a no-op and return immediately. * - * For sensors of type FingerprintSensorType::UNDER_DISPLAY_*, this method is used to notify the - * HAL of display touches. This method can be invoked when the HAL is performing any one of: - * ISession#authenticate, ISession#enroll, ISession#detectInteraction. + * This operation is used to notify the HAL of display touches. This operation can be invoked + * when the HAL is performing any one of: ISession#authenticate, ISession#enroll, + * ISession#detectInteraction. * - * Note that the framework will only invoke this method if the event occurred on the display on - * which this sensor is located. + * Note that the framework will only invoke this operation if the event occurred on the display + * on which this sensor is located. * * Note that for sensors which require illumination such as * FingerprintSensorType::UNDER_DISPLAY_OPTICAL, and where illumination is handled below the @@ -379,10 +423,13 @@ interface ISession { /** * onPointerUp: * - * This method only applies to sensors that are configured as + * This operation only applies to sensors that are configured as * FingerprintSensorType::UNDER_DISPLAY_*. If invoked for sensors of other types, the HAL must * treat this as a no-op and return immediately. * + * This operation can be invoked when the HAL is performing any one of: ISession#authenticate, + * ISession#enroll, ISession#detectInteraction. + * * @param pointerId See android.view.MotionEvent#getPointerId */ void onPointerUp(in int pointerId); @@ -390,12 +437,15 @@ interface ISession { /* * onUiReady: * - * This method only applies to sensors that are configured as + * This operation only applies to sensors that are configured as * FingerprintSensorType::UNDER_DISPLAY_OPTICAL. If invoked for sensors of other types, the HAL * must treat this as a no-op and return immediately. * + * This operation can be invoked when the HAL is performing any one of: ISession#authenticate, + * ISession#enroll, ISession#detectInteraction. + * * For FingerprintSensorType::UNDER_DISPLAY_OPTICAL where illumination is handled above the - * HAL, the framework will invoke this method to notify that the illumination has started. + * HAL, the framework will invoke this operation to notify when the illumination is showing. */ void onUiReady(); } diff --git a/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/ISessionCallback.aidl b/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/ISessionCallback.aidl index 95657b3d7b..f699966f59 100644 --- a/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/ISessionCallback.aidl +++ b/biometrics/fingerprint/aidl/android/hardware/biometrics/fingerprint/ISessionCallback.aidl @@ -34,12 +34,12 @@ interface ISessionCallback { /** * This method must only be used to notify the framework during the following operations: - * 1) ISession#enroll - * 2) ISession#authenticate - * 3) ISession#detectInteraction + * - ISession#enroll + * - ISession#authenticate + * - ISession#detectInteraction * - * These messages may be used to provide user guidance multiple times if necessary per - * operation. + * These messages may be used to provide user guidance multiple times per operation if + * necessary. * * @param info See the AcquiredInfo enum. * @param vendorCode Only valid if info == AcquiredInfo::VENDOR. The vendorCode must be used to @@ -51,18 +51,18 @@ interface ISessionCallback { /** * This method must only be used to notify the framework during the following operations: - * 1) ISession#enroll - * 2) ISession#authenticate - * 3) ISession#detectInteraction - * 4) ISession#invalidateAuthenticatorId - * 5) ISession#resetLockout + * - ISession#enroll + * - ISession#authenticate + * - ISession#detectInteraction + * - ISession#invalidateAuthenticatorId + * - ISession#resetLockout * * These messages may be used to notify the framework or user that a non-recoverable error * has occurred. The operation is finished, and the HAL can proceed with the next operation * or return to the idling state. * - * Note that cancellation (see common::ICancellationSignal) and preemption must be followed with - * an Error::CANCELED message. + * Note that cancellation (see common::ICancellationSignal) must be followed with an + * Error::CANCELED message. * * @param error See the Error enum. * @param vendorCode Only valid if error == Error::VENDOR. The vendorCode must be used to index @@ -100,8 +100,8 @@ interface ISessionCallback { * This method must only be used to notify the framework during ISession#authenticate. * * Used to notify the framework upon rejected attempts. Note that the authentication - * lifecycle ends when either 1) a fingerprint is accepted, or 2) an occurred. The - * authentication lifecycle does NOT end when a fingerprint is rejected. + * lifecycle ends when either 1) a fingerprint is accepted, or 2) an error occurred. + * The authentication lifecycle does NOT end when a fingerprint is rejected. */ void onAuthenticationFailed(); From 90e4a2f96e82a38871495e561f2c02cf0359cb05 Mon Sep 17 00:00:00 2001 From: Kalesh Singh Date: Tue, 29 Jun 2021 21:43:27 +0000 Subject: [PATCH 17/61] memtrack: Update AIDL memtrack hal documentation Fix Memtrack GRAPHICS type definition to also inculde GPU-mapped DMA-BUF memory. Clarify SMAPS_UNACCOUTNED should also include memory mapped with VM_PFNMAP set. Bug: 192384999 Test: N/A Change-Id: I5370efa731bc6307e4fe9b454796361e9a1ac5eb Merged-In: I5370efa731bc6307e4fe9b454796361e9a1ac5eb --- .../aidl/android/hardware/memtrack/IMemtrack.aidl | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/memtrack/aidl/android/hardware/memtrack/IMemtrack.aidl b/memtrack/aidl/android/hardware/memtrack/IMemtrack.aidl index e78d4d7c32..13c3389730 100644 --- a/memtrack/aidl/android/hardware/memtrack/IMemtrack.aidl +++ b/memtrack/aidl/android/hardware/memtrack/IMemtrack.aidl @@ -31,12 +31,14 @@ import android.hardware.memtrack.MemtrackType; * accounting for stride, bit depth, rounding up to page size, etc. * * The following getMemory() categories are important for memory accounting in - * `dumpsys meminfo` and should be reported as described below: + * Android frameworks (e.g. `dumpsys meminfo`) and should be reported as described + * below: * * - MemtrackType::GRAPHICS and MemtrackRecord::FLAG_SMAPS_UNACCOUNTED - * This should report the PSS of all DMA buffers mapped by the process - * with the specified PID. This PSS can be calculated using ReadDmaBufPss() - * form libdmabufinfo. + * This should report the PSS of all CPU-Mapped DMA-BUFs (buffers mapped into + * the process address space) and all GPU-Mapped DMA-BUFs (buffers mapped into + * the GPU device address space on behalf of the process), removing any overlap + * between the CPU-mapped and GPU-mapped sets. * * - MemtrackType::GL and MemtrackRecord::FLAG_SMAPS_UNACCOUNTED * This category should report all GPU private allocations for the specified @@ -46,6 +48,10 @@ import android.hardware.memtrack.MemtrackType; * Any other memory not accounted for in /proc//smaps if any, otherwise * this should return 0. * + * SMAPS_UNACCOUNTED memory should also include memory that is mapped with + * VM_PFNMAP flag set. For these mappings PSS and RSS are reported as 0 in smaps. + * Such mappings have no backing page structs from which PSS/RSS can be calculated. + * * Constructor for the interface should be used to perform memtrack management * setup actions and must be called once before any calls to getMemory(). */ From f5a84a23226468ced5a1faef81ff18d10522df31 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 1 Jul 2021 16:23:18 +0800 Subject: [PATCH 18/61] wifi: add 1.5 HIDL service in lazy rc 1.5 HIDL service is not declared in lazy init rc. Bug: 191940153 Test: atest VtsHalBaseV1_0TargetTest Change-Id: Idc4d01e9696c35c8fc2390a2639ff8d7ebf0bbf0 --- wifi/1.5/default/android.hardware.wifi@1.0-service-lazy.rc | 1 + 1 file changed, 1 insertion(+) diff --git a/wifi/1.5/default/android.hardware.wifi@1.0-service-lazy.rc b/wifi/1.5/default/android.hardware.wifi@1.0-service-lazy.rc index 061689dbe5..bc6bb6a7e6 100644 --- a/wifi/1.5/default/android.hardware.wifi@1.0-service-lazy.rc +++ b/wifi/1.5/default/android.hardware.wifi@1.0-service-lazy.rc @@ -4,6 +4,7 @@ service vendor.wifi_hal_legacy /vendor/bin/hw/android.hardware.wifi@1.0-service- interface android.hardware.wifi@1.2::IWifi default interface android.hardware.wifi@1.3::IWifi default interface android.hardware.wifi@1.4::IWifi default + interface android.hardware.wifi@1.5::IWifi default oneshot disabled class hal From 8b78dc5031a12819a477399374f3e3807777c7b2 Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Tue, 1 Jun 2021 11:30:24 -0700 Subject: [PATCH 19/61] Correct the description for getKeyCharacteristics The description should note that keystore-enforced tags are not to be returned. This is done so that the keymint implementation doesn't have to bother keeping track of tags it's not repsonsible for dealing with. Fixes: 192575557 Test: none (it's just a comment change) Change-Id: I3ff94201c262a5071d271b150dbbf21888d678aa Merged-In: I3ff94201c262a5071d271b150dbbf21888d678aa --- .../android/hardware/security/keymint/IKeyMintDevice.aidl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl b/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl index 4cecff7f7a..2241735928 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/IKeyMintDevice.aidl @@ -813,9 +813,10 @@ interface IKeyMintDevice { byte[] convertStorageKeyToEphemeral(in byte[] storageKeyBlob); /** - * Returns parameters associated with the provided key. This should match the - * KeyCharacteristics present in the KeyCreationResult returned by generateKey(), - * importKey(), or importWrappedKey(). + * Returns KeyMint-enforced parameters associated with the provided key. The returned tags are + * a subset of KeyCharacteristics found in the KeyCreationResult returned by generateKey(), + * importKey(), or importWrappedKey(). The returned value is a subset, as it does not include + * any Keystore-enforced parameters. * * @param keyBlob The opaque descriptor returned by generateKey, importKey or importWrappedKey. * From da9f5fe137ffaf1b020657e6a9ac8b820f28a089 Mon Sep 17 00:00:00 2001 From: Les Lee Date: Tue, 29 Jun 2021 22:48:11 +0800 Subject: [PATCH 20/61] WIFI: Set MAC address for bridged interface The MAC address of the bridged interface will be dynamically generated by kernel when any bridged iface is changed. This means that the bridged interface MAC address will be changed when we remove one of the instances from the bridged interface (shutdown unused interface case). The MAC change will break operation of bpf and it may cause the SAP client to send wrong ns packets because the tethering module is still using the old MAC in the ra packet. Always set MAC address so the bridged interface can avoid MAC changing. Bug: 191611764 Bug: 192315721 Test: Manual test with IPv6 tethering. Make sure client won't disconnect because it doesn't get na response. Test: Manual test in two scenarios: 1. MAC randomization 2. reset to factory MAC. Change-Id: I854fc74b6532824b7d7b5a1aa4bc20a3cf9fd588 --- wifi/1.5/default/wifi_ap_iface.cpp | 27 ++++++++++++++++++++------- wifi/1.5/default/wifi_iface_util.cpp | 4 ++-- wifi/1.5/default/wifi_iface_util.h | 4 ++-- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/wifi/1.5/default/wifi_ap_iface.cpp b/wifi/1.5/default/wifi_ap_iface.cpp index b438a4a832..1ae7905f74 100644 --- a/wifi/1.5/default/wifi_ap_iface.cpp +++ b/wifi/1.5/default/wifi_ap_iface.cpp @@ -136,24 +136,25 @@ WifiApIface::getValidFrequenciesForBandInternal(V1_0::WifiBand band) { WifiStatus WifiApIface::setMacAddressInternal( const std::array& mac) { - bool status; // Support random MAC up to 2 interfaces if (instances_.size() == 2) { int rbyte = 1; for (auto const& intf : instances_) { std::array rmac = mac; - // reverse the bits to avoid clision + // reverse the bits to avoid collision rmac[rbyte] = 0xff - rmac[rbyte]; - status = iface_util_.lock()->setMacAddress(intf, rmac); - if (!status) { + if (!iface_util_.lock()->setMacAddress(intf, rmac)) { LOG(INFO) << "Failed to set random mac address on " << intf; + return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN); } rbyte++; } - } else { - status = iface_util_.lock()->setMacAddress(ifname_, mac); } - if (!status) { + // It also needs to set mac address for bridged interface, otherwise the mac + // address of bridged interface will be changed after one of instance + // down. + if (!iface_util_.lock()->setMacAddress(ifname_, mac)) { + LOG(ERROR) << "Fail to config MAC for interface " << ifname_; return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN); } return createWifiStatus(WifiStatusCode::SUCCESS); @@ -181,6 +182,18 @@ WifiStatus WifiApIface::resetToFactoryMacAddressInternal() { return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN); } } + // It needs to set mac address for bridged interface, otherwise the mac + // address of the bridged interface will be changed after one of the + // instance down. Thus we are generating a random MAC address for the + // bridged interface even if we got the request to reset the Factory + // MAC. Since the bridged interface is an internal interface for the + // operation of bpf and others networking operation. + if (!iface_util_.lock()->setMacAddress( + ifname_, iface_util_.lock()->createRandomMacAddress())) { + LOG(ERROR) << "Fail to config MAC for bridged interface " + << ifname_; + return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN); + } } else { getMacResult = getFactoryMacAddressInternal(ifname_); LOG(DEBUG) << "Reset MAC to factory MAC on " << ifname_; diff --git a/wifi/1.5/default/wifi_iface_util.cpp b/wifi/1.5/default/wifi_iface_util.cpp index d1434e3a41..7bf830b875 100644 --- a/wifi/1.5/default/wifi_iface_util.cpp +++ b/wifi/1.5/default/wifi_iface_util.cpp @@ -86,9 +86,9 @@ bool WifiIfaceUtil::setMacAddress(const std::string& iface_name, event_handlers.on_state_toggle_off_on(iface_name); } if (!success) { - LOG(ERROR) << "SetMacAddress failed."; + LOG(ERROR) << "SetMacAddress failed on " << iface_name; } else { - LOG(DEBUG) << "SetMacAddress succeeded."; + LOG(DEBUG) << "SetMacAddress succeeded on " << iface_name; } return success; } diff --git a/wifi/1.5/default/wifi_iface_util.h b/wifi/1.5/default/wifi_iface_util.h index b449077e9e..544f575d41 100644 --- a/wifi/1.5/default/wifi_iface_util.h +++ b/wifi/1.5/default/wifi_iface_util.h @@ -71,10 +71,10 @@ class WifiIfaceUtil { virtual bool removeIfaceFromBridge(const std::string& br_name, const std::string& if_name); + // Get a random MAC address. + virtual std::array createRandomMacAddress(); private: - std::array createRandomMacAddress(); - std::weak_ptr iface_tool_; std::weak_ptr legacy_hal_; std::unique_ptr> random_mac_address_; From 1d52438592c4aa3533b495b48e1409f46f481018 Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Fri, 2 Jul 2021 20:33:51 +0000 Subject: [PATCH 21/61] IFingerprint: update default implementation Bug: 166800618 Test: atest VtsHalBiometricsFingerprintTargetTest Change-Id: I0ac3a019081f4f5db6943fc019165ad1aa2e0bc8 --- .../aidl/default/include/FakeFingerprintEngine.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/biometrics/fingerprint/aidl/default/include/FakeFingerprintEngine.h b/biometrics/fingerprint/aidl/default/include/FakeFingerprintEngine.h index 6667f7a7f0..b92777068c 100644 --- a/biometrics/fingerprint/aidl/default/include/FakeFingerprintEngine.h +++ b/biometrics/fingerprint/aidl/default/include/FakeFingerprintEngine.h @@ -17,14 +17,19 @@ #pragma once #include +#include namespace aidl::android::hardware::biometrics::fingerprint { class FakeFingerprintEngine { public: + FakeFingerprintEngine() : mRandom(std::mt19937::default_seed) {} + void generateChallengeImpl(ISessionCallback* cb) { LOG(INFO) << "generateChallengeImpl"; - cb->onChallengeGenerated(0 /* challenge */); + std::uniform_int_distribution dist; + auto challenge = dist(mRandom); + cb->onChallengeGenerated(challenge); } void revokeChallengeImpl(ISessionCallback* cb, int64_t challenge) { @@ -32,8 +37,13 @@ class FakeFingerprintEngine { cb->onChallengeRevoked(challenge); } - void enrollImpl(ISessionCallback* cb, const keymaster::HardwareAuthToken& /*hat*/) { + void enrollImpl(ISessionCallback* cb, const keymaster::HardwareAuthToken& hat) { LOG(INFO) << "enrollImpl"; + // Do proper HAT verification in the real implementation. + if (hat.mac.empty()) { + cb->onError(Error::UNABLE_TO_PROCESS, 0 /* vendorError */); + return; + } cb->onEnrollmentProgress(0 /* enrollmentId */, 0 /* remaining */); } @@ -71,6 +81,8 @@ class FakeFingerprintEngine { LOG(INFO) << "resetLockoutImpl"; cb->onLockoutCleared(); } + + std::mt19937 mRandom; }; } // namespace aidl::android::hardware::biometrics::fingerprint From 04247248ceb8b41950bcfc59e89cace7704565e5 Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Fri, 2 Jul 2021 20:34:52 +0000 Subject: [PATCH 22/61] IFingerprint: update VTS tests Bug: 166799066 Test: VtsHalBiometricsFingerprintTargetTest Change-Id: I2bf765890203a4edc016d1b7f0abd1d205b2a0ba --- .../VtsHalBiometricsFingerprintTargetTest.cpp | 204 +++++++++++++++--- 1 file changed, 177 insertions(+), 27 deletions(-) diff --git a/biometrics/fingerprint/aidl/vts/VtsHalBiometricsFingerprintTargetTest.cpp b/biometrics/fingerprint/aidl/vts/VtsHalBiometricsFingerprintTargetTest.cpp index f1cfb17837..1cd8c769f2 100644 --- a/biometrics/fingerprint/aidl/vts/VtsHalBiometricsFingerprintTargetTest.cpp +++ b/biometrics/fingerprint/aidl/vts/VtsHalBiometricsFingerprintTargetTest.cpp @@ -35,13 +35,19 @@ constexpr int kUserId = 0; class SessionCallback : public BnSessionCallback { public: - explicit SessionCallback(std::promise&& promise) : mPromise(std::move(promise)) {} - - ndk::ScopedAStatus onChallengeGenerated(int64_t /*challenge*/) override { + ndk::ScopedAStatus onChallengeGenerated(int64_t challenge) override { + auto lock = std::lock_guard{mMutex}; + mOnChallengeGeneratedInvoked = true; + mGeneratedChallenge = challenge; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } - ndk::ScopedAStatus onChallengeRevoked(int64_t /*challenge*/) override { + ndk::ScopedAStatus onChallengeRevoked(int64_t challenge) override { + auto lock = std::lock_guard{mMutex}; + mOnChallengeRevokedInvoked = true; + mRevokedChallenge = challenge; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } @@ -49,7 +55,11 @@ class SessionCallback : public BnSessionCallback { return ndk::ScopedAStatus::ok(); } - ndk::ScopedAStatus onError(Error /*error*/, int32_t /*vendorCode*/) override { + ndk::ScopedAStatus onError(Error error, int32_t /*vendorCode*/) override { + auto lock = std::lock_guard{mMutex}; + mError = error; + mOnErrorInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } @@ -77,63 +87,203 @@ class SessionCallback : public BnSessionCallback { ndk::ScopedAStatus onEnrollmentsEnumerated( const std::vector& /*enrollmentIds*/) override { + auto lock = std::lock_guard{mMutex}; + mOnEnrollmentsEnumeratedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus onEnrollmentsRemoved( const std::vector& /*enrollmentIds*/) override { + auto lock = std::lock_guard{mMutex}; + mOnEnrollmentsRemovedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus onAuthenticatorIdRetrieved(int64_t /*authenticatorId*/) override { + auto lock = std::lock_guard{mMutex}; + mOnAuthenticatorIdRetrievedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus onAuthenticatorIdInvalidated(int64_t /*newAuthenticatorId*/) override { + auto lock = std::lock_guard{mMutex}; + mOnAuthenticatorIdInvalidatedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus onSessionClosed() override { - mPromise.set_value(); + auto lock = std::lock_guard{mMutex}; + mOnSessionClosedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } - private: - std::promise mPromise; + std::mutex mMutex; + std::condition_variable mCv; + Error mError = Error::UNKNOWN; + int64_t mGeneratedChallenge = 0; + int64_t mRevokedChallenge = 0; + bool mOnChallengeGeneratedInvoked = false; + bool mOnChallengeRevokedInvoked = false; + bool mOnErrorInvoked = false; + bool mOnEnrollmentsEnumeratedInvoked = false; + bool mOnEnrollmentsRemovedInvoked = false; + bool mOnAuthenticatorIdRetrievedInvoked = false; + bool mOnAuthenticatorIdInvalidatedInvoked = false; + bool mOnSessionClosedInvoked = false; }; class Fingerprint : public testing::TestWithParam { protected: void SetUp() override { - AIBinder* binder = AServiceManager_waitForService(GetParam().c_str()); - ASSERT_NE(binder, nullptr); - mHal = IFingerprint::fromBinder(ndk::SpAIBinder(binder)); + // Prepare the callback. + mCb = ndk::SharedRefBase::make(); + + int retries = 0; + bool isOk = false; + // If the first attempt to create a session fails, we try to create a session again. The + // first attempt might fail if the framework already has an active session. The AIDL + // contract doesn't allow to create a new session without closing the old one. However, we + // can't close the framework's session from VTS. The expectation here is that the HAL will + // crash after the first illegal attempt to create a session, then it will restart, and then + // we'll be able to create a session. + do { + // Get an instance of the HAL. + AIBinder* binder = AServiceManager_waitForService(GetParam().c_str()); + ASSERT_NE(binder, nullptr); + mHal = IFingerprint::fromBinder(ndk::SpAIBinder(binder)); + + // Create a session. + isOk = mHal->createSession(kSensorId, kUserId, mCb, &mSession).isOk(); + ++retries; + } while (!isOk && retries < 2); + + ASSERT_TRUE(isOk); + } + + void TearDown() override { + // Close the mSession. + ASSERT_TRUE(mSession->close().isOk()); + + // Make sure the mSession is closed. + auto lock = std::unique_lock(mCb->mMutex); + mCb->mCv.wait(lock, [this] { return mCb->mOnSessionClosedInvoked; }); } std::shared_ptr mHal; + std::shared_ptr mCb; + std::shared_ptr mSession; }; -TEST_P(Fingerprint, AuthenticateTest) { - auto promise = std::promise{}; - auto future = promise.get_future(); - // Prepare the callback. - auto cb = ndk::SharedRefBase::make(std::move(promise)); +TEST_P(Fingerprint, GetSensorPropsWorksTest) { + std::vector sensorProps; - // Create a session - std::shared_ptr session; - ASSERT_TRUE(mHal->createSession(kSensorId, kUserId, cb, &session).isOk()); + // Call the method. + ASSERT_TRUE(mHal->getSensorProps(&sensorProps).isOk()); - // Call authenticate + // Make sure the sensorProps aren't empty. + ASSERT_FALSE(sensorProps.empty()); + ASSERT_FALSE(sensorProps[0].commonProps.componentInfo.empty()); +} + +TEST_P(Fingerprint, EnrollWithBadHatResultsInErrorTest) { + // Call the method. + auto hat = keymaster::HardwareAuthToken{}; std::shared_ptr cancellationSignal; - ASSERT_TRUE(session->authenticate(-1 /* operationId */, &cancellationSignal).isOk()); + ASSERT_TRUE(mSession->enroll(hat, &cancellationSignal).isOk()); - // Get the results - // TODO(b/166799066): test authenticate. + // Make sure an error is returned. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnErrorInvoked; }); +} - // Close the session - ASSERT_TRUE(session->close().isOk()); - auto status = future.wait_for(1s); - ASSERT_EQ(status, std::future_status::ready); +TEST_P(Fingerprint, GenerateChallengeProducesUniqueChallengesTest) { + static constexpr int kIterations = 100; + + auto challenges = std::set{}; + for (unsigned int i = 0; i < kIterations; ++i) { + // Call the method. + ASSERT_TRUE(mSession->generateChallenge().isOk()); + + // Check that the generated challenge is unique and not 0. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnChallengeGeneratedInvoked; }); + ASSERT_NE(mCb->mGeneratedChallenge, 0); + ASSERT_EQ(challenges.find(mCb->mGeneratedChallenge), challenges.end()); + + challenges.insert(mCb->mGeneratedChallenge); + mCb->mOnChallengeGeneratedInvoked = false; + } +} + +TEST_P(Fingerprint, RevokeChallengeWorksForNonexistentChallengeTest) { + const int64_t nonexistentChallenge = 123; + + // Call the method. + ASSERT_TRUE(mSession->revokeChallenge(nonexistentChallenge).isOk()); + + // Check that the challenge is revoked and matches the requested challenge. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnChallengeRevokedInvoked; }); + ASSERT_EQ(mCb->mRevokedChallenge, nonexistentChallenge); +} + +TEST_P(Fingerprint, RevokeChallengeWorksForExistentChallengeTest) { + // Generate a challenge. + ASSERT_TRUE(mSession->generateChallenge().isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnChallengeGeneratedInvoked; }); + lock.unlock(); + + // Revoke the challenge. + ASSERT_TRUE(mSession->revokeChallenge(mCb->mGeneratedChallenge).isOk()); + + // Check that the challenge is revoked and matches the requested challenge. + lock.lock(); + mCb->mCv.wait(lock, [this] { return mCb->mOnChallengeRevokedInvoked; }); + ASSERT_EQ(mCb->mRevokedChallenge, mCb->mGeneratedChallenge); +} + +TEST_P(Fingerprint, EnumerateEnrollmentsWorksTest) { + // Call the method. + ASSERT_TRUE(mSession->enumerateEnrollments().isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnEnrollmentsEnumeratedInvoked; }); +} + +TEST_P(Fingerprint, RemoveEnrollmentsWorksTest) { + // Call the method. + ASSERT_TRUE(mSession->removeEnrollments({}).isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnEnrollmentsRemovedInvoked; }); +} + +TEST_P(Fingerprint, GetAuthenticatorIdWorksTest) { + // Call the method. + ASSERT_TRUE(mSession->getAuthenticatorId().isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnAuthenticatorIdRetrievedInvoked; }); +} + +TEST_P(Fingerprint, InvalidateAuthenticatorIdWorksTest) { + // Call the method. + ASSERT_TRUE(mSession->invalidateAuthenticatorId().isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnAuthenticatorIdInvalidatedInvoked; }); } GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(Fingerprint); From 753a594aec166bb53a3e72f00ea0780297b38c86 Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Fri, 2 Jul 2021 11:38:20 -0700 Subject: [PATCH 23/61] Camera: Fix a typo in the 3.7 device comment Test: Build Bug: 192701691 Change-Id: Iba9ce48fc7e50bf632e93408072c89d3eb74ab99 --- camera/device/3.7/types.hal | 2 +- current.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/camera/device/3.7/types.hal b/camera/device/3.7/types.hal index 6910e6574c..55aceb8fa4 100644 --- a/camera/device/3.7/types.hal +++ b/camera/device/3.7/types.hal @@ -42,7 +42,7 @@ struct Stream { /** * The surface group id used for multi-resolution output streams. * - * This works simliar to the surfaceGroupId of OutputConfiguration in the + * This works similar to the surfaceGroupId of OutputConfiguration in the * public API, with the exception that this is for multi-resolution image * reader and is used by the camera HAL to choose a target stream within * the same group to which images are written. All streams in the same group diff --git a/current.txt b/current.txt index 908ecc4ed2..9fdd81dd67 100644 --- a/current.txt +++ b/current.txt @@ -838,7 +838,7 @@ c17d9e27abd37ae5a8ff8da08fc5c9b13a264670feef6bbbc9d3ab1915216130 android.hardwar 1a1dff6e8d25dbc02a69fed3c077dd0782b30331ca3f345848ec52fc67744224 android.hardware.camera.device@3.7::ICameraDevice 3be6faa3d11ad9c7ec01a1a0a009cf11cb65d701d109dab37613ce9cfb3cdd60 android.hardware.camera.device@3.7::ICameraDeviceSession 3740ec773b2eb8fa6bd8c6e879eedb56c4e4306b88f1c20fa51103d791d871b1 android.hardware.camera.device@3.7::ICameraInjectionSession -21f023685571daf46148097d98b89cea353f07e3ed83b2ed5685b23bd136c3ee android.hardware.camera.device@3.7::types +d272697484c41bbf76a0924d2aaebf065ce37a822fcb438316eb5dd2d112f052 android.hardware.camera.device@3.7::types e932e7ef95210142e1fd3a4504e1d19bdb1acc988450f1ced543f3401f67855a android.hardware.camera.metadata@3.6::types 98ff825a7d37e5ab983502d13cec1f2e5a9cac9b674b6ff1a52bcf540f4e315e android.hardware.camera.provider@2.7::ICameraProvider 51fd14005859b16be55872660c34f5d423c77a2abcc5d4bdd5a537c40f32516b android.hardware.camera.provider@2.7::types From 60406beac63ecbcd303a4b9a5f4b2010b8cffe59 Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Sat, 3 Jul 2021 00:04:43 +0000 Subject: [PATCH 24/61] IFace: update default implementation Bug: 170651283 Test: atest VtsHalBiometricsFaceTargetTest Change-Id: I6f8e6ec12e597034264e2b1383bc7325b0f697b7 --- biometrics/face/aidl/default/Session.cpp | 16 ++++++++++++++-- biometrics/face/aidl/default/Session.h | 3 +++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/biometrics/face/aidl/default/Session.cpp b/biometrics/face/aidl/default/Session.cpp index d980c5f892..0cb7c95de8 100644 --- a/biometrics/face/aidl/default/Session.cpp +++ b/biometrics/face/aidl/default/Session.cpp @@ -34,12 +34,15 @@ class CancellationSignal : public common::BnCancellationSignal { } }; -Session::Session(std::shared_ptr cb) : cb_(std::move(cb)) {} +Session::Session(std::shared_ptr cb) + : cb_(std::move(cb)), mRandom(std::mt19937::default_seed) {} ndk::ScopedAStatus Session::generateChallenge() { LOG(INFO) << "generateChallenge"; if (cb_) { - cb_->onChallengeGenerated(0); + std::uniform_int_distribution dist; + auto challenge = dist(mRandom); + cb_->onChallengeGenerated(challenge); } return ndk::ScopedAStatus::ok(); } @@ -63,6 +66,9 @@ ndk::ScopedAStatus Session::enroll( const std::vector& /*features*/, const NativeHandle& /*previewSurface*/, std::shared_ptr* /*return_val*/) { LOG(INFO) << "enroll"; + if (cb_) { + cb_->onError(Error::UNABLE_TO_PROCESS, 0 /* vendorError */); + } return ndk::ScopedAStatus::ok(); } @@ -100,6 +106,9 @@ ndk::ScopedAStatus Session::removeEnrollments(const std::vector& /*enro ndk::ScopedAStatus Session::getFeatures() { LOG(INFO) << "getFeatures"; + if (cb_) { + cb_->onFeaturesRetrieved({}); + } return ndk::ScopedAStatus::ok(); } @@ -119,6 +128,9 @@ ndk::ScopedAStatus Session::getAuthenticatorId() { ndk::ScopedAStatus Session::invalidateAuthenticatorId() { LOG(INFO) << "invalidateAuthenticatorId"; + if (cb_) { + cb_->onAuthenticatorIdInvalidated(0); + } return ndk::ScopedAStatus::ok(); } diff --git a/biometrics/face/aidl/default/Session.h b/biometrics/face/aidl/default/Session.h index caf81f8702..4d213e3860 100644 --- a/biometrics/face/aidl/default/Session.h +++ b/biometrics/face/aidl/default/Session.h @@ -16,6 +16,8 @@ #pragma once +#include + #include #include @@ -68,6 +70,7 @@ class Session : public BnSession { private: std::shared_ptr cb_; + std::mt19937 mRandom; }; } // namespace aidl::android::hardware::biometrics::face From 046020b149aaa9aca90d294356114cde7af94633 Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Sat, 3 Jul 2021 00:05:47 +0000 Subject: [PATCH 25/61] IFace: update VTS tests Bug: 170650876 Test: atest VtsHalBiometricsFaceTargetTest Change-Id: I0b38a98acaec654b144dbe56fc64c5c741bd4488 --- .../vts/VtsHalBiometricsFaceTargetTest.cpp | 203 +++++++++++++++--- 1 file changed, 179 insertions(+), 24 deletions(-) diff --git a/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp b/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp index 47a78139f2..4dc44f16c7 100644 --- a/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp +++ b/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp @@ -29,16 +29,26 @@ namespace { using namespace std::literals::chrono_literals; +using aidl::android::hardware::common::NativeHandle; + constexpr int kSensorId = 0; constexpr int kUserId = 0; class SessionCallback : public BnSessionCallback { public: - ndk::ScopedAStatus onChallengeGenerated(int64_t /*challenge*/) override { + ndk::ScopedAStatus onChallengeGenerated(int64_t challenge) override { + auto lock = std::lock_guard{mMutex}; + mOnChallengeGeneratedInvoked = true; + mGeneratedChallenge = challenge; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } - ndk::ScopedAStatus onChallengeRevoked(int64_t /*challenge*/) override { + ndk::ScopedAStatus onChallengeRevoked(int64_t challenge) override { + auto lock = std::lock_guard{mMutex}; + mOnChallengeRevokedInvoked = true; + mRevokedChallenge = challenge; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } @@ -50,10 +60,9 @@ class SessionCallback : public BnSessionCallback { return ndk::ScopedAStatus::ok(); } - ndk::ScopedAStatus onError(Error error, int32_t vendorCode) override { + ndk::ScopedAStatus onError(Error error, int32_t /*vendorCode*/) override { auto lock = std::lock_guard{mMutex}; mError = error; - mVendorCode = vendorCode; mOnErrorInvoked = true; mCv.notify_one(); return ndk::ScopedAStatus::ok(); @@ -83,15 +92,24 @@ class SessionCallback : public BnSessionCallback { ndk::ScopedAStatus onEnrollmentsEnumerated( const std::vector& /*enrollmentIds*/) override { + auto lock = std::lock_guard{mMutex}; + mOnEnrollmentsEnumeratedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus onEnrollmentsRemoved( const std::vector& /*enrollmentIds*/) override { + auto lock = std::lock_guard{mMutex}; + mOnEnrollmentsRemovedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus onFeaturesRetrieved(const std::vector& /*features*/) override { + auto lock = std::lock_guard{mMutex}; + mOnFeaturesRetrievedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } @@ -100,10 +118,16 @@ class SessionCallback : public BnSessionCallback { } ndk::ScopedAStatus onAuthenticatorIdRetrieved(int64_t /*authenticatorId*/) override { + auto lock = std::lock_guard{mMutex}; + mOnAuthenticatorIdRetrievedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus onAuthenticatorIdInvalidated(int64_t /*newAuthenticatorId*/) override { + auto lock = std::lock_guard{mMutex}; + mOnAuthenticatorIdInvalidatedInvoked = true; + mCv.notify_one(); return ndk::ScopedAStatus::ok(); } @@ -117,46 +141,177 @@ class SessionCallback : public BnSessionCallback { std::mutex mMutex; std::condition_variable mCv; Error mError = Error::UNKNOWN; - int32_t mVendorCode = 0; + int64_t mGeneratedChallenge = 0; + int64_t mRevokedChallenge = 0; + bool mOnChallengeGeneratedInvoked = false; + bool mOnChallengeRevokedInvoked = false; bool mOnErrorInvoked = false; + bool mOnEnrollmentsEnumeratedInvoked = false; + bool mOnEnrollmentsRemovedInvoked = false; + bool mOnFeaturesRetrievedInvoked = false; + bool mOnAuthenticatorIdRetrievedInvoked = false; + bool mOnAuthenticatorIdInvalidatedInvoked = false; bool mOnSessionClosedInvoked = false; }; class Face : public testing::TestWithParam { protected: void SetUp() override { - AIBinder* binder = AServiceManager_waitForService(GetParam().c_str()); - ASSERT_NE(binder, nullptr); - mHal = IFace::fromBinder(ndk::SpAIBinder(binder)); + // Prepare the callback. + mCb = ndk::SharedRefBase::make(); + + int retries = 0; + bool isOk = false; + // If the first attempt to create a session fails, we try to create a session again. The + // first attempt might fail if the framework already has an active session. The AIDL + // contract doesn't allow to create a new session without closing the old one. However, we + // can't close the framework's session from VTS. The expectation here is that the HAL will + // crash after the first illegal attempt to create a session, then it will restart, and then + // we'll be able to create a session. + do { + // Get an instance of the HAL. + AIBinder* binder = AServiceManager_waitForService(GetParam().c_str()); + ASSERT_NE(binder, nullptr); + mHal = IFace::fromBinder(ndk::SpAIBinder(binder)); + + // Create a session. + isOk = mHal->createSession(kSensorId, kUserId, mCb, &mSession).isOk(); + ++retries; + } while (!isOk && retries < 2); + + ASSERT_TRUE(isOk); + } + + void TearDown() override { + // Close the mSession. + ASSERT_TRUE(mSession->close().isOk()); + + // Make sure the mSession is closed. + auto lock = std::unique_lock(mCb->mMutex); + mCb->mCv.wait(lock, [this] { return mCb->mOnSessionClosedInvoked; }); } std::shared_ptr mHal; + std::shared_ptr mCb; + std::shared_ptr mSession; }; -TEST_P(Face, AuthenticateTest) { - // Prepare the callback. - auto cb = ndk::SharedRefBase::make(); +TEST_P(Face, GetSensorPropsWorksTest) { + std::vector sensorProps; - // Create a session - std::shared_ptr session; - ASSERT_TRUE(mHal->createSession(kSensorId, kUserId, cb, &session).isOk()); + // Call the method. + ASSERT_TRUE(mHal->getSensorProps(&sensorProps).isOk()); - // Call authenticate + // Make sure the sensorProps aren't empty. + ASSERT_FALSE(sensorProps.empty()); + ASSERT_FALSE(sensorProps[0].commonProps.componentInfo.empty()); +} + +TEST_P(Face, EnrollWithBadHatResultsInErrorTest) { + // Call the method. + auto hat = keymaster::HardwareAuthToken{}; std::shared_ptr cancellationSignal; - ASSERT_TRUE(session->authenticate(0 /* operationId */, &cancellationSignal).isOk()); + ASSERT_TRUE( + mSession->enroll(hat, EnrollmentType::DEFAULT, {}, NativeHandle{}, &cancellationSignal) + .isOk()); - auto lock = std::unique_lock(cb->mMutex); - cb->mCv.wait(lock, [&cb] { return cb->mOnErrorInvoked; }); - // Get the results - EXPECT_EQ(cb->mError, Error::UNABLE_TO_PROCESS); - EXPECT_EQ(cb->mVendorCode, 0); + // Make sure an error is returned. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnErrorInvoked; }); +} + +TEST_P(Face, GenerateChallengeProducesUniqueChallengesTest) { + static constexpr int kIterations = 100; + + auto challenges = std::set{}; + for (unsigned int i = 0; i < kIterations; ++i) { + // Call the method. + ASSERT_TRUE(mSession->generateChallenge().isOk()); + + // Check that the generated challenge is unique and not 0. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnChallengeGeneratedInvoked; }); + ASSERT_NE(mCb->mGeneratedChallenge, 0); + ASSERT_EQ(challenges.find(mCb->mGeneratedChallenge), challenges.end()); + + challenges.insert(mCb->mGeneratedChallenge); + mCb->mOnChallengeGeneratedInvoked = false; + } +} + +TEST_P(Face, RevokeChallengeWorksForNonexistentChallengeTest) { + const int64_t nonexistentChallenge = 123; + + // Call the method. + ASSERT_TRUE(mSession->revokeChallenge(nonexistentChallenge).isOk()); + + // Check that the challenge is revoked and matches the requested challenge. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnChallengeRevokedInvoked; }); + ASSERT_EQ(mCb->mRevokedChallenge, nonexistentChallenge); +} + +TEST_P(Face, RevokeChallengeWorksForExistentChallengeTest) { + // Generate a challenge. + ASSERT_TRUE(mSession->generateChallenge().isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnChallengeGeneratedInvoked; }); lock.unlock(); - // Close the session - ASSERT_TRUE(session->close().isOk()); + // Revoke the challenge. + ASSERT_TRUE(mSession->revokeChallenge(mCb->mGeneratedChallenge).isOk()); + // Check that the challenge is revoked and matches the requested challenge. lock.lock(); - cb->mCv.wait(lock, [&cb] { return cb->mOnSessionClosedInvoked; }); + mCb->mCv.wait(lock, [this] { return mCb->mOnChallengeRevokedInvoked; }); + ASSERT_EQ(mCb->mRevokedChallenge, mCb->mGeneratedChallenge); +} + +TEST_P(Face, EnumerateEnrollmentsWorksTest) { + // Call the method. + ASSERT_TRUE(mSession->enumerateEnrollments().isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnEnrollmentsEnumeratedInvoked; }); +} + +TEST_P(Face, RemoveEnrollmentsWorksTest) { + // Call the method. + ASSERT_TRUE(mSession->removeEnrollments({}).isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnEnrollmentsRemovedInvoked; }); +} + +TEST_P(Face, GetFeaturesWorksTest) { + // Call the method. + ASSERT_TRUE(mSession->getFeatures().isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnFeaturesRetrievedInvoked; }); +} + +TEST_P(Face, GetAuthenticatorIdWorksTest) { + // Call the method. + ASSERT_TRUE(mSession->getAuthenticatorId().isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnAuthenticatorIdRetrievedInvoked; }); +} + +TEST_P(Face, InvalidateAuthenticatorIdWorksTest) { + // Call the method. + ASSERT_TRUE(mSession->invalidateAuthenticatorId().isOk()); + + // Wait for the result. + auto lock = std::unique_lock{mCb->mMutex}; + mCb->mCv.wait(lock, [this] { return mCb->mOnAuthenticatorIdInvalidatedInvoked; }); } GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(Face); From d65b354deee2d3a90d670838180212329ec37abc Mon Sep 17 00:00:00 2001 From: Marin Shalamanov Date: Wed, 23 Jun 2021 14:12:37 +0200 Subject: [PATCH 26/61] VTS: Validate that getDisplayIdentificationData returns EDID Accroding to the documentation getDisplayIdentificationData "data is the EDID 1.3 blob identifying the display". This CL runs a basic validation that the returned data is an EDID. Test: atest VtsHalGraphicsComposerV2_3TargetTest Bug: 191851265 Change-Id: I7604f3dc68095612d79bb04243918d6348de6c89 --- .../VtsHalGraphicsComposerV2_3TargetTest.cpp | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/graphics/composer/2.3/vts/functional/VtsHalGraphicsComposerV2_3TargetTest.cpp b/graphics/composer/2.3/vts/functional/VtsHalGraphicsComposerV2_3TargetTest.cpp index 54ba79dcc1..ecfe66c80d 100644 --- a/graphics/composer/2.3/vts/functional/VtsHalGraphicsComposerV2_3TargetTest.cpp +++ b/graphics/composer/2.3/vts/functional/VtsHalGraphicsComposerV2_3TargetTest.cpp @@ -17,6 +17,7 @@ #define LOG_TAG "graphics_composer_hidl_hal_test@2.3" #include +#include #include #include @@ -155,16 +156,31 @@ class GraphicsComposerHidlCommandTest : public GraphicsComposerHidlTest { TEST_P(GraphicsComposerHidlTest, GetDisplayIdentificationData) { uint8_t port0; std::vector data0; - if (mComposerClient->getDisplayIdentificationData(mPrimaryDisplay, &port0, &data0)) { - uint8_t port1; - std::vector data1; - ASSERT_TRUE(mComposerClient->getDisplayIdentificationData(mPrimaryDisplay, &port1, &data1)); - ASSERT_EQ(port0, port1) << "ports are not stable"; - ASSERT_TRUE(data0.size() == data1.size() && - std::equal(data0.begin(), data0.end(), data1.begin())) - << "data is not stable"; + if (!mComposerClient->getDisplayIdentificationData(mPrimaryDisplay, &port0, &data0)) { + return; } + + ASSERT_FALSE(data0.empty()); + constexpr size_t kEdidBlockSize = 128; + ASSERT_TRUE(data0.size() % kEdidBlockSize == 0) + << "EDID blob length is not a multiple of " << kEdidBlockSize; + + const uint8_t kEdidHeader[] = {0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00}; + ASSERT_TRUE(std::equal(std::begin(kEdidHeader), std::end(kEdidHeader), data0.begin())) + << "EDID blob doesn't start with the fixed EDID header"; + ASSERT_EQ(0, std::accumulate(data0.begin(), data0.begin() + kEdidBlockSize, + static_cast(0))) + << "EDID base block doesn't checksum"; + + uint8_t port1; + std::vector data1; + ASSERT_TRUE(mComposerClient->getDisplayIdentificationData(mPrimaryDisplay, &port1, &data1)); + + ASSERT_EQ(port0, port1) << "ports are not stable"; + ASSERT_TRUE(data0.size() == data1.size() && + std::equal(data0.begin(), data0.end(), data1.begin())) + << "data is not stable"; } /** From dd0e20eba3a0b7a2fbbe6f0429b9eb3cadd8b883 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 7 Jul 2021 17:07:15 +0000 Subject: [PATCH 27/61] Revert "audio: exclude the echo reference device in capture position test" This reverts commit b22f307ccfd1072e3d61deb4e03659db7a168b7d. Reason for revert: As explained in b/192307382#comment12, the HAL must provide capture positions for the echo reference input regardless of whether there is any actual output. This should not affect O6/R4 as according to b/192307382#comment10, the VTS tests pass w/o this patch after the HAL has been fixed. Bug: 192307382 Change-Id: I224bd9de1dcb2e2c8dc138dbfd85f848378aea4f Test: VtsHalAudioV7_0TargetTest --gtest_filter=*PcmOnlyConfigInputStreamTest* --- .../android_audio_policy_configuration_V7_0-enums.h | 8 -------- .../vts/functional/7.0/AudioPrimaryHidlHalTest.cpp | 3 +-- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/audio/common/7.0/enums/include/android_audio_policy_configuration_V7_0-enums.h b/audio/common/7.0/enums/include/android_audio_policy_configuration_V7_0-enums.h index 79243b6e05..a92a277471 100644 --- a/audio/common/7.0/enums/include/android_audio_policy_configuration_V7_0-enums.h +++ b/audio/common/7.0/enums/include/android_audio_policy_configuration_V7_0-enums.h @@ -225,14 +225,6 @@ static inline bool isTelephonyDevice(const std::string& device) { return isTelephonyDevice(stringToAudioDevice(device)); } -static inline bool isEchoReferenceDevice(AudioDevice device) { - return device == AudioDevice::AUDIO_DEVICE_IN_ECHO_REFERENCE; -} - -static inline bool isEchoReferenceDevice(const std::string& device) { - return isEchoReferenceDevice(stringToAudioDevice(device)); -} - static inline bool maybeVendorExtension(const std::string& s) { // Only checks whether the string starts with the "vendor prefix". static const std::string vendorPrefix = "VX_"; diff --git a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp index 79ac295151..0b3098b872 100644 --- a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp @@ -710,8 +710,7 @@ class PcmOnlyConfigInputStreamTest : public InputStreamTest { // Returning 'true' when no source is found so the test can fail later with a more clear // problem description. return !maybeSourceAddress.has_value() || - !(xsd::isTelephonyDevice(maybeSourceAddress.value().deviceType) || - xsd::isEchoReferenceDevice(maybeSourceAddress.value().deviceType)); + !xsd::isTelephonyDevice(maybeSourceAddress.value().deviceType); } void createPatchIfNeeded() { From 13c679652834db45e2df4137f088ecd06351f017 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Sat, 3 Jul 2021 00:49:13 +0000 Subject: [PATCH 28/61] audio: Fix handling of relative XML include paths in VTS Pass "no fixup base dirs" flag to the XInclude processor to avoid modifications of the top-level elements from included XML files as a result of "fixup." Added tests to ensure that all relevant XInclude scenarios work. Bug: 192619060 Test: atest -host android.hardware.audio.common.test.utility_tests Change-Id: Id595c9fd30be378d76387ee55a8937e0bf28d1cd --- .../all-versions/test/utility/Android.bp | 31 ++- .../all-versions/test/utility/TEST_MAPPING | 7 + .../test/utility/src/ValidateXml.cpp | 3 +- .../test/utility/tests/utility_tests.cpp | 176 ++++++++++++++++++ 4 files changed, 214 insertions(+), 3 deletions(-) create mode 100644 audio/common/all-versions/test/utility/TEST_MAPPING create mode 100644 audio/common/all-versions/test/utility/tests/utility_tests.cpp diff --git a/audio/common/all-versions/test/utility/Android.bp b/audio/common/all-versions/test/utility/Android.bp index 1602d25b2f..757f8a853d 100644 --- a/audio/common/all-versions/test/utility/Android.bp +++ b/audio/common/all-versions/test/utility/Android.bp @@ -25,7 +25,7 @@ package { cc_library_static { name: "android.hardware.audio.common.test.utility", - defaults : ["hidl_defaults"], + defaults: ["hidl_defaults"], srcs: ["src/ValidateXml.cpp"], cflags: [ "-O0", @@ -34,7 +34,34 @@ cc_library_static { ], local_include_dirs: ["include/utility"], export_include_dirs: ["include"], - shared_libs: ["libxml2", "liblog"], + shared_libs: [ + "libxml2", + "liblog", + ], static_libs: ["libgtest"], export_static_lib_headers: ["libgtest"], } + +// Note: this isn't a VTS test, but rather a unit test +// to verify correctness of test utilities. +cc_test { + name: "android.hardware.audio.common.test.utility_tests", + host_supported: true, + local_include_dirs: ["include/utility"], + srcs: [ + "src/ValidateXml.cpp", + "tests/utility_tests.cpp", + ], + cflags: [ + "-Werror", + "-Wall", + "-g", + ], + shared_libs: [ + "libbase", + "libxml2", + "liblog", + ], + static_libs: ["libgtest"], + test_suites: ["general-tests"], +} diff --git a/audio/common/all-versions/test/utility/TEST_MAPPING b/audio/common/all-versions/test/utility/TEST_MAPPING new file mode 100644 index 0000000000..0bc187157a --- /dev/null +++ b/audio/common/all-versions/test/utility/TEST_MAPPING @@ -0,0 +1,7 @@ +{ + "presubmit": [ + { + "name": "android.hardware.audio.common.test.utility_tests" + } + ] +} diff --git a/audio/common/all-versions/test/utility/src/ValidateXml.cpp b/audio/common/all-versions/test/utility/src/ValidateXml.cpp index a866104b38..f111c011d9 100644 --- a/audio/common/all-versions/test/utility/src/ValidateXml.cpp +++ b/audio/common/all-versions/test/utility/src/ValidateXml.cpp @@ -112,7 +112,8 @@ struct Libxml2Global { return ::testing::AssertionFailure() << "Failed to parse xml\n" << context(); } - if (xmlXIncludeProcess(doc.get()) == -1) { + // Process 'include' directives w/o modifying elements loaded from included files. + if (xmlXIncludeProcessFlags(doc.get(), XML_PARSE_NOBASEFIX) == -1) { return ::testing::AssertionFailure() << "Failed to resolve xincludes in xml\n" << context(); } diff --git a/audio/common/all-versions/test/utility/tests/utility_tests.cpp b/audio/common/all-versions/test/utility/tests/utility_tests.cpp new file mode 100644 index 0000000000..c52306638f --- /dev/null +++ b/audio/common/all-versions/test/utility/tests/utility_tests.cpp @@ -0,0 +1,176 @@ +/* + * Copyright (C) 2021 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 + +using ::android::hardware::audio::common::test::utility::validateXml; + +const char* XSD_SOURCE = + "" + "" + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + ""; + +const char* INVALID_XML_SOURCE = + "" + ""; + +const char* VALID_XML_SOURCE = + "" + "" + " " + " " + " %s" + " " + ""; + +const char* MODULE_SOURCE = ""; + +const char* XI_INCLUDE = ""; + +const char* XML_INCLUDED_SOURCE = "%s"; + +namespace { + +std::string substitute(const char* fmt, const char* param) { + std::string buffer(static_cast(strlen(fmt) + strlen(param)), '\0'); + snprintf(buffer.data(), buffer.size(), fmt, param); + return buffer; +} + +std::string substitute(const char* fmt, const std::string& s) { + return substitute(fmt, s.c_str()); +} + +} // namespace + +TEST(ValidateXml, InvalidXml) { + TemporaryFile xml; + ASSERT_TRUE(android::base::WriteStringToFile(INVALID_XML_SOURCE, xml.path)) << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_FALSE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, ValidXml) { + TemporaryFile xml; + ASSERT_TRUE( + android::base::WriteStringToFile(substitute(VALID_XML_SOURCE, MODULE_SOURCE), xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, IncludeAbsolutePath) { + TemporaryFile xmlInclude; + ASSERT_TRUE(android::base::WriteStringToFile(substitute(XML_INCLUDED_SOURCE, MODULE_SOURCE), + xmlInclude.path)) + << strerror(errno); + TemporaryFile xml; + ASSERT_TRUE(android::base::WriteStringToFile( + substitute(VALID_XML_SOURCE, substitute(XI_INCLUDE, xmlInclude.path)), xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, IncludeSameDirRelativePath) { + TemporaryFile xmlInclude; + ASSERT_TRUE(android::base::WriteStringToFile(substitute(XML_INCLUDED_SOURCE, MODULE_SOURCE), + xmlInclude.path)) + << strerror(errno); + TemporaryFile xml; + ASSERT_EQ(android::base::Dirname(xml.path), android::base::Dirname(xmlInclude.path)); + ASSERT_TRUE(android::base::WriteStringToFile( + substitute(VALID_XML_SOURCE, + substitute(XI_INCLUDE, android::base::Basename(xmlInclude.path))), + xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, IncludeSubdirRelativePath) { + TemporaryDir xmlIncludeDir; + TemporaryFile xmlInclude(xmlIncludeDir.path); + ASSERT_TRUE(android::base::WriteStringToFile(substitute(XML_INCLUDED_SOURCE, MODULE_SOURCE), + xmlInclude.path)) + << strerror(errno); + TemporaryFile xml; + ASSERT_EQ(android::base::Dirname(xml.path), android::base::Dirname(xmlIncludeDir.path)); + ASSERT_TRUE(android::base::WriteStringToFile( + substitute(VALID_XML_SOURCE, + substitute(XI_INCLUDE, android::base::Basename(xmlIncludeDir.path) + "/" + + android::base::Basename(xmlInclude.path))), + xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, IncludeParentDirRelativePath) { + // An XML file from a subdirectory includes a file from the parent directory using '..' syntax. + TemporaryFile xmlInclude; + ASSERT_TRUE(android::base::WriteStringToFile(substitute(XML_INCLUDED_SOURCE, MODULE_SOURCE), + xmlInclude.path)) + << strerror(errno); + TemporaryDir xmlIncludeDir; + TemporaryFile xmlParentInclude(xmlIncludeDir.path); + ASSERT_TRUE(android::base::WriteStringToFile( + substitute(XML_INCLUDED_SOURCE, + substitute(XI_INCLUDE, "../" + android::base::Basename(xmlInclude.path))), + xmlParentInclude.path)) + << strerror(errno); + TemporaryFile xml; + ASSERT_EQ(android::base::Dirname(xml.path), android::base::Dirname(xmlInclude.path)); + ASSERT_EQ(android::base::Dirname(xml.path), android::base::Dirname(xmlIncludeDir.path)); + ASSERT_TRUE(android::base::WriteStringToFile( + substitute( + VALID_XML_SOURCE, + substitute(XI_INCLUDE, android::base::Basename(xmlIncludeDir.path) + "/" + + android::base::Basename(xmlParentInclude.path))), + xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +} From 12ee28322da50ee86484f401c70bd119cc4b4042 Mon Sep 17 00:00:00 2001 From: Eran Messeri Date: Mon, 28 Jun 2021 12:07:37 +0100 Subject: [PATCH 29/61] Annotate some TODOs There are two tags that cannot be currently removed but should be removed in KeyMint V2. Mark them as deprecated and point to the bug for deletion. Bug: 183737811 Test: That it compiles. Change-Id: I98b96cc8c49eb339a998d0abed9216aa57f6b19f Merged-In: I80ccaedeb777fdb249a8cb021db6628da32d6029 --- .../keymint/aidl/android/hardware/security/keymint/Tag.aidl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl index 270574bbfb..5600889393 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl @@ -293,6 +293,8 @@ enum Tag { * fails because the table is full, KeyMint returns ErrorCode::TOO_MANY_OPERATIONS. * * Must be hardware-enforced. + * + * TODO(b/191738660): Remove in KeyMint V2. Currently only used for FDE. */ MIN_SECONDS_BETWEEN_OPS = (3 << 28) /* TagType:UINT */ | 403, @@ -883,7 +885,8 @@ enum Tag { STORAGE_KEY = (7 << 28) /* TagType:BOOL */ | 722, /** - * TODO: Delete when keystore1 is deleted. + * OBSOLETE: Do not use. See IKeyMintOperation.updateAad instead. + * TODO(b/191738660): Remove in KeyMint v2. */ ASSOCIATED_DATA = (9 << 28) /* TagType:BYTES */ | 1000, From befeda6b00eef1042aba3a3d0e3d401636da5289 Mon Sep 17 00:00:00 2001 From: Eran Messeri Date: Tue, 15 Jun 2021 14:26:59 +0100 Subject: [PATCH 30/61] Use TagType constants Now that the aidl compiler supports it, use constants from TagType to indicate the type of each tag, rather than duplicating the values of the constants. Test: atest VtsAidlKeyMintTargetTest Bug: 183737811 Merged-In: Ie8af1f00d04fa05c59cfc72692caecbcf2fae483 Change-Id: Ie62b6ee8a8ced05a870711073bb3be16931f3d4d --- .../hardware/security/keymint/Tag.aidl | 134 +++++++++--------- 1 file changed, 65 insertions(+), 69 deletions(-) diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl index 270574bbfb..a785480dac 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl @@ -18,10 +18,6 @@ package android.hardware.security.keymint; import android.hardware.security.keymint.TagType; -// TODO(seleneh) : note aidl currently does not support double nested enum definitions such as -// ROOT_OF_TRUST = TagType:BYTES | 704. So we are forced to write definitions as -// ROOT_OF_TRUST = (9 << 28) for now. Will need to flip this back later when aidl support is added. - /** * Tag specifies various kinds of tags that can be set in KeyParameter to identify what kind of * data are stored in KeyParameter. @@ -33,7 +29,7 @@ enum Tag { /** * Tag::INVALID should never be set. It means you hit an error. */ - INVALID = (0 << 28) | 0, + INVALID = 0, /** * Tag::PURPOSE specifies the set of purposes for which the key may be used. Possible values @@ -47,7 +43,7 @@ enum Tag { * * Must be hardware-enforced. */ - PURPOSE = (2 << 28) /* TagType:ENUM_REP */ | 1, + PURPOSE = TagType.ENUM_REP | 1, /** * Tag::ALGORITHM specifies the cryptographic algorithm with which the key is used. This tag @@ -56,7 +52,7 @@ enum Tag { * * Must be hardware-enforced. */ - ALGORITHM = (1 << 28) /* TagType:ENUM */ | 2, + ALGORITHM = TagType.ENUM | 2, /** * Tag::KEY_SIZE specifies the size, in bits, of the key, measuring in the normal way for the @@ -68,7 +64,7 @@ enum Tag { * * Must be hardware-enforced. */ - KEY_SIZE = (3 << 28) /* TagType:UINT */ | 3, + KEY_SIZE = TagType.UINT | 3, /** * Tag::BLOCK_MODE specifies the block cipher mode(s) with which the key may be used. This tag @@ -81,7 +77,7 @@ enum Tag { * * Must be hardware-enforced. */ - BLOCK_MODE = (2 << 28) /* TagType:ENUM_REP */ | 4, + BLOCK_MODE = TagType.ENUM_REP | 4, /** * Tag::DIGEST specifies the digest algorithms that may be used with the key to perform signing @@ -95,7 +91,7 @@ enum Tag { * * Must be hardware-enforced. */ - DIGEST = (2 << 28) /* TagType:ENUM_REP */ | 5, + DIGEST = TagType.ENUM_REP | 5, /** * Tag::PADDING specifies the padding modes that may be used with the key. This tag is relevant @@ -123,7 +119,7 @@ enum Tag { * * Must be hardware-enforced. */ - PADDING = (2 << 28) /* TagType:ENUM_REP */ | 6, + PADDING = TagType.ENUM_REP | 6, /** * Tag::CALLER_NONCE specifies that the caller can provide a nonce for nonce-requiring @@ -136,7 +132,7 @@ enum Tag { * * Must be hardware-enforced. */ - CALLER_NONCE = (7 << 28) /* TagType:BOOL */ | 7, + CALLER_NONCE = TagType.BOOL | 7, /** * Tag::MIN_MAC_LENGTH specifies the minimum length of MAC that can be requested or verified @@ -149,7 +145,7 @@ enum Tag { * * Must be hardware-enforced. */ - MIN_MAC_LENGTH = (3 << 28) /* TagType:UINT */ | 8, + MIN_MAC_LENGTH = TagType.UINT | 8, // Tag 9 reserved @@ -159,7 +155,7 @@ enum Tag { * * Must be hardware-enforced. */ - EC_CURVE = (1 << 28) /* TagType:ENUM */ | 10, + EC_CURVE = TagType.ENUM | 10, /** * Tag::RSA_PUBLIC_EXPONENT specifies the value of the public exponent for an RSA key pair. @@ -173,7 +169,7 @@ enum Tag { * * Must be hardware-enforced. */ - RSA_PUBLIC_EXPONENT = (5 << 28) /* TagType:ULONG */ | 200, + RSA_PUBLIC_EXPONENT = TagType.ULONG | 200, // Tag 201 reserved @@ -184,7 +180,7 @@ enum Tag { * * Must be hardware-enforced. */ - INCLUDE_UNIQUE_ID = (7 << 28) /* TagType:BOOL */ | 202, + INCLUDE_UNIQUE_ID = TagType.BOOL | 202, /** * Tag::RSA_OAEP_MGF_DIGEST specifies the MGF1 digest algorithms that may be used with RSA @@ -197,7 +193,7 @@ enum Tag { * * Must be hardware-enforced. */ - RSA_OAEP_MGF_DIGEST = (2 << 28) /* TagType:ENUM_REP */ | 203, + RSA_OAEP_MGF_DIGEST = TagType.ENUM_REP | 203, // Tag 301 reserved @@ -209,7 +205,7 @@ enum Tag { * * Must be hardware-enforced. */ - BOOTLOADER_ONLY = (7 << 28) /* TagType:BOOL */ | 302, + BOOTLOADER_ONLY = TagType.BOOL | 302, /** * Tag::ROLLBACK_RESISTANCE specifies that the key has rollback resistance, meaning that when @@ -224,10 +220,10 @@ enum Tag { * * Must be hardware-enforced. */ - ROLLBACK_RESISTANCE = (7 << 28) /* TagType:BOOL */ | 303, + ROLLBACK_RESISTANCE = TagType.BOOL | 303, // Reserved for future use. - HARDWARE_TYPE = (1 << 28) /* TagType:ENUM */ | 304, + HARDWARE_TYPE = TagType.ENUM | 304, /** * Keys tagged with EARLY_BOOT_ONLY may only be used during early boot, until @@ -236,7 +232,7 @@ enum Tag { * provided to IKeyMintDevice::importKey, the import must fail with * ErrorCode::EARLY_BOOT_ENDED. */ - EARLY_BOOT_ONLY = (7 << 28) /* TagType:BOOL */ | 305, + EARLY_BOOT_ONLY = TagType.BOOL | 305, /** * Tag::ACTIVE_DATETIME specifies the date and time at which the key becomes active, in @@ -245,7 +241,7 @@ enum Tag { * * Need not be hardware-enforced. */ - ACTIVE_DATETIME = (6 << 28) /* TagType:DATE */ | 400, + ACTIVE_DATETIME = TagType.DATE | 400, /** * Tag::ORIGINATION_EXPIRE_DATETIME specifies the date and time at which the key expires for @@ -257,7 +253,7 @@ enum Tag { * * Need not be hardware-enforced. */ - ORIGINATION_EXPIRE_DATETIME = (6 << 28) /* TagType:DATE */ | 401, + ORIGINATION_EXPIRE_DATETIME = TagType.DATE | 401, /** * Tag::USAGE_EXPIRE_DATETIME specifies the date and time at which the key expires for @@ -269,7 +265,7 @@ enum Tag { * * Need not be hardware-enforced. */ - USAGE_EXPIRE_DATETIME = (6 << 28) /* TagType:DATE */ | 402, + USAGE_EXPIRE_DATETIME = TagType.DATE | 402, /** * TODO(seleneh) this tag need to be deleted. @@ -294,7 +290,7 @@ enum Tag { * * Must be hardware-enforced. */ - MIN_SECONDS_BETWEEN_OPS = (3 << 28) /* TagType:UINT */ | 403, + MIN_SECONDS_BETWEEN_OPS = TagType.UINT | 403, /** * Tag::MAX_USES_PER_BOOT specifies the maximum number of times that a key may be used between @@ -314,7 +310,7 @@ enum Tag { * * Must be hardware-enforced. */ - MAX_USES_PER_BOOT = (3 << 28) /* TagType:UINT */ | 404, + MAX_USES_PER_BOOT = TagType.UINT | 404, /** * Tag::USAGE_COUNT_LIMIT specifies the number of times that a key may be used. This can be @@ -343,14 +339,14 @@ enum Tag { * record. This tag must have the same SecurityLevel as the tag that is added to the key * characteristics. */ - USAGE_COUNT_LIMIT = (3 << 28) | 405, /* TagType:UINT */ + USAGE_COUNT_LIMIT = TagType.UINT | 405, /** * Tag::USER_ID specifies the ID of the Android user that is permitted to use the key. * * Must not be hardware-enforced. */ - USER_ID = (3 << 28) /* TagType:UINT */ | 501, + USER_ID = TagType.UINT | 501, /** * Tag::USER_SECURE_ID specifies that a key may only be used under a particular secure user @@ -383,7 +379,7 @@ enum Tag { * * Must be hardware-enforced. */ - USER_SECURE_ID = (10 << 28) /* TagType:ULONG_REP */ | 502, + USER_SECURE_ID = TagType.ULONG_REP | 502, /** * Tag::NO_AUTH_REQUIRED specifies that no authentication is required to use this key. This tag @@ -391,7 +387,7 @@ enum Tag { * * Must be hardware-enforced. */ - NO_AUTH_REQUIRED = (7 << 28) /* TagType:BOOL */ | 503, + NO_AUTH_REQUIRED = TagType.BOOL | 503, /** * Tag::USER_AUTH_TYPE specifies the types of user authenticators that may be used to authorize @@ -410,7 +406,7 @@ enum Tag { * * Must be hardware-enforced. */ - USER_AUTH_TYPE = (1 << 28) /* TagType:ENUM */ | 504, + USER_AUTH_TYPE = TagType.ENUM | 504, /** * Tag::AUTH_TIMEOUT specifies the time in seconds for which the key is authorized for use, @@ -424,7 +420,7 @@ enum Tag { * * Must be hardware-enforced. */ - AUTH_TIMEOUT = (3 << 28) /* TagType:UINT */ | 505, + AUTH_TIMEOUT = TagType.UINT | 505, /** * Tag::ALLOW_WHILE_ON_BODY specifies that the key may be used after authentication timeout if @@ -432,7 +428,7 @@ enum Tag { * * Cannot be hardware-enforced. */ - ALLOW_WHILE_ON_BODY = (7 << 28) /* TagType:BOOL */ | 506, + ALLOW_WHILE_ON_BODY = TagType.BOOL | 506, /** * TRUSTED_USER_PRESENCE_REQUIRED is an optional feature that specifies that this key must be @@ -479,7 +475,7 @@ enum Tag { * * Must be hardware-enforced. */ - TRUSTED_USER_PRESENCE_REQUIRED = (7 << 28) /* TagType:BOOL */ | 507, + TRUSTED_USER_PRESENCE_REQUIRED = TagType.BOOL | 507, /** * Tag::TRUSTED_CONFIRMATION_REQUIRED is only applicable to keys with KeyPurpose SIGN, and @@ -493,7 +489,7 @@ enum Tag { * * Must be hardware-enforced. */ - TRUSTED_CONFIRMATION_REQUIRED = (7 << 28) /* TagType:BOOL */ | 508, + TRUSTED_CONFIRMATION_REQUIRED = TagType.BOOL | 508, /** * Tag::UNLOCKED_DEVICE_REQUIRED specifies that the key may only be used when the device is @@ -503,7 +499,7 @@ enum Tag { * Must be hardware-enforced (but is also keystore-enforced on a per-user basis: see the * deviceLocked() documentation). */ - UNLOCKED_DEVICE_REQUIRED = (7 << 28) /* TagType:BOOL */ | 509, + UNLOCKED_DEVICE_REQUIRED = TagType.BOOL | 509, /** * Tag::APPLICATION_ID. When provided to generateKey or importKey, this tag specifies data @@ -519,7 +515,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - APPLICATION_ID = (9 << 28) /* TagType:BYTES */ | 601, + APPLICATION_ID = TagType.BYTES | 601, /* * Semantically unenforceable tags, either because they have no specific meaning or because @@ -540,7 +536,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - APPLICATION_DATA = (9 << 28) /* TagType:BYTES */ | 700, + APPLICATION_DATA = TagType.BYTES | 700, /** * Tag::CREATION_DATETIME specifies the date and time the key was created, in milliseconds since @@ -548,7 +544,7 @@ enum Tag { * * Must be in the software-enforced list, if provided. */ - CREATION_DATETIME = (6 << 28) /* TagType:DATE */ | 701, + CREATION_DATETIME = TagType.DATE | 701, /** * Tag::ORIGIN specifies where the key was created, if known. This tag must not be specified @@ -557,7 +553,7 @@ enum Tag { * * Must be hardware-enforced. */ - ORIGIN = (1 << 28) /* TagType:ENUM */ | 702, + ORIGIN = TagType.ENUM | 702, // 703 is unused. @@ -569,7 +565,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ROOT_OF_TRUST = (9 << 28) /* TagType:BYTES */ | 704, + ROOT_OF_TRUST = TagType.BYTES | 704, /** * Tag::OS_VERSION specifies the system OS version with which the key may be used. This tag is @@ -592,7 +588,7 @@ enum Tag { * * Must be hardware-enforced. */ - OS_VERSION = (3 << 28) /* TagType:UINT */ | 705, + OS_VERSION = TagType.UINT | 705, /** * Tag::OS_PATCHLEVEL specifies the system security patch level with which the key may be used. @@ -613,7 +609,7 @@ enum Tag { * * Must be hardware-enforced. */ - OS_PATCHLEVEL = (3 << 28) /* TagType:UINT */ | 706, + OS_PATCHLEVEL = TagType.UINT | 706, /** * Tag::UNIQUE_ID specifies a unique, time-based identifier. This tag is never provided to or @@ -648,7 +644,7 @@ enum Tag { * * Must be hardware-enforced. */ - UNIQUE_ID = (9 << 28) /* TagType:BYTES */ | 707, + UNIQUE_ID = TagType.BYTES | 707, /** * Tag::ATTESTATION_CHALLENGE is used to deliver a "challenge" value to the attested key @@ -657,7 +653,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ATTESTATION_CHALLENGE = (9 << 28) /* TagType:BYTES */ | 708, + ATTESTATION_CHALLENGE = TagType.BYTES | 708, /** * Tag::ATTESTATION_APPLICATION_ID identifies the set of applications which may use a key, used @@ -683,7 +679,7 @@ enum Tag { * * Cannot be hardware-enforced. */ - ATTESTATION_APPLICATION_ID = (9 << 28) /* TagType:BYTES */ | 709, + ATTESTATION_APPLICATION_ID = TagType.BYTES | 709, /** * Tag::ATTESTATION_ID_BRAND provides the device's brand name, as returned by Build.BRAND in @@ -696,7 +692,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ATTESTATION_ID_BRAND = (9 << 28) /* TagType:BYTES */ | 710, + ATTESTATION_ID_BRAND = TagType.BYTES | 710, /** * Tag::ATTESTATION_ID_DEVICE provides the device's device name, as returned by Build.DEVICE in @@ -709,7 +705,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ATTESTATION_ID_DEVICE = (9 << 28) /* TagType:BYTES */ | 711, + ATTESTATION_ID_DEVICE = TagType.BYTES | 711, /** * Tag::ATTESTATION_ID_PRODUCT provides the device's product name, as returned by Build.PRODUCT @@ -722,7 +718,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ATTESTATION_ID_PRODUCT = (9 << 28) /* TagType:BYTES */ | 712, + ATTESTATION_ID_PRODUCT = TagType.BYTES | 712, /** * Tag::ATTESTATION_ID_SERIAL the device's serial number. This field must be set only when @@ -734,7 +730,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ATTESTATION_ID_SERIAL = (9 << 28) /* TagType:BYTES */ | 713, + ATTESTATION_ID_SERIAL = TagType.BYTES | 713, /** * Tag::ATTESTATION_ID_IMEI provides the IMEIs for all radios on the device to attested key @@ -747,7 +743,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ATTESTATION_ID_IMEI = (9 << 28) /* TagType:BYTES */ | 714, + ATTESTATION_ID_IMEI = TagType.BYTES | 714, /** * Tag::ATTESTATION_ID_MEID provides the MEIDs for all radios on the device to attested key @@ -760,7 +756,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ATTESTATION_ID_MEID = (9 << 28) /* TagType:BYTES */ | 715, + ATTESTATION_ID_MEID = TagType.BYTES | 715, /** * Tag::ATTESTATION_ID_MANUFACTURER provides the device's manufacturer name, as returned by @@ -773,7 +769,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ATTESTATION_ID_MANUFACTURER = (9 << 28) /* TagType:BYTES */ | 716, + ATTESTATION_ID_MANUFACTURER = TagType.BYTES | 716, /** * Tag::ATTESTATION_ID_MODEL provides the device's model name, as returned by Build.MODEL in @@ -786,7 +782,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - ATTESTATION_ID_MODEL = (9 << 28) /* TagType:BYTES */ | 717, + ATTESTATION_ID_MODEL = TagType.BYTES | 717, /** * Tag::VENDOR_PATCHLEVEL specifies the vendor image security patch level with which the key may @@ -808,7 +804,7 @@ enum Tag { * * Must be hardware-enforced. */ - VENDOR_PATCHLEVEL = (3 << 28) /* TagType:UINT */ | 718, + VENDOR_PATCHLEVEL = TagType.UINT | 718, /** * Tag::BOOT_PATCHLEVEL specifies the boot image (kernel) security patch level with which the @@ -828,7 +824,7 @@ enum Tag { * * Must be hardware-enforced. */ - BOOT_PATCHLEVEL = (3 << 28) /* TagType:UINT */ | 719, + BOOT_PATCHLEVEL = TagType.UINT | 719, /** * DEVICE_UNIQUE_ATTESTATION is an argument to IKeyMintDevice::attested key generation/import @@ -854,7 +850,7 @@ enum Tag { * IKeyMintDevice implementations that support device-unique attestation MUST add the * DEVICE_UNIQUE_ATTESTATION tag to device-unique attestations. */ - DEVICE_UNIQUE_ATTESTATION = (7 << 28) /* TagType:BOOL */ | 720, + DEVICE_UNIQUE_ATTESTATION = TagType.BOOL | 720, /** * IDENTITY_CREDENTIAL_KEY is never used by IKeyMintDevice, is not a valid argument to key @@ -862,7 +858,7 @@ enum Tag { * attestation. It is used in attestations produced by the IIdentityCredential HAL when that * HAL attests to Credential Keys. IIdentityCredential produces KeyMint-style attestations. */ - IDENTITY_CREDENTIAL_KEY = (7 << 28) /* TagType:BOOL */ | 721, + IDENTITY_CREDENTIAL_KEY = TagType.BOOL | 721, /** * To prevent keys from being compromised if an attacker acquires read access to system / kernel @@ -880,12 +876,12 @@ enum Tag { * ErrorCode::INVALID_OPERATION is returned when a key with Tag::STORAGE_KEY is provided to * begin(). */ - STORAGE_KEY = (7 << 28) /* TagType:BOOL */ | 722, + STORAGE_KEY = TagType.BOOL | 722, /** * TODO: Delete when keystore1 is deleted. */ - ASSOCIATED_DATA = (9 << 28) /* TagType:BYTES */ | 1000, + ASSOCIATED_DATA = TagType.BYTES | 1000, /** * Tag::NONCE is used to provide or return a nonce or Initialization Vector (IV) for AES-GCM, @@ -900,7 +896,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - NONCE = (9 << 28) /* TagType:BYTES */ | 1001, + NONCE = TagType.BYTES | 1001, /** * Tag::MAC_LENGTH provides the requested length of a MAC or GCM authentication tag, in bits. @@ -911,7 +907,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - MAC_LENGTH = (3 << 28) /* TagType:UINT */ | 1003, + MAC_LENGTH = TagType.UINT | 1003, /** * Tag::RESET_SINCE_ID_ROTATION specifies whether the device has been factory reset since the @@ -919,7 +915,7 @@ enum Tag { * * Must never appear in KeyCharacteristics. */ - RESET_SINCE_ID_ROTATION = (7 << 28) /* TagType:BOOL */ | 1004, + RESET_SINCE_ID_ROTATION = TagType.BOOL | 1004, /** * OBSOLETE: Do not use. See the authToken parameter for IKeyMintDevice::begin and for @@ -927,7 +923,7 @@ enum Tag { * * TODO(b/191738660): Delete when keystore1 is deleted. */ - CONFIRMATION_TOKEN = (9 << 28) /* TagType:BYTES */ | 1005, + CONFIRMATION_TOKEN = TagType.BYTES | 1005, /** * Tag::CERTIFICATE_SERIAL specifies the serial number to be assigned to the attestation @@ -935,7 +931,7 @@ enum Tag { * keyMint in the attestation parameters during generateKey() and importKey(). If not provided, * the serial shall default to 1. */ - CERTIFICATE_SERIAL = (8 << 28) /* TagType:BIGNUM */ | 1006, + CERTIFICATE_SERIAL = TagType.BIGNUM | 1006, /** * Tag::CERTIFICATE_SUBJECT the certificate subject. The value is a DER encoded X509 NAME. @@ -943,7 +939,7 @@ enum Tag { * during generateKey and importKey. If not provided the subject name shall default to * CN="Android Keystore Key". */ - CERTIFICATE_SUBJECT = (9 << 28) /* TagType:BYTES */ | 1007, + CERTIFICATE_SUBJECT = TagType.BYTES | 1007, /** * Tag::CERTIFICATE_NOT_BEFORE the beginning of the validity of the certificate in UNIX epoch @@ -951,7 +947,7 @@ enum Tag { * certificates. ErrorCode::MISSING_NOT_BEFORE must be returned if this tag is not provided if * this tag is not provided to generateKey or importKey. */ - CERTIFICATE_NOT_BEFORE = (6 << 28) /* TagType:DATE */ | 1008, + CERTIFICATE_NOT_BEFORE = TagType.DATE | 1008, /** * Tag::CERTIFICATE_NOT_AFTER the end of the validity of the certificate in UNIX epoch time in @@ -959,7 +955,7 @@ enum Tag { * ErrorCode::MISSING_NOT_AFTER must be returned if this tag is not provided to generateKey or * importKey. */ - CERTIFICATE_NOT_AFTER = (6 << 28) /* TagType:DATE */ | 1009, + CERTIFICATE_NOT_AFTER = TagType.DATE | 1009, /** * Tag::MAX_BOOT_LEVEL specifies a maximum boot level at which a key should function. @@ -970,5 +966,5 @@ enum Tag { * * Cannot be hardware enforced in this version. */ - MAX_BOOT_LEVEL = (3 << 28) /* TagType:UINT */ | 1010, + MAX_BOOT_LEVEL = TagType.UINT | 1010, } From 17587b018339160071ca601db32e283dfe7d7abf Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Fri, 2 Jul 2021 15:38:17 -0700 Subject: [PATCH 31/61] Add test ensuring that BCC keys not unique ids Get two test BCCs, then ensure that no repeated keys are found. Ignore-AOSP-First: No merge path to AOSP, will manually port. Bug: 192687735 Test: VtsHalRemotelyProvisionedComponentTargetTest Change-Id: I48f86e7dfa9ab4bc6303a8d1b64ac7ca6ac76bbf --- .../VtsRemotelyProvisionedComponentTests.cpp | 50 ++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp index 78f8f08637..32765ad460 100644 --- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp +++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include "KeyMintAidlTestBase.h" @@ -297,7 +298,8 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { } void checkProtectedData(const DeviceInfo& deviceInfo, const cppbor::Array& keysToSign, - const bytevec& keysToSignMac, const ProtectedData& protectedData) { + const bytevec& keysToSignMac, const ProtectedData& protectedData, + std::vector* bccOutput = nullptr) { auto [parsedProtectedData, _, protDataErrMsg] = cppbor::parse(protectedData.protectedData); ASSERT_TRUE(parsedProtectedData) << protDataErrMsg; ASSERT_TRUE(parsedProtectedData->asArray()); @@ -354,6 +356,10 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { auto macPayload = verifyAndParseCoseMac0(&coseMac0, *macKey); ASSERT_TRUE(macPayload) << macPayload.message(); + + if (bccOutput) { + *bccOutput = std::move(*bccContents); + } } bytevec eekId_; @@ -386,6 +392,48 @@ TEST_P(CertificateRequestTest, EmptyRequest_testMode) { } } +/** + * Ensure that test mode outputs a unique BCC root key every time we request a + * certificate request. Else, it's possible that the test mode API could be used + * to fingerprint devices. Only the GEEK should be allowed to decrypt the same + * device public key multiple times. + */ +TEST_P(CertificateRequestTest, NewKeyPerCallInTestMode) { + constexpr bool testMode = true; + constexpr size_t eekLength = 2; + + generateEek(eekLength); + + bytevec keysToSignMac; + DeviceInfo deviceInfo; + ProtectedData protectedData; + auto status = provisionable_->generateCertificateRequest( + testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo, &protectedData, + &keysToSignMac); + ASSERT_TRUE(status.isOk()) << status.getMessage(); + + std::vector firstBcc; + checkProtectedData(deviceInfo, /*keysToSign=*/cppbor::Array(), keysToSignMac, protectedData, + &firstBcc); + + status = provisionable_->generateCertificateRequest(testMode, {} /* keysToSign */, + eekChain_.chain, challenge_, &deviceInfo, + &protectedData, &keysToSignMac); + ASSERT_TRUE(status.isOk()) << status.getMessage(); + + std::vector secondBcc; + checkProtectedData(deviceInfo, /*keysToSign=*/cppbor::Array(), keysToSignMac, protectedData, + &secondBcc); + + // Verify that none of the keys in the first BCC are repeated in the second one. + for (const auto& i : firstBcc) { + for (auto& j : secondBcc) { + ASSERT_THAT(i.pubKey, testing::Not(testing::ElementsAreArray(j.pubKey))) + << "Found a repeated pubkey in two generateCertificateRequest test mode calls"; + } + } +} + /** * Generate an empty certificate request in prod mode. Generation will fail because we don't have a * valid GEEK. From 3ed299dfcae89db2cc3e75f6e0a0efbb5a3910eb Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Fri, 23 Apr 2021 13:55:51 -0700 Subject: [PATCH 32/61] Camera: Add logical camera requirement test for GVF The test verifies that if more than one color camera is available for a particular facing, a logical mulit-camera must be supported consisting all color cameras facing that direction. Test: Run VTS test on Pixel4 and cuttlefish emulator Bug: 178633246 Change-Id: I7b02a4057064a7f4a236c1bbc49f768ac80232cf --- .../VtsHalCameraProviderV2_4TargetTest.cpp | 190 +++++++++++++++++- 1 file changed, 187 insertions(+), 3 deletions(-) diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp index 41a08f9843..49e00f4f4b 100644 --- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp +++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp @@ -230,10 +230,10 @@ namespace { return false; } - int getCameraDeviceVersion(const hidl_string& deviceName, - const hidl_string &providerType) { + int getCameraDeviceVersionAndId(const hidl_string& deviceName, + const hidl_string &providerType, std::string* id) { std::string version; - bool match = matchDeviceName(deviceName, providerType, &version, nullptr); + bool match = matchDeviceName(deviceName, providerType, &version, id); if (!match) { return -1; } @@ -256,6 +256,11 @@ namespace { return 0; } + int getCameraDeviceVersion(const hidl_string& deviceName, + const hidl_string &providerType) { + return getCameraDeviceVersionAndId(deviceName, providerType, nullptr); + } + bool parseProviderName(const std::string& name, std::string *type /*out*/, uint32_t *id /*out*/) { if (!type || !id) { @@ -930,6 +935,7 @@ public: camera_metadata_ro_entry* streamConfigs, camera_metadata_ro_entry* maxResolutionStreamConfigs, const camera_metadata_t* staticMetadata); + static bool isColorCamera(const camera_metadata_t *metadata); static V3_2::DataspaceFlags getDataspace(PixelFormat format); @@ -6179,6 +6185,167 @@ TEST_P(CameraHidlTest, configureInjectionStreamsWithSessionParameters) { } } +// Test the multi-camera API requirement for Google Requirement Freeze S +// Note that this requirement can only be partially tested. If a vendor +// device doesn't expose a physical camera in any shape or form, there is no way +// the test can catch it. +TEST_P(CameraHidlTest, grfSMultiCameraTest) { + const int socGrfApi = property_get_int32("ro.board.first_api_level", /*default*/ -1); + if (socGrfApi < 31 /*S*/) { + // Non-GRF devices, or version < 31 Skip + ALOGI("%s: socGrfApi level is %d. Skipping", __FUNCTION__, socGrfApi); + return; + } + + // Test that if more than one color cameras facing the same direction are + // supported, there must be at least one logical camera facing that + // direction. + hidl_vec cameraDeviceNames = getCameraDeviceNames(mProvider); + // Front and back facing non-logical color cameras + std::set frontColorCameras, rearColorCameras; + // Front and back facing logical cameras' physical camera Id sets + std::set> frontPhysicalIds, rearPhysicalIds; + for (const auto& name : cameraDeviceNames) { + std::string cameraId; + int deviceVersion = getCameraDeviceVersionAndId(name, mProviderType, &cameraId); + switch (deviceVersion) { + case CAMERA_DEVICE_API_VERSION_3_7: + case CAMERA_DEVICE_API_VERSION_3_6: + case CAMERA_DEVICE_API_VERSION_3_5: + case CAMERA_DEVICE_API_VERSION_3_4: + case CAMERA_DEVICE_API_VERSION_3_3: + case CAMERA_DEVICE_API_VERSION_3_2: { + ::android::sp<::android::hardware::camera::device::V3_2::ICameraDevice> device3_x; + ALOGI("getCameraCharacteristics: Testing camera device %s", name.c_str()); + Return ret; + ret = mProvider->getCameraDeviceInterface_V3_x( + name, [&](auto status, const auto& device) { + ALOGI("getCameraDeviceInterface_V3_x returns status:%d", (int)status); + ASSERT_EQ(Status::OK, status); + ASSERT_NE(device, nullptr); + device3_x = device; + }); + ASSERT_TRUE(ret.isOk()); + + ret = device3_x->getCameraCharacteristics([&](auto status, const auto& chars) { + ASSERT_EQ(Status::OK, status); + const camera_metadata_t* metadata = (camera_metadata_t*)chars.data(); + + // Skip if this is not a color camera. + if (!CameraHidlTest::isColorCamera(metadata)) { + return; + } + + // Check camera facing. Skip if facing is neither FRONT + // nor BACK. If this is not a logical camera, only note down + // the camera ID, and skip. + camera_metadata_ro_entry entry; + int retcode = find_camera_metadata_ro_entry( + metadata, ANDROID_LENS_FACING, &entry); + ASSERT_EQ(retcode, 0); + ASSERT_GT(entry.count, 0); + uint8_t facing = entry.data.u8[0]; + bool isLogicalCamera = (isLogicalMultiCamera(metadata) == Status::OK); + if (facing == ANDROID_LENS_FACING_FRONT) { + if (!isLogicalCamera) { + frontColorCameras.insert(cameraId); + return; + } + } else if (facing == ANDROID_LENS_FACING_BACK) { + if (!isLogicalCamera) { + rearColorCameras.insert(cameraId); + return; + } + } else { + // Not FRONT or BACK facing. Skip. + return; + } + + // Check logical camera's physical camera IDs for color + // cameras. + std::unordered_set physicalCameraIds; + Status s = getPhysicalCameraIds(metadata, &physicalCameraIds); + ASSERT_EQ(Status::OK, s); + if (facing == ANDROID_LENS_FACING_FRONT) { + frontPhysicalIds.emplace(physicalCameraIds.begin(), physicalCameraIds.end()); + } else { + rearPhysicalIds.emplace(physicalCameraIds.begin(), physicalCameraIds.end()); + } + for (const auto& physicalId : physicalCameraIds) { + // Skip if the physicalId is publicly available + for (auto& deviceName : cameraDeviceNames) { + std::string publicVersion, publicId; + ASSERT_TRUE(::matchDeviceName(deviceName, mProviderType, + &publicVersion, &publicId)); + if (physicalId == publicId) { + // Skip because public Ids will be iterated in outer loop. + return; + } + } + + auto castResult = device::V3_5::ICameraDevice::castFrom(device3_x); + ASSERT_TRUE(castResult.isOk()); + ::android::sp<::android::hardware::camera::device::V3_5::ICameraDevice> + device3_5 = castResult; + ASSERT_NE(device3_5, nullptr); + + // Check camera characteristics for hidden camera id + Return ret = device3_5->getPhysicalCameraCharacteristics( + physicalId, [&](auto status, const auto& chars) { + ASSERT_EQ(Status::OK, status); + const camera_metadata_t* physicalMetadata = + (camera_metadata_t*)chars.data(); + + if (CameraHidlTest::isColorCamera(physicalMetadata)) { + if (facing == ANDROID_LENS_FACING_FRONT) { + frontColorCameras.insert(physicalId); + } else if (facing == ANDROID_LENS_FACING_BACK) { + rearColorCameras.insert(physicalId); + } + } + }); + ASSERT_TRUE(ret.isOk()); + } + }); + ASSERT_TRUE(ret.isOk()); + } break; + case CAMERA_DEVICE_API_VERSION_1_0: { + // Not applicable + } break; + default: { + ALOGE("%s: Unsupported device version %d", __func__, deviceVersion); + ADD_FAILURE(); + } break; + } + } + + // If there are more than one color cameras facing one direction, a logical + // multi-camera must be defined consisting of all color cameras facing that + // direction. + if (frontColorCameras.size() > 1) { + bool hasFrontLogical = false; + for (const auto& physicalIds : frontPhysicalIds) { + if (std::includes(physicalIds.begin(), physicalIds.end(), + frontColorCameras.begin(), frontColorCameras.end())) { + hasFrontLogical = true; + break; + } + } + ASSERT_TRUE(hasFrontLogical); + } + if (rearColorCameras.size() > 1) { + bool hasRearLogical = false; + for (const auto& physicalIds : rearPhysicalIds) { + if (std::includes(physicalIds.begin(), physicalIds.end(), + rearColorCameras.begin(), rearColorCameras.end())) { + hasRearLogical = true; + break; + } + } + ASSERT_TRUE(hasRearLogical); + } +} + // Retrieve all valid output stream resolutions from the camera // static characteristics. Status CameraHidlTest::getAvailableOutputStreams(const camera_metadata_t* staticMeta, @@ -6651,6 +6818,23 @@ Status CameraHidlTest::isMonochromeCamera(const camera_metadata_t *staticMeta) { return ret; } +bool CameraHidlTest::isColorCamera(const camera_metadata_t *metadata) { + camera_metadata_ro_entry entry; + int retcode = find_camera_metadata_ro_entry( + metadata, ANDROID_REQUEST_AVAILABLE_CAPABILITIES, &entry); + if ((0 == retcode) && (entry.count > 0)) { + bool isBackwardCompatible = (std::find(entry.data.u8, entry.data.u8 + entry.count, + ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE) != + entry.data.u8 + entry.count); + bool isMonochrome = (std::find(entry.data.u8, entry.data.u8 + entry.count, + ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MONOCHROME) != + entry.data.u8 + entry.count); + bool isColor = isBackwardCompatible && !isMonochrome; + return isColor; + } + return false; +} + // Retrieve the reprocess input-output format map from the static // camera characteristics. Status CameraHidlTest::getZSLInputOutputMap(camera_metadata_t *staticMeta, From f594fce5ddb008143f09de27a3a4d942cbb0bb05 Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Thu, 24 Jun 2021 16:29:38 -0700 Subject: [PATCH 33/61] Add Attestation IDs State to DeviceInfo We will use the 'Attestation IDs State' field in DeviceInfo to determine whether a device is still provisionable or not. Once a production device has left the factory, certain attestated device ids should be fixed, and 'Attestation IDs State' should reflect this by reporting "locked". Remove stale, duplicated DeviceInfo description from ProtectedData.aidl Test: None, just a doc change Bug: 192017485 Change-Id: I4e0a840a8f415b3b410801805a158c46be30ec6a --- .../hardware/security/keymint/DeviceInfo.aidl | 6 ++++++ .../hardware/security/keymint/ProtectedData.aidl | 15 +-------------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/security/keymint/aidl/android/hardware/security/keymint/DeviceInfo.aidl b/security/keymint/aidl/android/hardware/security/keymint/DeviceInfo.aidl index 32d69cd227..b0761bf828 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/DeviceInfo.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/DeviceInfo.aidl @@ -44,6 +44,12 @@ parcelable DeviceInfo { * ? "vendor_patch_level" : uint, // YYYYMMDD * "version" : 1, // The CDDL schema version. * "security_level" : "tee" / "strongbox" + * "att_id_state": "locked" / "open", // Attestation IDs State. If "locked", this + * // indicates a device's attestable IDs are + * // factory-locked and immutable. If "open", + * // this indicates the device is still in a + * // provisionable state and the attestable IDs + * // are not yet frozen. * } */ byte[] deviceInfo; diff --git a/security/keymint/aidl/android/hardware/security/keymint/ProtectedData.aidl b/security/keymint/aidl/android/hardware/security/keymint/ProtectedData.aidl index 31dbb288ab..24cdbc1fa7 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/ProtectedData.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/ProtectedData.aidl @@ -158,20 +158,7 @@ parcelable ProtectedData { * payload: bstr .cbor BccPayload * ] * - * VerifiedDeviceInfo = { - * ? "brand" : tstr, - * ? "manufacturer" : tstr, - * ? "product" : tstr, - * ? "model" : tstr, - * ? "board" : tstr, - * ? "device" : tstr, - * ? "vb_state" : "green" / "yellow" / "orange", - * ? "bootloader_state" : "locked" / "unlocked", - * ? "os_version" : tstr, - * ? "system_patch_level" : uint, // YYYYMMDD - * ? "boot_patch_level" : uint, // YYYYMMDD - * ? "vendor_patch_level" : uint, // YYYYMMDD - * } + * VerifiedDeviceInfo = DeviceInfo // See DeviceInfo.aidl * * PubKeyX25519 = { // COSE_Key * 1 : 1, // Key type : Octet Key Pair From 3e6c2ef9c8b7a59055a604ab50d0c527675d6f86 Mon Sep 17 00:00:00 2001 From: Eran Messeri Date: Tue, 6 Jul 2021 12:07:57 +0100 Subject: [PATCH 34/61] KeyMint: Fix device-unique attestation chain specification Fix the device-unique attestation chain specification: The chain should have two or three certificates. In case of two certificates, the device-unique key should be used for the self-signed root. In case of three certificates, the device-unique key should be certified by another key (ideally shared by all StrongBox instances from the same manufacturer, to ease validation). Adjust the device-unique attestation tests to accept two or three certificates in the chain. Additionally, the current StrongBox KeyMint implementation can not yet generate fully-valid chains (with matching subjects and issuers), so relax that check. Bug: 191361618 Test: m VtsAidlKeyMintTargetTest Merged-In: I6e6bca33ebb4af67cac8e41a39e9c305d0f1345f Change-Id: Iebefafe72148c919d10308eff7a19fc1bc40c619 --- .../android/hardware/security/keymint/Tag.aidl | 16 +++++++++++++--- .../functional/DeviceUniqueAttestationTest.cpp | 13 +++++++++---- .../aidl/vts/functional/KeyMintAidlTestBase.cpp | 5 +++-- .../aidl/vts/functional/KeyMintAidlTestBase.h | 3 ++- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl index 270574bbfb..e5307fd812 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl @@ -833,11 +833,21 @@ enum Tag { /** * DEVICE_UNIQUE_ATTESTATION is an argument to IKeyMintDevice::attested key generation/import * operations. It indicates that attestation using a device-unique key is requested, rather - * than a batch key. When a device-unique key is used, the returned chain should contain two - * certificates: + * than a batch key. When a device-unique key is used, the returned chain contains two or + * three certificates. + * + * In case the chain contains two certificates, they should be: * * The attestation certificate, containing the attestation extension, as described in - KeyCreationResult.aidl. + * KeyCreationResult.aidl. * * A self-signed root certificate, signed by the device-unique key. + * + * In case the chain contains three certificates, they should be: + * * The attestation certificate, containing the attestation extension, as described in + * KeyCreationResult.aidl, signed by the device-unique key. + * * An intermediate certificate, containing the public portion of the device-unique key. + * * A self-signed root certificate, signed by a dedicated key, certifying the + * intermediate. + * * No additional chained certificates are provided. Only SecurityLevel::STRONGBOX * IKeyMintDevices may support device-unique attestations. SecurityLevel::TRUSTED_ENVIRONMENT * IKeyMintDevices must return ErrorCode::INVALID_ARGUMENT if they receive diff --git a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp index a3ed3ad4a0..d7abf0790c 100644 --- a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp +++ b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp @@ -40,11 +40,16 @@ class DeviceUniqueAttestationTest : public KeyMintAidlTestBase { AuthorizationSet crypto_params = SecLevelAuthorizations(key_characteristics); - // The device-unique attestation chain should contain exactly two certificates: + // The device-unique attestation chain should contain exactly three certificates: // * The leaf with the attestation extension. - // * A self-signed root, signed using the device-unique key. - ASSERT_EQ(cert_chain_.size(), 2); - EXPECT_TRUE(ChainSignaturesAreValid(cert_chain_)); + // * An intermediate, signing the leaf using the device-unique key. + // * A self-signed root, signed using some authority's key, certifying + // the device-unique key. + const size_t chain_length = cert_chain_.size(); + ASSERT_TRUE(chain_length == 2 || chain_length == 3); + // TODO(b/191361618): Once StrongBox implementations use a correctly-issued + // certificate chain, do not skip issuers matching. + EXPECT_TRUE(ChainSignaturesAreValid(cert_chain_, /* strict_issuer_check= */ false)); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(key_characteristics); EXPECT_TRUE(verify_attestation_record("challenge", "foo", sw_enforced, hw_enforced, diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index 5359b3b667..20324117b9 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -1493,7 +1493,8 @@ AuthorizationSet SwEnforcedAuthorizations(const vector& key_ return authList; } -AssertionResult ChainSignaturesAreValid(const vector& chain) { +AssertionResult ChainSignaturesAreValid(const vector& chain, + bool strict_issuer_check) { std::stringstream cert_data; for (size_t i = 0; i < chain.size(); ++i) { @@ -1520,7 +1521,7 @@ AssertionResult ChainSignaturesAreValid(const vector& chain) { string cert_issuer = x509NameToStr(X509_get_issuer_name(key_cert.get())); string signer_subj = x509NameToStr(X509_get_subject_name(signing_cert.get())); - if (cert_issuer != signer_subj) { + if (cert_issuer != signer_subj && strict_issuer_check) { return AssertionFailure() << "Cert " << i << " has wrong issuer.\n" << " Signer subject is " << signer_subj << " Issuer subject is " << cert_issuer << endl diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index d592d3686b..ec3fcf6a3e 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -349,7 +349,8 @@ void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey); AuthorizationSet HwEnforcedAuthorizations(const vector& key_characteristics); AuthorizationSet SwEnforcedAuthorizations(const vector& key_characteristics); -::testing::AssertionResult ChainSignaturesAreValid(const vector& chain); +::testing::AssertionResult ChainSignaturesAreValid(const vector& chain, + bool strict_issuer_check = true); #define INSTANTIATE_KEYMINT_AIDL_TEST(name) \ INSTANTIATE_TEST_SUITE_P(PerInstance, name, \ From 7e643777e125a0caf655629c8d993390bdeb7aa4 Mon Sep 17 00:00:00 2001 From: Lais Andrade Date: Fri, 9 Jul 2021 14:59:44 +0100 Subject: [PATCH 35/61] Update vibrator VTS to only validate support from required primitives Bug: 193196353 Test: VtsHalVibratorTargetTest Change-Id: I7ec2f0d82290f42259f8383db9ff00a126a2a7a4 --- vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp b/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp index c56bd9a4d4..553d7f0a4a 100644 --- a/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp +++ b/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp @@ -60,9 +60,10 @@ const std::vector kCompositePrimitives{ android::enum_range().begin(), android::enum_range().end()}; -const std::vector kOptionalPrimitives = { - CompositePrimitive::THUD, - CompositePrimitive::SPIN, +const std::vector kRequiredPrimitives = { + CompositePrimitive::CLICK, CompositePrimitive::LIGHT_TICK, + CompositePrimitive::QUICK_RISE, CompositePrimitive::SLOW_RISE, + CompositePrimitive::QUICK_FALL, }; const std::vector kInvalidPrimitives = { @@ -393,11 +394,11 @@ TEST_P(VibratorAidl, GetSupportedPrimitives) { for (auto primitive : kCompositePrimitives) { bool isPrimitiveSupported = std::find(supported.begin(), supported.end(), primitive) != supported.end(); - bool isPrimitiveOptional = - std::find(kOptionalPrimitives.begin(), kOptionalPrimitives.end(), primitive) != - kOptionalPrimitives.end(); + bool isPrimitiveRequired = + std::find(kRequiredPrimitives.begin(), kRequiredPrimitives.end(), primitive) != + kRequiredPrimitives.end(); - EXPECT_TRUE(isPrimitiveSupported || isPrimitiveOptional) << toString(primitive); + EXPECT_TRUE(isPrimitiveSupported || !isPrimitiveRequired) << toString(primitive); } } } From 87eb1dd928dd8e4934f18dfe3a6f7c45fbe95424 Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Wed, 23 Jun 2021 15:15:52 -0700 Subject: [PATCH 36/61] Update KeyMint VTS tests with prod GEEK Now that we have the production Google Endpoint Encryption Key, we can update the tests to use the correct GEEK cert chain where applicable. Ignore-AOSP-First: No merge path to aosp, will manually merge Test: VtsHalRemotelyProvisionedComponentTargetTest Test: VtsAidlKeyMintTargetTest Bug: 191301285 Change-Id: I84b557c6bad34741ffe6671fc941d9e266b73241 --- .../VtsRemotelyProvisionedComponentTests.cpp | 149 ++++++++---------- 1 file changed, 62 insertions(+), 87 deletions(-) diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp index 32765ad460..af951e8861 100644 --- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp +++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp @@ -103,8 +103,8 @@ ErrMsgOr corrupt_sig(const cppbor::Array* coseSign1) { return std::move(corruptSig); } -ErrMsgOr corrupt_sig_chain(const EekChain& eek, int which) { - auto [chain, _, parseErr] = cppbor::parse(eek.chain); +ErrMsgOr corrupt_sig_chain(const bytevec& encodedEekChain, int which) { + auto [chain, _, parseErr] = cppbor::parse(encodedEekChain); if (!chain || !chain->asArray()) { return "EekChain parse failed"; } @@ -126,7 +126,7 @@ ErrMsgOr corrupt_sig_chain(const EekChain& eek, int which) { corruptChain.add(eekChain->get(ii)->clone()); } } - return EekChain{corruptChain.encode(), eek.last_pubkey, eek.last_privkey}; + return corruptChain.encode(); } string device_suffix(const string& name) { @@ -272,14 +272,14 @@ TEST_P(GenerateKeyTests, generateEcdsaP256Key_testMode) { class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { protected: CertificateRequestTest() : eekId_(string_to_bytevec("eekid")), challenge_(randomBytes(32)) { - generateEek(3); + generateTestEekChain(3); } - void generateEek(size_t eekLength) { + void generateTestEekChain(size_t eekLength) { auto chain = generateEekChain(eekLength, eekId_); EXPECT_TRUE(chain) << chain.message(); - if (chain) eekChain_ = chain.moveValue(); - eekLength_ = eekLength; + if (chain) testEekChain_ = chain.moveValue(); + testEekLength_ = eekLength; } void generateKeys(bool testMode, size_t numKeys) { @@ -309,8 +309,9 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { ASSERT_TRUE(senderPubkey) << senderPubkey.message(); EXPECT_EQ(senderPubkey->second, eekId_); - auto sessionKey = x25519_HKDF_DeriveKey(eekChain_.last_pubkey, eekChain_.last_privkey, - senderPubkey->first, false /* senderIsA */); + auto sessionKey = + x25519_HKDF_DeriveKey(testEekChain_.last_pubkey, testEekChain_.last_privkey, + senderPubkey->first, false /* senderIsA */); ASSERT_TRUE(sessionKey) << sessionKey.message(); auto protectedDataPayload = @@ -363,8 +364,8 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { } bytevec eekId_; - size_t eekLength_; - EekChain eekChain_; + size_t testEekLength_; + EekChain testEekChain_; bytevec challenge_; std::vector keysToSign_; cppbor::Array cborKeysToSign_; @@ -378,13 +379,13 @@ TEST_P(CertificateRequestTest, EmptyRequest_testMode) { bool testMode = true; for (size_t eekLength : {2, 3, 7}) { SCOPED_TRACE(testing::Message() << "EEK of length " << eekLength); - generateEek(eekLength); + generateTestEekChain(eekLength); bytevec keysToSignMac; DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo, + testMode, {} /* keysToSign */, testEekChain_.chain, challenge_, &deviceInfo, &protectedData, &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); @@ -400,25 +401,22 @@ TEST_P(CertificateRequestTest, EmptyRequest_testMode) { */ TEST_P(CertificateRequestTest, NewKeyPerCallInTestMode) { constexpr bool testMode = true; - constexpr size_t eekLength = 2; - - generateEek(eekLength); bytevec keysToSignMac; DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); + testMode, {} /* keysToSign */, testEekChain_.chain, challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); std::vector firstBcc; checkProtectedData(deviceInfo, /*keysToSign=*/cppbor::Array(), keysToSignMac, protectedData, &firstBcc); - status = provisionable_->generateCertificateRequest(testMode, {} /* keysToSign */, - eekChain_.chain, challenge_, &deviceInfo, - &protectedData, &keysToSignMac); + status = provisionable_->generateCertificateRequest( + testMode, {} /* keysToSign */, testEekChain_.chain, challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); std::vector secondBcc; @@ -435,28 +433,20 @@ TEST_P(CertificateRequestTest, NewKeyPerCallInTestMode) { } /** - * Generate an empty certificate request in prod mode. Generation will fail because we don't have a - * valid GEEK. - * - * TODO(swillden): Get a valid GEEK and use it so the generation can succeed, though we won't be - * able to decrypt. + * Generate an empty certificate request in prod mode. This test must be run explicitly, and + * is not run by default. Not all devices are GMS devices, and therefore they do not all + * trust the Google EEK root. */ -TEST_P(CertificateRequestTest, EmptyRequest_prodMode) { +TEST_P(CertificateRequestTest, DISABLED_EmptyRequest_prodMode) { bool testMode = false; - for (size_t eekLength : {2, 3, 7}) { - SCOPED_TRACE(testing::Message() << "EEK of length " << eekLength); - generateEek(eekLength); - bytevec keysToSignMac; - DeviceInfo deviceInfo; - ProtectedData protectedData; - auto status = provisionable_->generateCertificateRequest( - testMode, {} /* keysToSign */, eekChain_.chain, challenge_, &deviceInfo, - &protectedData, &keysToSignMac); - EXPECT_FALSE(status.isOk()); - EXPECT_EQ(status.getServiceSpecificError(), - BnRemotelyProvisionedComponent::STATUS_INVALID_EEK); - } + bytevec keysToSignMac; + DeviceInfo deviceInfo; + ProtectedData protectedData; + auto status = provisionable_->generateCertificateRequest( + testMode, {} /* keysToSign */, getProdEekChain(), challenge_, &deviceInfo, + &protectedData, &keysToSignMac); + EXPECT_TRUE(status.isOk()); } /** @@ -468,13 +458,13 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_testMode) { for (size_t eekLength : {2, 3, 7}) { SCOPED_TRACE(testing::Message() << "EEK of length " << eekLength); - generateEek(eekLength); + generateTestEekChain(eekLength); bytevec keysToSignMac; DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, &protectedData, + testMode, keysToSign_, testEekChain_.chain, challenge_, &deviceInfo, &protectedData, &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); @@ -483,30 +473,21 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_testMode) { } /** - * Generate a non-empty certificate request in prod mode. Must fail because we don't have a valid - * GEEK. - * - * TODO(swillden): Get a valid GEEK and use it so the generation can succeed, though we won't be - * able to decrypt. + * Generate a non-empty certificate request in prod mode. This test must be run explicitly, and + * is not run by default. Not all devices are GMS devices, and therefore they do not all + * trust the Google EEK root. */ -TEST_P(CertificateRequestTest, NonEmptyRequest_prodMode) { +TEST_P(CertificateRequestTest, DISABLED_NonEmptyRequest_prodMode) { bool testMode = false; generateKeys(testMode, 4 /* numKeys */); - for (size_t eekLength : {2, 3, 7}) { - SCOPED_TRACE(testing::Message() << "EEK of length " << eekLength); - generateEek(eekLength); - - bytevec keysToSignMac; - DeviceInfo deviceInfo; - ProtectedData protectedData; - auto status = provisionable_->generateCertificateRequest( - testMode, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); - EXPECT_FALSE(status.isOk()); - EXPECT_EQ(status.getServiceSpecificError(), - BnRemotelyProvisionedComponent::STATUS_INVALID_EEK); - } + bytevec keysToSignMac; + DeviceInfo deviceInfo; + ProtectedData protectedData; + auto status = provisionable_->generateCertificateRequest( + testMode, keysToSign_, getProdEekChain(), challenge_, &deviceInfo, &protectedData, + &keysToSignMac); + EXPECT_TRUE(status.isOk()); } /** @@ -521,8 +502,8 @@ TEST_P(CertificateRequestTest, NonEmptyRequestCorruptMac_testMode) { DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, {keyWithCorruptMac}, eekChain_.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); + testMode, {keyWithCorruptMac}, testEekChain_.chain, challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()) << status.getMessage(); EXPECT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_MAC); } @@ -531,7 +512,7 @@ TEST_P(CertificateRequestTest, NonEmptyRequestCorruptMac_testMode) { * Generate a non-empty certificate request in prod mode, but with the MAC corrupted on the keypair. */ TEST_P(CertificateRequestTest, NonEmptyRequestCorruptMac_prodMode) { - bool testMode = true; + bool testMode = false; generateKeys(testMode, 1 /* numKeys */); MacedPublicKey keyWithCorruptMac = corrupt_maced_key(keysToSign_[0]).moveValue(); @@ -539,38 +520,35 @@ TEST_P(CertificateRequestTest, NonEmptyRequestCorruptMac_prodMode) { DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - testMode, {keyWithCorruptMac}, eekChain_.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); + testMode, {keyWithCorruptMac}, getProdEekChain(), challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()) << status.getMessage(); - auto rc = status.getServiceSpecificError(); - - // TODO(drysdale): drop the INVALID_EEK potential error code when a real GEEK is available. - EXPECT_TRUE(rc == BnRemotelyProvisionedComponent::STATUS_INVALID_EEK || - rc == BnRemotelyProvisionedComponent::STATUS_INVALID_MAC); + EXPECT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_MAC); } /** * Generate a non-empty certificate request in prod mode that has a corrupt EEK chain. * Confirm that the request is rejected. - * - * TODO(drysdale): Update to use a valid GEEK, so that the test actually confirms that the - * implementation is checking signatures. */ TEST_P(CertificateRequestTest, NonEmptyCorruptEekRequest_prodMode) { bool testMode = false; generateKeys(testMode, 4 /* numKeys */); - for (size_t ii = 0; ii < eekLength_; ii++) { - auto chain = corrupt_sig_chain(eekChain_, ii); + auto prodEekChain = getProdEekChain(); + auto [parsedChain, _, parseErr] = cppbor::parse(prodEekChain); + ASSERT_NE(parsedChain, nullptr) << parseErr; + ASSERT_NE(parsedChain->asArray(), nullptr); + + for (int ii = 0; ii < parsedChain->asArray()->size(); ++ii) { + auto chain = corrupt_sig_chain(prodEekChain, ii); ASSERT_TRUE(chain) << chain.message(); - EekChain corruptEek = chain.moveValue(); bytevec keysToSignMac; DeviceInfo deviceInfo; ProtectedData protectedData; - auto status = provisionable_->generateCertificateRequest( - testMode, keysToSign_, corruptEek.chain, challenge_, &deviceInfo, &protectedData, - &keysToSignMac); + auto status = provisionable_->generateCertificateRequest(testMode, keysToSign_, *chain, + challenge_, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_EEK); @@ -580,9 +558,6 @@ TEST_P(CertificateRequestTest, NonEmptyCorruptEekRequest_prodMode) { /** * Generate a non-empty certificate request in prod mode that has an incomplete EEK chain. * Confirm that the request is rejected. - * - * TODO(drysdale): Update to use a valid GEEK, so that the test actually confirms that the - * implementation is checking signatures. */ TEST_P(CertificateRequestTest, NonEmptyIncompleteEekRequest_prodMode) { bool testMode = false; @@ -590,7 +565,7 @@ TEST_P(CertificateRequestTest, NonEmptyIncompleteEekRequest_prodMode) { // Build an EEK chain that omits the first self-signed cert. auto truncatedChain = cppbor::Array(); - auto [chain, _, parseErr] = cppbor::parse(eekChain_.chain); + auto [chain, _, parseErr] = cppbor::parse(getProdEekChain()); ASSERT_TRUE(chain); auto eekChain = chain->asArray(); ASSERT_NE(eekChain, nullptr); @@ -619,7 +594,7 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_prodKeyInTestCert) { DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - true /* testMode */, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, + true /* testMode */, keysToSign_, testEekChain_.chain, challenge_, &deviceInfo, &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(), @@ -637,7 +612,7 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_testKeyInProdCert) { DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( - false /* testMode */, keysToSign_, eekChain_.chain, challenge_, &deviceInfo, + false /* testMode */, keysToSign_, testEekChain_.chain, challenge_, &deviceInfo, &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(), From b1bd1e8a3253242a164ce80476452027bc0da2eb Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Fri, 9 Jul 2021 11:15:18 -0700 Subject: [PATCH 37/61] Clear composition changes when a color mode is not supported Otherwise this may cause a test to spuriously fail during teardown. Bug: 184726169 Test: VtsHalGraphicsComposerV2_2TargetTest Change-Id: I569680a0fe6c866199ba0711e8cc263b9b3efd58 --- .../vts/functional/VtsHalGraphicsComposerV2_2ReadbackTest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/graphics/composer/2.2/vts/functional/VtsHalGraphicsComposerV2_2ReadbackTest.cpp b/graphics/composer/2.2/vts/functional/VtsHalGraphicsComposerV2_2ReadbackTest.cpp index 7a1568bd9f..7a053f1eb7 100644 --- a/graphics/composer/2.2/vts/functional/VtsHalGraphicsComposerV2_2ReadbackTest.cpp +++ b/graphics/composer/2.2/vts/functional/VtsHalGraphicsComposerV2_2ReadbackTest.cpp @@ -458,6 +458,7 @@ TEST_P(GraphicsCompositionTest, ClientComposition) { << " pixel format: PixelFormat::RGBA_8888 dataspace: " << ReadbackHelper::getDataspaceString(clientDataspace) << " unsupported for display" << std::endl; + mReader->mCompositionChanges.clear(); continue; } From c905ea66d41b9418a73cbaf860b7ea6661313f59 Mon Sep 17 00:00:00 2001 From: xshu Date: Sun, 11 Jul 2021 19:57:02 -0700 Subject: [PATCH 38/61] Clear ringbuffer after dumping to file Clear the in-memory ringbuffer after writing to file. Bug: 193007899 Test: Manually verified ringbuffers are cleared with command "adb shell lshal debug android.hardware.wifi@1.5::IWifi" Change-Id: Icfa08634e948d7155e231458edd394a4d699fbaa --- wifi/1.5/default/ringbuffer.cpp | 5 +++++ wifi/1.5/default/ringbuffer.h | 1 + wifi/1.5/default/wifi_chip.cpp | 5 +++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/wifi/1.5/default/ringbuffer.cpp b/wifi/1.5/default/ringbuffer.cpp index 26971ff25b..f554111e61 100644 --- a/wifi/1.5/default/ringbuffer.cpp +++ b/wifi/1.5/default/ringbuffer.cpp @@ -47,6 +47,11 @@ const std::list>& Ringbuffer::getData() const { return data_; } +void Ringbuffer::clear() { + data_.clear(); + size_ = 0; +} + } // namespace implementation } // namespace V1_5 } // namespace wifi diff --git a/wifi/1.5/default/ringbuffer.h b/wifi/1.5/default/ringbuffer.h index d8b87f2171..03fb37a6a2 100644 --- a/wifi/1.5/default/ringbuffer.h +++ b/wifi/1.5/default/ringbuffer.h @@ -37,6 +37,7 @@ class Ringbuffer { // within |maxSize_|. void append(const std::vector& input); const std::list>& getData() const; + void clear(); private: std::list> data_; diff --git a/wifi/1.5/default/wifi_chip.cpp b/wifi/1.5/default/wifi_chip.cpp index 961f9da4c2..024747af43 100644 --- a/wifi/1.5/default/wifi_chip.cpp +++ b/wifi/1.5/default/wifi_chip.cpp @@ -1940,8 +1940,8 @@ bool WifiChip::writeRingbufferFilesInternal() { // write ringbuffers to file { std::unique_lock lk(lock_t); - for (const auto& item : ringbuffer_map_) { - const Ringbuffer& cur_buffer = item.second; + for (auto& item : ringbuffer_map_) { + Ringbuffer& cur_buffer = item.second; if (cur_buffer.getData().empty()) { continue; } @@ -1959,6 +1959,7 @@ bool WifiChip::writeRingbufferFilesInternal() { PLOG(ERROR) << "Error writing to file"; } } + cur_buffer.clear(); } // unique_lock unlocked here } From 3dbdaa9717149eaf4f4be86443c78a3c1e72ac32 Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Mon, 12 Jul 2021 14:18:28 -0700 Subject: [PATCH 39/61] Don't fail if TAG_ALLOW_WHILE_ON_BODY is missing The TAG_ALLOW_WHILE_ON_BODY authorization is not required to be supported, and if it is not supported it's a noop. Don't expect the tag to fail with UNSUPPORTED_TAG on devices that don't support it. Test: VtsAidlKeyMintTargetTest Bug: 192222727 Change-Id: I2e80ca59151e79f595a65cae94ac966b4ba7020d --- security/keymint/aidl/vts/functional/KeyMintTest.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index d41d270764..5a87b83854 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -1487,9 +1487,8 @@ TEST_P(NewKeyGenerationTest, EcdsaAttestationTags) { tag.tag == TAG_ROLLBACK_RESISTANCE) { continue; } - if (result == ErrorCode::UNSUPPORTED_TAG && - (tag.tag == TAG_ALLOW_WHILE_ON_BODY || tag.tag == TAG_TRUSTED_USER_PRESENCE_REQUIRED)) { - // Optional tag not supported by this KeyMint implementation. + if (result == ErrorCode::UNSUPPORTED_TAG && tag.tag == TAG_TRUSTED_USER_PRESENCE_REQUIRED) { + // Tag not required to be supported by all KeyMint implementations. continue; } ASSERT_EQ(result, ErrorCode::OK); @@ -1501,9 +1500,8 @@ TEST_P(NewKeyGenerationTest, EcdsaAttestationTags) { AuthorizationSet hw_enforced = HwEnforcedAuthorizations(key_characteristics); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(key_characteristics); - if (tag.tag != TAG_ATTESTATION_APPLICATION_ID) { - // Expect to find most of the extra tags in the key characteristics - // of the generated key (but not for ATTESTATION_APPLICATION_ID). + // Some tags are optional, so don't require them to be in the enforcements. + if (tag.tag != TAG_ATTESTATION_APPLICATION_ID && tag.tag != TAG_ALLOW_WHILE_ON_BODY) { EXPECT_TRUE(hw_enforced.Contains(tag.tag) || sw_enforced.Contains(tag.tag)) << tag << " not in hw:" << hw_enforced << " nor sw:" << sw_enforced; } From 36f01d9083d71b70b021032adce9ac711cbd14eb Mon Sep 17 00:00:00 2001 From: Jayant Chowdhary Date: Mon, 12 Jul 2021 09:42:53 -0700 Subject: [PATCH 40/61] camera: Clarify that ANDROID_JPEG_MAX_SIZE applies to default sensor pixel mode. Bug: 193346383 Test: builds Change-Id: I451e4a8da7598f878f8fa5024e6bda5d8dbb1868 Signed-off-by: Jayant Chowdhary --- camera/metadata/3.2/types.hal | 4 +++- current.txt | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/camera/metadata/3.2/types.hal b/camera/metadata/3.2/types.hal index ad671d9db4..4b02830402 100644 --- a/camera/metadata/3.2/types.hal +++ b/camera/metadata/3.2/types.hal @@ -686,7 +686,9 @@ enum CameraMetadataTag : uint32_t { /** android.jpeg.maxSize [static, int32, system] * *

Maximum size in bytes for the compressed - * JPEG buffer

+ * JPEG buffer, in default sensor pixel mode (see ANDROID_SENSOR_PIXEL_MODE)

+ * + * @see ANDROID_SENSOR_PIXEL_MODE */ ANDROID_JPEG_MAX_SIZE, diff --git a/current.txt b/current.txt index 908ecc4ed2..2307c14b98 100644 --- a/current.txt +++ b/current.txt @@ -768,6 +768,8 @@ a64467bae843569f0d465c5be7f0c7a5b987985b55a3ef4794dd5afc68538650 android.hardwar 98592d193a717066facf91428426e5abe211e3bd718bc372e29fb944ddbe6e7c android.hardware.wifi.supplicant@1.3::types # ABI preserving changes to HALs during Android S +# b/193346383 +93d29fbe2fcc5e4e053a9db7c9abbd9190c46b85b443f2698a3460db2ee76c8d android.hardware.camera.metadata@3.2::types 159a0069336035852e9eca6354b86b7990680d1b239f23ef2f631b01807c4cb9 android.hardware.camera.metadata@3.5::types e042522daa4b5f7fd4a0a19bcdadb93c79a1b04c09ef2c9813a3a8941032f3f5 android.hardware.contexthub@1.0::IContexthub c2f64133b83ede65c9939ef97ab5bd867b73faf3dba0e7e69f77c3c43d9e487e android.hardware.contexthub@1.0::IContexthubCallback From 1c93999d3d6b6929aa1c812219c5df9925db1474 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Wed, 14 Jul 2021 16:50:14 +0100 Subject: [PATCH 41/61] KeyMaster 4.0 VTS: fix GSI detection Commit f18a8328a1ed ("keymaster: Relax testing under GSI") disabled some tag checks for devices running with GSI, but detected GSI by looking for an absence of the ro.boot.vbmeta.device_state property. This property is currently present on GSI, so instead detect GSI using the existing is_gsi() helper, which checks ro.product.system.name against "mainline". Bug: 192513934 Test: atest VtsHalKeymasterV4_0TargetTest:PerInstance/AttestationTest Change-Id: If3c7d84a9e091b9b0842e4d8919453600bc239ea Ignore-AOSP-First: manual merge to aosp/master to follow --- keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp b/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp index 01c502c586..476eed8b19 100644 --- a/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp +++ b/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp @@ -438,7 +438,7 @@ bool verify_attestation_record(const string& challenge, const string& app_id, // TODO(b/136282179): When running under VTS-on-GSI the TEE-backed // keymaster implementation will report YYYYMM dates instead of YYYYMMDD // for the BOOT_PATCH_LEVEL. - if (avb_verification_enabled()) { + if (!is_gsi()) { for (int i = 0; i < att_hw_enforced.size(); i++) { if (att_hw_enforced[i].tag == TAG_BOOT_PATCHLEVEL || att_hw_enforced[i].tag == TAG_VENDOR_PATCHLEVEL) { From eeac52c8f1da95cc6ed1e046c0f5ba7f04a194e0 Mon Sep 17 00:00:00 2001 From: Kalesh Singh Date: Tue, 13 Jul 2021 01:45:41 +0000 Subject: [PATCH 42/61] Memtrack HAL: Report global total GPU-private memory Update memtrack hal documentation to allow reporting the global total GPU-private memory from the getMemory() API. Specify how to handle unsupported memtrack operations. Bug: 193226716 Bug: 193465681 Bug: 192621117 Test: N/A Change-Id: I6fcebd16fafdc34cc662137784e86750ee907eee --- memtrack/aidl/android/hardware/memtrack/IMemtrack.aidl | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/memtrack/aidl/android/hardware/memtrack/IMemtrack.aidl b/memtrack/aidl/android/hardware/memtrack/IMemtrack.aidl index 13c3389730..88b090b68e 100644 --- a/memtrack/aidl/android/hardware/memtrack/IMemtrack.aidl +++ b/memtrack/aidl/android/hardware/memtrack/IMemtrack.aidl @@ -44,6 +44,12 @@ import android.hardware.memtrack.MemtrackType; * This category should report all GPU private allocations for the specified * PID that are not accounted in /proc//smaps. * + * getMemory() called with PID 0 should report the global total GPU-private + * memory, for MemtrackType::GL and MemtrackRecord::FLAG_SMAPS_UNACCOUNTED. + * + * getMemory() called with PID 0 for a MemtrackType other than GL should + * report 0. + * * - MemtrackType::OTHER and MemtrackRecord::FLAG_SMAPS_UNACCOUNTED * Any other memory not accounted for in /proc//smaps if any, otherwise * this should return 0. @@ -52,6 +58,9 @@ import android.hardware.memtrack.MemtrackType; * VM_PFNMAP flag set. For these mappings PSS and RSS are reported as 0 in smaps. * Such mappings have no backing page structs from which PSS/RSS can be calculated. * + * Any memtrack operation that is not supported should return a binder status with + * exception code EX_UNSUPPORTED_OPERATION. + * * Constructor for the interface should be used to perform memtrack management * setup actions and must be called once before any calls to getMemory(). */ From 238fbcc61a393b672cf59db6a4d29ef476cd56d3 Mon Sep 17 00:00:00 2001 From: David Li Date: Mon, 19 Jul 2021 12:48:06 +0800 Subject: [PATCH 43/61] audio: make sure to set back to AudioMode::NORMAL after the test case If the major version is greater than or equal to 6, the test tries to set the mode to AudioMode::CALL_SCREEN. However, it doesn't set back to AudioMode::NORMAL. Replace all ASSERT with EXPECT to ensure the test can reach to the explicit call to reset to AudioMode::NORMAL. Bug: 194022995 Test: atest VtsHalAudioV7_0TargetTest Change-Id: Ib9b6e310965a85b016853b72c60716fa054641c6 --- .../functional/4.0/AudioPrimaryHidlHalTest.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp index b96cc83673..f8af0304ad 100644 --- a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp @@ -343,18 +343,21 @@ TEST_P(AudioPrimaryHidlTest, setMode) { #endif for (int mode : {-2, -1, maxMode + 1}) { - ASSERT_RESULT(Result::INVALID_ARGUMENTS, getDevice()->setMode(AudioMode(mode))) + EXPECT_RESULT(Result::INVALID_ARGUMENTS, getDevice()->setMode(AudioMode(mode))) << "mode=" << mode; } - // Test valid values - for (AudioMode mode : {AudioMode::IN_CALL, AudioMode::IN_COMMUNICATION, AudioMode::RINGTONE, - AudioMode::NORMAL /* Make sure to leave the test in normal mode */}) { - ASSERT_OK(getDevice()->setMode(mode)) << "mode=" << toString(mode); - } + // AudioMode::CALL_SCREEN as support is optional #if MAJOR_VERSION >= 6 - ASSERT_RESULT(okOrNotSupportedOrInvalidArgs, getDevice()->setMode(AudioMode::CALL_SCREEN)); + EXPECT_RESULT(okOrNotSupportedOrInvalidArgs, getDevice()->setMode(AudioMode::CALL_SCREEN)); #endif + // Test valid values + for (AudioMode mode : {AudioMode::IN_CALL, AudioMode::IN_COMMUNICATION, AudioMode::RINGTONE, + AudioMode::NORMAL}) { + EXPECT_OK(getDevice()->setMode(mode)) << "mode=" << toString(mode); + } + // Make sure to leave the test in normal mode + getDevice()->setMode(AudioMode::NORMAL); } TEST_P(AudioPrimaryHidlTest, setBtHfpSampleRate) { From 70082bb98384fadc8da979e60630bab9d2cf7250 Mon Sep 17 00:00:00 2001 From: Yu-Han Yang Date: Thu, 15 Jul 2021 14:07:37 -0700 Subject: [PATCH 44/61] Ensure non-empty SvInfo is received Bug: 193806881 Test: atest VtsHalGnssV2_1TargetTest Change-Id: I79f0c7041af51403ec5a2d17a430cac6d7a88b80 --- .../vts/functional/gnss_hal_test_cases.cpp | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/gnss/2.1/vts/functional/gnss_hal_test_cases.cpp b/gnss/2.1/vts/functional/gnss_hal_test_cases.cpp index fcab8c4a04..8fa5f7e668 100644 --- a/gnss/2.1/vts/functional/gnss_hal_test_cases.cpp +++ b/gnss/2.1/vts/functional/gnss_hal_test_cases.cpp @@ -249,35 +249,40 @@ TEST_P(GnssHalTest, TestGnssAntennaInfo) { /* * TestGnssSvInfoFields: - * Gets 1 location and a GnssSvInfo, and verifies - * 1. basebandCN0DbHz is valid. + * Gets 1 location and a (non-empty) GnssSvInfo, and verifies basebandCN0DbHz is valid. */ TEST_P(GnssHalTest, TestGnssSvInfoFields) { gnss_cb_->location_cbq_.reset(); + gnss_cb_->sv_info_list_cbq_.reset(); StartAndCheckFirstLocation(/* min_interval_msec= */ 1000, /* low_power_mode= */ false); int location_called_count = gnss_cb_->location_cbq_.calledCount(); - - // Tolerate 1 less sv status to handle edge cases in reporting. - int sv_info_list_cbq_size = gnss_cb_->sv_info_list_cbq_.size(); - EXPECT_GE(sv_info_list_cbq_size, 0); ALOGD("Observed %d GnssSvStatus, while awaiting one location (%d received)", - sv_info_list_cbq_size, location_called_count); + gnss_cb_->sv_info_list_cbq_.size(), location_called_count); - // Get the last sv_info_list - std::list> sv_info_vec_list; - gnss_cb_->sv_info_list_cbq_.retrieve(sv_info_vec_list, sv_info_list_cbq_size, 1); - hidl_vec last_sv_info_list = sv_info_vec_list.back(); + // Wait for up to kNumSvInfoLists events for kTimeoutSeconds for each event. + int kTimeoutSeconds = 2; + int kNumSvInfoLists = 4; + std::list> sv_info_lists; + hidl_vec last_sv_info_list; + do { + EXPECT_GT(gnss_cb_->sv_info_list_cbq_.retrieve(sv_info_lists, kNumSvInfoLists, + kTimeoutSeconds), + 0); + last_sv_info_list = sv_info_lists.back(); + } while (last_sv_info_list.size() == 0); + + ALOGD("last_sv_info size = %d", (int)last_sv_info_list.size()); bool nonZeroCn0Found = false; for (auto sv_info : last_sv_info_list) { - ASSERT_TRUE(sv_info.basebandCN0DbHz >= 0.0 && sv_info.basebandCN0DbHz <= 65.0); + EXPECT_TRUE(sv_info.basebandCN0DbHz >= 0.0 && sv_info.basebandCN0DbHz <= 65.0); if (sv_info.basebandCN0DbHz > 0.0) { nonZeroCn0Found = true; } } // Assert at least one value is non-zero. Zero is ok in status as it's possibly // reporting a searched but not found satellite. - ASSERT_TRUE(nonZeroCn0Found); + EXPECT_TRUE(nonZeroCn0Found); StopAndClearLocations(); } From e4ce86bfb7bd81eea39da5d76dbf796e2c1dbe1d Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Mon, 19 Jul 2021 22:58:02 +0000 Subject: [PATCH 45/61] Audio VTS: Make the active microphone query test more robust in V7 Prior to V7 the test which exercises IStreamIn.getActiveMicrophones was using a hardcoded configuration for the input stream. This configuration no longer works for some of new devices. To fix that, the part of the test which calls getActiveMicrophones has been moved into a separate test--a descendant of InputStreamTest which is parametrized using the actual configuration of the DUT. Tests for HAL versions prior to V7 are not affected because they don't use a full parser for the DUT config. Bug: 193849687 Test: atest VtsHalAudioV7_0TargetTest Change-Id: I00fe8fedb6bfc6e034387b35c88f954cb2638dfa --- .../4.0/AudioPrimaryHidlHalTest.cpp | 23 ++----- .../7.0/AudioPrimaryHidlHalTest.cpp | 61 +++++++++++++++++++ 2 files changed, 67 insertions(+), 17 deletions(-) diff --git a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp index b96cc83673..28bcd0b713 100644 --- a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp @@ -53,6 +53,11 @@ TEST_P(AudioHidlDeviceTest, GetMicrophonesTest) { GTEST_SKIP() << "getMicrophones is not supported"; // returns } ASSERT_OK(res); + +#if MAJOR_VERSION <= 6 + // In V7, 'getActiveMicrophones' is tested by the 'MicrophoneInfoInputStream' + // test which uses the actual configuration of the device. + if (microphones.size() > 0) { // When there is microphone on the phone, try to open an input stream // and query for the active microphones. @@ -60,30 +65,13 @@ TEST_P(AudioHidlDeviceTest, GetMicrophonesTest) { "Make sure getMicrophones always succeeds" "and getActiveMicrophones always succeeds when recording from these microphones."); AudioConfig config{}; -#if MAJOR_VERSION <= 6 config.channelMask = mkEnumBitfield(AudioChannelMask::IN_MONO); config.sampleRateHz = 8000; config.format = AudioFormat::PCM_16_BIT; auto flags = hidl_bitfield(AudioInputFlag::NONE); const SinkMetadata initMetadata = {{{.source = AudioSource::MIC, .gain = 1}}}; -#elif MAJOR_VERSION >= 7 - config.base.channelMask = toString(xsd::AudioChannelMask::AUDIO_CHANNEL_IN_MONO); - config.base.sampleRateHz = 8000; - config.base.format = toString(xsd::AudioFormat::AUDIO_FORMAT_PCM_16_BIT); - hidl_vec flags; - const SinkMetadata initMetadata = { - {{.source = toString(xsd::AudioSource::AUDIO_SOURCE_MIC), - .gain = 1, - .tags = {}, - .channelMask = toString(xsd::AudioChannelMask::AUDIO_CHANNEL_IN_MONO)}}}; -#endif for (auto microphone : microphones) { -#if MAJOR_VERSION <= 6 if (microphone.deviceAddress.device != AudioDevice::IN_BUILTIN_MIC) { -#elif MAJOR_VERSION >= 7 - if (xsd::stringToAudioDevice(microphone.deviceAddress.deviceType) != - xsd::AudioDevice::AUDIO_DEVICE_IN_BUILTIN_MIC) { -#endif continue; } sp stream; @@ -106,6 +94,7 @@ TEST_P(AudioHidlDeviceTest, GetMicrophonesTest) { EXPECT_NE(0U, activeMicrophones.size()); } } +#endif // MAJOR_VERSION <= 6 } TEST_P(AudioHidlDeviceTest, SetConnectedState) { diff --git a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp index 0b3098b872..0cc6a5b964 100644 --- a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp @@ -839,3 +839,64 @@ INSTANTIATE_TEST_CASE_P(PcmOnlyConfigInputStream, PcmOnlyConfigInputStreamTest, ::testing::ValuesIn(getInputDevicePcmOnlyConfigParameters()), &DeviceConfigParameterToString); GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(PcmOnlyConfigInputStreamTest); + +static const std::vector& getBuiltinMicConfigParameters() { + static const std::vector parameters = [] { + auto allParams = getInputDeviceConfigParameters(); + std::vector builtinMicParams; + std::copy_if(allParams.begin(), allParams.end(), std::back_inserter(builtinMicParams), + [](auto cfg) { + // The built in mic may participate in various scenarios: + // FAST, HW_HOTWORD, MMAP NOIRQ, which are indicated by flags. + // We are only interested in testing the simplest scenario w/o any flags. + if (!std::get(cfg).empty()) return false; + auto maybeSourceDevice = getCachedPolicyConfig().getSourceDeviceForMixPort( + std::get(std::get(cfg)), + std::get(cfg)); + return maybeSourceDevice.has_value() && + xsd::stringToAudioDevice(maybeSourceDevice.value().deviceType) == + xsd::AudioDevice::AUDIO_DEVICE_IN_BUILTIN_MIC; + }); + return builtinMicParams; + }(); + return parameters; +} + +class MicrophoneInfoInputStreamTest : public InputStreamTest {}; + +TEST_P(MicrophoneInfoInputStreamTest, GetActiveMicrophones) { + doc::test( + "Make sure getActiveMicrophones always succeeds when recording " + "from the built-in microphone."); + hidl_vec microphones; + ASSERT_OK(getDevice()->getMicrophones(returnIn(res, microphones))); + if (res == Result::NOT_SUPPORTED) { + GTEST_SKIP() << "getMicrophones is not supported"; // returns + } + ASSERT_OK(res); + + auto maybeSourceAddress = + getCachedPolicyConfig().getSourceDeviceForMixPort(getDeviceName(), getMixPortName()); + ASSERT_TRUE(maybeSourceAddress.has_value()) + << "No source device found for mix port " << getMixPortName() << " (module " + << getDeviceName() << ")"; + + for (auto microphone : microphones) { + if (microphone.deviceAddress == maybeSourceAddress.value()) { + StreamReader reader(stream.get(), stream->getBufferSize()); + ASSERT_TRUE(reader.start()); + reader.pause(); // This ensures that at least one read has happened. + EXPECT_FALSE(reader.hasError()); + + hidl_vec activeMicrophones; + ASSERT_OK(stream->getActiveMicrophones(returnIn(res, activeMicrophones))); + ASSERT_OK(res); + EXPECT_NE(0U, activeMicrophones.size()); + } + } +} + +INSTANTIATE_TEST_CASE_P(MicrophoneInfoInputStream, MicrophoneInfoInputStreamTest, + ::testing::ValuesIn(getBuiltinMicConfigParameters()), + &DeviceConfigParameterToString); +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(MicrophoneInfoInputStreamTest); From 643a79417231ed837bea71571420204992e5b37e Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Tue, 20 Jul 2021 15:44:01 -0700 Subject: [PATCH 46/61] Add VtsRemotelyProvisionedComponentTests config VtsHalRemotelyProvisionedComponentTargetTest was picking up the same config file (AndroidTest.xml) as VtsAidlKeyMintTargetTest. When atest or TF was used to run VtsHalRemotelyProvisionedComponentTargetTest, it actually ran VtsAidlKeyMintTargetTest. Add a separate test config file so that we run the correct test binary. Test: atest VtsAidlKeyMintTargetTest Test: atest VtsHalRemotelyProvisionedComponentTargetTest Fixes: 192824779 Change-Id: I7ba0f8d364690209722f9a06c6c0ce2957781beb --- .../keymint/aidl/vts/functional/Android.bp | 1 + .../VtsRemotelyProvisionedComponentTests.xml | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml diff --git a/security/keymint/aidl/vts/functional/Android.bp b/security/keymint/aidl/vts/functional/Android.bp index 386029f306..77eea8afd6 100644 --- a/security/keymint/aidl/vts/functional/Android.bp +++ b/security/keymint/aidl/vts/functional/Android.bp @@ -94,6 +94,7 @@ cc_test { "libkeymint_vts_test_utils", "libpuresoftkeymasterdevice", ], + test_config: "VtsRemotelyProvisionedComponentTests.xml", test_suites: [ "general-tests", "vts", diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml new file mode 100644 index 0000000000..2375bde0ff --- /dev/null +++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.xml @@ -0,0 +1,34 @@ + + + + From 703ac9601ceacdaa7325c59c28586e3d6ff69534 Mon Sep 17 00:00:00 2001 From: Yu-Han Yang Date: Tue, 20 Jul 2021 16:47:37 -0700 Subject: [PATCH 47/61] Report GnssStatusValue when GNSS start/stop Also add carrier frequencies to the mock GnssStatus and GnssAntennaInfo Bug: 194117673 Test: atest GnssStatusTest Change-Id: Ib22aefe8e4dc8389a933e9009e36872287546c7b --- gnss/1.1/default/Gnss.cpp | 28 ++++++++++------ gnss/common/utils/default/Utils.cpp | 32 ++++++++++--------- gnss/common/utils/default/include/Constants.h | 4 +++ gnss/common/utils/default/include/Utils.h | 3 +- .../utils/default/include/v2_1/GnssTemplate.h | 17 ++++++++++ 5 files changed, 58 insertions(+), 26 deletions(-) diff --git a/gnss/1.1/default/Gnss.cpp b/gnss/1.1/default/Gnss.cpp index 5043649b2d..0d77ce4273 100644 --- a/gnss/1.1/default/Gnss.cpp +++ b/gnss/1.1/default/Gnss.cpp @@ -1,9 +1,9 @@ #define LOG_TAG "Gnss" +#include "Gnss.h" #include #include - -#include "Gnss.h" +#include "Constants.h" #include "GnssDebug.h" #include "GnssMeasurement.h" #include "Utils.h" @@ -16,6 +16,7 @@ namespace implementation { using ::android::hardware::gnss::common::Utils; using GnssSvFlags = IGnssCallback::GnssSvFlags; +using namespace ::android::hardware::gnss::common; const uint32_t MIN_INTERVAL_MILLIS = 100; sp<::android::hardware::gnss::V1_1::IGnssCallback> Gnss::sGnssCallback = nullptr; @@ -197,14 +198,21 @@ Return Gnss::injectBestLocation(const GnssLocation&) { Return Gnss::getMockSvStatus() const { std::unique_lock lock(mGnssConfiguration->getMutex()); GnssSvInfo mockGnssSvInfoList[] = { - Utils::getMockSvInfoV1_0(3, GnssConstellationType::GPS, 32.5, 59.1, 166.5), - Utils::getMockSvInfoV1_0(5, GnssConstellationType::GPS, 27.0, 29.0, 56.5), - Utils::getMockSvInfoV1_0(17, GnssConstellationType::GPS, 30.5, 71.0, 77.0), - Utils::getMockSvInfoV1_0(26, GnssConstellationType::GPS, 24.1, 28.0, 253.0), - Utils::getMockSvInfoV1_0(5, GnssConstellationType::GLONASS, 20.5, 11.5, 116.0), - Utils::getMockSvInfoV1_0(17, GnssConstellationType::GLONASS, 21.5, 28.5, 186.0), - Utils::getMockSvInfoV1_0(18, GnssConstellationType::GLONASS, 28.3, 38.8, 69.0), - Utils::getMockSvInfoV1_0(10, GnssConstellationType::GLONASS, 25.0, 66.0, 247.0)}; + Utils::getMockSvInfoV1_0(3, GnssConstellationType::GPS, 32.5, 59.1, 166.5, + kGpsL1FreqHz), + Utils::getMockSvInfoV1_0(5, GnssConstellationType::GPS, 27.0, 29.0, 56.5, kGpsL1FreqHz), + Utils::getMockSvInfoV1_0(17, GnssConstellationType::GPS, 30.5, 71.0, 77.0, + kGpsL5FreqHz), + Utils::getMockSvInfoV1_0(26, GnssConstellationType::GPS, 24.1, 28.0, 253.0, + kGpsL5FreqHz), + Utils::getMockSvInfoV1_0(5, GnssConstellationType::GLONASS, 20.5, 11.5, 116.0, + kGloG1FreqHz), + Utils::getMockSvInfoV1_0(17, GnssConstellationType::GLONASS, 21.5, 28.5, 186.0, + kGloG1FreqHz), + Utils::getMockSvInfoV1_0(18, GnssConstellationType::GLONASS, 28.3, 38.8, 69.0, + kGloG1FreqHz), + Utils::getMockSvInfoV1_0(10, GnssConstellationType::GLONASS, 25.0, 66.0, 247.0, + kGloG1FreqHz)}; GnssSvStatus svStatus = {.numSvs = sizeof(mockGnssSvInfoList) / sizeof(GnssSvInfo)}; for (uint32_t i = 0; i < svStatus.numSvs; i++) { diff --git a/gnss/common/utils/default/Utils.cpp b/gnss/common/utils/default/Utils.cpp index 569dac4c59..d136448ed9 100644 --- a/gnss/common/utils/default/Utils.cpp +++ b/gnss/common/utils/default/Utils.cpp @@ -265,50 +265,50 @@ V1_0::GnssLocation Utils::getMockLocationV1_0() { } hidl_vec Utils::getMockSvInfoListV2_1() { - GnssSvInfoV1_0 gnssSvInfoV1_0 = - Utils::getMockSvInfoV1_0(3, V1_0::GnssConstellationType::GPS, 32.5, 59.1, 166.5); + GnssSvInfoV1_0 gnssSvInfoV1_0 = Utils::getMockSvInfoV1_0(3, V1_0::GnssConstellationType::GPS, + 32.5, 59.1, 166.5, kGpsL1FreqHz); GnssSvInfoV2_0 gnssSvInfoV2_0 = Utils::getMockSvInfoV2_0(gnssSvInfoV1_0, V2_0::GnssConstellationType::GPS); hidl_vec gnssSvInfoList = { Utils::getMockSvInfoV2_1(gnssSvInfoV2_0, 27.5), getMockSvInfoV2_1( getMockSvInfoV2_0(getMockSvInfoV1_0(5, V1_0::GnssConstellationType::GPS, 27.0, - 29.0, 56.5), + 29.0, 56.5, kGpsL1FreqHz), V2_0::GnssConstellationType::GPS), 22.0), getMockSvInfoV2_1( getMockSvInfoV2_0(getMockSvInfoV1_0(17, V1_0::GnssConstellationType::GPS, 30.5, - 71.0, 77.0), + 71.0, 77.0, kGpsL5FreqHz), V2_0::GnssConstellationType::GPS), 25.5), getMockSvInfoV2_1( getMockSvInfoV2_0(getMockSvInfoV1_0(26, V1_0::GnssConstellationType::GPS, 24.1, - 28.0, 253.0), + 28.0, 253.0, kGpsL5FreqHz), V2_0::GnssConstellationType::GPS), 19.1), getMockSvInfoV2_1( getMockSvInfoV2_0(getMockSvInfoV1_0(5, V1_0::GnssConstellationType::GLONASS, - 20.5, 11.5, 116.0), + 20.5, 11.5, 116.0, kGloG1FreqHz), V2_0::GnssConstellationType::GLONASS), 15.5), getMockSvInfoV2_1( getMockSvInfoV2_0(getMockSvInfoV1_0(17, V1_0::GnssConstellationType::GLONASS, - 21.5, 28.5, 186.0), + 21.5, 28.5, 186.0, kGloG1FreqHz), V2_0::GnssConstellationType::GLONASS), 16.5), getMockSvInfoV2_1( getMockSvInfoV2_0(getMockSvInfoV1_0(18, V1_0::GnssConstellationType::GLONASS, - 28.3, 38.8, 69.0), + 28.3, 38.8, 69.0, kGloG1FreqHz), V2_0::GnssConstellationType::GLONASS), 25.3), getMockSvInfoV2_1( getMockSvInfoV2_0(getMockSvInfoV1_0(10, V1_0::GnssConstellationType::GLONASS, - 25.0, 66.0, 247.0), + 25.0, 66.0, 247.0, kGloG1FreqHz), V2_0::GnssConstellationType::GLONASS), 20.0), getMockSvInfoV2_1( getMockSvInfoV2_0(getMockSvInfoV1_0(3, V1_0::GnssConstellationType::UNKNOWN, - 22.0, 35.0, 112.0), + 22.0, 35.0, 112.0, kIrnssL5FreqHz), V2_0::GnssConstellationType::IRNSS), 19.7), }; @@ -333,21 +333,23 @@ GnssSvInfoV2_0 Utils::getMockSvInfoV2_0(GnssSvInfoV1_0 gnssSvInfoV1_0, } GnssSvInfoV1_0 Utils::getMockSvInfoV1_0(int16_t svid, V1_0::GnssConstellationType type, - float cN0DbHz, float elevationDegrees, - float azimuthDegrees) { + float cN0DbHz, float elevationDegrees, float azimuthDegrees, + float carrierFrequencyHz) { GnssSvInfoV1_0 svInfo = {.svid = svid, .constellation = type, .cN0Dbhz = cN0DbHz, .elevationDegrees = elevationDegrees, .azimuthDegrees = azimuthDegrees, + .carrierFrequencyHz = carrierFrequencyHz, .svFlag = GnssSvFlags::USED_IN_FIX | GnssSvFlags::HAS_EPHEMERIS_DATA | - GnssSvFlags::HAS_ALMANAC_DATA}; + GnssSvFlags::HAS_ALMANAC_DATA | + GnssSvFlags::HAS_CARRIER_FREQUENCY}; return svInfo; } hidl_vec Utils::getMockAntennaInfos() { GnssAntennaInfo mockAntennaInfo_1 = { - .carrierFrequencyMHz = 123412.12, + .carrierFrequencyMHz = kGpsL1FreqHz * 1e-6, .phaseCenterOffsetCoordinateMillimeters = Coord{.x = 1, .xUncertainty = 0.1, .y = 2, @@ -381,7 +383,7 @@ hidl_vec Utils::getMockAntennaInfos() { }; GnssAntennaInfo mockAntennaInfo_2 = { - .carrierFrequencyMHz = 532324.23, + .carrierFrequencyMHz = kGpsL5FreqHz * 1e-6, .phaseCenterOffsetCoordinateMillimeters = Coord{.x = 5, .xUncertainty = 0.1, .y = 6, diff --git a/gnss/common/utils/default/include/Constants.h b/gnss/common/utils/default/include/Constants.h index a290ed243f..22afee1ad1 100644 --- a/gnss/common/utils/default/include/Constants.h +++ b/gnss/common/utils/default/include/Constants.h @@ -29,6 +29,10 @@ const float kMockVerticalAccuracyMeters = 5; const float kMockSpeedAccuracyMetersPerSecond = 1; const float kMockBearingAccuracyDegrees = 90; const int64_t kMockTimestamp = 1519930775453L; +const float kGpsL1FreqHz = 1575.42 * 1e6; +const float kGpsL5FreqHz = 1176.45 * 1e6; +const float kGloG1FreqHz = 1602.0 * 1e6; +const float kIrnssL5FreqHz = 1176.45 * 1e6; } // namespace common } // namespace gnss diff --git a/gnss/common/utils/default/include/Utils.h b/gnss/common/utils/default/include/Utils.h index 771d39dbd1..43772ce11a 100644 --- a/gnss/common/utils/default/include/Utils.h +++ b/gnss/common/utils/default/include/Utils.h @@ -44,7 +44,8 @@ struct Utils { static V1_0::IGnssCallback::GnssSvInfo getMockSvInfoV1_0(int16_t svid, V1_0::GnssConstellationType type, float cN0DbHz, float elevationDegrees, - float azimuthDegrees); + float azimuthDegrees, + float carrierFrequencyHz); static hidl_vec getMockAntennaInfos(); }; diff --git a/gnss/common/utils/default/include/v2_1/GnssTemplate.h b/gnss/common/utils/default/include/v2_1/GnssTemplate.h index a1d698167c..131af24fbe 100644 --- a/gnss/common/utils/default/include/v2_1/GnssTemplate.h +++ b/gnss/common/utils/default/include/v2_1/GnssTemplate.h @@ -113,6 +113,7 @@ struct GnssTemplate : public T_IGnss { void reportLocation(const V2_0::GnssLocation&) const; void reportLocation(const V1_0::GnssLocation&) const; void reportSvStatus(const hidl_vec&) const; + void reportGnssStatusValue(const V1_0::IGnssCallback::GnssStatusValue) const; Return help(const hidl_handle& fd); Return setLocation(const hidl_handle& fd, const hidl_vec& options); @@ -215,6 +216,7 @@ Return GnssTemplate::start() { } mIsActive = true; + this->reportGnssStatusValue(V1_0::IGnssCallback::GnssStatusValue::SESSION_BEGIN); mThread = std::thread([this]() { while (mIsActive == true) { auto svStatus = filterBlocklistedSatellitesV2_1(Utils::getMockSvInfoListV2_1()); @@ -266,6 +268,7 @@ template Return GnssTemplate::stop() { ALOGD("stop"); mIsActive = false; + this->reportGnssStatusValue(V1_0::IGnssCallback::GnssStatusValue::SESSION_END); if (mThread.joinable()) { mThread.join(); } @@ -605,6 +608,20 @@ Return> GnssTemplate::getExtensionGnssAntenn return new V2_1::implementation::GnssAntennaInfo(); } +template +void GnssTemplate::reportGnssStatusValue( + const V1_0::IGnssCallback::GnssStatusValue gnssStatusValue) const { + std::unique_lock lock(mMutex); + if (sGnssCallback_2_1 == nullptr) { + ALOGE("%s: sGnssCallback v2.1 is null.", __func__); + return; + } + auto ret = sGnssCallback_2_1->gnssStatusCb(gnssStatusValue); + if (!ret.isOk()) { + ALOGE("%s: Unable to invoke callback", __func__); + } +} + template void GnssTemplate::reportSvStatus( const hidl_vec& svInfoList) const { From db3c830bc8be77b7cdf6a2fea17ecd31b2ffa632 Mon Sep 17 00:00:00 2001 From: Aaron Tsai Date: Tue, 13 Jul 2021 12:14:48 +0800 Subject: [PATCH 48/61] Waiting 10s at the beginning of getBarringInfo test if not yet in-service. If the previous setRadioPower_1_5_emergencyCall_cancelled test has just finished. Due to radio restarting, modem may need a little more time to acquire network service. Otherwise, the barring info will be empty. Bug: 191866257 Test: atest VtsHalRadioV1_5TargetTest Change-Id: I9ae4e7a07b9f47353554ffb63a23b6518aa344b7 --- radio/1.5/vts/functional/radio_hidl_hal_api.cpp | 12 ++++++++++++ radio/1.5/vts/functional/radio_hidl_hal_utils_v1_5.h | 3 +++ radio/1.5/vts/functional/radio_response.cpp | 6 ++++-- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/radio/1.5/vts/functional/radio_hidl_hal_api.cpp b/radio/1.5/vts/functional/radio_hidl_hal_api.cpp index 0b49b36927..d108951c0e 100644 --- a/radio/1.5/vts/functional/radio_hidl_hal_api.cpp +++ b/radio/1.5/vts/functional/radio_hidl_hal_api.cpp @@ -1251,8 +1251,20 @@ TEST_P(RadioHidlTest_v1_5, sendCdmaSmsExpectMore) { * Test IRadio.getBarringInfo() for the response returned. */ TEST_P(RadioHidlTest_v1_5, getBarringInfo) { + // If the previous setRadioPower_1_5_emergencyCall_cancelled test has just finished. + // Due to radio restarting, modem may need a little more time to acquire network service + // and barring infos. If voice status is in-service, waiting 3s to get barring infos ready. + // Or waiting 10s if voice status is not in-service. serial = GetRandomSerialNumber(); + radio_v1_5->getVoiceRegistrationState_1_5(serial); + EXPECT_EQ(std::cv_status::no_timeout, wait()); + if (isVoiceInService(radioRsp_v1_5->voiceRegResp.regState)) { + sleep(BARRING_INFO_MAX_WAIT_TIME_SECONDS); + } else { + sleep(VOICE_SERVICE_MAX_WAIT_TIME_SECONDS); + } + serial = GetRandomSerialNumber(); Return res = radio_v1_5->getBarringInfo(serial); EXPECT_EQ(std::cv_status::no_timeout, wait()); EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_v1_5->rspInfo.type); diff --git a/radio/1.5/vts/functional/radio_hidl_hal_utils_v1_5.h b/radio/1.5/vts/functional/radio_hidl_hal_utils_v1_5.h index 87ce675c5c..65442caff1 100644 --- a/radio/1.5/vts/functional/radio_hidl_hal_utils_v1_5.h +++ b/radio/1.5/vts/functional/radio_hidl_hal_utils_v1_5.h @@ -51,6 +51,8 @@ using ::android::hardware::Void; #define TIMEOUT_PERIOD 75 #define MODEM_EMERGENCY_CALL_ESTABLISH_TIME 3 #define MODEM_EMERGENCY_CALL_DISCONNECT_TIME 3 +#define VOICE_SERVICE_MAX_WAIT_TIME_SECONDS 10 +#define BARRING_INFO_MAX_WAIT_TIME_SECONDS 3 #define RADIO_SERVICE_NAME "slot1" @@ -69,6 +71,7 @@ class RadioResponse_v1_5 : public ::android::hardware::radio::V1_5::IRadioRespon // Call hidl_vec<::android::hardware::radio::V1_2::Call> currentCalls; + ::android::hardware::radio::V1_2::VoiceRegStateResult voiceRegResp; // Modem bool isModemEnabled; diff --git a/radio/1.5/vts/functional/radio_response.cpp b/radio/1.5/vts/functional/radio_response.cpp index 9b6d450e83..3d6fc17f5b 100644 --- a/radio/1.5/vts/functional/radio_response.cpp +++ b/radio/1.5/vts/functional/radio_response.cpp @@ -763,8 +763,9 @@ Return RadioResponse_v1_5::getCellInfoListResponse_1_2( Return RadioResponse_v1_5::getVoiceRegistrationStateResponse_1_2( const RadioResponseInfo& info, - const ::android::hardware::radio::V1_2::VoiceRegStateResult& /*voiceRegResponse*/) { + const ::android::hardware::radio::V1_2::VoiceRegStateResult& voiceRegResponse) { rspInfo = info; + voiceRegResp = voiceRegResponse; parent_v1_5.notify(info.serial); return Void(); } @@ -989,8 +990,9 @@ Return RadioResponse_v1_5::getBarringInfoResponse( Return RadioResponse_v1_5::getVoiceRegistrationStateResponse_1_5( const RadioResponseInfo& info, - const ::android::hardware::radio::V1_5::RegStateResult& /*regResponse*/) { + const ::android::hardware::radio::V1_5::RegStateResult& regResponse) { rspInfo = info; + voiceRegResp.regState = regResponse.regState; parent_v1_5.notify(info.serial); return Void(); } From 61a6d8d72effaedf8ed6bc5461e36859faac4f4f Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Wed, 21 Jul 2021 13:22:45 -0700 Subject: [PATCH 49/61] IFace: fix VTS test to match the interface contract Bug: 193849101 Test: atest VtsHalBiometricsFaceTargetTest Change-Id: I5b3ed1b87244d6265d23c15ef2c2efe4f6155973 --- .../face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp b/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp index 4dc44f16c7..8906b8d935 100644 --- a/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp +++ b/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp @@ -60,9 +60,10 @@ class SessionCallback : public BnSessionCallback { return ndk::ScopedAStatus::ok(); } - ndk::ScopedAStatus onError(Error error, int32_t /*vendorCode*/) override { + ndk::ScopedAStatus onError(Error error, int32_t vendorCode) override { auto lock = std::lock_guard{mMutex}; mError = error; + mVendorCode = vendorCode; mOnErrorInvoked = true; mCv.notify_one(); return ndk::ScopedAStatus::ok(); @@ -141,6 +142,7 @@ class SessionCallback : public BnSessionCallback { std::mutex mMutex; std::condition_variable mCv; Error mError = Error::UNKNOWN; + int32_t mVendorCode = 0; int64_t mGeneratedChallenge = 0; int64_t mRevokedChallenge = 0; bool mOnChallengeGeneratedInvoked = false; @@ -218,6 +220,8 @@ TEST_P(Face, EnrollWithBadHatResultsInErrorTest) { // Make sure an error is returned. auto lock = std::unique_lock{mCb->mMutex}; mCb->mCv.wait(lock, [this] { return mCb->mOnErrorInvoked; }); + EXPECT_EQ(mCb->mError, Error::UNABLE_TO_PROCESS); + EXPECT_EQ(mCb->mVendorCode, 0); } TEST_P(Face, GenerateChallengeProducesUniqueChallengesTest) { @@ -287,13 +291,15 @@ TEST_P(Face, RemoveEnrollmentsWorksTest) { mCb->mCv.wait(lock, [this] { return mCb->mOnEnrollmentsRemovedInvoked; }); } -TEST_P(Face, GetFeaturesWorksTest) { +TEST_P(Face, GetFeaturesWithoutEnrollmentsResultsInUnableToProcess) { // Call the method. ASSERT_TRUE(mSession->getFeatures().isOk()); // Wait for the result. auto lock = std::unique_lock{mCb->mMutex}; - mCb->mCv.wait(lock, [this] { return mCb->mOnFeaturesRetrievedInvoked; }); + mCb->mCv.wait(lock, [this] { return mCb->mOnErrorInvoked; }); + EXPECT_EQ(mCb->mError, Error::UNABLE_TO_PROCESS); + EXPECT_EQ(mCb->mVendorCode, 0); } TEST_P(Face, GetAuthenticatorIdWorksTest) { From 9208a095686a2b39e6ef3f6a664719ed933563f5 Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Wed, 21 Jul 2021 23:13:12 +0000 Subject: [PATCH 50/61] IFace: fix default implementation to pass VTS Bug: 193849101 Test: atest VtsHalBiometricsFaceTargetTest Change-Id: Ia90c639074a6a9ed270f7c499c8816a8c7224b7f --- biometrics/face/aidl/default/Session.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/biometrics/face/aidl/default/Session.cpp b/biometrics/face/aidl/default/Session.cpp index 0cb7c95de8..39f37538f7 100644 --- a/biometrics/face/aidl/default/Session.cpp +++ b/biometrics/face/aidl/default/Session.cpp @@ -107,7 +107,8 @@ ndk::ScopedAStatus Session::removeEnrollments(const std::vector& /*enro ndk::ScopedAStatus Session::getFeatures() { LOG(INFO) << "getFeatures"; if (cb_) { - cb_->onFeaturesRetrieved({}); + // Must error out with UNABLE_TO_PROCESS when no faces are enrolled. + cb_->onError(Error::UNABLE_TO_PROCESS, 0 /* vendorCode */); } return ndk::ScopedAStatus::ok(); } From 5beafca660e8ec4cf685f8acf20c84fb2a9f6c43 Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Wed, 21 Jul 2021 18:57:36 -0700 Subject: [PATCH 51/61] IFace: annotate the previewSurface as @nullable in enroll Bug: 194346408 Test: android.hardware.biometrics.face-update-api Change-Id: Id8809b27f121a738a41abeee66f5c1fd3840cc44 --- .../face/aidl/aidl_api/android.hardware.biometrics.face/1/.hash | 2 +- .../1/android/hardware/biometrics/face/ISession.aidl | 2 +- .../current/android/hardware/biometrics/face/ISession.aidl | 2 +- .../face/aidl/android/hardware/biometrics/face/ISession.aidl | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/1/.hash b/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/1/.hash index b8d5097cc9..f5ad87fca7 100644 --- a/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/1/.hash +++ b/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/1/.hash @@ -1 +1 @@ -945de3635b7f5a09244820eef56035c92fdbd324 +3b10f5094c5af9fe551093597fab007d1e148256 diff --git a/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/1/android/hardware/biometrics/face/ISession.aidl b/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/1/android/hardware/biometrics/face/ISession.aidl index d1c2c1dd47..78178642cd 100644 --- a/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/1/android/hardware/biometrics/face/ISession.aidl +++ b/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/1/android/hardware/biometrics/face/ISession.aidl @@ -37,7 +37,7 @@ interface ISession { void generateChallenge(); void revokeChallenge(in long challenge); android.hardware.biometrics.face.EnrollmentStageConfig[] getEnrollmentConfig(in android.hardware.biometrics.face.EnrollmentType enrollmentType); - android.hardware.biometrics.common.ICancellationSignal enroll(in android.hardware.keymaster.HardwareAuthToken hat, in android.hardware.biometrics.face.EnrollmentType type, in android.hardware.biometrics.face.Feature[] features, in android.hardware.common.NativeHandle previewSurface); + android.hardware.biometrics.common.ICancellationSignal enroll(in android.hardware.keymaster.HardwareAuthToken hat, in android.hardware.biometrics.face.EnrollmentType type, in android.hardware.biometrics.face.Feature[] features, in @nullable android.hardware.common.NativeHandle previewSurface); android.hardware.biometrics.common.ICancellationSignal authenticate(in long operationId); android.hardware.biometrics.common.ICancellationSignal detectInteraction(); void enumerateEnrollments(); diff --git a/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/current/android/hardware/biometrics/face/ISession.aidl b/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/current/android/hardware/biometrics/face/ISession.aidl index d1c2c1dd47..78178642cd 100644 --- a/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/current/android/hardware/biometrics/face/ISession.aidl +++ b/biometrics/face/aidl/aidl_api/android.hardware.biometrics.face/current/android/hardware/biometrics/face/ISession.aidl @@ -37,7 +37,7 @@ interface ISession { void generateChallenge(); void revokeChallenge(in long challenge); android.hardware.biometrics.face.EnrollmentStageConfig[] getEnrollmentConfig(in android.hardware.biometrics.face.EnrollmentType enrollmentType); - android.hardware.biometrics.common.ICancellationSignal enroll(in android.hardware.keymaster.HardwareAuthToken hat, in android.hardware.biometrics.face.EnrollmentType type, in android.hardware.biometrics.face.Feature[] features, in android.hardware.common.NativeHandle previewSurface); + android.hardware.biometrics.common.ICancellationSignal enroll(in android.hardware.keymaster.HardwareAuthToken hat, in android.hardware.biometrics.face.EnrollmentType type, in android.hardware.biometrics.face.Feature[] features, in @nullable android.hardware.common.NativeHandle previewSurface); android.hardware.biometrics.common.ICancellationSignal authenticate(in long operationId); android.hardware.biometrics.common.ICancellationSignal detectInteraction(); void enumerateEnrollments(); diff --git a/biometrics/face/aidl/android/hardware/biometrics/face/ISession.aidl b/biometrics/face/aidl/android/hardware/biometrics/face/ISession.aidl index 2a57e3aa46..5f06b408e8 100644 --- a/biometrics/face/aidl/android/hardware/biometrics/face/ISession.aidl +++ b/biometrics/face/aidl/android/hardware/biometrics/face/ISession.aidl @@ -154,7 +154,7 @@ interface ISession { * operation. */ ICancellationSignal enroll(in HardwareAuthToken hat, in EnrollmentType type, - in Feature[] features, in NativeHandle previewSurface); + in Feature[] features, in @nullable NativeHandle previewSurface); /** * authenticate: From bde61ca1d32ddde39989f0bcd944a51349bbcdad Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Wed, 21 Jul 2021 19:01:07 -0700 Subject: [PATCH 52/61] IFace: update default implementation to use optional previewSurface Bug: 194346408 Test: atest VtsHalBiometricsFaceTargetTest Change-Id: I72d3d6d638a1662ebd1a53a7029ea3bf200efe48 --- biometrics/face/aidl/default/Session.cpp | 3 ++- biometrics/face/aidl/default/Session.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/biometrics/face/aidl/default/Session.cpp b/biometrics/face/aidl/default/Session.cpp index 0cb7c95de8..bf5202850a 100644 --- a/biometrics/face/aidl/default/Session.cpp +++ b/biometrics/face/aidl/default/Session.cpp @@ -63,7 +63,8 @@ ndk::ScopedAStatus Session::getEnrollmentConfig(EnrollmentType /*enrollmentType* ndk::ScopedAStatus Session::enroll( const keymaster::HardwareAuthToken& /*hat*/, EnrollmentType /*enrollmentType*/, - const std::vector& /*features*/, const NativeHandle& /*previewSurface*/, + const std::vector& /*features*/, + const std::optional& /*previewSurface*/, std::shared_ptr* /*return_val*/) { LOG(INFO) << "enroll"; if (cb_) { diff --git a/biometrics/face/aidl/default/Session.h b/biometrics/face/aidl/default/Session.h index 4d213e3860..4152909a49 100644 --- a/biometrics/face/aidl/default/Session.h +++ b/biometrics/face/aidl/default/Session.h @@ -41,7 +41,7 @@ class Session : public BnSession { ndk::ScopedAStatus enroll(const keymaster::HardwareAuthToken& hat, EnrollmentType enrollmentType, const std::vector& features, - const NativeHandle& previewSurface, + const std::optional& previewSurface, std::shared_ptr* return_val) override; ndk::ScopedAStatus authenticate( From 88796f4992dc308102be052c9ab980ac51ed43ee Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Wed, 21 Jul 2021 22:54:40 -0700 Subject: [PATCH 53/61] IFace: update VTS test to pass std::nullopt for previewSurface Bug: 194346408 Test: atest VtsHalBiometricsFaceTargetTest Change-Id: I1e945d821d4b91b9a9ea8f74ae6f817ef04a0f85 --- biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp b/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp index 4dc44f16c7..f3eac8f460 100644 --- a/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp +++ b/biometrics/face/aidl/vts/VtsHalBiometricsFaceTargetTest.cpp @@ -212,7 +212,7 @@ TEST_P(Face, EnrollWithBadHatResultsInErrorTest) { auto hat = keymaster::HardwareAuthToken{}; std::shared_ptr cancellationSignal; ASSERT_TRUE( - mSession->enroll(hat, EnrollmentType::DEFAULT, {}, NativeHandle{}, &cancellationSignal) + mSession->enroll(hat, EnrollmentType::DEFAULT, {}, std::nullopt, &cancellationSignal) .isOk()); // Make sure an error is returned. From ac11a2e82480ffef7122f5d30d5e2901fb0955eb Mon Sep 17 00:00:00 2001 From: Ilya Matyukhin Date: Mon, 12 Jul 2021 18:25:11 -0700 Subject: [PATCH 54/61] Update comments for IBiometricsFingerprint@2.3 This CL adds a comment that discourages use of this interface. The AIDL interface, IFingerprint, should be used instead. Bug: 160189286 Test: hidl-gen -L check android.hardware.biometrics.fingerprint@2.3::IBiometricsFingerprint Change-Id: Ifafb191ac16e03e60d677fce6dc9e41e87bad0c0 --- biometrics/fingerprint/2.3/IBiometricsFingerprint.hal | 4 ++++ current.txt | 1 + 2 files changed, 5 insertions(+) diff --git a/biometrics/fingerprint/2.3/IBiometricsFingerprint.hal b/biometrics/fingerprint/2.3/IBiometricsFingerprint.hal index 13f03c5eea..378b564537 100644 --- a/biometrics/fingerprint/2.3/IBiometricsFingerprint.hal +++ b/biometrics/fingerprint/2.3/IBiometricsFingerprint.hal @@ -19,6 +19,10 @@ package android.hardware.biometrics.fingerprint@2.3; import @2.2::IBiometricsFingerprint; /** + * New use of this interface is strongly discouraged. The recommended option is + * to use the AIDL interface, android.hardware.biometrics.fingerprint + * (IFingerprint). + * * The interface for biometric fingerprint authentication. */ interface IBiometricsFingerprint extends @2.2::IBiometricsFingerprint { diff --git a/current.txt b/current.txt index fbad3dabf1..8c631d9e70 100644 --- a/current.txt +++ b/current.txt @@ -833,6 +833,7 @@ e3865e74cb1a6e6afd38c7aa84115cb109ce47b972132de5242bc3838d2771f6 android.hardwar b3caf524c46a47d67e6453a34419e1881942d059e146cda740502670e9a752c3 android.hardware.automotive.vehicle@2.0::IVehicle 7ce8728b27600e840cacf0a832f6942819fe535f9d3797ae052d5eef5065921c android.hardware.automotive.vehicle@2.0::IVehicleCallback b525e91d886379c13588f4975bb04d625d46e1f41b4453792c4b2db1e7ff4340 android.hardware.biometrics.fingerprint@2.3::IBiometricsFingerprint +7a78e9963bec0b071e7d46928c6100e2174270892d3f15a1eaad074997adf279 android.hardware.biometrics.fingerprint@2.3::IBiometricsFingerprint # Added for b/160189286 for Android S 4baf8e0eca4aa896cc9ceb7bb676aaf4fa21372ef8b49eed68eced1221c3dc0d android.hardware.bluetooth.audio@2.1::IBluetoothAudioProvider d417a9212c8f96e3a06a2f221c8c5756c765355b2b81de2b2a65d4c9eee85401 android.hardware.bluetooth.audio@2.1::IBluetoothAudioProvidersFactory c17d9e27abd37ae5a8ff8da08fc5c9b13a264670feef6bbbc9d3ab1915216130 android.hardware.bluetooth.audio@2.1::types From 97d1039dfd16414fd7f12868379e772b087b43ed Mon Sep 17 00:00:00 2001 From: garysungang Date: Wed, 7 Jul 2021 22:30:35 -0700 Subject: [PATCH 55/61] Update EVS VTS test case Update test case - CameraStreamExternalBuffering Use native resolution instead of fixed test buffer 640x320 Bug: 190127973 Test: Manually run VTS on seahawk Change-Id: I11043af4215fb9c5a2658591e9bdf9e468542a1b --- .../functional/VtsHalEvsV1_1TargetTest.cpp | 115 +++++++++++------- 1 file changed, 70 insertions(+), 45 deletions(-) diff --git a/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp b/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp index a3dc45bb5b..8cc18822f2 100644 --- a/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp +++ b/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp @@ -2269,48 +2269,74 @@ TEST_P(EvsHidlTest, CameraStreamExternalBuffering) { // Acquire the graphics buffer allocator android::GraphicBufferAllocator& alloc(android::GraphicBufferAllocator::get()); - const auto usage = GRALLOC_USAGE_HW_TEXTURE | - GRALLOC_USAGE_SW_READ_RARELY | - GRALLOC_USAGE_SW_WRITE_OFTEN; + const auto usage = + GRALLOC_USAGE_HW_TEXTURE | GRALLOC_USAGE_SW_READ_RARELY | GRALLOC_USAGE_SW_WRITE_OFTEN; const auto format = HAL_PIXEL_FORMAT_RGBA_8888; - const auto width = 640; - const auto height = 360; - - // Allocate buffers to use - hidl_vec buffers; - buffers.resize(kBuffersToHold); - for (auto i = 0; i < kBuffersToHold; ++i) { - unsigned pixelsPerLine; - buffer_handle_t memHandle = nullptr; - android::status_t result = alloc.allocate(width, - height, - format, - 1, - usage, - &memHandle, - &pixelsPerLine, - 0, - "EvsApp"); - if (result != android::NO_ERROR) { - LOG(ERROR) << __FUNCTION__ << " failed to allocate memory."; - } else { - BufferDesc buf; - AHardwareBuffer_Desc* pDesc = - reinterpret_cast(&buf.buffer.description); - pDesc->width = width; - pDesc->height = height; - pDesc->layers = 1; - pDesc->format = format; - pDesc->usage = usage; - pDesc->stride = pixelsPerLine; - buf.buffer.nativeHandle = memHandle; - buf.bufferId = i; // Unique number to identify this buffer - buffers[i] = buf; - } - } + uint32_t width = 640; + uint32_t height = 360; + camera_metadata_entry_t streamCfgs; // Test each reported camera - for (auto&& cam: cameraInfo) { + for (auto&& cam : cameraInfo) { + bool foundCfg = false; + if (!find_camera_metadata_entry(reinterpret_cast(cam.metadata.data()), + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, + &streamCfgs)) { + // Stream configurations are found in metadata + RawStreamConfig* ptr = reinterpret_cast(streamCfgs.data.i32); + + LOG(DEBUG) << __LINE__ << " start searching " << streamCfgs.count; + for (unsigned idx = 0; idx < streamCfgs.count; idx++) { + LOG(DEBUG) << "ptr->direction= " << ptr->direction + << " ptr->format= " << ptr->format; + if (ptr->direction == ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT && + ptr->format == HAL_PIXEL_FORMAT_RGBA_8888) { + width = ptr->width; + height = ptr->height; + foundCfg = true; + // Always use the 1st available configuration + break; + } + ++ptr; + } + } + + if (!foundCfg) { + LOG(INFO) << "No configuration found. Use default stream configurations."; + } + + // Allocate buffers to use + hidl_vec buffers; + buffers.resize(kBuffersToHold); + for (auto i = 0; i < kBuffersToHold; ++i) { + unsigned pixelsPerLine; + buffer_handle_t memHandle = nullptr; + android::status_t result = + alloc.allocate(width, height, format, 1, usage, &memHandle, &pixelsPerLine, 0, + "CameraStreamExternalBufferingTest"); + if (result != android::NO_ERROR) { + LOG(ERROR) << __FUNCTION__ << " failed to allocate memory."; + // Release previous allocated buffers + for (auto j = 0; j < i; j++) { + alloc.free(buffers[i].buffer.nativeHandle); + } + return; + } else { + BufferDesc buf; + AHardwareBuffer_Desc* pDesc = + reinterpret_cast(&buf.buffer.description); + pDesc->width = width; + pDesc->height = height; + pDesc->layers = 1; + pDesc->format = format; + pDesc->usage = usage; + pDesc->stride = pixelsPerLine; + buf.buffer.nativeHandle = memHandle; + buf.bufferId = i; // Unique number to identify this buffer + buffers[i] = buf; + } + } + bool isLogicalCam = false; getPhysicalCameraIds(cam.v1.cameraId, isLogicalCam); @@ -2374,13 +2400,12 @@ TEST_P(EvsHidlTest, CameraStreamExternalBuffering) { // Explicitly release the camera pEnumerator->closeCamera(pCam); activeCameras.clear(); + // Release buffers + for (auto& b : buffers) { + alloc.free(b.buffer.nativeHandle); + } + buffers.resize(0); } - - // Release buffers - for (auto& b : buffers) { - alloc.free(b.buffer.nativeHandle); - } - buffers.resize(0); } From 8aee4a857ca912dc8975e8f5e50634dcd342560e Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Tue, 27 Jul 2021 14:20:17 -0700 Subject: [PATCH 56/61] Allow uninstantiated remote provisioning tests Not all devices have an IRemotelyProvisionedComponent HAL, so on those devices 0 of the tests in VtsRemotelyProvisionedComponentTests will be run. Fixes: 194770385 Test: Ran tests on two devices: one with and one without the HAL. Change-Id: I8624096158f29058189dfab7cd876804ae178e60 --- .../aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp index af951e8861..38f3586862 100644 --- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp +++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp @@ -41,6 +41,7 @@ using ::std::vector; namespace { #define INSTANTIATE_REM_PROV_AIDL_TEST(name) \ + GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(name); \ INSTANTIATE_TEST_SUITE_P( \ PerInstance, name, \ testing::ValuesIn(VtsRemotelyProvisionedComponentTests::build_params()), \ From 6ac46d6bf22fe5a1339eac85666d865b63376999 Mon Sep 17 00:00:00 2001 From: Tyler Trephan Date: Thu, 29 Jul 2021 09:46:06 -0700 Subject: [PATCH 57/61] Fixed failing CTS tests related vehicle properties on the AAOS emulator. -Added supported gears to CURRENT_GEAR config. -Changed INFO_EXTERIOR_DIMENSIONS to int32 array Test: atest CarPropertyManagerTest Bug: 194182294 Change-Id: I96d241d0c388b8fc397af9f45a8a8072a05ee8d1 --- .../2.0/default/impl/vhal_v2_0/DefaultConfig.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h index a85cdf06a5..6a0288f3a4 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h @@ -124,7 +124,7 @@ const ConfigDeclaration kVehicleProperties[]{ .access = VehiclePropertyAccess::READ, .changeMode = VehiclePropertyChangeMode::STATIC, }, - .initialValue = {.floatValues = {1776, 4950, 2008, 2140, 2984, 1665, 1667, 11800}}}, + .initialValue = {.int32Values = {1776, 4950, 2008, 2140, 2984, 1665, 1667, 11800}}}, {.config = { .prop = toInt(VehicleProperty::PERF_VEHICLE_SPEED), @@ -328,6 +328,11 @@ const ConfigDeclaration kVehicleProperties[]{ .prop = toInt(VehicleProperty::CURRENT_GEAR), .access = VehiclePropertyAccess::READ, .changeMode = VehiclePropertyChangeMode::ON_CHANGE, + .configArray = {(int)VehicleGear::GEAR_PARK, + (int)VehicleGear::GEAR_NEUTRAL, + (int)VehicleGear::GEAR_REVERSE, (int)VehicleGear::GEAR_1, + (int)VehicleGear::GEAR_2, (int)VehicleGear::GEAR_3, + (int)VehicleGear::GEAR_4, (int)VehicleGear::GEAR_5}, }, .initialValue = {.int32Values = {toInt(VehicleGear::GEAR_PARK)}}}, @@ -1072,8 +1077,8 @@ const ConfigDeclaration kVehicleProperties[]{ .access = VehiclePropertyAccess::READ, .changeMode = VehiclePropertyChangeMode::ON_CHANGE, }, - .initialValue = {.int32Values = {0 /* Off */, -1, -1, -1, -1 /* Bounds */, - -1, -1, -1, -1 /* Insets */}}, + .initialValue = {.int32Values = {0 /* Off */, -1, -1, -1, -1 /* Bounds */, -1, -1, + -1, -1 /* Insets */}}, }, { .config = @@ -1126,9 +1131,9 @@ const ConfigDeclaration kVehicleProperties[]{ .changeMode = VehiclePropertyChangeMode::ON_CHANGE, .configArray = {0, 0, 0, 11, 0, 0, 0, 0, 16}, }, - .initialValue = {.int32Values = {0 /* Off */, -1, -1, -1, -1 /* Bounds */, - -1, -1, -1, -1 /* Insets */, - 0 /* ClusterHome */, -1 /* ClusterNone */}}, + .initialValue = {.int32Values = {0 /* Off */, -1, -1, -1, -1 /* Bounds */, -1, -1, + -1, -1 /* Insets */, 0 /* ClusterHome */, + -1 /* ClusterNone */}}, }, { .config = From 53ba71df99b4c41fef7e708632df3d6acd800a83 Mon Sep 17 00:00:00 2001 From: Cheney Ni Date: Thu, 29 Jul 2021 00:50:03 +0800 Subject: [PATCH 58/61] BluetoothAudio: Refine the FMQ size for A2DP software encoding For those high-resolution codecs, they are 24 or 32 bits per sample, so the buffer size must be the LCM of 16, 24, and 32. Bug: 194980528 Test: A2DP playing with aptX and / or LDAC Change-Id: I788e185496dea4a1b40fa369e032a4f8491b835b Merged-In: I788e185496dea4a1b40fa369e032a4f8491b835b (cherry picked from commit a0cc24730eafe91021643b9b36889d61728f58f0) --- bluetooth/audio/2.0/default/A2dpSoftwareAudioProvider.cpp | 8 ++++++-- bluetooth/audio/2.1/default/A2dpSoftwareAudioProvider.cpp | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/bluetooth/audio/2.0/default/A2dpSoftwareAudioProvider.cpp b/bluetooth/audio/2.0/default/A2dpSoftwareAudioProvider.cpp index f71a73e233..0c0b85fdf3 100644 --- a/bluetooth/audio/2.0/default/A2dpSoftwareAudioProvider.cpp +++ b/bluetooth/audio/2.0/default/A2dpSoftwareAudioProvider.cpp @@ -32,10 +32,14 @@ namespace implementation { using ::android::bluetooth::audio::BluetoothAudioSessionReport; using ::android::hardware::Void; +// Here the buffer size is based on SBC static constexpr uint32_t kPcmFrameSize = 4; // 16 bits per sample / stereo -static constexpr uint32_t kPcmFrameCount = 128; +// SBC is 128, and here choose the LCM of 16, 24, and 32 +static constexpr uint32_t kPcmFrameCount = 96; static constexpr uint32_t kRtpFrameSize = kPcmFrameSize * kPcmFrameCount; -static constexpr uint32_t kRtpFrameCount = 7; // max counts by 1 tick (20ms) +// The max counts by 1 tick (20ms) for SBC is about 7. Since using 96 for the +// PCM counts, here we just choose a greater number +static constexpr uint32_t kRtpFrameCount = 10; static constexpr uint32_t kBufferSize = kRtpFrameSize * kRtpFrameCount; static constexpr uint32_t kBufferCount = 2; // double buffer static constexpr uint32_t kDataMqSize = kBufferSize * kBufferCount; diff --git a/bluetooth/audio/2.1/default/A2dpSoftwareAudioProvider.cpp b/bluetooth/audio/2.1/default/A2dpSoftwareAudioProvider.cpp index a37176ba4d..4928cea66d 100644 --- a/bluetooth/audio/2.1/default/A2dpSoftwareAudioProvider.cpp +++ b/bluetooth/audio/2.1/default/A2dpSoftwareAudioProvider.cpp @@ -34,10 +34,14 @@ using ::android::bluetooth::audio::BluetoothAudioSessionReport_2_1; using ::android::hardware::Void; using ::android::hardware::bluetooth::audio::V2_0::AudioConfiguration; +// Here the buffer size is based on SBC static constexpr uint32_t kPcmFrameSize = 4; // 16 bits per sample / stereo -static constexpr uint32_t kPcmFrameCount = 128; +// SBC is 128, and here we choose the LCM of 16, 24, and 32 +static constexpr uint32_t kPcmFrameCount = 96; static constexpr uint32_t kRtpFrameSize = kPcmFrameSize * kPcmFrameCount; -static constexpr uint32_t kRtpFrameCount = 7; // max counts by 1 tick (20ms) +// The max counts by 1 tick (20ms) for SBC is about 7. Since using 96 for the +// PCM counts, here we just choose a greater number +static constexpr uint32_t kRtpFrameCount = 10; static constexpr uint32_t kBufferSize = kRtpFrameSize * kRtpFrameCount; static constexpr uint32_t kBufferCount = 2; // double buffer static constexpr uint32_t kDataMqSize = kBufferSize * kBufferCount; From 237a6ea5eab4be6500cb447b7f6712ab145f032b Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 5 Aug 2021 11:29:18 +0800 Subject: [PATCH 59/61] wifi: use 1.4 ISupplicant object for 1.4 vts tests This would ensure that 1.4 vts tests are run with ISupplicant 1.4 support. Bug: 194979754 Test: atest VtsHalWifiSupplicantV1_4TargetTest Change-Id: Ifaa3e1bb27f1df350b83fb7a4c05b6251a7c2d10 --- .../1.4/vts/functional/supplicant_sta_network_hidl_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wifi/supplicant/1.4/vts/functional/supplicant_sta_network_hidl_test.cpp b/wifi/supplicant/1.4/vts/functional/supplicant_sta_network_hidl_test.cpp index e3fbaf36f1..49d471bbc4 100644 --- a/wifi/supplicant/1.4/vts/functional/supplicant_sta_network_hidl_test.cpp +++ b/wifi/supplicant/1.4/vts/functional/supplicant_sta_network_hidl_test.cpp @@ -155,5 +155,6 @@ INSTANTIATE_TEST_CASE_P( testing::ValuesIn( android::hardware::getAllHalInstanceNames(IWifi::descriptor)), testing::ValuesIn(android::hardware::getAllHalInstanceNames( - ISupplicant::descriptor))), + android::hardware::wifi::supplicant::V1_4::ISupplicant:: + descriptor))), android::hardware::PrintInstanceTupleNameToString<>); From d1705d569a82a1fc67cb0beaacae50e2e8928d5b Mon Sep 17 00:00:00 2001 From: Tyler Trephan Date: Fri, 6 Aug 2021 13:27:27 -0700 Subject: [PATCH 60/61] Set the minSampleRate > 0 for continuous properties on the AAOS emulator. Test: atest CarPropertyManagerTest Bug: 194680297 Change-Id: I3b9c359885d173c89e825b206b629ed9dfa38d13 --- .../vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h index 6a0288f3a4..f09d75ba21 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h @@ -174,7 +174,7 @@ const ConfigDeclaration kVehicleProperties[]{ .prop = toInt(VehicleProperty::PERF_ODOMETER), .access = VehiclePropertyAccess::READ, .changeMode = VehiclePropertyChangeMode::CONTINUOUS, - .minSampleRate = 0.0f, + .minSampleRate = 1.0f, .maxSampleRate = 10.0f, }, .initialValue = {.floatValues = {0.0f}}}, @@ -183,7 +183,7 @@ const ConfigDeclaration kVehicleProperties[]{ .prop = toInt(VehicleProperty::PERF_STEERING_ANGLE), .access = VehiclePropertyAccess::READ, .changeMode = VehiclePropertyChangeMode::CONTINUOUS, - .minSampleRate = 0.0f, + .minSampleRate = 1.0f, .maxSampleRate = 10.0f, }, .initialValue = {.floatValues = {0.0f}}}, @@ -192,7 +192,7 @@ const ConfigDeclaration kVehicleProperties[]{ .prop = toInt(VehicleProperty::PERF_REAR_STEERING_ANGLE), .access = VehiclePropertyAccess::READ, .changeMode = VehiclePropertyChangeMode::CONTINUOUS, - .minSampleRate = 0.0f, + .minSampleRate = 1.0f, .maxSampleRate = 10.0f, }, .initialValue = {.floatValues = {0.0f}}}, @@ -213,7 +213,7 @@ const ConfigDeclaration kVehicleProperties[]{ .prop = toInt(VehicleProperty::FUEL_LEVEL), .access = VehiclePropertyAccess::READ, .changeMode = VehiclePropertyChangeMode::CONTINUOUS, - .minSampleRate = 0.0f, + .minSampleRate = 1.0f, .maxSampleRate = 100.0f, }, .initialValue = {.floatValues = {15000.0f}}}, @@ -231,7 +231,7 @@ const ConfigDeclaration kVehicleProperties[]{ .prop = toInt(VehicleProperty::EV_BATTERY_LEVEL), .access = VehiclePropertyAccess::READ, .changeMode = VehiclePropertyChangeMode::CONTINUOUS, - .minSampleRate = 0.0f, + .minSampleRate = 1.0f, .maxSampleRate = 100.0f, }, .initialValue = {.floatValues = {150000.0f}}}, From e7a2d287b2abc7f8e566e7758f61b6ea6a04f291 Mon Sep 17 00:00:00 2001 From: Marin Shalamanov Date: Mon, 9 Aug 2021 17:51:12 +0200 Subject: [PATCH 61/61] Increase timeout for VtsHalGraphicsComposerV2_4TargetTest The current timout of 5 mins is not enough for setActiveConfigWithConstraints to complete on devices which have ~30 display modes and which can't do seamless switching. The test needs 8 mins to complete locally -- increasing the timeout to 15 to have some margin. Bug: 188686850 Test: atest VtsHalGraphicsComposerV2_4TargetTest Change-Id: I07cf1825c23e2467852d5cd93436eb9dee23b408 --- graphics/composer/2.4/vts/functional/AndroidTest.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphics/composer/2.4/vts/functional/AndroidTest.xml b/graphics/composer/2.4/vts/functional/AndroidTest.xml index 583aa6866c..773db93255 100644 --- a/graphics/composer/2.4/vts/functional/AndroidTest.xml +++ b/graphics/composer/2.4/vts/functional/AndroidTest.xml @@ -31,6 +31,6 @@