From fdbb904a0c0994a67e1bf9224325434188ae303d Mon Sep 17 00:00:00 2001 From: Max Bires Date: Tue, 23 Mar 2021 12:43:38 -0700 Subject: [PATCH] IRemotelyProvisionedComponent returns DeviceInfo This alters the AIDL interface and underlying functionality to have the component return the DeviceInfo CBOR blob that is used as AAD in verification of a signature over a MAC key. Trying to reconstruct this from userspace is very likely to lead to pain and subtle errors in the future as adoption of this HAL increases, and multiple instances of this HAL may exist on device simultaneously. Test: atest VtsRemotelyProvisionedComponentTests Change-Id: I44bd588586652630ed31a87cfda7e9c01cbf0a2f --- .../hardware/security/keymint/DeviceInfo.aidl | 39 +++++++++++++++ .../IRemotelyProvisionedComponent.aidl | 2 +- .../hardware/security/keymint/DeviceInfo.aidl | 47 +++++++++++++++++++ .../IRemotelyProvisionedComponent.aidl | 5 +- .../default/RemotelyProvisionedComponent.cpp | 7 +-- .../default/RemotelyProvisionedComponent.h | 4 +- .../VtsRemotelyProvisionedComponentTests.cpp | 36 ++++++++------ 7 files changed, 118 insertions(+), 22 deletions(-) create mode 100644 security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/DeviceInfo.aidl create mode 100644 security/keymint/aidl/android/hardware/security/keymint/DeviceInfo.aidl diff --git a/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/DeviceInfo.aidl b/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/DeviceInfo.aidl new file mode 100644 index 0000000000..d04d49cea8 --- /dev/null +++ b/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/DeviceInfo.aidl @@ -0,0 +1,39 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL file. Do not edit it manually. There are +// two cases: +// 1). this is a frozen version file - do not edit this in any case. +// 2). this is a 'current' file. If you make a backwards compatible change to +// the interface (from the latest frozen version), the build system will +// prompt you to update this file with `m -update-api`. +// +// You must not make a backward incompatible change to any AIDL file built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.hardware.security.keymint; +/* @hide */ +@VintfStability +parcelable DeviceInfo { + byte[] deviceInfo; +} diff --git a/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl b/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl index 8387ecc9dd..da150628b7 100644 --- a/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl +++ b/security/keymint/aidl/aidl_api/android.hardware.security.keymint/current/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl @@ -35,7 +35,7 @@ package android.hardware.security.keymint; @VintfStability interface IRemotelyProvisionedComponent { byte[] generateEcdsaP256KeyPair(in boolean testMode, out android.hardware.security.keymint.MacedPublicKey macedPublicKey); - void generateCertificateRequest(in boolean testMode, in android.hardware.security.keymint.MacedPublicKey[] keysToSign, in byte[] endpointEncryptionCertChain, in byte[] challenge, out byte[] keysToSignMac, out android.hardware.security.keymint.ProtectedData protectedData); + byte[] generateCertificateRequest(in boolean testMode, in android.hardware.security.keymint.MacedPublicKey[] keysToSign, in byte[] endpointEncryptionCertChain, in byte[] challenge, out android.hardware.security.keymint.DeviceInfo deviceInfo, out android.hardware.security.keymint.ProtectedData protectedData); const int STATUS_FAILED = 1; const int STATUS_INVALID_MAC = 2; const int STATUS_PRODUCTION_KEY_IN_TEST_REQUEST = 3; diff --git a/security/keymint/aidl/android/hardware/security/keymint/DeviceInfo.aidl b/security/keymint/aidl/android/hardware/security/keymint/DeviceInfo.aidl new file mode 100644 index 0000000000..3ea14a1b97 --- /dev/null +++ b/security/keymint/aidl/android/hardware/security/keymint/DeviceInfo.aidl @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.security.keymint; + +/** + * DeviceInfo contains information about the device that's fed in as AAD in the signature of the + * device private key over the MAC key used for the bundle of public keys. These values are intended + * to be checked by the server to verify that the certificate signing request crafted by + * an IRemotelyProvisionedComponent HAL instance is coming from the expected device based + * on values initially uploaded during device manufacture in the factory. + * @hide + */ +@VintfStability +parcelable DeviceInfo { + /** + * DeviceInfo is a CBOR Map structure described by the following CDDL. + * + * DeviceInfo = { + * ? "brand" : tstr, + * ? "manufacturer" : tstr, + * ? "product" : tstr, + * ? "model" : tstr, + * ? "board" : tstr, + * ? "vb_state" : "green" / "yellow" / "orange", // Taken from the AVB values + * ? "bootloader_state" : "locked" / "unlocked", // Taken from the AVB values + * ? "os_version" : tstr, // Same as android.os.Build.VERSION.release + * ? "system_patch_level" : uint, // YYYYMMDD + * ? "boot_patch_level" : uint, // YYYYMMDD + * ? "vendor_patch_level" : uint, // YYYYMMDD + * } + */ + byte[] deviceInfo; +} diff --git a/security/keymint/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl b/security/keymint/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl index 327e4a1e5d..7d749e3a37 100644 --- a/security/keymint/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl +++ b/security/keymint/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl @@ -16,6 +16,7 @@ package android.hardware.security.keymint; +import android.hardware.security.keymint.DeviceInfo; import android.hardware.security.keymint.MacedPublicKey; import android.hardware.security.keymint.ProtectedData; @@ -256,7 +257,7 @@ interface IRemotelyProvisionedComponent { * @param out ProtectedData contains the encrypted BCC and the ephemeral MAC key used to * authenticate the keysToSign (see keysToSignMac output argument). */ - void generateCertificateRequest(in boolean testMode, in MacedPublicKey[] keysToSign, - in byte[] endpointEncryptionCertChain, in byte[] challenge, out byte[] keysToSignMac, + byte[] generateCertificateRequest(in boolean testMode, in MacedPublicKey[] keysToSign, + in byte[] endpointEncryptionCertChain, in byte[] challenge, out DeviceInfo deviceInfo, out ProtectedData protectedData); } diff --git a/security/keymint/aidl/default/RemotelyProvisionedComponent.cpp b/security/keymint/aidl/default/RemotelyProvisionedComponent.cpp index 749f0bc15a..4dbaa05d54 100644 --- a/security/keymint/aidl/default/RemotelyProvisionedComponent.cpp +++ b/security/keymint/aidl/default/RemotelyProvisionedComponent.cpp @@ -322,8 +322,8 @@ ScopedAStatus RemotelyProvisionedComponent::generateEcdsaP256KeyPair(bool testMo ScopedAStatus RemotelyProvisionedComponent::generateCertificateRequest( bool testMode, const vector& keysToSign, - const bytevec& endpointEncCertChain, const bytevec& challenge, bytevec* keysToSignMac, - ProtectedData* protectedData) { + const bytevec& endpointEncCertChain, const bytevec& challenge, DeviceInfo* deviceInfo, + ProtectedData* protectedData, bytevec* keysToSignMac) { auto pubKeysToSign = validateAndExtractPubkeys(testMode, keysToSign, testMode ? remote_prov::kTestMacKey : macKey_); if (!pubKeysToSign.isOk()) return pubKeysToSign.moveError(); @@ -343,11 +343,12 @@ ScopedAStatus RemotelyProvisionedComponent::generateCertificateRequest( bcc = bcc_.clone(); } + deviceInfo->deviceInfo = createDeviceInfo(); auto signedMac = constructCoseSign1(devicePrivKey /* Signing key */, // ephemeralMacKey /* Payload */, cppbor::Array() /* AAD */ .add(challenge) - .add(createDeviceInfo()) + .add(deviceInfo->deviceInfo) .encode()); if (!signedMac) return Status(signedMac.moveMessage()); diff --git a/security/keymint/aidl/default/RemotelyProvisionedComponent.h b/security/keymint/aidl/default/RemotelyProvisionedComponent.h index e8d2343091..65b1bbc87e 100644 --- a/security/keymint/aidl/default/RemotelyProvisionedComponent.h +++ b/security/keymint/aidl/default/RemotelyProvisionedComponent.h @@ -39,8 +39,8 @@ class RemotelyProvisionedComponent : public BnRemotelyProvisionedComponent { const std::vector& keysToSign, const std::vector& endpointEncCertChain, const std::vector& challenge, - std::vector* keysToSignMac, - ProtectedData* protectedData) override; + DeviceInfo* deviceInfo, ProtectedData* protectedData, + std::vector* keysToSignMac) override; private: // TODO(swillden): Move these into an appropriate Context class. diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp index 50e6cceb4c..9b797de802 100644 --- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp +++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp @@ -227,11 +227,12 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests { TEST_P(CertificateRequestTest, EmptyRequest_testMode) { bool testMode = true; bytevec keysToSignMac; + DeviceInfo deviceInfo; ProtectedData protectedData; auto challenge = randomBytes(32); - auto status = provisionable_->generateCertificateRequest(testMode, {} /* keysToSign */, - eekChain_.chain, challenge, - &keysToSignMac, &protectedData); + auto status = provisionable_->generateCertificateRequest( + testMode, {} /* keysToSign */, eekChain_.chain, challenge, &deviceInfo, &protectedData, + &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); auto [parsedProtectedData, _, protDataErrMsg] = cppbor::parse(protectedData.protectedData); @@ -297,11 +298,12 @@ TEST_P(CertificateRequestTest, EmptyRequest_testMode) { TEST_P(CertificateRequestTest, EmptyRequest_prodMode) { bool testMode = false; bytevec keysToSignMac; + DeviceInfo deviceInfo; ProtectedData protectedData; auto challenge = randomBytes(32); - auto status = provisionable_->generateCertificateRequest(testMode, {} /* keysToSign */, - eekChain_.chain, challenge, - &keysToSignMac, &protectedData); + auto status = provisionable_->generateCertificateRequest( + testMode, {} /* keysToSign */, eekChain_.chain, challenge, &deviceInfo, &protectedData, + &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_EEK); } @@ -314,10 +316,12 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_testMode) { generateKeys(testMode, 4 /* numKeys */); bytevec keysToSignMac; + DeviceInfo deviceInfo; ProtectedData protectedData; auto challenge = randomBytes(32); - auto status = provisionable_->generateCertificateRequest( - testMode, keysToSign_, eekChain_.chain, challenge, &keysToSignMac, &protectedData); + auto status = provisionable_->generateCertificateRequest(testMode, keysToSign_, eekChain_.chain, + challenge, &deviceInfo, &protectedData, + &keysToSignMac); ASSERT_TRUE(status.isOk()) << status.getMessage(); auto [parsedProtectedData, _, protDataErrMsg] = cppbor::parse(protectedData.protectedData); @@ -384,10 +388,12 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_prodMode) { generateKeys(testMode, 4 /* numKeys */); bytevec keysToSignMac; + DeviceInfo deviceInfo; ProtectedData protectedData; auto challenge = randomBytes(32); - auto status = provisionable_->generateCertificateRequest( - testMode, keysToSign_, eekChain_.chain, challenge, &keysToSignMac, &protectedData); + auto status = provisionable_->generateCertificateRequest(testMode, keysToSign_, eekChain_.chain, + challenge, &deviceInfo, &protectedData, + &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_INVALID_EEK); } @@ -400,11 +406,12 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_prodKeyInTestCert) { generateKeys(false /* testMode */, 2 /* numKeys */); bytevec keysToSignMac; + DeviceInfo deviceInfo; ProtectedData protectedData; auto challenge = randomBytes(32); - auto status = provisionable_->generateCertificateRequest(true /* testMode */, keysToSign_, - eekChain_.chain, challenge, - &keysToSignMac, &protectedData); + auto status = provisionable_->generateCertificateRequest( + true /* testMode */, keysToSign_, eekChain_.chain, challenge, &deviceInfo, + &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_PRODUCTION_KEY_IN_TEST_REQUEST); @@ -418,10 +425,11 @@ TEST_P(CertificateRequestTest, NonEmptyRequest_testKeyInProdCert) { generateKeys(true /* testMode */, 2 /* numKeys */); bytevec keysToSignMac; + DeviceInfo deviceInfo; ProtectedData protectedData; auto status = provisionable_->generateCertificateRequest( false /* testMode */, keysToSign_, eekChain_.chain, randomBytes(32) /* challenge */, - &keysToSignMac, &protectedData); + &deviceInfo, &protectedData, &keysToSignMac); ASSERT_FALSE(status.isOk()); ASSERT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_TEST_KEY_IN_PRODUCTION_REQUEST);