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 f3c4bbfc28..10f9aa5886 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; @@ -316,13 +320,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, @@ -779,7 +786,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 = @@ -797,17 +804,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); }