From e35797ffcae45788f80e0705ef8187a9b1ce74d1 Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Thu, 27 Feb 2020 14:25:54 -0500 Subject: [PATCH] Identity: Move signingKeyBlob from finishRetrieval() to startRetrieval(). The implementation of the Identity Credential TA in constrained environments may need to incrementally update the HMAC-SHA256 of DeviceAuthencation CBOR to avoid keeping the entire CBOR structure in memory. To do this they need to calculate the derived key before starting to build the CBOR so they need access to the signingKey earlier on. Bug: 150390415 Test: atest android.security.identity.cts Test: VtsHalIdentityTargetTest Change-Id: I72ad30ec3ccec0b8161cbea360ef8c9212f8cbbc --- .../hardware/identity/IIdentityCredential.aidl | 13 ++++++------- identity/aidl/default/IdentityCredential.cpp | 15 +++++++-------- identity/aidl/default/IdentityCredential.h | 7 ++++--- identity/aidl/vts/VtsHalIdentityTargetTest.cpp | 16 ++++++++-------- 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/identity/aidl/android/hardware/identity/IIdentityCredential.aidl b/identity/aidl/android/hardware/identity/IIdentityCredential.aidl index 10ce4c2867..cc142713cd 100644 --- a/identity/aidl/android/hardware/identity/IIdentityCredential.aidl +++ b/identity/aidl/android/hardware/identity/IIdentityCredential.aidl @@ -176,6 +176,10 @@ interface IIdentityCredential { * @param itemsRequest * If non-empty, contains request data that is signed by the reader. See above. * + * @param signingKeyBlob is either empty or a signingKeyBlob (see generateSigningKeyPair(), + * below) containing the signing key to use to sign the data retrieved. If this + * is not in the right format the call fails with STATUS_INVALID_DATA. + * * @param sessionTranscript * Either empty or the CBOR of the SessionTranscript. See above. * @@ -195,8 +199,7 @@ interface IIdentityCredential { * and remove the corresponding requests from the counts. */ void startRetrieval(in SecureAccessControlProfile[] accessControlProfiles, - in HardwareAuthToken authToken, - in byte[] itemsRequest, + in HardwareAuthToken authToken, in byte[] itemsRequest, in byte[] signingKeyBlob, in byte[] sessionTranscript, in byte[] readerSignature, in int[] requestCounts); /** @@ -254,10 +257,6 @@ interface IIdentityCredential { * If signingKeyBlob or the sessionTranscript parameter passed to startRetrieval() is * empty then the returned MAC will be empty. * - * @param signingKeyBlob is either empty or a signingKeyBlob (see generateSigningKeyPair(), - * below) containing the signing key to use to sign the data retrieved. If this - * is not in the right format the call fails with STATUS_INVALID_DATA. - * * @param out mac is empty if signingKeyBlob or the sessionTranscript passed to * startRetrieval() is empty. Otherwise it is a COSE_Mac0 with empty payload * and the detached content is set to DeviceAuthentication as defined below. @@ -304,7 +303,7 @@ interface IIdentityCredential { * * @param out deviceNameSpaces the bytes of DeviceNameSpaces. */ - void finishRetrieval(in byte[] signingKeyBlob, out byte[] mac, out byte[] deviceNameSpaces); + void finishRetrieval(out byte[] mac, out byte[] deviceNameSpaces); /** * Generate a key pair to be used for signing session data and retrieved data items. diff --git a/identity/aidl/default/IdentityCredential.cpp b/identity/aidl/default/IdentityCredential.cpp index d5b3a0ffee..341fae6e93 100644 --- a/identity/aidl/default/IdentityCredential.cpp +++ b/identity/aidl/default/IdentityCredential.cpp @@ -256,8 +256,8 @@ bool checkUserAuthentication(const SecureAccessControlProfile& profile, ndk::ScopedAStatus IdentityCredential::startRetrieval( const vector& accessControlProfiles, const HardwareAuthToken& authToken, const vector& itemsRequestS, - const vector& sessionTranscriptS, const vector& readerSignatureS, - const vector& requestCounts) { + const vector& signingKeyBlobS, const vector& sessionTranscriptS, + const vector& readerSignatureS, const vector& requestCounts) { auto sessionTranscript = byteStringToUnsigned(sessionTranscriptS); auto itemsRequest = byteStringToUnsigned(itemsRequestS); auto readerSignature = byteStringToUnsigned(readerSignatureS); @@ -498,6 +498,7 @@ ndk::ScopedAStatus IdentityCredential::startRetrieval( currentNameSpace_ = ""; itemsRequest_ = itemsRequest; + signingKeyBlob_ = byteStringToUnsigned(signingKeyBlobS); numStartRetrievalCalls_ += 1; return ndk::ScopedAStatus::ok(); @@ -650,11 +651,8 @@ ndk::ScopedAStatus IdentityCredential::retrieveEntryValue(const vector& return ndk::ScopedAStatus::ok(); } -ndk::ScopedAStatus IdentityCredential::finishRetrieval(const vector& signingKeyBlobS, - vector* outMac, +ndk::ScopedAStatus IdentityCredential::finishRetrieval(vector* outMac, vector* outDeviceNameSpaces) { - auto signingKeyBlob = byteStringToUnsigned(signingKeyBlobS); - if (currentNameSpaceDeviceNameSpacesMap_.size() > 0) { deviceNameSpacesMap_.add(currentNameSpace_, std::move(currentNameSpaceDeviceNameSpacesMap_)); @@ -664,7 +662,8 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(const vector& sig // If there's no signing key or no sessionTranscript or no reader ephemeral // public key, we return the empty MAC. optional> mac; - if (signingKeyBlob.size() > 0 && sessionTranscript_.size() > 0 && readerPublicKey_.size() > 0) { + if (signingKeyBlob_.size() > 0 && sessionTranscript_.size() > 0 && + readerPublicKey_.size() > 0) { cppbor::Array array; array.add("DeviceAuthentication"); array.add(sessionTranscriptItem_->clone()); @@ -674,7 +673,7 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(const vector& sig vector docTypeAsBlob(docType_.begin(), docType_.end()); optional> signingKey = - support::decryptAes128Gcm(storageKey_, signingKeyBlob, docTypeAsBlob); + support::decryptAes128Gcm(storageKey_, signingKeyBlob_, docTypeAsBlob); if (!signingKey) { return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage( IIdentityCredentialStore::STATUS_INVALID_DATA, diff --git a/identity/aidl/default/IdentityCredential.h b/identity/aidl/default/IdentityCredential.h index 49ed0d45b8..fc29254f4b 100644 --- a/identity/aidl/default/IdentityCredential.h +++ b/identity/aidl/default/IdentityCredential.h @@ -54,14 +54,14 @@ class IdentityCredential : public BnIdentityCredential { ndk::ScopedAStatus startRetrieval( const vector& accessControlProfiles, const HardwareAuthToken& authToken, const vector& itemsRequest, - const vector& sessionTranscript, const vector& readerSignature, - const vector& requestCounts) override; + const vector& signingKeyBlob, const vector& sessionTranscript, + const vector& readerSignature, const vector& requestCounts) override; ndk::ScopedAStatus startRetrieveEntryValue( const string& nameSpace, const string& name, int32_t entrySize, const vector& accessControlProfileIds) override; ndk::ScopedAStatus retrieveEntryValue(const vector& encryptedContent, vector* outContent) override; - ndk::ScopedAStatus finishRetrieval(const vector& signingKeyBlob, vector* outMac, + ndk::ScopedAStatus finishRetrieval(vector* outMac, vector* outDeviceNameSpaces) override; ndk::ScopedAStatus generateSigningKeyPair(vector* outSigningKeyBlob, Certificate* outSigningKeyCertificate) override; @@ -88,6 +88,7 @@ class IdentityCredential : public BnIdentityCredential { // Set at startRetrieval() time. map profileIdToAccessCheckResult_; + vector signingKeyBlob_; vector sessionTranscript_; std::unique_ptr sessionTranscriptItem_; vector itemsRequest_; diff --git a/identity/aidl/vts/VtsHalIdentityTargetTest.cpp b/identity/aidl/vts/VtsHalIdentityTargetTest.cpp index 5abe5a2f66..ea37fdc7a5 100644 --- a/identity/aidl/vts/VtsHalIdentityTargetTest.cpp +++ b/identity/aidl/vts/VtsHalIdentityTargetTest.cpp @@ -352,10 +352,15 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { readerCertificate.value()); ASSERT_TRUE(readerSignature); + // Generate the key that will be used to sign AuthenticatedData. + vector signingKeyBlob; + Certificate signingKeyCertificate; + ASSERT_TRUE(credential->generateSigningKeyPair(&signingKeyBlob, &signingKeyCertificate).isOk()); + ASSERT_TRUE(credential ->startRetrieval(returnedSecureProfiles, authToken, itemsRequestBytes, - sessionTranscriptBytes, readerSignature.value(), - testEntriesEntryCounts) + signingKeyBlob, sessionTranscriptBytes, + readerSignature.value(), testEntriesEntryCounts) .isOk()); for (const auto& entry : testEntries) { @@ -377,14 +382,9 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { EXPECT_EQ(content, entry.valueCbor); } - // Generate the key that will be used to sign AuthenticatedData. - vector signingKeyBlob; - Certificate signingKeyCertificate; - ASSERT_TRUE(credential->generateSigningKeyPair(&signingKeyBlob, &signingKeyCertificate).isOk()); - vector mac; vector deviceNameSpacesBytes; - ASSERT_TRUE(credential->finishRetrieval(signingKeyBlob, &mac, &deviceNameSpacesBytes).isOk()); + ASSERT_TRUE(credential->finishRetrieval(&mac, &deviceNameSpacesBytes).isOk()); cborPretty = support::cborPrettyPrint(deviceNameSpacesBytes, 32, {}); ASSERT_EQ( "{\n"