Merge "Identity: Update for changes to ISO 18013-5." into rvc-dev am: 494c937d78 am: c5ab6a92d2

Original change: https://googleplex-android-review.googlesource.com/c/platform/hardware/interfaces/+/11987943

Change-Id: I3e738173455ccc3123985397c463bd81f0477c7b
diff --git a/identity/aidl/android/hardware/identity/IIdentityCredential.aidl b/identity/aidl/android/hardware/identity/IIdentityCredential.aidl
index 3b8fbd9..730b601 100644
--- a/identity/aidl/android/hardware/identity/IIdentityCredential.aidl
+++ b/identity/aidl/android/hardware/identity/IIdentityCredential.aidl
@@ -151,8 +151,8 @@
      *   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 @@
      *
      *     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 @@
      *
      * @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 @@
      *
      *        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 bd664e8..33e25b1 100644
--- a/identity/aidl/android/hardware/identity/IIdentityCredentialStore.aidl
+++ b/identity/aidl/android/hardware/identity/IIdentityCredentialStore.aidl
@@ -99,7 +99,7 @@
  * 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 f3c4bbf..10f9aa5 100644
--- a/identity/aidl/default/IdentityCredential.cpp
+++ b/identity/aidl/default/IdentityCredential.cpp
@@ -39,6 +39,10 @@
 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 @@
         }
 
         const vector<uint8_t>& itemsRequestBytes = itemsRequest;
-        vector<uint8_t> dataThatWasSigned = cppbor::Array()
-                                                    .add("ReaderAuthentication")
-                                                    .add(sessionTranscriptItem_->clone())
-                                                    .add(cppbor::Semantic(24, itemsRequestBytes))
-                                                    .encode();
+        vector<uint8_t> encodedReaderAuthentication =
+                cppbor::Array()
+                        .add("ReaderAuthentication")
+                        .add(sessionTranscriptItem_->clone())
+                        .add(cppbor::Semantic(24, itemsRequestBytes))
+                        .encode();
+        vector<uint8_t> 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 @@
         array.add(sessionTranscriptItem_->clone());
         array.add(docType_);
         array.add(cppbor::Semantic(24, encodedDeviceNameSpaces));
-        vector<uint8_t> encodedDeviceAuthentication = array.encode();
+        vector<uint8_t> deviceAuthenticationBytes = cppbor::Semantic(24, array.encode()).encode();
 
         vector<uint8_t> docTypeAsBlob(docType_.begin(), docType_.end());
         optional<vector<uint8_t>> signingKey =
@@ -797,17 +804,24 @@
                     IIdentityCredentialStore::STATUS_FAILED, "Error doing ECDH"));
         }
 
+        // Mix-in SessionTranscriptBytes
+        vector<uint8_t> sessionTranscriptBytes = cppbor::Semantic(24, sessionTranscript_).encode();
+        vector<uint8_t> sharedSecretWithSessionTranscriptBytes = sharedSecret.value();
+        std::copy(sessionTranscriptBytes.begin(), sessionTranscriptBytes.end(),
+                  std::back_inserter(sharedSecretWithSessionTranscriptBytes));
+
         vector<uint8_t> salt = {0x00};
         vector<uint8_t> info = {};
-        optional<vector<uint8_t>> derivedKey = support::hkdf(sharedSecret.value(), salt, info, 32);
+        optional<vector<uint8_t>> 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 680ba5b..b11f6c5 100644
--- a/identity/aidl/vts/ReaderAuthTests.cpp
+++ b/identity/aidl/vts/ReaderAuthTests.cpp
@@ -289,16 +289,19 @@
                                                             .add("Accessible by None", false)))
                         .encode();
     }
-    vector<uint8_t> dataToSign = cppbor::Array()
-                                         .add("ReaderAuthentication")
-                                         .add(sessionTranscript.clone())
-                                         .add(cppbor::Semantic(24, itemsRequestBytes))
-                                         .encode();
+    vector<uint8_t> encodedReaderAuthentication =
+            cppbor::Array()
+                    .add("ReaderAuthentication")
+                    .add(sessionTranscript.clone())
+                    .add(cppbor::Semantic(24, itemsRequestBytes))
+                    .encode();
+    vector<uint8_t> encodedReaderAuthenticationBytes =
+            cppbor::Semantic(24, encodedReaderAuthentication).encode();
 
     optional<vector<uint8_t>> 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 @@
                                                         .add("Accessible by C", false)
                                                         .add("Accessible by None", false)))
                     .encode();
-    vector<uint8_t> dataToSign = cppbor::Array()
-                                         .add("ReaderAuthentication")
-                                         .add(sessionTranscript.clone())
-                                         .add(cppbor::Semantic(24, itemsRequestBytes))
-                                         .encode();
+    vector<uint8_t> encodedReaderAuthentication =
+            cppbor::Array()
+                    .add("ReaderAuthentication")
+                    .add(sessionTranscript.clone())
+                    .add(cppbor::Semantic(24, itemsRequestBytes))
+                    .encode();
+    vector<uint8_t> encodedReaderAuthenticationBytes =
+            cppbor::Semantic(24, encodedReaderAuthentication).encode();
 
     vector<vector<uint8_t>> readerCertChain = {cert_reader_SelfSigned_};
     optional<vector<uint8_t>> 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 a0c4416..1577293 100644
--- a/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp
+++ b/identity/aidl/vts/VtsHalIdentityEndToEndTest.cpp
@@ -319,7 +319,7 @@
     cppbor::Array sessionTranscript = cppbor::Array()
                                               .add(cppbor::Semantic(24, deviceEngagementBytes))
                                               .add(cppbor::Semantic(24, eReaderPubBytes));
-    vector<uint8_t> sessionTranscriptBytes = sessionTranscript.encode();
+    vector<uint8_t> sessionTranscriptEncoded = sessionTranscript.encode();
 
     vector<uint8_t> itemsRequestBytes =
             cppbor::Map("nameSpaces",
@@ -347,14 +347,17 @@
             "  },\n"
             "}",
             cborPretty);
-    vector<uint8_t> dataToSign = cppbor::Array()
-                                         .add("ReaderAuthentication")
-                                         .add(sessionTranscript.clone())
-                                         .add(cppbor::Semantic(24, itemsRequestBytes))
-                                         .encode();
+    vector<uint8_t> encodedReaderAuthentication =
+            cppbor::Array()
+                    .add("ReaderAuthentication")
+                    .add(sessionTranscript.clone())
+                    .add(cppbor::Semantic(24, itemsRequestBytes))
+                    .encode();
+    vector<uint8_t> encodedReaderAuthenticationBytes =
+            cppbor::Semantic(24, encodedReaderAuthentication).encode();
     optional<vector<uint8_t>> readerSignature =
-            support::coseSignEcDsa(readerKey, {},  // content
-                                   dataToSign,     // detached content
+            support::coseSignEcDsa(readerKey, {},                     // content
+                                   encodedReaderAuthenticationBytes,  // detached content
                                    readerCertificate.value());
     ASSERT_TRUE(readerSignature);
 
@@ -388,7 +391,7 @@
     credential->setVerificationToken(verificationToken);
     ASSERT_TRUE(credential
                         ->startRetrieval(secureProfiles.value(), authToken, itemsRequestBytes,
-                                         signingKeyBlob, sessionTranscriptBytes,
+                                         signingKeyBlob, sessionTranscriptEncoded,
                                          readerSignature.value(), testEntriesEntryCounts)
                         .isOk());
 
@@ -432,7 +435,7 @@
             "  },\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 @@
     string docType = "org.iso.18013-5.2019.mdl";
     deviceAuthentication.add(docType);
     deviceAuthentication.add(cppbor::Semantic(24, deviceNameSpacesBytes));
-    vector<uint8_t> encodedDeviceAuthentication = deviceAuthentication.encode();
+    vector<uint8_t> deviceAuthenticationBytes =
+            cppbor::Semantic(24, deviceAuthentication.encode()).encode();
 
     // Derive the key used for MACing.
     optional<vector<uint8_t>> readerEphemeralPrivateKey =
@@ -449,13 +453,20 @@
     optional<vector<uint8_t>> sharedSecret =
             support::ecdh(signingPubKey.value(), readerEphemeralPrivateKey.value());
     ASSERT_TRUE(sharedSecret);
+    // Mix-in SessionTranscriptBytes
+    vector<uint8_t> sessionTranscriptBytes =
+            cppbor::Semantic(24, sessionTranscript.encode()).encode();
+    vector<uint8_t> sharedSecretWithSessionTranscriptBytes = sharedSecret.value();
+    std::copy(sessionTranscriptBytes.begin(), sessionTranscriptBytes.end(),
+              std::back_inserter(sharedSecretWithSessionTranscriptBytes));
     vector<uint8_t> salt = {0x00};
     vector<uint8_t> info = {};
-    optional<vector<uint8_t>> derivedKey = support::hkdf(sharedSecret.value(), salt, info, 32);
+    optional<vector<uint8_t>> derivedKey =
+            support::hkdf(sharedSecretWithSessionTranscriptBytes, salt, info, 32);
     ASSERT_TRUE(derivedKey);
     optional<vector<uint8_t>> calculatedMac =
-            support::coseMac0(derivedKey.value(), {},        // payload
-                              encodedDeviceAuthentication);  // detached content
+            support::coseMac0(derivedKey.value(), {},      // payload
+                              deviceAuthenticationBytes);  // detached content
     ASSERT_TRUE(calculatedMac);
     EXPECT_EQ(mac, calculatedMac);
 }