Fix loading of CD signing certs in chip-tool. (project-chip#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 project-chip#32337
Sarthak-Shaha · Mar 6, 2024 · b742587 · b742587
1 parent e48ed00
commit b742587
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
 {