From 30f8443b4154a87653e72a301d53f6cf22eb8a18 Mon Sep 17 00:00:00 2001 From: Changyeon Jo Date: Thu, 1 Oct 2020 13:28:55 -0700 Subject: [PATCH 01/13] Handle the empty display information When the underlying display is not ready yet, IEvsDisplay::getDisplayInfo_1_1() returns an empty display information. When this happens, CameraToDisplayRoundTrip test case must fail because it verifies the camera and display hardware devices both. Fix: 169877399 Test: m -j vts and run CameraToDisplayRoundTrip Change-Id: I2ecb03d19a9088436e7701003944cd76af6c260a --- .../evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp b/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp index 8b68fd6f0a..948d45ff85 100644 --- a/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp +++ b/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp @@ -72,6 +72,8 @@ using ::android::hardware::automotive::evs::V1_1::BufferDesc; using ::android::hardware::automotive::evs::V1_0::DisplayDesc; using ::android::hardware::automotive::evs::V1_0::DisplayState; using ::android::hardware::graphics::common::V1_0::PixelFormat; +using ::android::frameworks::automotive::display::V1_0::HwDisplayConfig; +using ::android::frameworks::automotive::display::V1_0::HwDisplayState; using IEvsCamera_1_0 = ::android::hardware::automotive::evs::V1_0::IEvsCamera; using IEvsCamera_1_1 = ::android::hardware::automotive::evs::V1_1::IEvsCamera; using IEvsDisplay_1_0 = ::android::hardware::automotive::evs::V1_0::IEvsDisplay; @@ -605,7 +607,10 @@ TEST_P(EvsHidlTest, CameraToDisplayRoundTrip) { LOG(INFO) << "Display " << targetDisplayId << " is alreay in use."; // Get the display descriptor - pDisplay->getDisplayInfo_1_1([](const auto& config, const auto& state) { + pDisplay->getDisplayInfo_1_1([](const HwDisplayConfig& config, const HwDisplayState& state) { + ASSERT_GT(config.size(), 0); + ASSERT_GT(state.size(), 0); + android::DisplayConfig* pConfig = (android::DisplayConfig*)config.data(); const auto width = pConfig->resolution.getWidth(); const auto height = pConfig->resolution.getHeight(); From af491ffdf830061c6c11a833b963eeb3495f5249 Mon Sep 17 00:00:00 2001 From: josephjang Date: Thu, 24 Sep 2020 09:51:08 +0800 Subject: [PATCH 02/13] fastboot: add a new oem command for post wipe userdata When user input 'fastboot erase userdata' in fastbootd, may need an oem specific API doOemSpecificErase() to wipe other userdata in device. If oem doesn't need this specific API, oem could return NOT_SUPPORTED to fastbootd. Bug: 169173873 Change-Id: Ie12ede31ef071a3c15265777b55746536a861292 --- .../exclude/fcm_exclude.cpp | 1 + fastboot/1.1/Android.bp | 14 ++++ fastboot/1.1/IFastboot.hal | 32 +++++++++ fastboot/1.1/default/Android.bp | 31 +++++++++ fastboot/1.1/default/Fastboot.cpp | 66 +++++++++++++++++++ fastboot/1.1/default/Fastboot.h | 52 +++++++++++++++ 6 files changed, 196 insertions(+) create mode 100644 fastboot/1.1/Android.bp create mode 100644 fastboot/1.1/IFastboot.hal create mode 100644 fastboot/1.1/default/Android.bp create mode 100644 fastboot/1.1/default/Fastboot.cpp create mode 100644 fastboot/1.1/default/Fastboot.h diff --git a/compatibility_matrices/exclude/fcm_exclude.cpp b/compatibility_matrices/exclude/fcm_exclude.cpp index 4240788ea1..459a6e2fc8 100644 --- a/compatibility_matrices/exclude/fcm_exclude.cpp +++ b/compatibility_matrices/exclude/fcm_exclude.cpp @@ -58,6 +58,7 @@ bool ShouldCheckMissingHalsInFcm(const std::string& package) { // Fastboot HAL is only used by recovery. Recovery is owned by OEM. Framework // does not depend on this HAL, hence it is not declared in any manifests or matrices. "android.hardware.fastboot@1.0", + "android.hardware.fastboot@1.1", }; auto package_has_prefix = [&](const std::string& prefix) { diff --git a/fastboot/1.1/Android.bp b/fastboot/1.1/Android.bp new file mode 100644 index 0000000000..46306cfe81 --- /dev/null +++ b/fastboot/1.1/Android.bp @@ -0,0 +1,14 @@ +// This file is autogenerated by hidl-gen -Landroidbp. + +hidl_interface { + name: "android.hardware.fastboot@1.1", + root: "android.hardware", + srcs: [ + "IFastboot.hal", + ], + interfaces: [ + "android.hardware.fastboot@1.0", + "android.hidl.base@1.0", + ], + gen_java: true, +} diff --git a/fastboot/1.1/IFastboot.hal b/fastboot/1.1/IFastboot.hal new file mode 100644 index 0000000000..c8f1ad01f8 --- /dev/null +++ b/fastboot/1.1/IFastboot.hal @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package android.hardware.fastboot@1.1; +import android.hardware.fastboot@1.0; + +/** + * IFastboot interface implements vendor specific fastboot commands. + */ +interface IFastboot extends @1.0::IFastboot { + /** + * Executes an OEM specific erase after fastboot erase userdata. + * + * @return result Returns the status SUCCESS if the operation is successful, + * NOT_SUPPORTED for unsupported command. + * INVALID_ARGUMENT for bad arguments, + * FAILURE_UNKNOWN for unknown error in the oem specific command. + */ + doOemSpecificErase() generates (Result result); +}; diff --git a/fastboot/1.1/default/Android.bp b/fastboot/1.1/default/Android.bp new file mode 100644 index 0000000000..980586ba4b --- /dev/null +++ b/fastboot/1.1/default/Android.bp @@ -0,0 +1,31 @@ +// +// Copyright (C) 2020 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. + +cc_library { + name: "android.hardware.fastboot@1.1-impl-mock", + recovery: true, + srcs: [ + "Fastboot.cpp", + ], + relative_install_path: "hw", + shared_libs: [ + "libbase", + "libhidlbase", + "libutils", + "libcutils", + "android.hardware.fastboot@1.0", + "android.hardware.fastboot@1.1", + ], +} diff --git a/fastboot/1.1/default/Fastboot.cpp b/fastboot/1.1/default/Fastboot.cpp new file mode 100644 index 0000000000..0b502e0c3c --- /dev/null +++ b/fastboot/1.1/default/Fastboot.cpp @@ -0,0 +1,66 @@ +/* + * Copyright (C) 2020 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 "Fastboot.h" + +namespace android { +namespace hardware { +namespace fastboot { +namespace V1_1 { +namespace implementation { + +// Methods from ::android::hardware::fastboot::V1_1::IFastboot follow. +Return Fastboot::getPartitionType(const hidl_string& /* partitionName */, + getPartitionType_cb _hidl_cb) { + _hidl_cb(FileSystemType::RAW, {Status::SUCCESS, ""}); + return Void(); +} + +Return Fastboot::doOemCommand(const hidl_string& /* oemCmd */, doOemCommand_cb _hidl_cb) { + _hidl_cb({Status::FAILURE_UNKNOWN, "Command not supported in default implementation"}); + return Void(); +} + +Return Fastboot::getVariant(getVariant_cb _hidl_cb) { + _hidl_cb("NA", {Status::SUCCESS, ""}); + return Void(); +} + +Return Fastboot::getOffModeChargeState(getOffModeChargeState_cb _hidl_cb) { + _hidl_cb(false, {Status::SUCCESS, ""}); + return Void(); +} + +Return Fastboot::getBatteryVoltageFlashingThreshold( + getBatteryVoltageFlashingThreshold_cb _hidl_cb) { + _hidl_cb(0, {Status::SUCCESS, ""}); + return Void(); +} + +Return Fastboot::doOemSpecificErase(doOemSpecificErase_cb _hidl_cb) { + _hidl_cb({Status::NOT_SUPPORTED, "Command not supported in default implementation"}); + return Void(); +} + +extern "C" IFastboot* HIDL_FETCH_IFastboot(const char* /* name */) { + return new Fastboot(); +} + +} // namespace implementation +} // namespace V1_1 +} // namespace fastboot +} // namespace hardware +} // namespace android diff --git a/fastboot/1.1/default/Fastboot.h b/fastboot/1.1/default/Fastboot.h new file mode 100644 index 0000000000..09b39c2e35 --- /dev/null +++ b/fastboot/1.1/default/Fastboot.h @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2020 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. + */ +#pragma once + +#include +#include + +namespace android { +namespace hardware { +namespace fastboot { +namespace V1_1 { +namespace implementation { + +using ::android::hardware::hidl_string; +using ::android::hardware::Return; +using ::android::hardware::Void; +using ::android::hardware::fastboot::V1_0::FileSystemType; +using ::android::hardware::fastboot::V1_0::Status; +using ::android::hardware::fastboot::V1_0::Result; + +struct Fastboot : public IFastboot { + // Methods from ::android::hardware::fastboot::V1_0::IFastboot follow. + Return getPartitionType(const hidl_string& partitionName, + getPartitionType_cb _hidl_cb) override; + Return doOemCommand(const hidl_string& oemCmd, doOemCommand_cb _hidl_cb) override; + Return getVariant(getVariant_cb _hidl_cb) override; + Return getOffModeChargeState(getOffModeChargeState_cb _hidl_cb) override; + Return getBatteryVoltageFlashingThreshold( + getBatteryVoltageFlashingThreshold_cb _hidl_cb) override; + Return doOemSpecificErase(doOemSpecificErase_cb _hidl_cb) override; +}; + +extern "C" IFastboot* HIDL_FETCH_IFastboot(const char* name); + +} // namespace implementation +} // namespace V1_1 +} // namespace fastboot +} // namespace hardware +} // namespace android From 1368c2921334b951bcbf09f945ad5930cc35f434 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Fri, 25 Sep 2020 14:28:36 -0700 Subject: [PATCH 03/13] composer: hold a sp from the service itself IComposerClient assumes that IComposer will outlive its life cycle and holds a simple pointer to HwcHal. This change is taking the same approach of newer composer versions (2.2, 2.3, and 2.4) to make sure that IComposer would outlive IComposerClient. Test: coral booting with this change Fixes: 155769496 Change-Id: I3962ede51ce823368c62c4e4e5fb30f7a5680bdf Merged-In: I3962ede51ce823368c62c4e4e5fb30f7a5680bdf (cherry picked from commit 43e42ff6ec685038921bc375bd660c245e4bc4e7) --- graphics/composer/2.1/default/Android.bp | 38 +++++-------------- graphics/composer/2.1/default/passthrough.cpp | 25 ------------ graphics/composer/2.1/default/service.cpp | 19 +++++++++- 3 files changed, 27 insertions(+), 55 deletions(-) delete mode 100644 graphics/composer/2.1/default/passthrough.cpp diff --git a/graphics/composer/2.1/default/Android.bp b/graphics/composer/2.1/default/Android.bp index 533687bb7d..a367457ae9 100644 --- a/graphics/composer/2.1/default/Android.bp +++ b/graphics/composer/2.1/default/Android.bp @@ -1,31 +1,3 @@ -cc_library_shared { - name: "android.hardware.graphics.composer@2.1-impl", - defaults: ["hidl_defaults"], - vendor: true, - relative_install_path: "hw", - srcs: ["passthrough.cpp"], - header_libs: [ - "android.hardware.graphics.composer@2.1-passthrough", - ], - shared_libs: [ - "android.hardware.graphics.composer@2.1", - "android.hardware.graphics.composer@2.1-resources", - "libbase", - "libcutils", - "libfmq", - "libhardware", - "libhidlbase", - "liblog", - "libsync", - "libutils", - "libhwc2on1adapter", - "libhwc2onfbadapter", - ], - cflags: [ - "-DLOG_TAG=\"ComposerHal\"" - ], -} - cc_binary { name: "android.hardware.graphics.composer@2.1-service", defaults: ["hidl_defaults"], @@ -33,10 +5,20 @@ cc_binary { relative_install_path: "hw", srcs: ["service.cpp"], init_rc: ["android.hardware.graphics.composer@2.1-service.rc"], + header_libs: [ + "android.hardware.graphics.composer@2.1-passthrough", + ], shared_libs: [ "android.hardware.graphics.composer@2.1", + "android.hardware.graphics.composer@2.1-resources", + "libbase", "libbinder", + "libcutils", + "libfmq", + "libhardware", "libhidlbase", + "libhwc2on1adapter", + "libhwc2onfbadapter", "liblog", "libsync", "libutils", diff --git a/graphics/composer/2.1/default/passthrough.cpp b/graphics/composer/2.1/default/passthrough.cpp deleted file mode 100644 index ef7ed7c87a..0000000000 --- a/graphics/composer/2.1/default/passthrough.cpp +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright 2016 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 - -using android::hardware::graphics::composer::V2_1::IComposer; -using android::hardware::graphics::composer::V2_1::passthrough::HwcLoader; - -extern "C" IComposer* HIDL_FETCH_IComposer(const char* /* name */) { - return HwcLoader::load(); -} diff --git a/graphics/composer/2.1/default/service.cpp b/graphics/composer/2.1/default/service.cpp index 82a33f6a4e..1276d2df7e 100644 --- a/graphics/composer/2.1/default/service.cpp +++ b/graphics/composer/2.1/default/service.cpp @@ -21,10 +21,11 @@ #include #include +#include #include using android::hardware::graphics::composer::V2_1::IComposer; -using android::hardware::defaultPassthroughServiceImplementation; +using android::hardware::graphics::composer::V2_1::passthrough::HwcLoader; int main() { // the conventional HAL might start binder services @@ -40,5 +41,19 @@ int main() { ALOGE("Couldn't set SCHED_FIFO: %d", errno); } - return defaultPassthroughServiceImplementation(4); + android::hardware::configureRpcThreadpool(4, true /* will join */); + + android::sp composer = HwcLoader::load(); + if (composer == nullptr) { + return 1; + } + if (composer->registerAsService() != android::NO_ERROR) { + ALOGE("failed to register service"); + return 1; + } + + android::hardware::joinRpcThreadpool(); + + ALOGE("service is terminating"); + return 1; } From bc2d16b9f50bff90b5d35486a021cb4b040c4516 Mon Sep 17 00:00:00 2001 From: Hao Chen Date: Thu, 24 Sep 2020 17:23:02 -0700 Subject: [PATCH 04/13] Move Emulated User HAL to Emulated Vehicle Connector Class Test: build; manually tested the following commands ``` > adb shell lshal debug android.hardware.automotive.vehicle@2.0::IVehicle/default --user-hal > adb shell lshal debug android.hardware.automotive.vehicle@2.0::IVehicle/default --set 299896583 a 1 i 666 i 1 i 11 ``` Bug: 166706927 Change-Id: Ic5774e56dec7febcfeaf496111ba77907e1b7fac Change-Id: Ib2545b7e0d6b2eea0734fe013451b1365ee0e8ff Merged-In: Ib2545b7e0d6b2eea0734fe013451b1365ee0e8ff (cherry picked from commit 872784629bd3018a1ca61d0ab5e85c4953c08be7) --- automotive/vehicle/2.0/default/Android.bp | 1 - .../vehicle/2.0/default/VehicleService.cpp | 2 +- .../vhal_v2_0/EmulatedVehicleConnector.cpp | 36 +++++++++++++------ .../impl/vhal_v2_0/EmulatedVehicleConnector.h | 17 +++++++-- .../impl/vhal_v2_0/VehicleHalServer.cpp | 20 ----------- .../default/impl/vhal_v2_0/VehicleHalServer.h | 8 ----- 6 files changed, 41 insertions(+), 43 deletions(-) diff --git a/automotive/vehicle/2.0/default/Android.bp b/automotive/vehicle/2.0/default/Android.bp index 9a0d89d209..590adc58b0 100644 --- a/automotive/vehicle/2.0/default/Android.bp +++ b/automotive/vehicle/2.0/default/Android.bp @@ -137,7 +137,6 @@ cc_library_static { local_include_dirs: ["common/include/vhal_v2_0"], export_include_dirs: ["impl"], srcs: [ - "impl/vhal_v2_0/EmulatedUserHal.cpp", "impl/vhal_v2_0/GeneratorHub.cpp", "impl/vhal_v2_0/JsonFakeValueGenerator.cpp", "impl/vhal_v2_0/LinearFakeValueGenerator.cpp", diff --git a/automotive/vehicle/2.0/default/VehicleService.cpp b/automotive/vehicle/2.0/default/VehicleService.cpp index 32e5e703ff..cf1e4baac6 100644 --- a/automotive/vehicle/2.0/default/VehicleService.cpp +++ b/automotive/vehicle/2.0/default/VehicleService.cpp @@ -33,7 +33,7 @@ using namespace android::hardware::automotive::vehicle::V2_0; int main(int /* argc */, char* /* argv */ []) { auto store = std::make_unique(); - auto connector = impl::makeEmulatedPassthroughConnector(); + auto connector = std::make_unique(); auto hal = std::make_unique(store.get(), connector.get()); auto emulator = std::make_unique(hal.get()); auto service = std::make_unique(hal.get()); diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.cpp b/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.cpp index 7f9362fdf4..ed3f4a2e8f 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.cpp +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.cpp @@ -35,13 +35,33 @@ namespace V2_0 { namespace impl { -class EmulatedPassthroughConnector : public PassthroughConnector { - public: - bool onDump(const hidl_handle& fd, const hidl_vec& options) override; -}; +EmulatedUserHal* EmulatedVehicleConnector::getEmulatedUserHal() { + return &mEmulatedUserHal; +} -bool EmulatedPassthroughConnector::onDump(const hidl_handle& handle, - const hidl_vec& options) { +StatusCode EmulatedVehicleConnector::onSetProperty(const VehiclePropValue& value, + bool updateStatus) { + if (mEmulatedUserHal.isSupported(value.prop)) { + LOG(INFO) << "onSetProperty(): property " << value.prop << " will be handled by UserHal"; + + const auto& ret = mEmulatedUserHal.onSetProperty(value); + if (!ret.ok()) { + LOG(ERROR) << "onSetProperty(): HAL returned error: " << ret.error().message(); + return StatusCode(ret.error().code()); + } + auto updatedValue = ret.value().get(); + if (updatedValue != nullptr) { + LOG(INFO) << "onSetProperty(): updating property returned by HAL: " + << toString(*updatedValue); + onPropertyValueFromCar(*updatedValue, updateStatus); + } + return StatusCode::OK; + } + return this->VehicleHalServer::onSetProperty(value, updateStatus); +} + +bool EmulatedVehicleConnector::onDump(const hidl_handle& handle, + const hidl_vec& options) { int fd = handle->data[0]; if (options.size() > 0) { @@ -68,10 +88,6 @@ bool EmulatedPassthroughConnector::onDump(const hidl_handle& handle, return true; } -PassthroughConnectorPtr makeEmulatedPassthroughConnector() { - return std::make_unique(); -} - } // namespace impl } // namespace V2_0 diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.h index 57cbb8b893..4c6c66150b 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.h @@ -19,6 +19,7 @@ #include +#include "EmulatedUserHal.h" #include "VehicleHalClient.h" #include "VehicleHalServer.h" @@ -30,10 +31,20 @@ namespace V2_0 { namespace impl { -using PassthroughConnector = IPassThroughConnector; -using PassthroughConnectorPtr = std::unique_ptr; +class EmulatedVehicleConnector : public IPassThroughConnector { + public: + EmulatedVehicleConnector() {} -PassthroughConnectorPtr makeEmulatedPassthroughConnector(); + EmulatedUserHal* getEmulatedUserHal(); + + // Methods from VehicleHalServer + StatusCode onSetProperty(const VehiclePropValue& value, bool updateStatus) override; + + bool onDump(const hidl_handle& fd, const hidl_vec& options) override; + + private: + EmulatedUserHal mEmulatedUserHal; +}; } // namespace impl diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.cpp b/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.cpp index 36f25345ae..0ee183596a 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.cpp +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.cpp @@ -41,10 +41,6 @@ VehiclePropValuePool* VehicleHalServer::getValuePool() const { return mValuePool; } -EmulatedUserHal* VehicleHalServer::getEmulatedUserHal() { - return &mEmulatedUserHal; -} - void VehicleHalServer::setValuePool(VehiclePropValuePool* valuePool) { if (!valuePool) { LOG(WARNING) << __func__ << ": Setting value pool to nullptr!"; @@ -185,22 +181,6 @@ VehicleHalServer::VehiclePropValuePtr VehicleHalServer::createHwInputKeyProp( } StatusCode VehicleHalServer::onSetProperty(const VehiclePropValue& value, bool updateStatus) { - if (mEmulatedUserHal.isSupported(value.prop)) { - LOG(INFO) << "onSetProperty(): property " << value.prop << " will be handled by UserHal"; - - const auto& ret = mEmulatedUserHal.onSetProperty(value); - if (!ret.ok()) { - LOG(ERROR) << "onSetProperty(): HAL returned error: " << ret.error().message(); - return StatusCode(ret.error().code()); - } - auto updatedValue = ret.value().get(); - if (updatedValue != nullptr) { - LOG(INFO) << "onSetProperty(): updating property returned by HAL: " - << toString(*updatedValue); - onPropertyValueFromCar(*updatedValue, updateStatus); - } - return StatusCode::OK; - } LOG(DEBUG) << "onSetProperty(" << value.prop << ")"; // Some properties need to be treated non-trivially diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.h index fca78bc822..117eadb1e2 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.h @@ -19,7 +19,6 @@ #include #include -#include "EmulatedUserHal.h" #include "GeneratorHub.h" namespace android::hardware::automotive::vehicle::V2_0::impl { @@ -38,8 +37,6 @@ class VehicleHalServer : public IVehicleServer { // Set the Property Value Pool used in this server void setValuePool(VehiclePropValuePool* valuePool); - EmulatedUserHal* getEmulatedUserHal(); - private: using VehiclePropValuePtr = recyclable_ptr; @@ -56,11 +53,6 @@ class VehicleHalServer : public IVehicleServer { VehiclePropValuePtr createHwInputKeyProp(VehicleHwKeyInputAction action, int32_t keyCode, int32_t targetDisplay); - // data members - - protected: - EmulatedUserHal mEmulatedUserHal; - private: GeneratorHub mGeneratorHub{ std::bind(&VehicleHalServer::onFakeValueGenerated, this, std::placeholders::_1)}; From 057e2e8e7f3e5901ba6b6e5dbbabc5b1bb38778b Mon Sep 17 00:00:00 2001 From: Hao Chen Date: Thu, 24 Sep 2020 17:23:02 -0700 Subject: [PATCH 05/13] Move Emulated User HAL to Emulated Vehicle Connector Class Test: build; manually tested the following commands ``` > adb shell lshal debug android.hardware.automotive.vehicle@2.0::IVehicle/default --user-hal > adb shell lshal debug android.hardware.automotive.vehicle@2.0::IVehicle/default --set 299896583 a 1 i 666 i 1 i 11 ``` Bug: 166706927 Change-Id: Ic5774e56dec7febcfeaf496111ba77907e1b7fac Change-Id: Ib2545b7e0d6b2eea0734fe013451b1365ee0e8ff Merged-In: Ib2545b7e0d6b2eea0734fe013451b1365ee0e8ff (cherry picked from commit 872784629bd3018a1ca61d0ab5e85c4953c08be7) --- automotive/vehicle/2.0/default/Android.bp | 1 - .../vehicle/2.0/default/VehicleService.cpp | 2 +- .../vhal_v2_0/EmulatedVehicleConnector.cpp | 36 +++++++++++++------ .../impl/vhal_v2_0/EmulatedVehicleConnector.h | 17 +++++++-- .../impl/vhal_v2_0/VehicleHalServer.cpp | 20 ----------- .../default/impl/vhal_v2_0/VehicleHalServer.h | 8 ----- 6 files changed, 41 insertions(+), 43 deletions(-) diff --git a/automotive/vehicle/2.0/default/Android.bp b/automotive/vehicle/2.0/default/Android.bp index 9a0d89d209..590adc58b0 100644 --- a/automotive/vehicle/2.0/default/Android.bp +++ b/automotive/vehicle/2.0/default/Android.bp @@ -137,7 +137,6 @@ cc_library_static { local_include_dirs: ["common/include/vhal_v2_0"], export_include_dirs: ["impl"], srcs: [ - "impl/vhal_v2_0/EmulatedUserHal.cpp", "impl/vhal_v2_0/GeneratorHub.cpp", "impl/vhal_v2_0/JsonFakeValueGenerator.cpp", "impl/vhal_v2_0/LinearFakeValueGenerator.cpp", diff --git a/automotive/vehicle/2.0/default/VehicleService.cpp b/automotive/vehicle/2.0/default/VehicleService.cpp index 47133fd162..ef29560588 100644 --- a/automotive/vehicle/2.0/default/VehicleService.cpp +++ b/automotive/vehicle/2.0/default/VehicleService.cpp @@ -34,7 +34,7 @@ using namespace android::hardware::automotive::vehicle::V2_0; int main(int /* argc */, char* /* argv */ []) { auto store = std::make_unique(); - auto connector = impl::makeEmulatedPassthroughConnector(); + auto connector = std::make_unique(); auto userHal = connector->getEmulatedUserHal(); auto hal = std::make_unique(store.get(), connector.get(), userHal); auto emulator = std::make_unique(hal.get()); diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.cpp b/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.cpp index 7f9362fdf4..ed3f4a2e8f 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.cpp +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.cpp @@ -35,13 +35,33 @@ namespace V2_0 { namespace impl { -class EmulatedPassthroughConnector : public PassthroughConnector { - public: - bool onDump(const hidl_handle& fd, const hidl_vec& options) override; -}; +EmulatedUserHal* EmulatedVehicleConnector::getEmulatedUserHal() { + return &mEmulatedUserHal; +} -bool EmulatedPassthroughConnector::onDump(const hidl_handle& handle, - const hidl_vec& options) { +StatusCode EmulatedVehicleConnector::onSetProperty(const VehiclePropValue& value, + bool updateStatus) { + if (mEmulatedUserHal.isSupported(value.prop)) { + LOG(INFO) << "onSetProperty(): property " << value.prop << " will be handled by UserHal"; + + const auto& ret = mEmulatedUserHal.onSetProperty(value); + if (!ret.ok()) { + LOG(ERROR) << "onSetProperty(): HAL returned error: " << ret.error().message(); + return StatusCode(ret.error().code()); + } + auto updatedValue = ret.value().get(); + if (updatedValue != nullptr) { + LOG(INFO) << "onSetProperty(): updating property returned by HAL: " + << toString(*updatedValue); + onPropertyValueFromCar(*updatedValue, updateStatus); + } + return StatusCode::OK; + } + return this->VehicleHalServer::onSetProperty(value, updateStatus); +} + +bool EmulatedVehicleConnector::onDump(const hidl_handle& handle, + const hidl_vec& options) { int fd = handle->data[0]; if (options.size() > 0) { @@ -68,10 +88,6 @@ bool EmulatedPassthroughConnector::onDump(const hidl_handle& handle, return true; } -PassthroughConnectorPtr makeEmulatedPassthroughConnector() { - return std::make_unique(); -} - } // namespace impl } // namespace V2_0 diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.h index 57cbb8b893..4c6c66150b 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/EmulatedVehicleConnector.h @@ -19,6 +19,7 @@ #include +#include "EmulatedUserHal.h" #include "VehicleHalClient.h" #include "VehicleHalServer.h" @@ -30,10 +31,20 @@ namespace V2_0 { namespace impl { -using PassthroughConnector = IPassThroughConnector; -using PassthroughConnectorPtr = std::unique_ptr; +class EmulatedVehicleConnector : public IPassThroughConnector { + public: + EmulatedVehicleConnector() {} -PassthroughConnectorPtr makeEmulatedPassthroughConnector(); + EmulatedUserHal* getEmulatedUserHal(); + + // Methods from VehicleHalServer + StatusCode onSetProperty(const VehiclePropValue& value, bool updateStatus) override; + + bool onDump(const hidl_handle& fd, const hidl_vec& options) override; + + private: + EmulatedUserHal mEmulatedUserHal; +}; } // namespace impl diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.cpp b/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.cpp index 36f25345ae..0ee183596a 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.cpp +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.cpp @@ -41,10 +41,6 @@ VehiclePropValuePool* VehicleHalServer::getValuePool() const { return mValuePool; } -EmulatedUserHal* VehicleHalServer::getEmulatedUserHal() { - return &mEmulatedUserHal; -} - void VehicleHalServer::setValuePool(VehiclePropValuePool* valuePool) { if (!valuePool) { LOG(WARNING) << __func__ << ": Setting value pool to nullptr!"; @@ -185,22 +181,6 @@ VehicleHalServer::VehiclePropValuePtr VehicleHalServer::createHwInputKeyProp( } StatusCode VehicleHalServer::onSetProperty(const VehiclePropValue& value, bool updateStatus) { - if (mEmulatedUserHal.isSupported(value.prop)) { - LOG(INFO) << "onSetProperty(): property " << value.prop << " will be handled by UserHal"; - - const auto& ret = mEmulatedUserHal.onSetProperty(value); - if (!ret.ok()) { - LOG(ERROR) << "onSetProperty(): HAL returned error: " << ret.error().message(); - return StatusCode(ret.error().code()); - } - auto updatedValue = ret.value().get(); - if (updatedValue != nullptr) { - LOG(INFO) << "onSetProperty(): updating property returned by HAL: " - << toString(*updatedValue); - onPropertyValueFromCar(*updatedValue, updateStatus); - } - return StatusCode::OK; - } LOG(DEBUG) << "onSetProperty(" << value.prop << ")"; // Some properties need to be treated non-trivially diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.h index fca78bc822..117eadb1e2 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/VehicleHalServer.h @@ -19,7 +19,6 @@ #include #include -#include "EmulatedUserHal.h" #include "GeneratorHub.h" namespace android::hardware::automotive::vehicle::V2_0::impl { @@ -38,8 +37,6 @@ class VehicleHalServer : public IVehicleServer { // Set the Property Value Pool used in this server void setValuePool(VehiclePropValuePool* valuePool); - EmulatedUserHal* getEmulatedUserHal(); - private: using VehiclePropValuePtr = recyclable_ptr; @@ -56,11 +53,6 @@ class VehicleHalServer : public IVehicleServer { VehiclePropValuePtr createHwInputKeyProp(VehicleHwKeyInputAction action, int32_t keyCode, int32_t targetDisplay); - // data members - - protected: - EmulatedUserHal mEmulatedUserHal; - private: GeneratorHub mGeneratorHub{ std::bind(&VehicleHalServer::onFakeValueGenerated, this, std::placeholders::_1)}; From 4c521b0745890516b37e1e1b6fc2523569528838 Mon Sep 17 00:00:00 2001 From: josephjang Date: Tue, 13 Oct 2020 13:40:45 +0800 Subject: [PATCH 06/13] authsecret: Notify LSS to generate secret In order to prevent storing testing secret to Citadel during authsecret testing, we try to notify lock setting service to generate PIN code send corresponding secret to Citadel before authsecret VTS testing. LSS will not generate new secret if it had ever generated PIN code/Password/Pattern before. Bug: 150929955 Bug: 163064036 atest VtsHalAuthSecretV1_0TargetTest Change-Id: I6b58bf90d16db16ce2a78d8b73298e042c049fc3 --- .../1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp b/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp index 0bff9df7dd..f27f26658a 100644 --- a/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp +++ b/authsecret/1.0/vts/functional/VtsHalAuthSecretV1_0TargetTest.cpp @@ -34,11 +34,19 @@ class AuthSecretHidlTest : public testing::TestWithParam { authsecret = IAuthSecret::getService(GetParam()); ASSERT_NE(authsecret, nullptr); + // Notify LSS to generate PIN code '1234' and corresponding secret. + (void)system("cmd lock_settings set-pin 1234"); + // All tests must enroll the correct secret first as this cannot be changed // without a factory reset and the order of tests could change. authsecret->primaryUserCredential(CORRECT_SECRET); } + static void TearDownTestSuite() { + // clean up PIN code after testing + (void)system("cmd lock_settings clear --old 1234"); + } + sp authsecret; hidl_vec CORRECT_SECRET{61, 93, 124, 240, 5, 0, 7, 201, 9, 129, 11, 12, 0, 14, 0, 16}; hidl_vec WRONG_SECRET{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; From 6006eb1b82cb08d8d77b5660bf29910e30978255 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 16 Oct 2020 23:39:53 +0000 Subject: [PATCH 07/13] Update VNDK version for common graphics HALs. We are adding things to them, and in order for core libraries like libui to use them, the newer versions need to be in the VNDK. Bug: 170435409 Test: build Change-Id: I4094240656f357b5ae52194befd0e10627b9fe23 Merged-In: I4094240656f357b5ae52194befd0e10627b9fe23 --- common/aidl/Android.bp | 1 + graphics/common/aidl/Android.bp | 1 + graphics/mapper/4.0/vts/functional/Android.bp | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/common/aidl/Android.bp b/common/aidl/Android.bp index 9ea4cdf5c1..0f0fa82cfc 100644 --- a/common/aidl/Android.bp +++ b/common/aidl/Android.bp @@ -6,6 +6,7 @@ aidl_interface { enabled: true, support_system_process: true, }, + vndk_use_version: "2", srcs: [ "android/hardware/common/*.aidl", ], diff --git a/graphics/common/aidl/Android.bp b/graphics/common/aidl/Android.bp index e5942339e7..22e609d758 100644 --- a/graphics/common/aidl/Android.bp +++ b/graphics/common/aidl/Android.bp @@ -6,6 +6,7 @@ aidl_interface { enabled: true, support_system_process: true, }, + vndk_use_version: "2", srcs: [ "android/hardware/graphics/common/*.aidl", ], diff --git a/graphics/mapper/4.0/vts/functional/Android.bp b/graphics/mapper/4.0/vts/functional/Android.bp index 03abc891c0..8bda42556b 100644 --- a/graphics/mapper/4.0/vts/functional/Android.bp +++ b/graphics/mapper/4.0/vts/functional/Android.bp @@ -19,7 +19,7 @@ cc_test { defaults: ["VtsHalTargetTestDefaults"], srcs: ["VtsHalGraphicsMapperV4_0TargetTest.cpp"], static_libs: [ - "android.hardware.graphics.common-ndk_platform", + "android.hardware.graphics.common-unstable-ndk_platform", "android.hardware.graphics.mapper@4.0-vts", "libgralloctypes", "libsync", From c27cc544dba07132812851f38b296b495ce4ca37 Mon Sep 17 00:00:00 2001 From: Jordan Liu Date: Wed, 21 Oct 2020 16:28:40 -0700 Subject: [PATCH 08/13] Update CardState HAL and make setSimPower synchronous Bug: 171433370 Test: make; make vts Change-Id: I0d298209a00f5f194547b0d6e368baa44b0c18ec Merged-In: I0d298209a00f5f194547b0d6e368baa44b0c18ec --- current.txt | 2 +- radio/1.0/types.hal | 8 +++- radio/1.6/IRadio.hal | 37 +++++++++++++++++++ radio/1.6/IRadioResponse.hal | 16 ++++++++ .../functional/radio_hidl_hal_utils_v1_6.h | 3 ++ radio/1.6/vts/functional/radio_response.cpp | 7 ++++ 6 files changed, 70 insertions(+), 3 deletions(-) diff --git a/current.txt b/current.txt index d10adcabd1..5ec35579ad 100644 --- a/current.txt +++ b/current.txt @@ -771,7 +771,7 @@ a64467bae843569f0d465c5be7f0c7a5b987985b55a3ef4794dd5afc68538650 android.hardwar cd84ab19c590e0e73dd2307b591a3093ee18147ef95e6d5418644463a6620076 android.hardware.neuralnetworks@1.2::IDevice 9625e85f56515ad2cf87b6a1847906db669f746ea4ab02cd3d4ca25abc9b0109 android.hardware.neuralnetworks@1.2::types 9e758e208d14f7256e0885d6d8ad0b61121b21d8c313864f981727ae55bffd16 android.hardware.neuralnetworks@1.3::types -7da2707d4cf93818eaf8038eb65e2180116a399c310e594a00935c5c981aa340 android.hardware.radio@1.0::types +0f53d70e1eadf8d987766db4bf6ae2048004682168f4cab118da576787def3fa android.hardware.radio@1.0::types # HALs released in Android S # NOTE: waiting to freeze HALs until later in the release diff --git a/radio/1.0/types.hal b/radio/1.0/types.hal index 025aa7ccca..cafa5a62d1 100644 --- a/radio/1.0/types.hal +++ b/radio/1.0/types.hal @@ -166,11 +166,15 @@ enum RestrictedState : int32_t { }; enum CardState : int32_t { + /* card is physically absent from device. (Some old modems use CardState.ABSENT when the SIM + is powered off. This is no longer correct, however the platform will still support this + legacy behavior.) */ ABSENT, + /* card is inserted in the device */ PRESENT, ERROR, - RESTRICTED, // card is present but not usable due to carrier - // restrictions + /* card is present but not usable due to carrier restrictions */ + RESTRICTED, }; enum PinState : int32_t { diff --git a/radio/1.6/IRadio.hal b/radio/1.6/IRadio.hal index ca40a17b24..66007b48ac 100644 --- a/radio/1.6/IRadio.hal +++ b/radio/1.6/IRadio.hal @@ -19,6 +19,7 @@ package android.hardware.radio@1.6; import @1.0::CdmaSmsMessage; import @1.0::GsmSmsMessage; +import @1.1::CardPowerState; import @1.2::DataRequestReason; import @1.5::IRadio; import @1.5::AccessNetwork; @@ -182,6 +183,42 @@ interface IRadio extends @1.5::IRadio { */ oneway sendCdmaSmsExpectMore_1_6(int32_t serial, CdmaSmsMessage sms); + /** + * Set SIM card power state. + * Request is used to power off or power on the card. It should not generate + * a CardState.CARDSTATE_ABSENT indication, since the SIM is still physically + * inserted. + * + * @param serial Serial number of request + * @param powerUp POWER_DOWN if powering down the SIM card, + * POWER_UP if powering up the SIM card, + * POWER_UP_PASS_THROUGH if powering up the SIM card in + * pass through mode. + * + * When SIM card is in POWER_UP_PASS_THROUGH, the modem does not send + * any command to it (for example SELECT of MF, or TERMINAL + * CAPABILITY), and the SIM card is controlled completely by Telephony + * sending APDUs directly. The SIM card state must be + * RIL_CARDSTATE_PRESENT and the number of card apps will be 0. + * No new error code is generated. Emergency calls are supported in + * the same way as if the SIM card is absent. + * Pass-through mode is valid only for the specific card session where + * it is activated, and normal behavior occurs at the next SIM + * initialization, unless POWER_UP_PASS_THROUGH is requested again. + * + * The device is required to power down the SIM card before it can + * switch the mode between POWER_UP and POWER_UP_PASS_THROUGH. + * At device power up, the SIM interface is powered up automatically. + * Each subsequent request to this method is processed only after the + * completion of the previous one. + * + * Response callback is IRadioResponse.setSimCardPowerResponse_1_6(). + * Note that this differs from setSimCardPower_1_1 in that the response + * callback should only be sent once the device has finished executing + * the request (the SIM has finished powering on or off). + */ + oneway setSimCardPower_1_6(int32_t serial, CardPowerState powerUp); + /** * Enable or disable E-UTRA-NR dual connectivity. If disabled then UE will not connect * to secondary carrier. diff --git a/radio/1.6/IRadioResponse.hal b/radio/1.6/IRadioResponse.hal index 4ff7e47f12..bc6db11d56 100644 --- a/radio/1.6/IRadioResponse.hal +++ b/radio/1.6/IRadioResponse.hal @@ -201,6 +201,22 @@ interface IRadioResponse extends @1.5::IRadioResponse { */ oneway sendCdmaSmsExpectMoreResponse_1_6(RadioResponseInfo info, SendSmsResult sms); + /** + * @param info Response info struct containing response type, serial no. and error + * + * Valid errors returned: + * RadioError:NONE + * RadioError:RADIO_NOT_AVAILABLE + * RadioError:REQUEST_NOT_SUPPORTED + * RadioError:INVALID_ARGUMENTS + * RadioError:SIM_ERR (indicates a timeout or other issue making the SIM unresponsive) + * + * Note that this differs from setSimCardPowerResponse_1_1 in that the response + * should only be sent once the request from setSimCardPower_1_6 is complete + * (the SIM has finished powering on or off). + */ + oneway setSimCardPowerResponse_1_6(RadioResponseInfo info); + /** * @param info Response info struct containing response type, serial no. and error * diff --git a/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h b/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h index 784bcd91e8..553e5b7daa 100644 --- a/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h +++ b/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h @@ -763,6 +763,9 @@ class RadioResponse_v1_6 : public ::android::hardware::radio::V1_6::IRadioRespon const ::android::hardware::radio::V1_6::RadioResponseInfo& info, const SendSmsResult& sms); + Return setSimCardPowerResponse_1_6( + const ::android::hardware::radio::V1_6::RadioResponseInfo& info); + Return sendCdmaSmsExpectMoreResponse_1_6( const ::android::hardware::radio::V1_6::RadioResponseInfo& info, const SendSmsResult& sms); diff --git a/radio/1.6/vts/functional/radio_response.cpp b/radio/1.6/vts/functional/radio_response.cpp index ffa384ed9b..c684584370 100644 --- a/radio/1.6/vts/functional/radio_response.cpp +++ b/radio/1.6/vts/functional/radio_response.cpp @@ -1097,6 +1097,13 @@ Return RadioResponse_v1_6::sendCdmaSmsResponse_1_6( return Void(); } +Return RadioResponse_v1_6::setSimCardPowerResponse_1_6( + const ::android::hardware::radio::V1_6::RadioResponseInfo& info) { + rspInfo = info; + parent_v1_6.notify(info.serial); + return Void(); +} + Return RadioResponse_v1_6::sendCdmaSmsExpectMoreResponse_1_6( const ::android::hardware::radio::V1_6::RadioResponseInfo& info, const SendSmsResult& sms) { From da8cd066ca426b68b97c7f255c7b5b306e8fd261 Mon Sep 17 00:00:00 2001 From: Jordan Liu Date: Wed, 21 Oct 2020 16:28:40 -0700 Subject: [PATCH 09/13] Update CardState HAL and make setSimPower synchronous Bug: 171433370 Test: make; make vts Change-Id: I0d298209a00f5f194547b0d6e368baa44b0c18ec Merged-In: I0d298209a00f5f194547b0d6e368baa44b0c18ec --- current.txt | 2 +- radio/1.0/types.hal | 8 +++- radio/1.6/IRadio.hal | 37 +++++++++++++++++++ radio/1.6/IRadioResponse.hal | 16 ++++++++ .../functional/radio_hidl_hal_utils_v1_6.h | 3 ++ radio/1.6/vts/functional/radio_response.cpp | 7 ++++ 6 files changed, 70 insertions(+), 3 deletions(-) diff --git a/current.txt b/current.txt index d10adcabd1..5ec35579ad 100644 --- a/current.txt +++ b/current.txt @@ -771,7 +771,7 @@ a64467bae843569f0d465c5be7f0c7a5b987985b55a3ef4794dd5afc68538650 android.hardwar cd84ab19c590e0e73dd2307b591a3093ee18147ef95e6d5418644463a6620076 android.hardware.neuralnetworks@1.2::IDevice 9625e85f56515ad2cf87b6a1847906db669f746ea4ab02cd3d4ca25abc9b0109 android.hardware.neuralnetworks@1.2::types 9e758e208d14f7256e0885d6d8ad0b61121b21d8c313864f981727ae55bffd16 android.hardware.neuralnetworks@1.3::types -7da2707d4cf93818eaf8038eb65e2180116a399c310e594a00935c5c981aa340 android.hardware.radio@1.0::types +0f53d70e1eadf8d987766db4bf6ae2048004682168f4cab118da576787def3fa android.hardware.radio@1.0::types # HALs released in Android S # NOTE: waiting to freeze HALs until later in the release diff --git a/radio/1.0/types.hal b/radio/1.0/types.hal index 025aa7ccca..cafa5a62d1 100644 --- a/radio/1.0/types.hal +++ b/radio/1.0/types.hal @@ -166,11 +166,15 @@ enum RestrictedState : int32_t { }; enum CardState : int32_t { + /* card is physically absent from device. (Some old modems use CardState.ABSENT when the SIM + is powered off. This is no longer correct, however the platform will still support this + legacy behavior.) */ ABSENT, + /* card is inserted in the device */ PRESENT, ERROR, - RESTRICTED, // card is present but not usable due to carrier - // restrictions + /* card is present but not usable due to carrier restrictions */ + RESTRICTED, }; enum PinState : int32_t { diff --git a/radio/1.6/IRadio.hal b/radio/1.6/IRadio.hal index ca40a17b24..66007b48ac 100644 --- a/radio/1.6/IRadio.hal +++ b/radio/1.6/IRadio.hal @@ -19,6 +19,7 @@ package android.hardware.radio@1.6; import @1.0::CdmaSmsMessage; import @1.0::GsmSmsMessage; +import @1.1::CardPowerState; import @1.2::DataRequestReason; import @1.5::IRadio; import @1.5::AccessNetwork; @@ -182,6 +183,42 @@ interface IRadio extends @1.5::IRadio { */ oneway sendCdmaSmsExpectMore_1_6(int32_t serial, CdmaSmsMessage sms); + /** + * Set SIM card power state. + * Request is used to power off or power on the card. It should not generate + * a CardState.CARDSTATE_ABSENT indication, since the SIM is still physically + * inserted. + * + * @param serial Serial number of request + * @param powerUp POWER_DOWN if powering down the SIM card, + * POWER_UP if powering up the SIM card, + * POWER_UP_PASS_THROUGH if powering up the SIM card in + * pass through mode. + * + * When SIM card is in POWER_UP_PASS_THROUGH, the modem does not send + * any command to it (for example SELECT of MF, or TERMINAL + * CAPABILITY), and the SIM card is controlled completely by Telephony + * sending APDUs directly. The SIM card state must be + * RIL_CARDSTATE_PRESENT and the number of card apps will be 0. + * No new error code is generated. Emergency calls are supported in + * the same way as if the SIM card is absent. + * Pass-through mode is valid only for the specific card session where + * it is activated, and normal behavior occurs at the next SIM + * initialization, unless POWER_UP_PASS_THROUGH is requested again. + * + * The device is required to power down the SIM card before it can + * switch the mode between POWER_UP and POWER_UP_PASS_THROUGH. + * At device power up, the SIM interface is powered up automatically. + * Each subsequent request to this method is processed only after the + * completion of the previous one. + * + * Response callback is IRadioResponse.setSimCardPowerResponse_1_6(). + * Note that this differs from setSimCardPower_1_1 in that the response + * callback should only be sent once the device has finished executing + * the request (the SIM has finished powering on or off). + */ + oneway setSimCardPower_1_6(int32_t serial, CardPowerState powerUp); + /** * Enable or disable E-UTRA-NR dual connectivity. If disabled then UE will not connect * to secondary carrier. diff --git a/radio/1.6/IRadioResponse.hal b/radio/1.6/IRadioResponse.hal index 4ff7e47f12..bc6db11d56 100644 --- a/radio/1.6/IRadioResponse.hal +++ b/radio/1.6/IRadioResponse.hal @@ -201,6 +201,22 @@ interface IRadioResponse extends @1.5::IRadioResponse { */ oneway sendCdmaSmsExpectMoreResponse_1_6(RadioResponseInfo info, SendSmsResult sms); + /** + * @param info Response info struct containing response type, serial no. and error + * + * Valid errors returned: + * RadioError:NONE + * RadioError:RADIO_NOT_AVAILABLE + * RadioError:REQUEST_NOT_SUPPORTED + * RadioError:INVALID_ARGUMENTS + * RadioError:SIM_ERR (indicates a timeout or other issue making the SIM unresponsive) + * + * Note that this differs from setSimCardPowerResponse_1_1 in that the response + * should only be sent once the request from setSimCardPower_1_6 is complete + * (the SIM has finished powering on or off). + */ + oneway setSimCardPowerResponse_1_6(RadioResponseInfo info); + /** * @param info Response info struct containing response type, serial no. and error * diff --git a/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h b/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h index 784bcd91e8..553e5b7daa 100644 --- a/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h +++ b/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h @@ -763,6 +763,9 @@ class RadioResponse_v1_6 : public ::android::hardware::radio::V1_6::IRadioRespon const ::android::hardware::radio::V1_6::RadioResponseInfo& info, const SendSmsResult& sms); + Return setSimCardPowerResponse_1_6( + const ::android::hardware::radio::V1_6::RadioResponseInfo& info); + Return sendCdmaSmsExpectMoreResponse_1_6( const ::android::hardware::radio::V1_6::RadioResponseInfo& info, const SendSmsResult& sms); diff --git a/radio/1.6/vts/functional/radio_response.cpp b/radio/1.6/vts/functional/radio_response.cpp index ffa384ed9b..c684584370 100644 --- a/radio/1.6/vts/functional/radio_response.cpp +++ b/radio/1.6/vts/functional/radio_response.cpp @@ -1097,6 +1097,13 @@ Return RadioResponse_v1_6::sendCdmaSmsResponse_1_6( return Void(); } +Return RadioResponse_v1_6::setSimCardPowerResponse_1_6( + const ::android::hardware::radio::V1_6::RadioResponseInfo& info) { + rspInfo = info; + parent_v1_6.notify(info.serial); + return Void(); +} + Return RadioResponse_v1_6::sendCdmaSmsExpectMoreResponse_1_6( const ::android::hardware::radio::V1_6::RadioResponseInfo& info, const SendSmsResult& sms) { From c9e720b367c3dee73876b3c55c4004d251c3556d Mon Sep 17 00:00:00 2001 From: Jordan Jozwiak Date: Wed, 11 Nov 2020 18:44:08 -0800 Subject: [PATCH 10/13] Use a better RANGE_REMAINING default Test: adb shell cmd car_service get-property-value | grep 0x11600308 Bug: 173114224 Change-Id: Ice9a53b57e79a0166602249b5be9e3bcb03371ec --- automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 cf1840499c..dec2d8cb78 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 @@ -417,7 +417,7 @@ const ConfigDeclaration kVehicleProperties[]{ .minSampleRate = 1.0f, .maxSampleRate = 2.0f, }, - .initialValue = {.floatValues = {100.0f}}}, // units in meters + .initialValue = {.floatValues = {50000.0f}}}, // units in meters {.config = { From e4f58f898fa2fd50c056d5ceac4235eefa41e415 Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Wed, 11 Nov 2020 12:34:27 -0500 Subject: [PATCH 11/13] identity: Fix attestation and documentation problems. - The docs said that IdentityCredential.createEphemeralKey() returned data encoded PKCS#8 which is wrong. It's supposed to be in DER format which is also what the VTS tests and credstore expects. - Clarify that createEphemeralKeyPair(), setReaderEphemeralPublicKey(), and createAuthChallenge() are all optional. - Avoid passing an invalid profile ID in the IdentityCredentialTests. verifyOneProfileAndEntryPass test. - Update requirements for which tags must be present in the attestation for CredentialKey as well as the requirements on expiration date and the issuer name. Update default implementation to satisfy these requirements. Update VTS tests to carefully verify these requrements are met. - Clarify requirements for X.509 cert for AuthenticationKey. Add VTS test to verify. - Mandate that TAG_IDENTITY_CREDENTIAL_KEY must not be set for test credentials. Add VTS test to verify this. - Make default implementation pretend to be implemented in a trusted environment and streamline VTS tests to not special-case for the default implementation. - Switch to using the attestation extension parser from the KM 4.1 support library instead of the one from system/keymaster. The latter one did not support the latest attestation extension and thus would fail for pretty much anything that wasn't the default HAL impl. - Fix a couple of bugs in keymaster::V4_1::parse_attestation_record(): - Report root_of_trust.security_level - Add support for Tag::IDENTITY_CREDENTIAL_KEY - Fix how EMacKey is calculated. - Add test vectors to verify how EMacKey and DeviceMac is calculated. Test: atest VtsHalIdentityTargetTest Test: atest android.security.identity.cts Bug: 171745570 Change-Id: Ic906fa24baa3a475585e543dc03aaf1d0f8d19a0 Merged-In: I2f8bd772de078556733f769cec2021918d1d7de6 --- .../identity/IIdentityCredential.aidl | 19 +- .../identity/IWritableIdentityCredential.aidl | 39 ++- identity/aidl/default/IdentityCredential.cpp | 46 +-- identity/aidl/default/IdentityCredential.h | 1 - .../default/WritableIdentityCredential.cpp | 2 +- identity/aidl/vts/Android.bp | 4 +- .../aidl/vts/VtsAttestationParserSupport.cpp | 187 ---------- .../aidl/vts/VtsAttestationParserSupport.h | 122 ------- identity/aidl/vts/VtsAttestationTests.cpp | 46 +-- .../aidl/vts/VtsHalIdentityEndToEndTest.cpp | 84 ++--- .../VtsIWritableIdentityCredentialTests.cpp | 85 +++-- identity/aidl/vts/VtsIdentityTestUtils.cpp | 318 +++++++++++++++--- identity/aidl/vts/VtsIdentityTestUtils.h | 16 +- .../support/IdentityCredentialSupport.h | 49 ++- .../support/src/IdentityCredentialSupport.cpp | 222 +++++++++--- .../tests/IdentityCredentialSupportTest.cpp | 294 ++++++++++++++++ keymaster/4.1/support/attestation_record.cpp | 12 +- 17 files changed, 930 insertions(+), 616 deletions(-) delete mode 100644 identity/aidl/vts/VtsAttestationParserSupport.cpp delete mode 100644 identity/aidl/vts/VtsAttestationParserSupport.h diff --git a/identity/aidl/android/hardware/identity/IIdentityCredential.aidl b/identity/aidl/android/hardware/identity/IIdentityCredential.aidl index 730b601c69..702334d0b6 100644 --- a/identity/aidl/android/hardware/identity/IIdentityCredential.aidl +++ b/identity/aidl/android/hardware/identity/IIdentityCredential.aidl @@ -55,7 +55,7 @@ interface IIdentityCredential { * This method may only be called once per instance. If called more than once, STATUS_FAILED * will be returned. * - * @return the unencrypted key-pair in PKCS#8 format. + * @return the private key, in DER format as specified in RFC 5915. */ byte[] createEphemeralKeyPair(); @@ -88,10 +88,10 @@ interface IIdentityCredential { * The setRequestedNamespaces() and setVerificationToken() methods will be called before * this method is called. * - * This method be called after createEphemeralKeyPair(), setReaderEphemeralPublicKey(), - * createAuthChallenge() and before startRetrieveEntry(). This method call is followed by - * multiple calls of startRetrieveEntryValue(), retrieveEntryValue(), and finally - * finishRetrieval(). + * This method is called after createEphemeralKeyPair(), setReaderEphemeralPublicKey(), + * createAuthChallenge() (note that those calls are optional) and before startRetrieveEntry(). + * This method call is followed by multiple calls of startRetrieveEntryValue(), + * retrieveEntryValue(), and finally finishRetrieval(). * * It is permissible to perform data retrievals multiple times using the same instance (e.g. * startRetrieval(), then multiple calls of startRetrieveEntryValue(), retrieveEntryValue(), @@ -343,12 +343,13 @@ interface IIdentityCredential { * * - signature: must be set to ECDSA. * - * - subject: CN shall be set to "Android Identity Credential Authentication Key". + * - subject: CN shall be set to "Android Identity Credential Authentication Key". (fixed + * value: same on all certs) * - * - issuer: shall be set to "credentialStoreName (credentialStoreAuthorName)" using the - * values returned in HardwareInformation. + * - issuer: CN shall be set to "Android Identity Credential Key". (fixed value: + * same on all certs) * - * - validity: should be from current time and one year in the future. + * - validity: should be from current time and one year in the future (365 days). * * - subjectPublicKeyInfo: must contain attested public key. * diff --git a/identity/aidl/android/hardware/identity/IWritableIdentityCredential.aidl b/identity/aidl/android/hardware/identity/IWritableIdentityCredential.aidl index 297fd1d8ec..c48cb6682e 100644 --- a/identity/aidl/android/hardware/identity/IWritableIdentityCredential.aidl +++ b/identity/aidl/android/hardware/identity/IWritableIdentityCredential.aidl @@ -37,12 +37,12 @@ interface IWritableIdentityCredential { * * - signature: must be set to ECDSA. * - * - subject: CN shall be set to "Android Identity Credential Key". + * - subject: CN shall be set to "Android Identity Credential Key". (fixed value: + * same on all certs) * - * - issuer: shall be set to "credentialStoreName (credentialStoreAuthorName)" using the - * values returned in HardwareInformation. + * - issuer: Same as the subject field of the batch attestation key. * - * - validity: should be from current time and expire at the same time as the + * - validity: Should be set to current time and expire at the same time as the * attestation batch certificate used. * * - subjectPublicKeyInfo: must contain attested public key. @@ -55,19 +55,14 @@ interface IWritableIdentityCredential { * * - The attestationSecurityLevel field must be set to either Software (0), * TrustedEnvironment (1), or StrongBox (2) depending on how attestation is - * implemented. Only the default AOSP implementation of this HAL may use - * value 0 (additionally, this implementation must not be used on production - * devices). + * implemented. * - * - The keymasterVersion field in the attestation extension must be set to (10*major + minor) - * where major and minor are the Identity Credential interface major and minor versions. - * Specifically for this version of the interface (1.0) this value is 10. + * - The keymasterVersion field in the attestation extension must be set to the. + * same value as used for Android Keystore keys. * * - The keymasterSecurityLevel field in the attestation extension must be set to * either Software (0), TrustedEnvironment (1), or StrongBox (2) depending on how - * the Trusted Application backing the HAL implementation is implemented. Only - * the default AOSP implementation of this HAL may use value 0 (additionally, this - * implementation must not be used on production devices) + * the Trusted Application backing the HAL implementation is implemented. * * - The attestationChallenge field must be set to the passed-in challenge. * @@ -81,7 +76,8 @@ interface IWritableIdentityCredential { * * - Tag::IDENTITY_CREDENTIAL_KEY which indicates that the key is an Identity * Credential key (which can only sign/MAC very specific messages) and not an Android - * Keystore key (which can be used to sign/MAC anything). + * Keystore key (which can be used to sign/MAC anything). This must not be set + * for test credentials. * * - Tag::PURPOSE must be set to SIGN * @@ -95,10 +91,13 @@ interface IWritableIdentityCredential { * * - Tag::EC_CURVE must be set to P_256 * - * Additional authorizations may be needed in the softwareEnforced and teeEnforced - * fields - the above is not an exhaustive list. Specifically, authorizations containing - * information about the root of trust, OS version, verified boot state, and so on should - * be included. + * - Tag::ROOT_OF_TRUST must be set + * + * - Tag::OS_VERSION and Tag::OS_PATCHLEVEL must be set + * + * Additional authorizations may be appear in the softwareEnforced and teeEnforced + * fields. For example if the device has a boot or vendor partitions, then BOOT_PATCHLEVEL + * and VENDOR_PATCHLEVEL should be set. * * Since the chain is required to be generated using Keymaster Attestation, the returned * certificate chain has the following properties: @@ -112,8 +111,8 @@ interface IWritableIdentityCredential { * As with any user of attestation, the Issuing Authority (as a relying party) wishing * to issue a credential to a device using these APIs, must carefully examine the * returned certificate chain for all of the above (and more). In particular, the Issuing - * Authority should check the root of trust, verified boot state, patch level, - * application id, etc. + * Authority should check the root of trust (which include verified boot state), patch level, + * attestation application id, etc. * * This all depends on the needs of the Issuing Authority and the kind of credential but * in general an Issuing Authority should never issue a credential to a device without diff --git a/identity/aidl/default/IdentityCredential.cpp b/identity/aidl/default/IdentityCredential.cpp index 10f9aa5886..41d410d6c9 100644 --- a/identity/aidl/default/IdentityCredential.cpp +++ b/identity/aidl/default/IdentityCredential.cpp @@ -276,6 +276,7 @@ ndk::ScopedAStatus IdentityCredential::startRetrieval( auto itemsRequest = byteStringToUnsigned(itemsRequestS); auto readerSignature = byteStringToUnsigned(readerSignatureS); + std::unique_ptr sessionTranscriptItem; if (sessionTranscript.size() > 0) { auto [item, _, message] = cppbor::parse(sessionTranscript); if (item == nullptr) { @@ -283,7 +284,7 @@ ndk::ScopedAStatus IdentityCredential::startRetrieval( IIdentityCredentialStore::STATUS_INVALID_DATA, "SessionTranscript contains invalid CBOR")); } - sessionTranscriptItem_ = std::move(item); + sessionTranscriptItem = std::move(item); } if (numStartRetrievalCalls_ > 0) { if (sessionTranscript_ != sessionTranscript) { @@ -323,7 +324,7 @@ ndk::ScopedAStatus IdentityCredential::startRetrieval( vector encodedReaderAuthentication = cppbor::Array() .add("ReaderAuthentication") - .add(sessionTranscriptItem_->clone()) + .add(std::move(sessionTranscriptItem)) .add(cppbor::Semantic(24, itemsRequestBytes)) .encode(); vector encodedReaderAuthenticationBytes = @@ -781,13 +782,6 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(vector* outMac, optional> mac; if (signingKeyBlob_.size() > 0 && sessionTranscript_.size() > 0 && readerPublicKey_.size() > 0) { - cppbor::Array array; - array.add("DeviceAuthentication"); - array.add(sessionTranscriptItem_->clone()); - array.add(docType_); - array.add(cppbor::Semantic(24, encodedDeviceNameSpaces)); - vector deviceAuthenticationBytes = cppbor::Semantic(24, array.encode()).encode(); - vector docTypeAsBlob(docType_.begin(), docType_.end()); optional> signingKey = support::decryptAes128Gcm(storageKey_, signingKeyBlob_, docTypeAsBlob); @@ -797,31 +791,15 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(vector* outMac, "Error decrypting signingKeyBlob")); } - optional> sharedSecret = - support::ecdh(readerPublicKey_, signingKey.value()); - if (!sharedSecret) { - return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage( - IIdentityCredentialStore::STATUS_FAILED, "Error doing ECDH")); - } - - // Mix-in SessionTranscriptBytes vector sessionTranscriptBytes = cppbor::Semantic(24, sessionTranscript_).encode(); - vector sharedSecretWithSessionTranscriptBytes = sharedSecret.value(); - std::copy(sessionTranscriptBytes.begin(), sessionTranscriptBytes.end(), - std::back_inserter(sharedSecretWithSessionTranscriptBytes)); - - vector salt = {0x00}; - vector info = {}; - optional> derivedKey = - support::hkdf(sharedSecretWithSessionTranscriptBytes, salt, info, 32); - if (!derivedKey) { + optional> eMacKey = + support::calcEMacKey(signingKey.value(), readerPublicKey_, sessionTranscriptBytes); + if (!eMacKey) { return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage( - IIdentityCredentialStore::STATUS_FAILED, - "Error deriving key from shared secret")); + IIdentityCredentialStore::STATUS_FAILED, "Error calculating EMacKey")); } - - mac = support::coseMac0(derivedKey.value(), {}, // payload - deviceAuthenticationBytes); // detached content + mac = support::calcMac(sessionTranscript_, docType_, encodedDeviceNameSpaces, + eMacKey.value()); if (!mac) { return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage( IIdentityCredentialStore::STATUS_FAILED, "Error MACing data")); @@ -835,9 +813,9 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(vector* outMac, ndk::ScopedAStatus IdentityCredential::generateSigningKeyPair( vector* outSigningKeyBlob, Certificate* outSigningKeyCertificate) { - string serialDecimal = "0"; // TODO: set serial to something unique - string issuer = "Android Open Source Project"; - string subject = "Android IdentityCredential Reference Implementation"; + string serialDecimal = "1"; + string issuer = "Android Identity Credential Key"; + string subject = "Android Identity Credential Authentication Key"; time_t validityNotBefore = time(nullptr); time_t validityNotAfter = validityNotBefore + 365 * 24 * 3600; diff --git a/identity/aidl/default/IdentityCredential.h b/identity/aidl/default/IdentityCredential.h index 40070c0074..f44d731f31 100644 --- a/identity/aidl/default/IdentityCredential.h +++ b/identity/aidl/default/IdentityCredential.h @@ -103,7 +103,6 @@ class IdentityCredential : public BnIdentityCredential { map profileIdToAccessCheckResult_; vector signingKeyBlob_; vector sessionTranscript_; - std::unique_ptr sessionTranscriptItem_; vector itemsRequest_; vector requestCountsRemaining_; map> requestedNameSpacesAndNames_; diff --git a/identity/aidl/default/WritableIdentityCredential.cpp b/identity/aidl/default/WritableIdentityCredential.cpp index c218866ace..4428543611 100644 --- a/identity/aidl/default/WritableIdentityCredential.cpp +++ b/identity/aidl/default/WritableIdentityCredential.cpp @@ -74,7 +74,7 @@ ndk::ScopedAStatus WritableIdentityCredential::getAttestationCertificate( vector appId(attestationApplicationId.begin(), attestationApplicationId.end()); optional, vector>>> keyAttestationPair = - support::createEcKeyPairAndAttestation(challenge, appId); + support::createEcKeyPairAndAttestation(challenge, appId, testCredential_); if (!keyAttestationPair) { LOG(ERROR) << "Error creating credentialKey and attestation"; return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage( diff --git a/identity/aidl/vts/Android.bp b/identity/aidl/vts/Android.bp index c1f44e742e..03966de549 100644 --- a/identity/aidl/vts/Android.bp +++ b/identity/aidl/vts/Android.bp @@ -9,7 +9,6 @@ cc_test { "VtsIWritableIdentityCredentialTests.cpp", "VtsIdentityTestUtils.cpp", "VtsAttestationTests.cpp", - "VtsAttestationParserSupport.cpp", "UserAuthTests.cpp", "ReaderAuthTests.cpp", ], @@ -20,13 +19,14 @@ cc_test { static_libs: [ "libcppbor", "libkeymaster_portable", - "libsoft_attestation_cert", "libpuresoftkeymasterdevice", "android.hardware.keymaster@4.0", "android.hardware.identity-support-lib", "android.hardware.identity-cpp", "android.hardware.keymaster-cpp", "android.hardware.keymaster-ndk_platform", + "libkeymaster4support", + "libkeymaster4_1support", ], test_suites: [ "general-tests", diff --git a/identity/aidl/vts/VtsAttestationParserSupport.cpp b/identity/aidl/vts/VtsAttestationParserSupport.cpp deleted file mode 100644 index 71fe7331ba..0000000000 --- a/identity/aidl/vts/VtsAttestationParserSupport.cpp +++ /dev/null @@ -1,187 +0,0 @@ -/* - * Copyright 2019, 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 "VtsAttestationParserSupport.h" - -#include -#include - -namespace android::hardware::identity::test_utils { - -using std::endl; -using std::map; -using std::optional; -using std::string; -using std::vector; - -using ::android::sp; -using ::android::String16; -using ::android::binder::Status; - -using ::keymaster::ASN1_OBJECT_Ptr; -using ::keymaster::AuthorizationSet; -using ::keymaster::EVP_PKEY_Ptr; -using ::keymaster::kAttestionRecordOid; -using ::keymaster::TAG_ATTESTATION_APPLICATION_ID; -using ::keymaster::TAG_IDENTITY_CREDENTIAL_KEY; -using ::keymaster::TAG_INCLUDE_UNIQUE_ID; -using ::keymaster::TypedTag; -using ::keymaster::X509_Ptr; - -using support::certificateChainSplit; - -optional AttestationCertificateParser::certificateChainToKeymasterChain( - const vector& certificates) { - if (certificates.size() <= 0) { - return {}; - } - - keymaster_cert_chain_t kCert; - kCert.entry_count = certificates.size(); - kCert.entries = (keymaster_blob_t*)malloc(sizeof(keymaster_blob_t) * kCert.entry_count); - - int index = 0; - for (const auto& c : certificates) { - kCert.entries[index].data_length = c.encodedCertificate.size(); - uint8_t* data = (uint8_t*)malloc(c.encodedCertificate.size()); - - memcpy(data, c.encodedCertificate.data(), c.encodedCertificate.size()); - kCert.entries[index].data = (const uint8_t*)data; - index++; - } - - return kCert; -} - -bool AttestationCertificateParser::parse() { - optional cert_chain = certificateChainToKeymasterChain(origCertChain_); - if (!cert_chain) { - return false; - } - - if (cert_chain.value().entry_count < 3) { - return false; - } - - if (!verifyChain(cert_chain.value())) { - return false; - } - - if (!verifyAttestationRecord(cert_chain.value().entries[0])) { - return false; - } - - keymaster_free_cert_chain(&cert_chain.value()); - return true; -} - -ASN1_OCTET_STRING* AttestationCertificateParser::getAttestationRecord(X509* certificate) { - ASN1_OBJECT_Ptr oid(OBJ_txt2obj(kAttestionRecordOid, 1)); - if (!oid.get()) return nullptr; - - int location = X509_get_ext_by_OBJ(certificate, oid.get(), -1); - if (location == -1) return nullptr; - - X509_EXTENSION* attest_rec_ext = X509_get_ext(certificate, location); - if (!attest_rec_ext) return nullptr; - - ASN1_OCTET_STRING* attest_rec = X509_EXTENSION_get_data(attest_rec_ext); - return attest_rec; -} - -X509* AttestationCertificateParser::parseCertBlob(const keymaster_blob_t& blob) { - const uint8_t* p = blob.data; - return d2i_X509(nullptr, &p, blob.data_length); -} - -bool AttestationCertificateParser::verifyAttestationRecord( - const keymaster_blob_t& attestation_cert) { - X509_Ptr cert(parseCertBlob(attestation_cert)); - if (!cert.get()) { - return false; - } - - ASN1_OCTET_STRING* attest_rec = getAttestationRecord(cert.get()); - if (!attest_rec) { - return false; - } - - keymaster_blob_t att_unique_id = {}; - keymaster_blob_t att_challenge; - keymaster_error_t ret = parse_attestation_record( - attest_rec->data, attest_rec->length, &att_attestation_version_, - &att_attestation_security_level_, &att_keymaster_version_, - &att_keymaster_security_level_, &att_challenge, &att_sw_enforced_, &att_hw_enforced_, - &att_unique_id); - if (ret) { - return false; - } - - att_challenge_.assign(att_challenge.data, att_challenge.data + att_challenge.data_length); - return true; -} - -uint32_t AttestationCertificateParser::getKeymasterVersion() { - return att_keymaster_version_; -} - -uint32_t AttestationCertificateParser::getAttestationVersion() { - return att_attestation_version_; -} - -vector AttestationCertificateParser::getAttestationChallenge() { - return att_challenge_; -} - -keymaster_security_level_t AttestationCertificateParser::getKeymasterSecurityLevel() { - return att_keymaster_security_level_; -} - -keymaster_security_level_t AttestationCertificateParser::getAttestationSecurityLevel() { - return att_attestation_security_level_; -} - -// Verify the Attestation certificates are correctly chained. -bool AttestationCertificateParser::verifyChain(const keymaster_cert_chain_t& chain) { - for (size_t i = 0; i < chain.entry_count - 1; ++i) { - keymaster_blob_t& key_cert_blob = chain.entries[i]; - keymaster_blob_t& signing_cert_blob = chain.entries[i + 1]; - - X509_Ptr key_cert(parseCertBlob(key_cert_blob)); - X509_Ptr signing_cert(parseCertBlob(signing_cert_blob)); - if (!key_cert.get() || !signing_cert.get()) { - return false; - } - - EVP_PKEY_Ptr signing_pubkey(X509_get_pubkey(signing_cert.get())); - if (!signing_pubkey.get()) return false; - - if (X509_verify(key_cert.get(), signing_pubkey.get()) != 1) { - return false; - } - - if (i + 1 == chain.entry_count - 1) { - // Last entry is self-signed. - if (X509_verify(signing_cert.get(), signing_pubkey.get()) != 1) { - return false; - } - } - } - - return true; -} - -} // namespace android::hardware::identity::test_utils diff --git a/identity/aidl/vts/VtsAttestationParserSupport.h b/identity/aidl/vts/VtsAttestationParserSupport.h deleted file mode 100644 index 7c7e1b6ec1..0000000000 --- a/identity/aidl/vts/VtsAttestationParserSupport.h +++ /dev/null @@ -1,122 +0,0 @@ - -/* - * Copyright 2019, 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. - */ - -#ifndef VTS_ATTESTATION_PARSER_SUPPORT_H -#define VTS_ATTESTATION_PARSER_SUPPORT_H - -//#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace android::hardware::identity::test_utils { - -using ::std::optional; -using ::std::string; -using ::std::vector; - -using ::keymaster::AuthorizationSet; -using ::keymaster::TypedTag; - -class AttestationCertificateParser { - public: - AttestationCertificateParser(const vector& certChain) - : origCertChain_(certChain) {} - - bool parse(); - - uint32_t getKeymasterVersion(); - uint32_t getAttestationVersion(); - vector getAttestationChallenge(); - keymaster_security_level_t getKeymasterSecurityLevel(); - keymaster_security_level_t getAttestationSecurityLevel(); - - template - bool getSwEnforcedBool(TypedTag tag) { - if (att_sw_enforced_.GetTagValue(tag)) { - return true; - } - - return false; - } - - template - bool getHwEnforcedBool(TypedTag tag) { - if (att_hw_enforced_.GetTagValue(tag)) { - return true; - } - - return false; - } - - template - optional> getHwEnforcedBlob(TypedTag tag) { - keymaster_blob_t blob; - if (att_hw_enforced_.GetTagValue(tag, &blob)) { - return {}; - } - - vector ret(blob.data, blob.data + blob.data_length); - return ret; - } - - template - optional> getSwEnforcedBlob(TypedTag tag) { - keymaster_blob_t blob; - if (!att_sw_enforced_.GetTagValue(tag, &blob)) { - return {}; - } - - vector ret(blob.data, blob.data + blob.data_length); - return ret; - } - - private: - // Helper functions. - bool verifyChain(const keymaster_cert_chain_t& chain); - - ASN1_OCTET_STRING* getAttestationRecord(X509* certificate); - - X509* parseCertBlob(const keymaster_blob_t& blob); - - bool verifyAttestationRecord(const keymaster_blob_t& attestation_cert); - - optional certificateChainToKeymasterChain( - const vector& certificates); - - // Private variables. - vector origCertChain_; - AuthorizationSet att_sw_enforced_; - AuthorizationSet att_hw_enforced_; - uint32_t att_attestation_version_; - uint32_t att_keymaster_version_; - keymaster_security_level_t att_attestation_security_level_; - keymaster_security_level_t att_keymaster_security_level_; - vector att_challenge_; -}; - -} // namespace android::hardware::identity::test_utils - -#endif // VTS_ATTESTATION_PARSER_SUPPORT_H diff --git a/identity/aidl/vts/VtsAttestationTests.cpp b/identity/aidl/vts/VtsAttestationTests.cpp index c7cdfc7714..e5871fb435 100644 --- a/identity/aidl/vts/VtsAttestationTests.cpp +++ b/identity/aidl/vts/VtsAttestationTests.cpp @@ -29,7 +29,6 @@ #include #include -#include "VtsAttestationParserSupport.h" #include "VtsIdentityTestUtils.h" namespace android::hardware::identity { @@ -44,7 +43,6 @@ using ::android::sp; using ::android::String16; using ::android::binder::Status; -using test_utils::AttestationCertificateParser; using test_utils::setupWritableCredential; using test_utils::validateAttestationCertificate; @@ -61,38 +59,12 @@ class VtsAttestationTests : public testing::TestWithParam { sp credentialStore_; }; -TEST_P(VtsAttestationTests, verifyAttestationWithNonemptyChallengeEmptyId) { - Status result; - - HardwareInformation hwInfo; - ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); - - sp writableCredential; - ASSERT_TRUE(setupWritableCredential(writableCredential, credentialStore_)); - - string challenge = "NotSoRandomChallenge"; - vector attestationChallenge(challenge.begin(), challenge.end()); - vector attestationCertificate; - vector attestationApplicationId = {}; - - result = writableCredential->getAttestationCertificate( - attestationApplicationId, attestationChallenge, &attestationCertificate); - - ASSERT_TRUE(result.isOk()) << result.exceptionCode() << "; " << result.exceptionMessage() - << endl; - - EXPECT_TRUE(validateAttestationCertificate(attestationCertificate, attestationChallenge, - attestationApplicationId, hwInfo)); -} - TEST_P(VtsAttestationTests, verifyAttestationWithNonemptyChallengeNonemptyId) { Status result; - HardwareInformation hwInfo; - ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); - sp writableCredential; - ASSERT_TRUE(setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); string challenge = "NotSoRandomChallenge1NotSoRandomChallenge1NotSoRandomChallenge1"; vector attestationChallenge(challenge.begin(), challenge.end()); @@ -106,18 +78,16 @@ TEST_P(VtsAttestationTests, verifyAttestationWithNonemptyChallengeNonemptyId) { ASSERT_TRUE(result.isOk()) << result.exceptionCode() << "; " << result.exceptionMessage() << endl; - EXPECT_TRUE(validateAttestationCertificate(attestationCertificate, attestationChallenge, - attestationApplicationId, hwInfo)); + validateAttestationCertificate(attestationCertificate, attestationChallenge, + attestationApplicationId, false); } TEST_P(VtsAttestationTests, verifyAttestationWithVeryShortChallengeAndId) { Status result; - HardwareInformation hwInfo; - ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); - sp writableCredential; - ASSERT_TRUE(setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); string challenge = "c"; vector attestationChallenge(challenge.begin(), challenge.end()); @@ -131,8 +101,8 @@ TEST_P(VtsAttestationTests, verifyAttestationWithVeryShortChallengeAndId) { ASSERT_TRUE(result.isOk()) << result.exceptionCode() << "; " << result.exceptionMessage() << endl; - EXPECT_TRUE(validateAttestationCertificate(attestationCertificate, attestationChallenge, - attestationApplicationId, hwInfo)); + validateAttestationCertificate(attestationCertificate, attestationChallenge, + attestationApplicationId, false); } INSTANTIATE_TEST_SUITE_P( diff --git a/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp b/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp index e347654d9a..765794353e 100644 --- a/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp +++ b/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp @@ -174,16 +174,17 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { string cborPretty; sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + true /* testCredential */)); string challenge = "attestationChallenge"; - test_utils::AttestationData attData(writableCredential, challenge, {}); + test_utils::AttestationData attData(writableCredential, challenge, + {1} /* atteestationApplicationId */); ASSERT_TRUE(attData.result.isOk()) << attData.result.exceptionCode() << "; " << attData.result.exceptionMessage() << endl; - EXPECT_TRUE(validateAttestationCertificate(attData.attestationCertificate, - attData.attestationChallenge, - attData.attestationApplicationId, hwInfo)); + validateAttestationCertificate(attData.attestationCertificate, attData.attestationChallenge, + attData.attestationApplicationId, true); // This is kinda of a hack but we need to give the size of // ProofOfProvisioning that we'll expect to receive. @@ -368,6 +369,7 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { optional> signingPubKey = support::certificateChainGetTopMostKey(signingKeyCertificate.encodedCertificate); EXPECT_TRUE(signingPubKey); + test_utils::verifyAuthKeyCertificate(signingKeyCertificate.encodedCertificate); // Since we're using a test-credential we know storageKey meaning we can get the // private key. Do this, derive the public key from it, and check this matches what @@ -418,9 +420,9 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { } vector mac; - vector deviceNameSpacesBytes; - ASSERT_TRUE(credential->finishRetrieval(&mac, &deviceNameSpacesBytes).isOk()); - cborPretty = support::cborPrettyPrint(deviceNameSpacesBytes, 32, {}); + vector deviceNameSpacesEncoded; + ASSERT_TRUE(credential->finishRetrieval(&mac, &deviceNameSpacesEncoded).isOk()); + cborPretty = support::cborPrettyPrint(deviceNameSpacesEncoded, 32, {}); ASSERT_EQ( "{\n" " 'PersonalData' : {\n" @@ -435,37 +437,19 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { " },\n" "}", cborPretty); - // The data that is MACed is ["DeviceAuthentication", sessionTranscript, docType, - // deviceNameSpacesBytes] so build up that structure - cppbor::Array deviceAuthentication; - deviceAuthentication.add("DeviceAuthentication"); - deviceAuthentication.add(sessionTranscript.clone()); string docType = "org.iso.18013-5.2019.mdl"; - deviceAuthentication.add(docType); - deviceAuthentication.add(cppbor::Semantic(24, deviceNameSpacesBytes)); - vector deviceAuthenticationBytes = - cppbor::Semantic(24, deviceAuthentication.encode()).encode(); - // Derive the key used for MACing. optional> readerEphemeralPrivateKey = support::ecKeyPairGetPrivateKey(readerEphemeralKeyPair.value()); - optional> sharedSecret = - support::ecdh(signingPubKey.value(), readerEphemeralPrivateKey.value()); - ASSERT_TRUE(sharedSecret); - // Mix-in SessionTranscriptBytes - vector sessionTranscriptBytes = - cppbor::Semantic(24, sessionTranscript.encode()).encode(); - vector sharedSecretWithSessionTranscriptBytes = sharedSecret.value(); - std::copy(sessionTranscriptBytes.begin(), sessionTranscriptBytes.end(), - std::back_inserter(sharedSecretWithSessionTranscriptBytes)); - vector salt = {0x00}; - vector info = {}; - optional> derivedKey = - support::hkdf(sharedSecretWithSessionTranscriptBytes, salt, info, 32); - ASSERT_TRUE(derivedKey); + optional> eMacKey = support::calcEMacKey( + readerEphemeralPrivateKey.value(), // Private Key + signingPubKey.value(), // Public Key + cppbor::Semantic(24, sessionTranscript.encode()).encode()); // SessionTranscriptBytes optional> calculatedMac = - support::coseMac0(derivedKey.value(), {}, // payload - deviceAuthenticationBytes); // detached content + support::calcMac(sessionTranscript.encode(), // SessionTranscript + docType, // DocType + deviceNameSpacesEncoded, // DeviceNamespaces + eMacKey.value()); // EMacKey ASSERT_TRUE(calculatedMac); EXPECT_EQ(mac, calculatedMac); @@ -480,18 +464,14 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { signingKeyBlob, sessionTranscriptEncoded, {}, // readerSignature, testEntriesEntryCounts) .isOk()); - ASSERT_TRUE(credential->finishRetrieval(&mac, &deviceNameSpacesBytes).isOk()); - cborPretty = support::cborPrettyPrint(deviceNameSpacesBytes, 32, {}); + ASSERT_TRUE(credential->finishRetrieval(&mac, &deviceNameSpacesEncoded).isOk()); + cborPretty = support::cborPrettyPrint(deviceNameSpacesEncoded, 32, {}); ASSERT_EQ("{}", cborPretty); // Calculate DeviceAuthentication and MAC (MACing key hasn't changed) - deviceAuthentication = cppbor::Array(); - deviceAuthentication.add("DeviceAuthentication"); - deviceAuthentication.add(sessionTranscript.clone()); - deviceAuthentication.add(docType); - deviceAuthentication.add(cppbor::Semantic(24, deviceNameSpacesBytes)); - deviceAuthenticationBytes = cppbor::Semantic(24, deviceAuthentication.encode()).encode(); - calculatedMac = support::coseMac0(derivedKey.value(), {}, // payload - deviceAuthenticationBytes); // detached content + calculatedMac = support::calcMac(sessionTranscript.encode(), // SessionTranscript + docType, // DocType + deviceNameSpacesEncoded, // DeviceNamespaces + eMacKey.value()); // EMacKey ASSERT_TRUE(calculatedMac); EXPECT_EQ(mac, calculatedMac); @@ -506,18 +486,14 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { signingKeyBlob, sessionTranscriptEncoded, {}, // readerSignature, testEntriesEntryCounts) .isOk()); - ASSERT_TRUE(credential->finishRetrieval(&mac, &deviceNameSpacesBytes).isOk()); - cborPretty = support::cborPrettyPrint(deviceNameSpacesBytes, 32, {}); + ASSERT_TRUE(credential->finishRetrieval(&mac, &deviceNameSpacesEncoded).isOk()); + cborPretty = support::cborPrettyPrint(deviceNameSpacesEncoded, 32, {}); ASSERT_EQ("{}", cborPretty); // Calculate DeviceAuthentication and MAC (MACing key hasn't changed) - deviceAuthentication = cppbor::Array(); - deviceAuthentication.add("DeviceAuthentication"); - deviceAuthentication.add(sessionTranscript.clone()); - deviceAuthentication.add(docType); - deviceAuthentication.add(cppbor::Semantic(24, deviceNameSpacesBytes)); - deviceAuthenticationBytes = cppbor::Semantic(24, deviceAuthentication.encode()).encode(); - calculatedMac = support::coseMac0(derivedKey.value(), {}, // payload - deviceAuthenticationBytes); // detached content + calculatedMac = support::calcMac(sessionTranscript.encode(), // SessionTranscript + docType, // DocType + deviceNameSpacesEncoded, // DeviceNamespaces + eMacKey.value()); // EMacKey ASSERT_TRUE(calculatedMac); EXPECT_EQ(mac, calculatedMac); } diff --git a/identity/aidl/vts/VtsIWritableIdentityCredentialTests.cpp b/identity/aidl/vts/VtsIWritableIdentityCredentialTests.cpp index b572b0fbec..5435ed85a1 100644 --- a/identity/aidl/vts/VtsIWritableIdentityCredentialTests.cpp +++ b/identity/aidl/vts/VtsIWritableIdentityCredentialTests.cpp @@ -61,7 +61,8 @@ TEST_P(IdentityCredentialTests, verifyAttestationWithEmptyChallenge) { ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); vector attestationChallenge; vector attestationCertificate; @@ -82,12 +83,13 @@ TEST_P(IdentityCredentialTests, verifyAttestationSuccessWithChallenge) { ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); string challenge = "NotSoRandomChallenge1NotSoRandomChallenge1NotSoRandomChallenge1"; vector attestationChallenge(challenge.begin(), challenge.end()); vector attestationCertificate; - vector attestationApplicationId = {}; + vector attestationApplicationId = {1}; result = writableCredential->getAttestationCertificate( attestationApplicationId, attestationChallenge, &attestationCertificate); @@ -95,27 +97,27 @@ TEST_P(IdentityCredentialTests, verifyAttestationSuccessWithChallenge) { EXPECT_TRUE(result.isOk()) << result.exceptionCode() << "; " << result.exceptionMessage() << endl; - EXPECT_TRUE(test_utils::validateAttestationCertificate( - attestationCertificate, attestationChallenge, attestationApplicationId, hwInfo)); + test_utils::validateAttestationCertificate(attestationCertificate, attestationChallenge, + attestationApplicationId, false); } TEST_P(IdentityCredentialTests, verifyAttestationDoubleCallFails) { Status result; - HardwareInformation hwInfo; - ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); - sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); string challenge = "NotSoRandomChallenge1"; - test_utils::AttestationData attData(writableCredential, challenge, {}); - ASSERT_TRUE(test_utils::validateAttestationCertificate( - attData.attestationCertificate, attData.attestationChallenge, - attData.attestationApplicationId, hwInfo)); + test_utils::AttestationData attData(writableCredential, challenge, + {1} /* atteestationApplicationId */); + test_utils::validateAttestationCertificate(attData.attestationCertificate, + attData.attestationChallenge, + attData.attestationApplicationId, false); string challenge2 = "NotSoRandomChallenge2"; - test_utils::AttestationData attData2(writableCredential, challenge2, {}); + test_utils::AttestationData attData2(writableCredential, challenge2, + {} /* atteestationApplicationId */); EXPECT_FALSE(attData2.result.isOk()) << attData2.result.exceptionCode() << "; " << attData2.result.exceptionMessage() << endl; EXPECT_EQ(binder::Status::EX_SERVICE_SPECIFIC, attData2.result.exceptionCode()); @@ -125,7 +127,8 @@ TEST_P(IdentityCredentialTests, verifyAttestationDoubleCallFails) { TEST_P(IdentityCredentialTests, verifyStartPersonalization) { Status result; sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); // First call should go through const vector entryCounts = {2, 4}; @@ -147,7 +150,8 @@ TEST_P(IdentityCredentialTests, verifyStartPersonalization) { TEST_P(IdentityCredentialTests, verifyStartPersonalizationMin) { Status result; sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); // Verify minimal number of profile count and entry count const vector entryCounts = {1, 1}; @@ -160,7 +164,8 @@ TEST_P(IdentityCredentialTests, verifyStartPersonalizationMin) { TEST_P(IdentityCredentialTests, verifyStartPersonalizationOne) { Status result; sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); // Verify minimal number of profile count and entry count const vector entryCounts = {1}; @@ -173,7 +178,8 @@ TEST_P(IdentityCredentialTests, verifyStartPersonalizationOne) { TEST_P(IdentityCredentialTests, verifyStartPersonalizationLarge) { Status result; sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); // Verify set a large number of profile count and entry count is ok const vector entryCounts = {3000}; @@ -186,7 +192,8 @@ TEST_P(IdentityCredentialTests, verifyStartPersonalizationLarge) { TEST_P(IdentityCredentialTests, verifyProfileNumberMismatchShouldFail) { Status result; sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); // Enter mismatched entry and profile numbers const vector entryCounts = {5, 6}; @@ -224,7 +231,8 @@ TEST_P(IdentityCredentialTests, verifyProfileNumberMismatchShouldFail) { TEST_P(IdentityCredentialTests, verifyDuplicateProfileId) { Status result; sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); const vector entryCounts = {3, 6}; writableCredential->setExpectedProofOfProvisioningSize(123456); @@ -283,10 +291,12 @@ TEST_P(IdentityCredentialTests, verifyOneProfileAndEntryPass) { ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); string challenge = "NotSoRandomChallenge1"; - test_utils::AttestationData attData(writableCredential, challenge, {}); + test_utils::AttestationData attData(writableCredential, challenge, + {} /* atteestationApplicationId */); EXPECT_TRUE(attData.result.isOk()) << attData.result.exceptionCode() << "; " << attData.result.exceptionMessage() << endl; @@ -294,7 +304,7 @@ TEST_P(IdentityCredentialTests, verifyOneProfileAndEntryPass) { ASSERT_TRUE(readerCertificate1); const vector entryCounts = {1u}; - size_t expectedPoPSize = 186 + readerCertificate1.value().size(); + size_t expectedPoPSize = 185 + readerCertificate1.value().size(); // OK to fail, not available in v1 HAL writableCredential->setExpectedProofOfProvisioningSize(expectedPoPSize); result = writableCredential->startPersonalization(1, entryCounts); @@ -308,7 +318,7 @@ TEST_P(IdentityCredentialTests, verifyOneProfileAndEntryPass) { ASSERT_TRUE(secureProfiles); const vector testEntries1 = { - {"Name Space", "Last name", string("Turing"), vector{0, 1}}, + {"Name Space", "Last name", string("Turing"), vector{1}}, }; map>> encryptedBlobs; @@ -347,11 +357,11 @@ TEST_P(IdentityCredentialTests, verifyOneProfileAndEntryPass) { " {\n" " 'name' : 'Last name',\n" " 'value' : 'Turing',\n" - " 'accessControlProfiles' : [0, 1, ],\n" + " 'accessControlProfiles' : [1, ],\n" " },\n" " ],\n" " },\n" - " true,\n" + " false,\n" "]", cborPretty); @@ -370,10 +380,12 @@ TEST_P(IdentityCredentialTests, verifyManyProfilesAndEntriesPass) { ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); string challenge = "NotSoRandomChallenge"; - test_utils::AttestationData attData(writableCredential, challenge, {}); + test_utils::AttestationData attData(writableCredential, challenge, + {} /* atteestationApplicationId */); EXPECT_TRUE(attData.result.isOk()) << attData.result.exceptionCode() << "; " << attData.result.exceptionMessage() << endl; @@ -510,7 +522,7 @@ TEST_P(IdentityCredentialTests, verifyManyProfilesAndEntriesPass) { " },\n" " ],\n" " },\n" - " true,\n" + " false,\n" "]", cborPretty); @@ -529,10 +541,12 @@ TEST_P(IdentityCredentialTests, verifyEmptyNameSpaceMixedWithNonEmptyWorks) { ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); string challenge = "NotSoRandomChallenge"; - test_utils::AttestationData attData(writableCredential, challenge, {}); + test_utils::AttestationData attData(writableCredential, challenge, + {} /* atteestationApplicationId */); ASSERT_TRUE(attData.result.isOk()) << attData.result.exceptionCode() << "; " << attData.result.exceptionMessage() << endl; @@ -591,10 +605,12 @@ TEST_P(IdentityCredentialTests, verifyInterleavingEntryNameSpaceOrderingFails) { ASSERT_TRUE(credentialStore_->getHardwareInformation(&hwInfo).isOk()); sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); string challenge = "NotSoRandomChallenge"; - test_utils::AttestationData attData(writableCredential, challenge, {}); + test_utils::AttestationData attData(writableCredential, challenge, + {} /* atteestationApplicationId */); ASSERT_TRUE(attData.result.isOk()) << attData.result.exceptionCode() << "; " << attData.result.exceptionMessage() << endl; @@ -667,7 +683,8 @@ TEST_P(IdentityCredentialTests, verifyInterleavingEntryNameSpaceOrderingFails) { TEST_P(IdentityCredentialTests, verifyAccessControlProfileIdOutOfRange) { sp writableCredential; - ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_)); + ASSERT_TRUE(test_utils::setupWritableCredential(writableCredential, credentialStore_, + false /* testCredential */)); const vector entryCounts = {1}; writableCredential->setExpectedProofOfProvisioningSize(123456); diff --git a/identity/aidl/vts/VtsIdentityTestUtils.cpp b/identity/aidl/vts/VtsIdentityTestUtils.cpp index b6ed80f4b1..3b106514b4 100644 --- a/identity/aidl/vts/VtsIdentityTestUtils.cpp +++ b/identity/aidl/vts/VtsIdentityTestUtils.cpp @@ -14,13 +14,17 @@ * limitations under the License. */ +#define LOG_TAG "VtsIdentityTestUtils" + #include "VtsIdentityTestUtils.h" #include +#include +#include +#include +#include #include -#include "VtsAttestationParserSupport.h" - namespace android::hardware::identity::test_utils { using std::endl; @@ -32,15 +36,15 @@ using std::vector; using ::android::sp; using ::android::String16; using ::android::binder::Status; +using ::keymaster::X509_Ptr; bool setupWritableCredential(sp& writableCredential, - sp& credentialStore) { + sp& credentialStore, bool testCredential) { if (credentialStore == nullptr) { return false; } string docType = "org.iso.18013-5.2019.mdl"; - bool testCredential = true; Status result = credentialStore->createCredential(docType, testCredential, &writableCredential); if (result.isOk() && writableCredential != nullptr) { @@ -178,63 +182,269 @@ void setImageData(vector& image) { } } -bool validateAttestationCertificate(const vector& inputCertificates, - const vector& expectedChallenge, - const vector& expectedAppId, - const HardwareInformation& hwInfo) { - AttestationCertificateParser certParser_(inputCertificates); - bool ret = certParser_.parse(); - EXPECT_TRUE(ret); - if (!ret) { +string x509NameToRfc2253String(X509_NAME* name) { + char* buf; + size_t bufSize; + BIO* bio; + + bio = BIO_new(BIO_s_mem()); + X509_NAME_print_ex(bio, name, 0, XN_FLAG_RFC2253); + bufSize = BIO_get_mem_data(bio, &buf); + string ret = string(buf, bufSize); + BIO_free(bio); + + return ret; +} + +int parseDigits(const char** s, int numDigits) { + int result; + auto [_, ec] = std::from_chars(*s, *s + numDigits, result); + if (ec != std::errc()) { + LOG(ERROR) << "Error parsing " << numDigits << " digits " + << " from " << s; + return 0; + } + *s += numDigits; + return result; +} + +bool parseAsn1Time(const ASN1_TIME* asn1Time, time_t* outTime) { + struct tm tm; + + memset(&tm, '\0', sizeof(tm)); + const char* timeStr = (const char*)asn1Time->data; + const char* s = timeStr; + if (asn1Time->type == V_ASN1_UTCTIME) { + tm.tm_year = parseDigits(&s, 2); + if (tm.tm_year < 70) { + tm.tm_year += 100; + } + } else if (asn1Time->type == V_ASN1_GENERALIZEDTIME) { + tm.tm_year = parseDigits(&s, 4) - 1900; + tm.tm_year -= 1900; + } else { + LOG(ERROR) << "Unsupported ASN1_TIME type " << asn1Time->type; + return false; + } + tm.tm_mon = parseDigits(&s, 2) - 1; + tm.tm_mday = parseDigits(&s, 2); + tm.tm_hour = parseDigits(&s, 2); + tm.tm_min = parseDigits(&s, 2); + tm.tm_sec = parseDigits(&s, 2); + // This may need to be updated if someone create certificates using +/- instead of Z. + // + if (*s != 'Z') { + LOG(ERROR) << "Expected Z in string '" << timeStr << "' at offset " << (s - timeStr); return false; } - // As per the IC HAL, the version of the Identity - // Credential HAL is 1.0 - and this is encoded as major*10 + minor. This field is used by - // Keymaster which is known to report integers less than or equal to 4 (for KM up to 4.0) - // and integers greater or equal than 41 (for KM starting with 4.1). - // - // Since we won't get to version 4.0 of the IC HAL for a while, let's also check that a KM - // version isn't errornously returned. - EXPECT_LE(10, certParser_.getKeymasterVersion()); - EXPECT_GT(40, certParser_.getKeymasterVersion()); - EXPECT_LE(3, certParser_.getAttestationVersion()); - - // Verify the app id matches to whatever we set it to be. - optional> appId = - certParser_.getSwEnforcedBlob(::keymaster::TAG_ATTESTATION_APPLICATION_ID); - if (appId) { - EXPECT_EQ(expectedAppId.size(), appId.value().size()); - EXPECT_EQ(0, memcmp(expectedAppId.data(), appId.value().data(), expectedAppId.size())); - } else { - // app id not found - EXPECT_EQ(0, expectedAppId.size()); - } - - EXPECT_TRUE(certParser_.getHwEnforcedBool(::keymaster::TAG_IDENTITY_CREDENTIAL_KEY)); - EXPECT_FALSE(certParser_.getHwEnforcedBool(::keymaster::TAG_INCLUDE_UNIQUE_ID)); - - // Verify the challenge always matches in size and data of what is passed - // in. - vector attChallenge = certParser_.getAttestationChallenge(); - EXPECT_EQ(expectedChallenge.size(), attChallenge.size()); - EXPECT_EQ(0, memcmp(expectedChallenge.data(), attChallenge.data(), expectedChallenge.size())); - - // Ensure the attestation conveys that it's implemented in secure hardware (with carve-out - // for the reference implementation which cannot be implemented in secure hardware). - if (hwInfo.credentialStoreName == "Identity Credential Reference Implementation" && - hwInfo.credentialStoreAuthorName == "Google") { - EXPECT_LE(KM_SECURITY_LEVEL_SOFTWARE, certParser_.getKeymasterSecurityLevel()); - EXPECT_LE(KM_SECURITY_LEVEL_SOFTWARE, certParser_.getAttestationSecurityLevel()); - - } else { - // Actual devices should use TrustedEnvironment or StrongBox. - EXPECT_LE(KM_SECURITY_LEVEL_TRUSTED_ENVIRONMENT, certParser_.getKeymasterSecurityLevel()); - EXPECT_LE(KM_SECURITY_LEVEL_TRUSTED_ENVIRONMENT, certParser_.getAttestationSecurityLevel()); + time_t t = timegm(&tm); + if (t == -1) { + LOG(ERROR) << "Error converting broken-down time to time_t"; + return false; } + *outTime = t; return true; } +void validateAttestationCertificate(const vector& credentialKeyCertChain, + const vector& expectedChallenge, + const vector& expectedAppId, bool isTestCredential) { + ASSERT_GE(credentialKeyCertChain.size(), 2); + + vector certBytes = credentialKeyCertChain[0].encodedCertificate; + const uint8_t* certData = certBytes.data(); + X509_Ptr cert = X509_Ptr(d2i_X509(nullptr, &certData, certBytes.size())); + + vector batchCertBytes = credentialKeyCertChain[1].encodedCertificate; + const uint8_t* batchCertData = batchCertBytes.data(); + X509_Ptr batchCert = X509_Ptr(d2i_X509(nullptr, &batchCertData, batchCertBytes.size())); + + // First get some values from the batch certificate which is checked + // against the top-level certificate (subject, notAfter) + // + + X509_NAME* batchSubject = X509_get_subject_name(batchCert.get()); + ASSERT_NE(nullptr, batchSubject); + time_t batchNotAfter; + ASSERT_TRUE(parseAsn1Time(X509_get0_notAfter(batchCert.get()), &batchNotAfter)); + + // Check all the requirements from IWritableIdentityCredential::getAttestationCertificate()... + // + + // - version: INTEGER 2 (means v3 certificate). + EXPECT_EQ(2, X509_get_version(cert.get())); + + // - serialNumber: INTEGER 1 (fixed value: same on all certs). + EXPECT_EQ(1, ASN1_INTEGER_get(X509_get_serialNumber(cert.get()))); + + // - signature: must be set to ECDSA. + EXPECT_EQ(NID_ecdsa_with_SHA256, X509_get_signature_nid(cert.get())); + + // - subject: CN shall be set to "Android Identity Credential Key". (fixed value: + // same on all certs) + X509_NAME* subject = X509_get_subject_name(cert.get()); + ASSERT_NE(nullptr, subject); + EXPECT_EQ("CN=Android Identity Credential Key", x509NameToRfc2253String(subject)); + + // - issuer: Same as the subject field of the batch attestation key. + X509_NAME* issuer = X509_get_issuer_name(cert.get()); + ASSERT_NE(nullptr, issuer); + EXPECT_EQ(x509NameToRfc2253String(batchSubject), x509NameToRfc2253String(issuer)); + + // - validity: Should be from current time and expire at the same time as the + // attestation batch certificate used. + // + // Allow for 10 seconds drift to account for the time drift between Secure HW + // and this environment plus the difference between when the certificate was + // created and until now + // + time_t notBefore; + ASSERT_TRUE(parseAsn1Time(X509_get0_notBefore(cert.get()), ¬Before)); + uint64_t now = time(nullptr); + int64_t diffSecs = now - notBefore; + int64_t allowDriftSecs = 10; + EXPECT_LE(-allowDriftSecs, diffSecs); + EXPECT_GE(allowDriftSecs, diffSecs); + + time_t notAfter; + ASSERT_TRUE(parseAsn1Time(X509_get0_notAfter(cert.get()), ¬After)); + EXPECT_EQ(notAfter, batchNotAfter); + + auto [err, attRec] = keymaster::V4_1::parse_attestation_record(certBytes); + ASSERT_EQ(keymaster::V4_1::ErrorCode::OK, err); + + // - subjectPublicKeyInfo: must contain attested public key. + + // - The attestationVersion field in the attestation extension must be at least 3. + EXPECT_GE(attRec.attestation_version, 3); + + // - The attestationSecurityLevel field must be set to either Software (0), + // TrustedEnvironment (1), or StrongBox (2) depending on how attestation is + // implemented. + EXPECT_GE(attRec.attestation_security_level, + keymaster::V4_0::SecurityLevel::TRUSTED_ENVIRONMENT); + + // - The keymasterVersion field in the attestation extension must be set to the. + // same value as used for Android Keystore keys. + // + // Nothing to check here... + + // - The keymasterSecurityLevel field in the attestation extension must be set to + // either Software (0), TrustedEnvironment (1), or StrongBox (2) depending on how + // the Trusted Application backing the HAL implementation is implemented. + EXPECT_GE(attRec.keymaster_security_level, keymaster::V4_0::SecurityLevel::TRUSTED_ENVIRONMENT); + + // - The attestationChallenge field must be set to the passed-in challenge. + EXPECT_EQ(expectedChallenge.size(), attRec.attestation_challenge.size()); + EXPECT_TRUE(memcmp(expectedChallenge.data(), attRec.attestation_challenge.data(), + attRec.attestation_challenge.size()) == 0); + + // - The uniqueId field must be empty. + EXPECT_EQ(attRec.unique_id.size(), 0); + + // - The softwareEnforced field in the attestation extension must include + // Tag::ATTESTATION_APPLICATION_ID which must be set to the bytes of the passed-in + // attestationApplicationId. + EXPECT_TRUE(attRec.software_enforced.Contains(keymaster::V4_0::TAG_ATTESTATION_APPLICATION_ID, + expectedAppId)); + + // - The teeEnforced field in the attestation extension must include + // + // - Tag::IDENTITY_CREDENTIAL_KEY which indicates that the key is an Identity + // Credential key (which can only sign/MAC very specific messages) and not an Android + // Keystore key (which can be used to sign/MAC anything). This must not be set + // for test credentials. + bool hasIcKeyTag = + attRec.hardware_enforced.Contains(static_cast( + keymaster::V4_1::Tag::IDENTITY_CREDENTIAL_KEY)); + if (isTestCredential) { + EXPECT_FALSE(hasIcKeyTag); + } else { + EXPECT_TRUE(hasIcKeyTag); + } + + // - Tag::PURPOSE must be set to SIGN + EXPECT_TRUE(attRec.hardware_enforced.Contains(keymaster::V4_0::TAG_PURPOSE, + keymaster::V4_0::KeyPurpose::SIGN)); + + // - Tag::KEY_SIZE must be set to the appropriate key size, in bits (e.g. 256) + EXPECT_TRUE(attRec.hardware_enforced.Contains(keymaster::V4_0::TAG_KEY_SIZE, 256)); + + // - Tag::ALGORITHM must be set to EC + EXPECT_TRUE(attRec.hardware_enforced.Contains(keymaster::V4_0::TAG_ALGORITHM, + keymaster::V4_0::Algorithm::EC)); + + // - Tag::NO_AUTH_REQUIRED must be set + EXPECT_TRUE(attRec.hardware_enforced.Contains(keymaster::V4_0::TAG_NO_AUTH_REQUIRED)); + + // - Tag::DIGEST must be include SHA_2_256 + EXPECT_TRUE(attRec.hardware_enforced.Contains(keymaster::V4_0::TAG_DIGEST, + keymaster::V4_0::Digest::SHA_2_256)); + + // - Tag::EC_CURVE must be set to P_256 + EXPECT_TRUE(attRec.hardware_enforced.Contains(keymaster::V4_0::TAG_EC_CURVE, + keymaster::V4_0::EcCurve::P_256)); + + // - Tag::ROOT_OF_TRUST must be set + // + EXPECT_GE(attRec.root_of_trust.security_level, + keymaster::V4_0::SecurityLevel::TRUSTED_ENVIRONMENT); + + // - Tag::OS_VERSION and Tag::OS_PATCHLEVEL must be set + EXPECT_TRUE(attRec.hardware_enforced.Contains(keymaster::V4_0::TAG_OS_VERSION)); + EXPECT_TRUE(attRec.hardware_enforced.Contains(keymaster::V4_0::TAG_OS_PATCHLEVEL)); + + // TODO: we could retrieve osVersion and osPatchLevel from Android itself and compare it + // with what was reported in the certificate. +} + +void verifyAuthKeyCertificate(const vector& authKeyCertChain) { + const uint8_t* data = authKeyCertChain.data(); + auto cert = X509_Ptr(d2i_X509(nullptr, &data, authKeyCertChain.size())); + + // - version: INTEGER 2 (means v3 certificate). + EXPECT_EQ(X509_get_version(cert.get()), 2); + + // - serialNumber: INTEGER 1 (fixed value: same on all certs). + EXPECT_EQ(ASN1_INTEGER_get(X509_get_serialNumber(cert.get())), 1); + + // - signature: must be set to ECDSA. + EXPECT_EQ(X509_get_signature_nid(cert.get()), NID_ecdsa_with_SHA256); + + // - subject: CN shall be set to "Android Identity Credential Authentication Key". (fixed + // value: same on all certs) + X509_NAME* subject = X509_get_subject_name(cert.get()); + ASSERT_NE(subject, nullptr); + EXPECT_EQ(x509NameToRfc2253String(subject), + "CN=Android Identity Credential Authentication Key"); + + // - issuer: CN shall be set to "Android Identity Credential Key". (fixed value: + // same on all certs) + X509_NAME* issuer = X509_get_issuer_name(cert.get()); + ASSERT_NE(issuer, nullptr); + EXPECT_EQ(x509NameToRfc2253String(issuer), "CN=Android Identity Credential Key"); + + // - subjectPublicKeyInfo: must contain attested public key. + + // - validity: should be from current time and one year in the future (365 days). + time_t notBefore, notAfter; + ASSERT_TRUE(parseAsn1Time(X509_get0_notAfter(cert.get()), ¬After)); + ASSERT_TRUE(parseAsn1Time(X509_get0_notBefore(cert.get()), ¬Before)); + + // Allow for 10 seconds drift to account for the time drift between Secure HW + // and this environment plus the difference between when the certificate was + // created and until now + // + uint64_t now = time(nullptr); + int64_t diffSecs = now - notBefore; + int64_t allowDriftSecs = 10; + EXPECT_LE(-allowDriftSecs, diffSecs); + EXPECT_GE(allowDriftSecs, diffSecs); + constexpr uint64_t kSecsInOneYear = 365 * 24 * 60 * 60; + EXPECT_EQ(notBefore + kSecsInOneYear, notAfter); +} + vector buildRequestNamespaces(const vector entries) { vector ret; RequestNamespace curNs; diff --git a/identity/aidl/vts/VtsIdentityTestUtils.h b/identity/aidl/vts/VtsIdentityTestUtils.h index 673b736842..85c24f86b5 100644 --- a/identity/aidl/vts/VtsIdentityTestUtils.h +++ b/identity/aidl/vts/VtsIdentityTestUtils.h @@ -34,8 +34,8 @@ using ::android::binder::Status; struct AttestationData { AttestationData(sp& writableCredential, string challenge, - vector applicationId) - : attestationApplicationId(applicationId) { + vector attestationAppId) + : attestationApplicationId(attestationAppId) { // ASSERT_NE(writableCredential, nullptr); if (!challenge.empty()) { @@ -94,7 +94,7 @@ struct TestProfile { }; bool setupWritableCredential(sp& writableCredential, - sp& credentialStore); + sp& credentialStore, bool testCredential); optional> generateReaderCertificate(string serialDecimal); @@ -111,13 +111,17 @@ bool addEntry(sp& writableCredential, const TestEnt void setImageData(vector& image); -bool validateAttestationCertificate(const vector& inputCertificates, +void validateAttestationCertificate(const vector& credentialKeyCertChain, const vector& expectedChallenge, - const vector& expectedAppId, - const HardwareInformation& hwInfo); + const vector& expectedAppId, bool isTestCredential); vector buildRequestNamespaces(const vector entries); +// Verifies that the X.509 certificate for a just created authentication key +// is valid. +// +void verifyAuthKeyCertificate(const vector& authKeyCertChain); + } // namespace android::hardware::identity::test_utils #endif // VTS_IDENTITY_TEST_UTILS_H diff --git a/identity/support/include/android/hardware/identity/support/IdentityCredentialSupport.h b/identity/support/include/android/hardware/identity/support/IdentityCredentialSupport.h index f7ec7c516f..3aa5bb641e 100644 --- a/identity/support/include/android/hardware/identity/support/IdentityCredentialSupport.h +++ b/identity/support/include/android/hardware/identity/support/IdentityCredentialSupport.h @@ -35,6 +35,9 @@ using ::std::tuple; using ::std::vector; using ::std::pair; +// The semantic tag for a bstr which includes Encoded CBOR (RFC 7049, section 2.4) +const int kSemanticTagEncodedCbor = 24; + // --------------------------------------------------------------------------- // Miscellaneous utilities. // --------------------------------------------------------------------------- @@ -108,45 +111,47 @@ optional> encryptAes128Gcm(const vector& key, const vec // --------------------------------------------------------------------------- // EC crypto functionality / abstraction (only supports P-256). // --------------------------------------------------------------------------- + // Creates an 256-bit EC key using the NID_X9_62_prime256v1 curve, returns the -// PKCS#8 encoded key-pair. Also generates an attestation -// certificate using the |challenge| and |applicationId|, and returns the generated -// certificate in X.509 certificate chain format. +// DER encoded private key. Also generates an attestation using the |challenge| +// and |applicationId|, and returns the generated certificate chain. // -// The attestation time fields used will be the current time, and expires in one year. +// The notBeffore field will be the current time and the notAfter will be the same +// same time as the batch certificate. // // The first parameter of the return value is the keyPair generated, second return in // the pair is the attestation certificate generated. -optional, vector>>> createEcKeyPairAndAttestation( - const vector& challenge, const vector& applicationId); - -// Like createEcKeyPairAndAttestation() but allows you to choose the public key. // +optional, vector>>> createEcKeyPairAndAttestation( + const vector& challenge, const vector& applicationId, + bool isTestCredential); + +// (TODO: remove when no longer used by 3rd party.) optional>> createAttestationForEcPublicKey( const vector& publicKey, const vector& challenge, const vector& applicationId); // Creates an 256-bit EC key using the NID_X9_62_prime256v1 curve, returns the -// PKCS#8 encoded key-pair. +// private key in DER format (as specified in RFC 5915). // optional> createEcKeyPair(); -// For an EC key |keyPair| encoded in PKCS#8 format, extracts the public key in +// For an EC key |keyPair| encoded in DER format, extracts the public key in // uncompressed point form. // optional> ecKeyPairGetPublicKey(const vector& keyPair); -// For an EC key |keyPair| encoded in PKCS#8 format, extracts the private key as +// For an EC key |keyPair| encoded in DER format, extracts the private key as // an EC uncompressed key. // optional> ecKeyPairGetPrivateKey(const vector& keyPair); -// Creates a PKCS#8 encoded key-pair from a private key (which must be uncompressed, -// e.g. 32 bytes). The public key is derived from the given private key.. +// Creates a DER encoded representation from a private key (which must be uncompressed, +// e.g. 32 bytes). // optional> ecPrivateKeyToKeyPair(const vector& privateKey); -// For an EC key |keyPair| encoded in PKCS#8 format, creates a PKCS#12 structure +// For an EC key |keyPair| encoded in DER format, creates a PKCS#12 structure // with the key-pair (not using a password to encrypt the data). The public key // in the created structure is included as a certificate, using the given fields // |serialDecimal|, |issuer|, |subject|, |validityNotBefore|, and @@ -209,6 +214,13 @@ optional> certificateTbsCertificate(const vector& // optional> certificateFindSignature(const vector& x509Certificate); +// Extracts notBefore and notAfter from the top-most certificate in |certificateChain +// (which should be a concatenated chain of DER-encoded X.509 certificates). +// +// Returns notBefore and notAfter in that order. +// +optional> certificateGetValidity(const vector& x509Certificate); + // Generates a X.509 certificate for |publicKey| (which must be in the format // returned by ecKeyPairGetPublicKey()). // @@ -351,6 +363,15 @@ optional> coseMacWithDigest(const vector& digestToBeMac // Utility functions specific to IdentityCredential. // --------------------------------------------------------------------------- +optional> calcMac(const vector& sessionTranscriptEncoded, + const string& docType, + const vector& deviceNameSpacesEncoded, + const vector& eMacKey); + +optional> calcEMacKey(const vector& privateKey, + const vector& publicKey, + const vector& sessionTranscriptBytes); + // Returns the testing AES-128 key where all bits are set to 0. const vector& getTestHardwareBoundKey(); diff --git a/identity/support/src/IdentityCredentialSupport.cpp b/identity/support/src/IdentityCredentialSupport.cpp index 8e099e7d2f..08559c6fce 100644 --- a/identity/support/src/IdentityCredentialSupport.cpp +++ b/identity/support/src/IdentityCredentialSupport.cpp @@ -44,6 +44,7 @@ #include #include +#include #include #include @@ -870,16 +871,97 @@ optional> hmacSha256(const vector& key, const vectordata; + const char* s = timeStr; + if (asn1Time->type == V_ASN1_UTCTIME) { + tm.tm_year = parseDigits(&s, 2); + if (tm.tm_year < 70) { + tm.tm_year += 100; + } + } else if (asn1Time->type == V_ASN1_GENERALIZEDTIME) { + tm.tm_year = parseDigits(&s, 4) - 1900; + tm.tm_year -= 1900; + } else { + LOG(ERROR) << "Unsupported ASN1_TIME type " << asn1Time->type; + return false; + } + tm.tm_mon = parseDigits(&s, 2) - 1; + tm.tm_mday = parseDigits(&s, 2); + tm.tm_hour = parseDigits(&s, 2); + tm.tm_min = parseDigits(&s, 2); + tm.tm_sec = parseDigits(&s, 2); + // This may need to be updated if someone create certificates using +/- instead of Z. + // + if (*s != 'Z') { + LOG(ERROR) << "Expected Z in string '" << timeStr << "' at offset " << (s - timeStr); + return false; + } + + time_t t = timegm(&tm); + if (t == -1) { + LOG(ERROR) << "Error converting broken-down time to time_t"; + return false; + } + *outTime = t; + return true; +} + // Generates the attestation certificate with the parameters passed in. Note // that the passed in |activeTimeMilliSeconds| |expireTimeMilliSeconds| are in // milli seconds since epoch. We are setting them to milliseconds due to // requirement in AuthorizationSet KM_DATE fields. The certificate created is // actually in seconds. -optional>> createAttestation(const EVP_PKEY* key, - const vector& applicationId, - const vector& challenge, - uint64_t activeTimeMilliSeconds, - uint64_t expireTimeMilliSeconds) { +// +// If 0 is passed for expiration time, the expiration time from batch +// certificate will be used. +// +optional>> createAttestation( + const EVP_PKEY* key, const vector& applicationId, const vector& challenge, + uint64_t activeTimeMilliSeconds, uint64_t expireTimeMilliSeconds, bool isTestCredential) { + const keymaster_cert_chain_t* attestation_chain = + ::keymaster::getAttestationChain(KM_ALGORITHM_EC, nullptr); + if (attestation_chain == nullptr) { + LOG(ERROR) << "Error getting attestation chain"; + return {}; + } + if (expireTimeMilliSeconds == 0) { + if (attestation_chain->entry_count < 1) { + LOG(ERROR) << "Expected at least one entry in attestation chain"; + return {}; + } + keymaster_blob_t* bcBlob = &(attestation_chain->entries[0]); + const uint8_t* bcData = bcBlob->data; + auto bc = X509_Ptr(d2i_X509(nullptr, &bcData, bcBlob->data_length)); + time_t bcNotAfter; + if (!parseAsn1Time(X509_get0_notAfter(bc.get()), &bcNotAfter)) { + LOG(ERROR) << "Error getting notAfter from batch certificate"; + return {}; + } + expireTimeMilliSeconds = bcNotAfter * 1000; + } + const keymaster_key_blob_t* attestation_signing_key = + ::keymaster::getAttestationKey(KM_ALGORITHM_EC, nullptr); + if (attestation_signing_key == nullptr) { + LOG(ERROR) << "Error getting attestation key"; + return {}; + } + ::keymaster::AuthorizationSet auth_set( ::keymaster::AuthorizationSetBuilder() .Authorization(::keymaster::TAG_ATTESTATION_CHALLENGE, challenge.data(), @@ -901,7 +983,7 @@ optional>> createAttestation(const EVP_PKEY* key, ::keymaster::AuthorizationSet swEnforced(::keymaster::AuthorizationSetBuilder().Authorization( ::keymaster::TAG_CREATION_DATETIME, activeTimeMilliSeconds)); - ::keymaster::AuthorizationSet hwEnforced( + ::keymaster::AuthorizationSetBuilder hwEnforcedBuilder = ::keymaster::AuthorizationSetBuilder() .Authorization(::keymaster::TAG_PURPOSE, KM_PURPOSE_SIGN) .Authorization(::keymaster::TAG_KEY_SIZE, 256) @@ -909,34 +991,29 @@ optional>> createAttestation(const EVP_PKEY* key, .Authorization(::keymaster::TAG_NO_AUTH_REQUIRED) .Authorization(::keymaster::TAG_DIGEST, KM_DIGEST_SHA_2_256) .Authorization(::keymaster::TAG_EC_CURVE, KM_EC_CURVE_P_256) - .Authorization(::keymaster::TAG_IDENTITY_CREDENTIAL_KEY)); + .Authorization(::keymaster::TAG_OS_VERSION, 42) + .Authorization(::keymaster::TAG_OS_PATCHLEVEL, 43); - const keymaster_cert_chain_t* attestation_chain = - ::keymaster::getAttestationChain(KM_ALGORITHM_EC, nullptr); - - if (attestation_chain == nullptr) { - LOG(ERROR) << "Error getting attestation chain"; - return {}; - } - - const keymaster_key_blob_t* attestation_signing_key = - ::keymaster::getAttestationKey(KM_ALGORITHM_EC, nullptr); - if (attestation_signing_key == nullptr) { - LOG(ERROR) << "Error getting attestation key"; - return {}; + // Only include TAG_IDENTITY_CREDENTIAL_KEY if it's not a test credential + if (!isTestCredential) { + hwEnforcedBuilder.Authorization(::keymaster::TAG_IDENTITY_CREDENTIAL_KEY); } + ::keymaster::AuthorizationSet hwEnforced(hwEnforcedBuilder); keymaster_error_t error; ::keymaster::CertChainPtr cert_chain_out; - ::keymaster::PureSoftKeymasterContext context; - // set identity version to 10 per hal requirements specified in IWriteableCredential.hal - // For now, the identity version in the attestation is set in the keymaster - // version field in the portable keymaster lib, which is a bit misleading. - uint identity_version = 10; + // Pretend to be implemented in a trusted environment just so we can pass + // the VTS tests. Of course, this is a pretend-only game since hopefully no + // relying party is ever going to trust our batch key and those keys above + // it. + // + ::keymaster::PureSoftKeymasterContext context(KM_SECURITY_LEVEL_TRUSTED_ENVIRONMENT); + error = generate_attestation_from_EVP(key, swEnforced, hwEnforced, auth_set, context, - identity_version, *attestation_chain, - *attestation_signing_key, &cert_chain_out); + ::keymaster::kCurrentKeymasterVersion, *attestation_chain, + *attestation_signing_key, + "Android Identity Credential Key", &cert_chain_out); if (KM_ERROR_OK != error || !cert_chain_out) { LOG(ERROR) << "Error generate attestation from EVP key" << error; @@ -957,7 +1034,8 @@ optional>> createAttestation(const EVP_PKEY* key, } optional, vector>>> createEcKeyPairAndAttestation( - const vector& challenge, const vector& applicationId) { + const vector& challenge, const vector& applicationId, + bool isTestCredential) { auto ec_key = ::keymaster::EC_KEY_Ptr(EC_KEY_new()); auto pkey = ::keymaster::EVP_PKEY_Ptr(EVP_PKEY_new()); auto group = ::keymaster::EC_GROUP_Ptr(EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1)); @@ -978,12 +1056,11 @@ optional, vector>>> createEcKeyPairAnd return {}; } - uint64_t now = time(nullptr); - uint64_t secondsInOneYear = 365 * 24 * 60 * 60; - uint64_t expireTimeMs = (now + secondsInOneYear) * 1000; + uint64_t nowMs = time(nullptr) * 1000; + uint64_t expireTimeMs = 0; // Set to same as batch certificate - optional>> attestationCert = - createAttestation(pkey.get(), applicationId, challenge, now * 1000, expireTimeMs); + optional>> attestationCert = createAttestation( + pkey.get(), applicationId, challenge, nowMs, expireTimeMs, isTestCredential); if (!attestationCert) { LOG(ERROR) << "Error create attestation from key and challenge"; return {}; @@ -1031,14 +1108,12 @@ optional>> createAttestationForEcPublicKey( return {}; } - uint64_t now = (std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch()). - count()/ 1000000000); - uint64_t secondsInOneYear = 365 * 24 * 60 * 60; - uint64_t expireTimeMs = (now + secondsInOneYear) * 1000; + uint64_t nowMs = time(nullptr) * 1000; + uint64_t expireTimeMs = 0; // Set to same as batch certificate optional>> attestationCert = - createAttestation(pkey.get(), applicationId, challenge, now * 1000, expireTimeMs); + createAttestation(pkey.get(), applicationId, challenge, nowMs, expireTimeMs, + false /* isTestCredential */); if (!attestationCert) { LOG(ERROR) << "Error create attestation from key and challenge"; return {}; @@ -1652,6 +1727,32 @@ optional> certificateTbsCertificate(const vector& return std::make_pair(tbsCertificateOffset, tbsCertificateSize); } +optional> certificateGetValidity(const vector& x509Certificate) { + vector certs; + if (!parseX509Certificates(x509Certificate, certs)) { + LOG(ERROR) << "Error parsing certificates"; + return {}; + } + if (certs.size() < 1) { + LOG(ERROR) << "No certificates in chain"; + return {}; + } + + time_t notBefore; + time_t notAfter; + if (!parseAsn1Time(X509_get0_notBefore(certs[0].get()), ¬Before)) { + LOG(ERROR) << "Error parsing notBefore"; + return {}; + } + + if (!parseAsn1Time(X509_get0_notAfter(certs[0].get()), ¬After)) { + LOG(ERROR) << "Error parsing notAfter"; + return {}; + } + + return std::make_pair(notBefore, notAfter); +} + optional> certificateFindSignature(const vector& x509Certificate) { vector certs; if (!parseX509Certificates(x509Certificate, certs)) { @@ -2224,6 +2325,49 @@ optional> coseMacWithDigest(const vector& digestToBeMac // Utility functions specific to IdentityCredential. // --------------------------------------------------------------------------- +optional> calcEMacKey(const vector& privateKey, + const vector& publicKey, + const vector& sessionTranscriptBytes) { + optional> sharedSecret = support::ecdh(publicKey, privateKey); + if (!sharedSecret) { + LOG(ERROR) << "Error performing ECDH"; + return {}; + } + vector salt = support::sha256(sessionTranscriptBytes); + vector info = {'E', 'M', 'a', 'c', 'K', 'e', 'y'}; + optional> derivedKey = support::hkdf(sharedSecret.value(), salt, info, 32); + if (!derivedKey) { + LOG(ERROR) << "Error performing HKDF"; + return {}; + } + return derivedKey.value(); +} + +optional> calcMac(const vector& sessionTranscriptEncoded, + const string& docType, + const vector& deviceNameSpacesEncoded, + const vector& eMacKey) { + auto [sessionTranscriptItem, _, errMsg] = cppbor::parse(sessionTranscriptEncoded); + if (sessionTranscriptItem == nullptr) { + LOG(ERROR) << "Error parsing sessionTranscriptEncoded: " << errMsg; + return {}; + } + // The data that is MACed is ["DeviceAuthentication", sessionTranscript, docType, + // deviceNameSpacesBytes] so build up that structure + cppbor::Array deviceAuthentication = + cppbor::Array() + .add("DeviceAuthentication") + .add(std::move(sessionTranscriptItem)) + .add(docType) + .add(cppbor::Semantic(kSemanticTagEncodedCbor, deviceNameSpacesEncoded)); + vector deviceAuthenticationBytes = + cppbor::Semantic(kSemanticTagEncodedCbor, deviceAuthentication.encode()).encode(); + optional> calculatedMac = + support::coseMac0(eMacKey, {}, // payload + deviceAuthenticationBytes); // detached content + return calculatedMac; +} + vector> chunkVector(const vector& content, size_t maxChunkSize) { vector> ret; diff --git a/identity/support/tests/IdentityCredentialSupportTest.cpp b/identity/support/tests/IdentityCredentialSupportTest.cpp index c356549d00..266f263203 100644 --- a/identity/support/tests/IdentityCredentialSupportTest.cpp +++ b/identity/support/tests/IdentityCredentialSupportTest.cpp @@ -436,6 +436,300 @@ TEST(IdentityCredentialSupport, CoseMac0DetachedContent) { support::cborPrettyPrint(mac.value())); } +// Generates a private key in DER format for a small value of 'd'. +// +// Used for test vectors. +// +vector p256PrivateKeyFromD(uint8_t d) { + vector privateUncompressed; + privateUncompressed.resize(32); + privateUncompressed[31] = d; + optional> privateKey = support::ecPrivateKeyToKeyPair(privateUncompressed); + return privateKey.value(); +} + +std::pair, vector> p256PrivateKeyGetXandY( + const vector privateKey) { + optional> publicUncompressed = support::ecKeyPairGetPublicKey(privateKey); + vector x = vector(publicUncompressed.value().begin() + 1, + publicUncompressed.value().begin() + 33); + vector y = vector(publicUncompressed.value().begin() + 33, + publicUncompressed.value().begin() + 65); + return std::make_pair(x, y); +} + +const cppbor::Item* findValueForTstr(const cppbor::Map* map, const string& keyValue) { + // TODO: Need cast until libcppbor's Map::get() is marked as const + auto [item, found] = ((cppbor::Map*)map)->get(keyValue); + if (!found) { + return nullptr; + } + return item.get(); +} + +const cppbor::Array* findArrayValueForTstr(const cppbor::Map* map, const string& keyValue) { + const cppbor::Item* item = findValueForTstr(map, keyValue); + if (item == nullptr) { + return nullptr; + } + return item->asArray(); +} + +const cppbor::Map* findMapValueForTstr(const cppbor::Map* map, const string& keyValue) { + const cppbor::Item* item = findValueForTstr(map, keyValue); + if (item == nullptr) { + return nullptr; + } + return item->asMap(); +} + +const cppbor::Semantic* findSemanticValueForTstr(const cppbor::Map* map, const string& keyValue) { + const cppbor::Item* item = findValueForTstr(map, keyValue); + if (item == nullptr) { + return nullptr; + } + return item->asSemantic(); +} + +const std::string findStringValueForTstr(const cppbor::Map* map, const string& keyValue) { + const cppbor::Item* item = findValueForTstr(map, keyValue); + if (item == nullptr) { + return nullptr; + } + const cppbor::Tstr* tstr = item->asTstr(); + if (tstr == nullptr) { + return ""; + } + return tstr->value(); +} + +TEST(IdentityCredentialSupport, testVectors_18013_5) { + // This is a test against known vectors for ISO 18013-5. + // + // The objective of this test is to verify that support::calcEMacKey() and + // support::calcMac() agree with the given test vectors. + // + + // We're given static device key: + // + // x: 28412803729898893058558238221310261427084375743576167377786533380249859400145 + // y: 65403602826180996396520286939226973026599920614829401631985882360676038096704 + // d: 11 + // + vector deviceKey = p256PrivateKeyFromD(11); + auto [deviceKeyX, deviceKeyY] = p256PrivateKeyGetXandY(deviceKey); + EXPECT_EQ(support::encodeHex(deviceKeyX), + "3ed113b7883b4c590638379db0c21cda16742ed0255048bf433391d374bc21d1"); + EXPECT_EQ(support::encodeHex(deviceKeyY), + "9099209accc4c8a224c843afa4f4c68a090d04da5e9889dae2f8eefce82a3740"); + + // We're given Ephemeral reader key: + // + // x: 59535862115950685744176693329402396749019581632805653266809849538337418304154 + // y: 53776829996815113213100700404832701936765102413212294632483274374518863708344 + // d: 20 + // + vector ephemeralReaderKey = p256PrivateKeyFromD(20); + auto [ephemeralReaderKeyX, ephemeralReaderKeyY] = p256PrivateKeyGetXandY(ephemeralReaderKey); + EXPECT_EQ(support::encodeHex(ephemeralReaderKeyX), + "83a01a9378395bab9bcd6a0ad03cc56d56e6b19250465a94a234dc4c6b28da9a"); + EXPECT_EQ(support::encodeHex(ephemeralReaderKeyY), + "76e49b6de2f73234ae6a5eb9d612b75c9f2202bb6923f54ff8240aaa86f640b8"); + vector ephemeralReaderKeyPublic = + support::ecKeyPairGetPublicKey(ephemeralReaderKey).value(); + + // We're given SessionEstablishment. + // + // SessionEstablishment = { + // "eReaderKey" : EReaderKeyBytes, + // "data" : bstr ; Encrypted mdoc request + // } + // + // Fish out EReaderKey from this. + // + // Note that the test vector below is incorrect insofar that it uses + // "eReaderKeyBytes" instead of just "eReaderKey". This will be corrected in + // the future. + // + optional> sessionEstablishmentEncoded = support::decodeHex( + "a26f655265616465724b65794279746573d818584ba40102200121582083a01a9378395bab9bcd6a0ad03c" + "c56d56e6b19250465a94a234dc4c6b28da9a22582076e49b6de2f73234ae6a5eb9d612b75c9f2202bb6923" + "f54ff8240aaa86f640b864646174615902d945b31040c57491acb6d46a71f6c1f67a0b837df1bda9089fd0" + "3d0b1fdac3eeb2874a4ef6f90c97d03397186ba00a91102faae7e992e15f761d5662c3c37e3c6c2cfd2ebc" + "0bf59dbb8795e377bd7dd353230a41ba2d82294b45871a39b42ca531f26b52f46e356fbaf5075c8fd5b8b0" + "8a0df4a1d2e1bdd2e5d69169c1efbb51e393e608d833d325bebfbccb2e15ec08f94b264582fa7b93f7cebc" + "aa69f4f0cac2744d4fe35b04df26b2ae69273eed33024949080c1c95a6ef046beede959e9494297dd770af" + "4ac6fdd56783aa012555c213dc05cf0f41d1c95119720fcfe1621027f80e2ddd56ea3c1fc596f7b2579333" + "5a887ec788092b4a69d23b6219e27d0249b50b3fdcb95b5227007689362e0416b3bae3dae7cb56b4394666" + "4e3a3f60dce8d0b678fcd754bebf87bd2b0278dd782d952488a46f2874e34c2dd97bb74084a62b850e9719" + "252cd1dca7dbf1858193f6cf093cb3735312bbe1138cf29d8f350e285923f8ef07065299926720b42264e8" + "fd5d4b133e72f47c4e999ea689c353f8b41e50a59838e1a0d09eca4a557f77a9c389a0591ad1639119ce86" + "edc3320130480ee5101effae6066e8c85aac9ead2ae83e49c1e508aab02f753decbb522ea2200d62fd5d26" + "094bd35100bffaa1cdc6af9f7e9cfe7b63da6b5671cd5ac2cf5da450c72addc64cde441f3b7f7fdaf930ad" + "1e13388e8a7308d8ca4607e59e082db431a232e7e12cb692baeb4b2127e110ff24cea322ffdbc2e4d9c4c6" + "bed27753137d07897c8613627a799a560cf1a2d1edb3de029442862940a5ed7785eea8b6ace93aa6af0792" + "fd82877f62d07b757d0179ecbb7347004ecc9c0690d41f75f188cb17ffd2cec2ad8c9675466bb33b737a2a" + "e7592b2dcb8132aced2e572266f3f5413a5f9d6d4339a1e4662622af2e7e157a4ea3bfd5c4247e2ec91d8c" + "5c3c17427d5edfae673d0e0f782a8d40fa805fd8bc82ae3cb21a65cdad863e02309f6b01d1753fa884b778" + "f6e019a2004d8964deeb11f1fd478fcb"); + ASSERT_TRUE(sessionEstablishmentEncoded); + auto [sessionEstablishmentItem, _se, _se2] = cppbor::parse(sessionEstablishmentEncoded.value()); + const cppbor::Map* sessionEstablishment = sessionEstablishmentItem->asMap(); + ASSERT_NE(sessionEstablishment, nullptr); + const cppbor::Semantic* eReaderKeyBytes = + findSemanticValueForTstr(sessionEstablishment, "eReaderKeyBytes"); + ASSERT_NE(eReaderKeyBytes, nullptr); + ASSERT_EQ(eReaderKeyBytes->value(), 24); + const cppbor::Bstr* eReaderKeyBstr = eReaderKeyBytes->child()->asBstr(); + ASSERT_NE(eReaderKeyBstr, nullptr); + vector eReaderKeyEncoded = eReaderKeyBstr->value(); + // TODO: verify this agrees with ephemeralReaderKeyX and ephemeralReaderKeyY + + // We're given DeviceEngagement. + // + vector deviceEngagementEncoded = + support::decodeHex( + "a20063312e30018201d818584ba401022001215820cef66d6b2a3a993e591214d1ea223fb545ca" + "6c471c48306e4c36069404c5723f225820878662a229aaae906e123cdd9d3b4c10590ded29fe75" + "1eeeca34bbaa44af0773") + .value(); + + // Now calculate SessionTranscriptBytes. It is defined as + // + // SessionTranscript = [ + // DeviceEngagementBytes, + // EReaderKeyBytes, + // Handover + // ] + // + // SessionTranscriptBytes = #6.24(bstr .cbor SessionTranscript) + // + cppbor::Array sessionTranscript; + sessionTranscript.add(cppbor::Semantic(24, deviceEngagementEncoded)); + sessionTranscript.add(cppbor::Semantic(24, eReaderKeyEncoded)); + sessionTranscript.add(cppbor::Null()); + vector sessionTranscriptEncoded = sessionTranscript.encode(); + vector sessionTranscriptBytes = + cppbor::Semantic(24, sessionTranscriptEncoded).encode(); + + // The expected EMacKey is 4c1ebb8aacc633465390fa44edfdb49cb57f2e079aaa771d812584699c0b97e2 + // + // Verify that support::calcEMacKey() gets the same result. + // + optional> eMacKey = + support::calcEMacKey(support::ecKeyPairGetPrivateKey(deviceKey).value(), // private key + ephemeralReaderKeyPublic, // public key + sessionTranscriptBytes); // sessionTranscriptBytes + ASSERT_TRUE(eMacKey); + ASSERT_EQ(support::encodeHex(eMacKey.value()), + "4c1ebb8aacc633465390fa44edfdb49cb57f2e079aaa771d812584699c0b97e2"); + + // Also do it the other way around + // + optional> eMacKey2 = support::calcEMacKey( + support::ecKeyPairGetPrivateKey(ephemeralReaderKey).value(), // private key + support::ecKeyPairGetPublicKey(deviceKey).value(), // public key + sessionTranscriptBytes); // sessionTranscriptBytes + ASSERT_TRUE(eMacKey2); + ASSERT_EQ(support::encodeHex(eMacKey2.value()), + "4c1ebb8aacc633465390fa44edfdb49cb57f2e079aaa771d812584699c0b97e2"); + + // We're given DeviceResponse + // + vector deviceResponseEncoded = + support::decodeHex( + "a36776657273696f6e63312e3069646f63756d656e747381a367646f6354797065756f72672e69" + "736f2e31383031332e352e312e6d444c6c6973737565725369676e6564a26a6e616d6553706163" + "6573a2716f72672e69736f2e31383031332e352e3181d8185863a4686469676573744944016672" + "616e646f6d58208798645b20ea200e19ffabac92624bee6aec63aceedecfb1b80077d22bfc20e9" + "71656c656d656e744964656e7469666965726b66616d696c795f6e616d656c656c656d656e7456" + "616c756563446f656b636f6d2e6578616d706c6581d8185864a468646967657374494401667261" + "6e646f6d5820218ecf13521b53f4b96abaebe56417afec0e4c91fc8fb26086cd1e5cdc1a94ff71" + "656c656d656e744964656e7469666965726f616e6f746865725f656c656d656e746c656c656d65" + "6e7456616c75650a6a697373756572417574688443a10126a118215901d2308201ce30820174a0" + "0302010202141f7d44f4f107c5ee3f566049cf5d72de294b0d23300a06082a8648ce3d04030230" + "233114301206035504030c0b75746f7069612069616361310b3009060355040613025553301e17" + "0d3230313030313030303030305a170d3231313030313030303030305a30213112301006035504" + "030c0975746f706961206473310b30090603550406130255533059301306072a8648ce3d020106" + "082a8648ce3d03010703420004301d9e502dc7e05da85da026a7ae9aa0fac9db7d52a95b3e3e3f" + "9aa0a1b45b8b6551b6f6b3061223e0d23c026b017d72298d9ae46887ca61d58db6aea17ee267a3" + "8187308184301e0603551d120417301581136578616d706c65406578616d706c652e636f6d301c" + "0603551d1f041530133011a00fa00d820b6578616d706c652e636f6d301d0603551d0e04160414" + "7bef4db59a1ffb07592bfc57f4743b8a73aea792300e0603551d0f0101ff040403020780301506" + "03551d250101ff040b3009060728818c5d050102300a06082a8648ce3d04030203480030450220" + "21d52fb1fbda80e5bfda1e8dfb1bc7bf0acb7261d5c9ff54425af76eb21571c602210082bf301f" + "89e0a2cb9ca9c9050352de80b47956764f7a3e07bf6a8cd87528a3b55901d2d8185901cda66776" + "657273696f6e63312e306f646967657374416c676f726974686d675348412d3235366c76616c75" + "6544696765737473a2716f72672e69736f2e31383031332e352e31a20058203b22af1126771f02" + "f0ea0d546d4ee3c5b51637381154f5211b79daf5f9facaa8015820f2cba0ce3cde5df901a3da75" + "13a4d7f7225fdfe5a306544529bf3dbcce655ca06b636f6d2e6578616d706c65a200582072636d" + "ddc282424a63499f4b3927aaa3b74da7b9c0134178bf735e949e4a761e01582006322d3cbe6603" + "876bdacc5b6679b51b0fc53d029c244fd5ea719d9028459c916d6465766963654b6579496e666f" + "a1696465766963654b6579a4010220012158203ed113b7883b4c590638379db0c21cda16742ed0" + "255048bf433391d374bc21d12258209099209accc4c8a224c843afa4f4c68a090d04da5e9889da" + "e2f8eefce82a374067646f6354797065756f72672e69736f2e31383031332e352e312e6d444c6c" + "76616c6964697479496e666fa3667369676e6564c074323032302d31302d30315431333a33303a" + "30325a6976616c696446726f6dc074323032302d31302d30315431333a33303a30325a6a76616c" + "6964556e74696cc074323032312d31302d30315431333a33303a30325a5840273ec1b59817d571" + "b5a2c5c0ab0ea213d42acb18547fd7097afcc888a22ecbb863c6461ce0e240880895b4aaa84308" + "784571c7be7aa3a2e7e3a2ea1a145ed1966c6465766963655369676e6564a26a6e616d65537061" + "636573d81841a06a64657669636541757468a1696465766963654d61638443a10105a0f6582009" + "da7c964ac004ec36ec64edd0c1abf50c03433c215c3ddb144768abcdf20a60667374617475730" + "0") + .value(); + auto [deviceResponseItem, _, _2] = cppbor::parse(deviceResponseEncoded); + const cppbor::Map* deviceResponse = deviceResponseItem->asMap(); + ASSERT_NE(deviceResponse, nullptr); + const cppbor::Array* documents = findArrayValueForTstr(deviceResponse, "documents"); + ASSERT_NE(documents, nullptr); + ASSERT_EQ(documents->size(), 1); + const cppbor::Map* document = ((*documents)[0])->asMap(); + ASSERT_NE(document, nullptr); + + // Get docType + string docType = findStringValueForTstr(document, "docType"); + ASSERT_EQ(docType, "org.iso.18013.5.1.mDL"); + + // Drill down... + const cppbor::Map* deviceSigned = findMapValueForTstr(document, "deviceSigned"); + ASSERT_NE(deviceSigned, nullptr); + + // Dig out the encoded form of DeviceNameSpaces + // + const cppbor::Semantic* deviceNameSpacesBytes = + findSemanticValueForTstr(deviceSigned, "nameSpaces"); + ASSERT_NE(deviceNameSpacesBytes, nullptr); + ASSERT_EQ(deviceNameSpacesBytes->value(), 24); + const cppbor::Bstr* deviceNameSpacesBstr = deviceNameSpacesBytes->child()->asBstr(); + ASSERT_NE(deviceNameSpacesBstr, nullptr); + vector deviceNameSpacesEncoded = deviceNameSpacesBstr->value(); + + // (For this version of 18013-5, DeviceNameSpaces is always supposed to be empty, check that.) + EXPECT_EQ(deviceNameSpacesEncoded, cppbor::Map().encode()); + + const cppbor::Map* deviceAuth = findMapValueForTstr(deviceSigned, "deviceAuth"); + ASSERT_NE(deviceAuth, nullptr); + // deviceMac is is the COSE_Mac0.. dig out the encoded form to check that + // support::calcMac() gives exactly the same bytes. + // + const cppbor::Array* deviceMac = findArrayValueForTstr(deviceAuth, "deviceMac"); + ASSERT_NE(deviceMac, nullptr); + vector deviceMacEncoded = deviceMac->encode(); + + // Now we calculate what it should be.. + optional> calculatedMac = + support::calcMac(sessionTranscriptEncoded, // SessionTranscript + docType, // DocType + deviceNameSpacesEncoded, // DeviceNamespaces + eMacKey.value()); // EMacKey + ASSERT_TRUE(calculatedMac); + + // ... and hopefully it's the same! + ASSERT_EQ(calculatedMac.value().size(), deviceMacEncoded.size()); + EXPECT_TRUE(memcmp(calculatedMac.value().data(), deviceMacEncoded.data(), + deviceMacEncoded.size()) == 0); +} + } // namespace identity } // namespace hardware } // namespace android diff --git a/keymaster/4.1/support/attestation_record.cpp b/keymaster/4.1/support/attestation_record.cpp index 598b6b50c4..207a7e8264 100644 --- a/keymaster/4.1/support/attestation_record.cpp +++ b/keymaster/4.1/support/attestation_record.cpp @@ -102,6 +102,7 @@ typedef struct km_auth_list { ASN1_INTEGER* boot_patchlevel; ASN1_NULL* early_boot_only; ASN1_NULL* device_unique_attestation; + ASN1_NULL* identity_credential_key; } KM_AUTH_LIST; ASN1_SEQUENCE(KM_AUTH_LIST) = { @@ -145,6 +146,8 @@ ASN1_SEQUENCE(KM_AUTH_LIST) = { ASN1_EXP_OPT(KM_AUTH_LIST, early_boot_only, ASN1_NULL, TAG_EARLY_BOOT_ONLY.maskedTag()), ASN1_EXP_OPT(KM_AUTH_LIST, device_unique_attestation, ASN1_NULL, TAG_DEVICE_UNIQUE_ATTESTATION.maskedTag()), + ASN1_EXP_OPT(KM_AUTH_LIST, identity_credential_key, ASN1_NULL, + TAG_IDENTITY_CREDENTIAL_KEY.maskedTag()), } ASN1_SEQUENCE_END(KM_AUTH_LIST); IMPLEMENT_ASN1_FUNCTIONS(KM_AUTH_LIST); @@ -285,6 +288,7 @@ static ErrorCode extract_auth_list(const KM_AUTH_LIST* record, AuthorizationSet* copyAuthTag(record->unlocked_device_required, TAG_UNLOCKED_DEVICE_REQUIRED, auth_list); copyAuthTag(record->early_boot_only, TAG_EARLY_BOOT_ONLY, auth_list); copyAuthTag(record->device_unique_attestation, TAG_DEVICE_UNIQUE_ATTESTATION, auth_list); + copyAuthTag(record->identity_credential_key, TAG_IDENTITY_CREDENTIAL_KEY, auth_list); return ErrorCode::OK; } @@ -327,7 +331,10 @@ std::tuple parse_attestation_record(const hidl_vec p = attest_rec->data; KM_KEY_DESCRIPTION_Ptr record(d2i_KM_KEY_DESCRIPTION(nullptr, &p, attest_rec->length)); - if (!record.get()) return {ErrorCode::UNKNOWN_ERROR, {}}; + if (!record.get()) { + LOG(ERROR) << "Unable to get key description"; + return {ErrorCode::UNKNOWN_ERROR, {}}; + } AttestationRecord result; @@ -352,10 +359,12 @@ std::tuple parse_attestation_record(const hidl_vec if (error != ErrorCode::OK) return {error, {}}; KM_ROOT_OF_TRUST* root_of_trust = nullptr; + SecurityLevel root_of_trust_security_level = SecurityLevel::TRUSTED_ENVIRONMENT; if (record->tee_enforced && record->tee_enforced->root_of_trust) { root_of_trust = record->tee_enforced->root_of_trust; } else if (record->software_enforced && record->software_enforced->root_of_trust) { root_of_trust = record->software_enforced->root_of_trust; + root_of_trust_security_level = SecurityLevel::SOFTWARE; } else { LOG(ERROR) << AT << " Failed root of trust parsing"; return {ErrorCode::INVALID_ARGUMENT, {}}; @@ -373,6 +382,7 @@ std::tuple parse_attestation_record(const hidl_vec rot.verified_boot_state = static_cast( ASN1_ENUMERATED_get(root_of_trust->verified_boot_state)); rot.device_locked = root_of_trust->device_locked; + rot.security_level = root_of_trust_security_level; auto& vb_hash = root_of_trust->verified_boot_hash; if (!vb_hash) { From de05fd53f46fc7f8419eeb6007e43913ea394e9a Mon Sep 17 00:00:00 2001 From: Yin-Chia Yeh Date: Wed, 2 Dec 2020 14:38:19 -0800 Subject: [PATCH 12/13] Camera: fix HAL1 removeCamera crash Test: partner testing Bug: 173511749 Merged-In: Ifd5fc7c63e3835945194291d161b491bd5acb342 Change-Id: Ifd5fc7c63e3835945194291d161b491bd5acb342 --- camera/common/1.0/default/CameraModule.cpp | 33 +++++++++++++--------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/camera/common/1.0/default/CameraModule.cpp b/camera/common/1.0/default/CameraModule.cpp index 86f26e480c..27e74f1480 100644 --- a/camera/common/1.0/default/CameraModule.cpp +++ b/camera/common/1.0/default/CameraModule.cpp @@ -529,24 +529,29 @@ status_t CameraModule::filterOpenErrorCode(status_t err) { } void CameraModule::removeCamera(int cameraId) { - std::unordered_set physicalIds; - camera_metadata_t *metadata = const_cast( - mCameraInfoMap.valueFor(cameraId).static_camera_characteristics); - common::V1_0::helper::CameraMetadata hidlMetadata(metadata); + // Skip HAL1 devices which isn't cached in mCameraInfoMap and don't advertise + // static_camera_characteristics + if (getDeviceVersion(cameraId) >= CAMERA_DEVICE_API_VERSION_3_0) { + std::unordered_set physicalIds; + camera_metadata_t *metadata = const_cast( + mCameraInfoMap.valueFor(cameraId).static_camera_characteristics); + common::V1_0::helper::CameraMetadata hidlMetadata(metadata); - if (isLogicalMultiCamera(hidlMetadata, &physicalIds)) { - for (const auto& id : physicalIds) { - int idInt = std::stoi(id); - if (mPhysicalCameraInfoMap.indexOfKey(idInt) >= 0) { - free_camera_metadata(mPhysicalCameraInfoMap[idInt]); - mPhysicalCameraInfoMap.removeItem(idInt); - } else { - ALOGE("%s: Cannot find corresponding static metadata for physical id %s", - __FUNCTION__, id.c_str()); + if (isLogicalMultiCamera(hidlMetadata, &physicalIds)) { + for (const auto& id : physicalIds) { + int idInt = std::stoi(id); + if (mPhysicalCameraInfoMap.indexOfKey(idInt) >= 0) { + free_camera_metadata(mPhysicalCameraInfoMap[idInt]); + mPhysicalCameraInfoMap.removeItem(idInt); + } else { + ALOGE("%s: Cannot find corresponding static metadata for physical id %s", + __FUNCTION__, id.c_str()); + } } } + free_camera_metadata(metadata); } - free_camera_metadata(metadata); + mCameraInfoMap.removeItem(cameraId); mDeviceVersionMap.removeItem(cameraId); } From cf0e945ac2a3014499613cc983e527d66e8cac73 Mon Sep 17 00:00:00 2001 From: Changyeon Jo Date: Wed, 9 Dec 2020 17:32:29 -0800 Subject: [PATCH 13/13] Correct EVS VTS test case This change updates CameraOpenClean test case to handle logical camera devices properly when it attempts to set and get an extended info. Fix: 175260026 Test: atest VtsHalEvsV1_1TargetTest Change-Id: Ie18436afa722dd24abcaa1a4e0180955827ee499 --- .../functional/VtsHalEvsV1_1TargetTest.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp b/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp index 948d45ff85..f1c8f9f6c7 100644 --- a/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp +++ b/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp @@ -305,11 +305,22 @@ TEST_P(EvsHidlTest, CameraOpenClean) { const auto id = 0xFFFFFFFF; // meaningless id hidl_vec values; auto err = pCam->setExtendedInfo_1_1(id, values); - ASSERT_NE(EvsResult::INVALID_ARG, err); + if (isLogicalCam) { + // Logical camera device does not support setExtendedInfo + // method. + ASSERT_EQ(EvsResult::INVALID_ARG, err); + } else { + ASSERT_NE(EvsResult::INVALID_ARG, err); + } - pCam->getExtendedInfo_1_1(id, [](const auto& result, const auto& data) { - ASSERT_NE(EvsResult::INVALID_ARG, result); - ASSERT_EQ(0, data.size()); + + pCam->getExtendedInfo_1_1(id, [&isLogicalCam](const auto& result, const auto& data) { + if (isLogicalCam) { + ASSERT_EQ(EvsResult::INVALID_ARG, result); + } else { + ASSERT_NE(EvsResult::INVALID_ARG, result); + ASSERT_EQ(0, data.size()); + } }); // Explicitly close the camera so resources are released right away