commit 798188aba0fe1d37cf22968bd209b222f1f9f9c7
author Seth Moore <sethmo@google.com> Thu Jun 17 10:58:27 2021 -0700
committer Max Bires <jbires@google.com> Tue Jun 22 18:32:56 2021 -0700
tree bffe9a210281bd11289c3440ccb626313e9f5fc1
parent e3dc2f88b281ed597fbecd8e462c00e718a39100

Remove ignoreSignature for cose signature checks

This flag is never used anywhere, so just remove it. When used, it would
bypass signature checks. This is something we generally don't want to
do, even in testing. So remove the flag so there's no temptation to use
it.

Bug: 190942528
Test: VtsHalRemotelyProvisionedComponentTargetTest
Change-Id: I0433c1eedc08e9a5a5ad71347154867dba61689e
Merged-In: I0433c1eedc08e9a5a5ad71347154867dba61689e

security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp

diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
index a177317..78f8f08 100644
--- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
+++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
@@ -335,8 +335,7 @@
         ASSERT_TRUE(deviceInfoMap->asMap());
 
         auto& signingKey = bccContents->back().pubKey;
-        auto macKey = verifyAndParseCoseSign1(/* ignore_signature = */ false, signedMac->asArray(),
-                                              signingKey,
+        auto macKey = verifyAndParseCoseSign1(signedMac->asArray(), signingKey,
                                               cppbor::Array()  // SignedMacAad
                                                       .add(challenge_)
                                                       .add(std::move(deviceInfoMap))

security/keymint/support/remote_prov_utils.cpp

diff --git a/security/keymint/support/remote_prov_utils.cpp b/security/keymint/support/remote_prov_utils.cpp
index da10eb2..33f1ed3 100644
--- a/security/keymint/support/remote_prov_utils.cpp
+++ b/security/keymint/support/remote_prov_utils.cpp
@@ -78,7 +78,7 @@
     return EekChain{eekChain.encode(), pub_key, priv_key};
 }
 
-ErrMsgOr<bytevec> verifyAndParseCoseSign1Cwt(bool ignoreSignature, const cppbor::Array* coseSign1,
+ErrMsgOr<bytevec> verifyAndParseCoseSign1Cwt(const cppbor::Array* coseSign1,
                                              const bytevec& signingCoseKey, const bytevec& aad) {
     if (!coseSign1 || coseSign1->size() != kCoseSign1EntryCount) {
         return "Invalid COSE_Sign1";
@@ -115,27 +115,22 @@
     auto serializedKey = parsedPayload->asMap()->get(-4670552)->clone();
     if (!serializedKey || !serializedKey->asBstr()) return "Could not find key entry";
 
-    if (!ignoreSignature) {
-        bool selfSigned = signingCoseKey.empty();
-        auto key = CoseKey::parseEd25519(selfSigned ? serializedKey->asBstr()->value()
-                                                    : signingCoseKey);
-        if (!key) return "Bad signing key: " + key.moveMessage();
+    bool selfSigned = signingCoseKey.empty();
+    auto key =
+            CoseKey::parseEd25519(selfSigned ? serializedKey->asBstr()->value() : signingCoseKey);
+    if (!key) return "Bad signing key: " + key.moveMessage();
 
-        bytevec signatureInput = cppbor::Array()
-                                         .add("Signature1")
-                                         .add(*protectedParams)
-                                         .add(aad)
-                                         .add(*payload)
-                                         .encode();
+    bytevec signatureInput =
+            cppbor::Array().add("Signature1").add(*protectedParams).add(aad).add(*payload).encode();
 
-        if (!ED25519_verify(signatureInput.data(), signatureInput.size(), signature->value().data(),
-                            key->getBstrValue(CoseKey::PUBKEY_X)->data())) {
-            return "Signature verification failed";
-        }
+    if (!ED25519_verify(signatureInput.data(), signatureInput.size(), signature->value().data(),
+                        key->getBstrValue(CoseKey::PUBKEY_X)->data())) {
+        return "Signature verification failed";
     }
 
     return serializedKey->asBstr()->value();
 }
+
 ErrMsgOr<std::vector<BccEntryData>> validateBcc(const cppbor::Array* bcc) {
     if (!bcc || bcc->size() == 0) return "Invalid BCC";
 
@@ -148,8 +143,7 @@
         if (!entry || entry->size() != kCoseSign1EntryCount) {
             return "Invalid BCC entry " + std::to_string(i) + ": " + prettyPrint(entry);
         }
-        auto payload = verifyAndParseCoseSign1Cwt(false /* ignoreSignature */, entry,
-                                                  std::move(prevKey), bytevec{} /* AAD */);
+        auto payload = verifyAndParseCoseSign1Cwt(entry, std::move(prevKey), bytevec{} /* AAD */);
         if (!payload) {
             return "Failed to verify entry " + std::to_string(i) + ": " + payload.moveMessage();
         }