From b742587091630534313e0403d87c649db690496d Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Wed, 6 Mar 2024 13:47:17 -0500
Subject: [PATCH] Fix loading of CD signing certs in chip-tool. (#32395)

We were calling a function that tried to validate that the certs are valid PAA
certs, which is very much not required for CD signing certs.

Fixes https://github.com/project-chip/connectedhomeip/issues/32337
---
 .../chip-tool/commands/common/CHIPCommand.cpp |  3 +-
 .../FileAttestationTrustStore.cpp             | 34 ++++++++++++++-----
 .../FileAttestationTrustStore.h               | 16 +++++++--
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/examples/chip-tool/commands/common/CHIPCommand.cpp b/examples/chip-tool/commands/common/CHIPCommand.cpp
index 125ab46433e0ef..29de06b5e0069c 100644
--- a/examples/chip-tool/commands/common/CHIPCommand.cpp
+++ b/examples/chip-tool/commands/common/CHIPCommand.cpp
@@ -166,7 +166,8 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()
             cdTrustStorePath = getenv(kCDTrustStorePathVariable);
         }
 
-        auto additionalCdCerts = chip::Credentials::LoadAllX509DerCerts(cdTrustStorePath);
+        auto additionalCdCerts =
+            chip::Credentials::LoadAllX509DerCerts(cdTrustStorePath, chip::Credentials::CertificateValidationMode::kPublicKeyOnly);
         if (cdTrustStorePath != nullptr && additionalCdCerts.size() == 0)
         {
             ChipLogError(chipTool, "Warning: no CD signing certs found in path: %s, only defaults will be used", cdTrustStorePath);
diff --git a/src/credentials/attestation_verifier/FileAttestationTrustStore.cpp b/src/credentials/attestation_verifier/FileAttestationTrustStore.cpp
index c5e59153491378..7707cb4d2aa495 100644
--- a/src/credentials/attestation_verifier/FileAttestationTrustStore.cpp
+++ b/src/credentials/attestation_verifier/FileAttestationTrustStore.cpp
@@ -53,7 +53,7 @@ FileAttestationTrustStore::FileAttestationTrustStore(const char * paaTrustStoreP
     mIsInitialized = true;
 }
 
-std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePath)
+std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePath, CertificateValidationMode validationMode)
 {
     std::vector<std::vector<uint8_t>> certs;
     if (trustStorePath == nullptr)
@@ -89,21 +89,39 @@ std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePat
                 if ((certificateLength > 0) && (certificateLength <= kMaxDERCertLength))
                 {
                     certificate.resize(certificateLength);
-                    // Only accumulate certificate if it has a subject key ID extension
-                    {
-                        uint8_t kidBuf[Crypto::kSubjectKeyIdentifierLength] = { 0 };
-                        MutableByteSpan kidSpan{ kidBuf };
-                        ByteSpan certSpan{ certificate.data(), certificate.size() };
+                    ByteSpan certSpan{ certificate.data(), certificate.size() };
 
+                    // Only accumulate certificate if it passes validation.
+                    bool isValid = false;
+                    switch (validationMode)
+                    {
+                    case CertificateValidationMode::kPAA: {
                         if (CHIP_NO_ERROR != VerifyAttestationCertificateFormat(certSpan, Crypto::AttestationCertType::kPAA))
                         {
-                            continue;
+                            break;
                         }
 
+                        uint8_t kidBuf[Crypto::kSubjectKeyIdentifierLength] = { 0 };
+                        MutableByteSpan kidSpan{ kidBuf };
                         if (CHIP_NO_ERROR == Crypto::ExtractSKIDFromX509Cert(certSpan, kidSpan))
                         {
-                            certs.push_back(certificate);
+                            isValid = true;
                         }
+                        break;
+                    }
+                    case CertificateValidationMode::kPublicKeyOnly: {
+                        Crypto::P256PublicKey publicKey;
+                        if (CHIP_NO_ERROR == Crypto::ExtractPubkeyFromX509Cert(certSpan, publicKey))
+                        {
+                            isValid = true;
+                        }
+                        break;
+                    }
+                    }
+
+                    if (isValid)
+                    {
+                        certs.push_back(certificate);
                     }
                 }
                 fclose(file);
diff --git a/src/credentials/attestation_verifier/FileAttestationTrustStore.h b/src/credentials/attestation_verifier/FileAttestationTrustStore.h
index 0446d7a6ebb6b8..a0f3b974488414 100644
--- a/src/credentials/attestation_verifier/FileAttestationTrustStore.h
+++ b/src/credentials/attestation_verifier/FileAttestationTrustStore.h
@@ -25,17 +25,29 @@
 namespace chip {
 namespace Credentials {
 
+enum class CertificateValidationMode
+{
+    // Validate that the certificate is a valid PAA certificate.
+    kPAA,
+    // Validate just that the certificate has a public key we can extract
+    // (e.g. it's a CD signing certificate).
+    kPublicKeyOnly,
+};
+
 /**
  * @brief Load all X.509 DER certificates in a given path.
  *
- * Silently ignores non-X.509 files and X.509 files without a subject key identifier.
+ * Silently ignores non-X.509 files and X.509 files that fail validation as
+ * determined by the provided validation mode.
  *
  * Returns an empty vector if no files are found or unrecoverable errors arise.
  *
  * @param trustStorePath - path from where to search for certificates.
+ * @param validationMode - how the certificate files should be validated.
  * @return a vector of certificate DER data
  */
-std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePath);
+std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePath,
+                                                      CertificateValidationMode validationMode = CertificateValidationMode::kPAA);
 
 class FileAttestationTrustStore : public AttestationTrustStore
 {