From 2e4533e5c1e0b2d4c5f4ce0e3d4d720a19044d21 Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Sat, 20 Jun 2020 17:04:41 -0400 Subject: [PATCH] Identity: Update for changes to ISO 18013-5. Key derivation for session encryption and MACing now involves mixing in SessionTranscriptBytes. Update docs, default implementation, and VTS tests to reflect this. Also, the standard changed such that instead of DeviceAuthentication being MACed or signed, it's instead DeviceAuthenticationBytes which is defined as #6.24(bstr .cbor DeviceAuthentication). The same also for ReaderAuthentication, now ReaderAuthenticationBytes is the CBOR which is signed by the reader. Also update the URL for CDDL since it's now a published RFC. Bug: 159482543 Test: atest VtsHalIdentityTargetTest Test: atest android.security.identity.cts Change-Id: I73fc7eb48ffb71e00a8b54849266ed814295fa39 --- .../identity/IIdentityCredential.aidl | 10 +++-- .../identity/IIdentityCredentialStore.aidl | 2 +- identity/aidl/default/IdentityCredential.cpp | 34 +++++++++++----- identity/aidl/vts/ReaderAuthTests.cpp | 38 ++++++++++-------- .../aidl/vts/VtsHalIdentityEndToEndTest.cpp | 39 ++++++++++++------- 5 files changed, 79 insertions(+), 44 deletions(-) diff --git a/identity/aidl/android/hardware/identity/IIdentityCredential.aidl b/identity/aidl/android/hardware/identity/IIdentityCredential.aidl index 3b8fbd9e1f..730b601c69 100644 --- a/identity/aidl/android/hardware/identity/IIdentityCredential.aidl +++ b/identity/aidl/android/hardware/identity/IIdentityCredential.aidl @@ -151,8 +151,8 @@ interface IIdentityCredential { * IntentToRetain = bool * * For the readerSignature parameter, this can either be empty or if non-empty it - * must be a COSE_Sign1 structure with an ECDSA signature over the content of the - * CBOR conforming to the following CDDL: + * must be a COSE_Sign1 where the payload is the bytes of the + * ReaderAuthenticationBytes CBOR defined below: * * ReaderAuthentication = [ * "ReaderAuthentication", @@ -164,6 +164,8 @@ interface IIdentityCredential { * * ItemsRequestBytes = #6.24(bstr .cbor ItemsRequest) * + * ReaderAuthenticationBytes = #6.24(bstr .cbor ReaderAuthentication) + * * The public key corresponding to the key used to made signature, can be found in the * 'x5chain' unprotected header element of the COSE_Sign1 structure (as as described * in 'draft-ietf-cose-x509-04'). There will be at least one certificate in said element @@ -278,7 +280,7 @@ interface IIdentityCredential { * * @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. + * and the detached content is set to DeviceAuthenticationBytes as defined below. * This code is produced by using the key agreement and key derivation function * from the ciphersuite with the authentication private key and the reader * ephemeral public key to compute a shared message authentication code (MAC) @@ -299,6 +301,8 @@ interface IIdentityCredential { * * DeviceNameSpacesBytes = #6.24(bstr .cbor DeviceNameSpaces) * + * DeviceAuthenticationBytes = #6.24(bstr .cbor DeviceAuthentication) + * * where * * DeviceNameSpaces = { diff --git a/identity/aidl/android/hardware/identity/IIdentityCredentialStore.aidl b/identity/aidl/android/hardware/identity/IIdentityCredentialStore.aidl index bd664e86ea..33e25b1adf 100644 --- a/identity/aidl/android/hardware/identity/IIdentityCredentialStore.aidl +++ b/identity/aidl/android/hardware/identity/IIdentityCredentialStore.aidl @@ -99,7 +99,7 @@ import android.hardware.identity.CipherSuite; * Various fields need to be encoded as precisely-specified byte arrays. Where existing standards * define appropriate encodings, those are used. For example, X.509 certificates. Where new * encodings are needed, CBOR is used. CBOR maps are described in CDDL notation - * (https://tools.ietf.org/html/draft-ietf-cbor-cddl-06). + * (https://tools.ietf.org/html/rfc8610). * * All binder calls in the HAL may return a ServiceSpecificException with statuses from the * STATUS_* integers defined in this interface. Each method states which status can be returned diff --git a/identity/aidl/default/IdentityCredential.cpp b/identity/aidl/default/IdentityCredential.cpp index 4e9e0e698c..87d9a935de 100644 --- a/identity/aidl/default/IdentityCredential.cpp +++ b/identity/aidl/default/IdentityCredential.cpp @@ -39,6 +39,10 @@ using ::std::optional; using namespace ::android::hardware::identity; int IdentityCredential::initialize() { + if (credentialData_.size() == 0) { + LOG(ERROR) << "CredentialData is empty"; + return IIdentityCredentialStore::STATUS_INVALID_DATA; + } auto [item, _, message] = cppbor::parse(credentialData_); if (item == nullptr) { LOG(ERROR) << "CredentialData is not valid CBOR: " << message; @@ -312,13 +316,16 @@ ndk::ScopedAStatus IdentityCredential::startRetrieval( } const vector& itemsRequestBytes = itemsRequest; - vector dataThatWasSigned = cppbor::Array() - .add("ReaderAuthentication") - .add(sessionTranscriptItem_->clone()) - .add(cppbor::Semantic(24, itemsRequestBytes)) - .encode(); + vector encodedReaderAuthentication = + cppbor::Array() + .add("ReaderAuthentication") + .add(sessionTranscriptItem_->clone()) + .add(cppbor::Semantic(24, itemsRequestBytes)) + .encode(); + vector encodedReaderAuthenticationBytes = + cppbor::Semantic(24, encodedReaderAuthentication).encode(); if (!support::coseCheckEcDsaSignature(readerSignature, - dataThatWasSigned, // detached content + encodedReaderAuthenticationBytes, // detached content readerPublicKey.value())) { return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage( IIdentityCredentialStore::STATUS_READER_SIGNATURE_CHECK_FAILED, @@ -774,7 +781,7 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(vector* outMac, array.add(sessionTranscriptItem_->clone()); array.add(docType_); array.add(cppbor::Semantic(24, encodedDeviceNameSpaces)); - vector encodedDeviceAuthentication = array.encode(); + vector deviceAuthenticationBytes = cppbor::Semantic(24, array.encode()).encode(); vector docTypeAsBlob(docType_.begin(), docType_.end()); optional> signingKey = @@ -792,17 +799,24 @@ ndk::ScopedAStatus IdentityCredential::finishRetrieval(vector* outMac, 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(sharedSecret.value(), salt, info, 32); + optional> derivedKey = + support::hkdf(sharedSecretWithSessionTranscriptBytes, salt, info, 32); if (!derivedKey) { return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage( IIdentityCredentialStore::STATUS_FAILED, "Error deriving key from shared secret")); } - mac = support::coseMac0(derivedKey.value(), {}, // payload - encodedDeviceAuthentication); // additionalData + mac = support::coseMac0(derivedKey.value(), {}, // payload + deviceAuthenticationBytes); // detached content if (!mac) { return ndk::ScopedAStatus(AStatus_fromServiceSpecificErrorWithMessage( IIdentityCredentialStore::STATUS_FAILED, "Error MACing data")); diff --git a/identity/aidl/vts/ReaderAuthTests.cpp b/identity/aidl/vts/ReaderAuthTests.cpp index 680ba5b7f9..b11f6c5e8f 100644 --- a/identity/aidl/vts/ReaderAuthTests.cpp +++ b/identity/aidl/vts/ReaderAuthTests.cpp @@ -289,16 +289,19 @@ void ReaderAuthTests::retrieveData(const vector& readerPrivateKey, .add("Accessible by None", false))) .encode(); } - vector dataToSign = cppbor::Array() - .add("ReaderAuthentication") - .add(sessionTranscript.clone()) - .add(cppbor::Semantic(24, itemsRequestBytes)) - .encode(); + vector encodedReaderAuthentication = + cppbor::Array() + .add("ReaderAuthentication") + .add(sessionTranscript.clone()) + .add(cppbor::Semantic(24, itemsRequestBytes)) + .encode(); + vector encodedReaderAuthenticationBytes = + cppbor::Semantic(24, encodedReaderAuthentication).encode(); optional> readerSignature = - support::coseSignEcDsa(readerPrivateKey, // private key for reader - {}, // content - dataToSign, // detached content + support::coseSignEcDsa(readerPrivateKey, // private key for reader + {}, // content + encodedReaderAuthenticationBytes, // detached content support::certificateChainJoin(readerCertChain)); ASSERT_TRUE(readerSignature); @@ -528,17 +531,20 @@ TEST_P(ReaderAuthTests, ephemeralKeyNotInSessionTranscript) { .add("Accessible by C", false) .add("Accessible by None", false))) .encode(); - vector dataToSign = cppbor::Array() - .add("ReaderAuthentication") - .add(sessionTranscript.clone()) - .add(cppbor::Semantic(24, itemsRequestBytes)) - .encode(); + vector encodedReaderAuthentication = + cppbor::Array() + .add("ReaderAuthentication") + .add(sessionTranscript.clone()) + .add(cppbor::Semantic(24, itemsRequestBytes)) + .encode(); + vector encodedReaderAuthenticationBytes = + cppbor::Semantic(24, encodedReaderAuthentication).encode(); vector> readerCertChain = {cert_reader_SelfSigned_}; optional> readerSignature = - support::coseSignEcDsa(readerPrivateKey_, // private key for reader - {}, // content - dataToSign, // detached content + support::coseSignEcDsa(readerPrivateKey_, // private key for reader + {}, // content + encodedReaderAuthenticationBytes, // detached content support::certificateChainJoin(readerCertChain)); ASSERT_TRUE(readerSignature); diff --git a/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp b/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp index a0c4416115..1577293521 100644 --- a/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp +++ b/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp @@ -319,7 +319,7 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { cppbor::Array sessionTranscript = cppbor::Array() .add(cppbor::Semantic(24, deviceEngagementBytes)) .add(cppbor::Semantic(24, eReaderPubBytes)); - vector sessionTranscriptBytes = sessionTranscript.encode(); + vector sessionTranscriptEncoded = sessionTranscript.encode(); vector itemsRequestBytes = cppbor::Map("nameSpaces", @@ -347,14 +347,17 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { " },\n" "}", cborPretty); - vector dataToSign = cppbor::Array() - .add("ReaderAuthentication") - .add(sessionTranscript.clone()) - .add(cppbor::Semantic(24, itemsRequestBytes)) - .encode(); + vector encodedReaderAuthentication = + cppbor::Array() + .add("ReaderAuthentication") + .add(sessionTranscript.clone()) + .add(cppbor::Semantic(24, itemsRequestBytes)) + .encode(); + vector encodedReaderAuthenticationBytes = + cppbor::Semantic(24, encodedReaderAuthentication).encode(); optional> readerSignature = - support::coseSignEcDsa(readerKey, {}, // content - dataToSign, // detached content + support::coseSignEcDsa(readerKey, {}, // content + encodedReaderAuthenticationBytes, // detached content readerCertificate.value()); ASSERT_TRUE(readerSignature); @@ -388,7 +391,7 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { credential->setVerificationToken(verificationToken); ASSERT_TRUE(credential ->startRetrieval(secureProfiles.value(), authToken, itemsRequestBytes, - signingKeyBlob, sessionTranscriptBytes, + signingKeyBlob, sessionTranscriptEncoded, readerSignature.value(), testEntriesEntryCounts) .isOk()); @@ -432,7 +435,7 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { " },\n" "}", cborPretty); - // The data that is MACed is ["DeviceAuthentication", sessionTranscriptBytes, docType, + // The data that is MACed is ["DeviceAuthentication", sessionTranscript, docType, // deviceNameSpacesBytes] so build up that structure cppbor::Array deviceAuthentication; deviceAuthentication.add("DeviceAuthentication"); @@ -441,7 +444,8 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { string docType = "org.iso.18013-5.2019.mdl"; deviceAuthentication.add(docType); deviceAuthentication.add(cppbor::Semantic(24, deviceNameSpacesBytes)); - vector encodedDeviceAuthentication = deviceAuthentication.encode(); + vector deviceAuthenticationBytes = + cppbor::Semantic(24, deviceAuthentication.encode()).encode(); // Derive the key used for MACing. optional> readerEphemeralPrivateKey = @@ -449,13 +453,20 @@ TEST_P(IdentityAidl, createAndRetrieveCredential) { 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(sharedSecret.value(), salt, info, 32); + optional> derivedKey = + support::hkdf(sharedSecretWithSessionTranscriptBytes, salt, info, 32); ASSERT_TRUE(derivedKey); optional> calculatedMac = - support::coseMac0(derivedKey.value(), {}, // payload - encodedDeviceAuthentication); // detached content + support::coseMac0(derivedKey.value(), {}, // payload + deviceAuthenticationBytes); // detached content ASSERT_TRUE(calculatedMac); EXPECT_EQ(mac, calculatedMac); }