Implemented PR #16124 review comments

credentials/development/fetch-development-paa-certs-from-dcl.py

@@ -37,6 +37,27 @@
 
 
 def parse_paa_root_certs(cmdpipe, paa_list):
+    """
+    example output of a query to all x509 root certs in DCL:
+
+    certs:
+    - subject: CN=Non Production ONLY - XFN PAA Class 3
+      subject_key_id: F8:99:A9:D5:AD:71:71:E4:C3:81:7F:14:10:7F:78:F0:D9:F7:62:E9
+    - subject: CN=Matter Development PAA
+      subject_key_id: FA:92:CF:9:5E:FA:42:E1:14:30:65:16:32:FE:FE:1B:2C:77:A7:C8
+    - subject: CN=Matter PAA 1,O=Google,C=US,1.3.6.1.4.1.37244.2.1=#130436303036
+      subject_key_id: B0:0:56:81:B8:88:62:89:62:80:E1:21:18:A1:A8:BE:9:DE:93:21
+    - subject: CN=Matter Test PAA,1.3.6.1.4.1.37244.2.1=#130431323544
+      subject_key_id: E2:90:8D:36:9C:3C:A3:C1:13:BB:9:E2:4D:C1:CC:C5:A6:66:91:D4
+
+    Brief:
+    This method will search for the first line that contains ': ' char sequence.
+    From there, it assumes every 2 lines contain subject and subject key id info of
+    a valid PAA root certificate.
+    The paa_list parameter will contain a list of all valid PAA Root certificates
+    from DCL.
+    """
+
     result = {}
 
     while True:
@@ -55,7 +76,7 @@ def parse_paa_root_certs(cmdpipe, paa_list):
 def write_paa_root_cert(cmdpipe, subject):
     filename = 'paa-root-certs/dcld_mirror_' + \
         re.sub('[^a-zA-Z0-9_-]', '', re.sub('[=, ]', '_', subject))
-    with open(filename + '.pem', 'wb') as outfile:
+    with open(filename + '.pem', 'wb+') as outfile:
         while True:
             line = cmdpipe.stdout.readline()
             if not line:
@@ -70,7 +91,7 @@ def write_paa_root_cert(cmdpipe, subject):
     # convert pem file to der
     with open(filename + '.pem', 'rb') as infile:
         pem_certificate = x509.load_pem_x509_certificate(infile.read())
-    with open(filename + '.der', 'wb') as outfile:
+    with open(filename + '.der', 'wb+') as outfile:
         der_certificate = pem_certificate.public_bytes(
             serialization.Encoding.DER)
         outfile.write(der_certificate)

examples/chip-tool/commands/common/CHIPCommand.cpp

@@ -42,7 +42,14 @@ const chip::Credentials::AttestationTrustStore * GetTestFileAttestationTrustStor
 {
     static chip::Credentials::FileAttestationTrustStore attestationTrustStore{ paaTrustStorePath };
 
-    return &attestationTrustStore;
+    if (attestationTrustStore.IsInitialized())
+    {
+        return &attestationTrustStore;
+    }
+    else
+    {
+        return nullptr;
+    }
 }
 } // namespace
 
@@ -62,9 +69,17 @@ CHIP_ERROR CHIPCommand::Run()
     factoryInitParams.listenPort               = static_cast<uint16_t>(mDefaultStorage.GetListenPort() + CurrentCommissionerId());
     ReturnLogErrorOnFailure(DeviceControllerFactory::GetInstance().Init(factoryInitParams));
 
-    // we assume that chip-tool is being run by default under the examples/chip-tool/out/host directory.
-    const chip::Credentials::AttestationTrustStore * trustStore = GetTestFileAttestationTrustStore(
-        mPaaTrustStorePath.HasValue() ? mPaaTrustStorePath.Value() : "../../../../credentials/development/paa-root-certs");
+    const chip::Credentials::AttestationTrustStore * trustStore =
+        GetTestFileAttestationTrustStore(mPaaTrustStorePath.HasValue() ? mPaaTrustStorePath.Value() : ".");
+    if (trustStore == nullptr)
+    {
+        ChipLogError(chipTool, "No PAAs found in path: %s", mPaaTrustStorePath.HasValue() ? mPaaTrustStorePath.Value() : ".");
+        ChipLogError(chipTool,
+                     "Please specify a valid path containing trusted PAA certificates using [--paa-trust-store-path paa/file/path] "
+                     "argument");
+
+        return CHIP_ERROR_INVALID_ARGUMENT;
+    }
 
     ReturnLogErrorOnFailure(InitializeCommissioner(kIdentityNull, kIdentityNullFabricId, trustStore));
     ReturnLogErrorOnFailure(InitializeCommissioner(kIdentityAlpha, kIdentityAlphaFabricId, trustStore));

scripts/tests/chiptest/test_definition.py

@@ -24,6 +24,7 @@
 from random import randrange
 
 TEST_NODE_ID = '0x12344321'
+DEVELOPMENT_PAA_LIST = './credentials/development/paa-root-certs'
 
 
 class App:
@@ -239,11 +240,13 @@ def Run(self, runner, apps_register, paths: ApplicationPaths):
             apps_register.add("default", app)
 
             runner.RunSubprocess(
-                tool_cmd + ['pairing', 'qrcode', TEST_NODE_ID, app.setupCode],
+                tool_cmd + ['pairing', 'qrcode', TEST_NODE_ID, app.setupCode] +
+                ['--paa-trust-store-path', DEVELOPMENT_PAA_LIST],
                 name='PAIR', dependencies=[apps_register])
 
             runner.RunSubprocess(
-                tool_cmd + ['tests', self.run_name],
+                tool_cmd + ['tests', self.run_name] +
+                ['--paa-trust-store-path', DEVELOPMENT_PAA_LIST],
                 name='TEST', dependencies=[apps_register])
 
         except Exception:

src/credentials/attestation_verifier/FileAttestationTrustStore.cpp

@@ -46,6 +46,7 @@ FileAttestationTrustStore::FileAttestationTrustStore(const char * paaTrustStoreP
     dir = opendir(paaTrustStorePath);
     if (dir != NULL)
     {
+        // Nested directories are not handled.
         dirent * entry;
         while ((entry = readdir(dir)) != NULL)
         {
@@ -55,58 +56,62 @@ FileAttestationTrustStore::FileAttestationTrustStore(const char * paaTrustStoreP
                 FILE * file;
 
                 std::array<uint8_t, kMaxDERCertLength> certificate;
-                uint32_t certificateLength = 0;
                 std::string filename(paaTrustStorePath);
 
                 filename += std::string("/") + std::string(entry->d_name);
 
-                file              = fopen(filename.c_str(), "rb");
-                certificateLength = fread(certificate.data(), sizeof(uint8_t), kMaxDERCertLength, file);
-
-                mDerCertsRaw.push_back(certificate);
-                mDerCerts.push_back(ByteSpan(mDerCertsRaw.back().data(), certificateLength));
-
-                fclose(file);
-
-                AlignStartAddresses();
+                file = fopen(filename.c_str(), "rb");
+                if (file != NULL)
+                {
+                    uint32_t certificateLength = fread(certificate.data(), sizeof(uint8_t), kMaxDERCertLength, file);
+                    if (certificateLength > 0)
+                    {
+                        mDerCerts.push_back(certificate);
+                        mIsInitialized = true;
+                    }
+                    fclose(file);
+                }
+                else
+                {
+                    Cleanup();
+                    break;
+                }
             }
         }
         closedir(dir);
     }
 }
 
-CHIP_ERROR FileAttestationTrustStore::AlignStartAddresses()
+FileAttestationTrustStore::~FileAttestationTrustStore()
 {
-    VerifyOrReturnError(mDerCerts.size() == mDerCertsRaw.size(), CHIP_ERROR_INTERNAL);
-
-    // align start address of bytespans w/ raw certificates
-    auto rawCertificateIt  = std::begin(mDerCertsRaw);
-    auto certificateSpanIt = std::begin(mDerCerts);
-    for (; rawCertificateIt != std::end(mDerCertsRaw) && certificateSpanIt != std::end(mDerCerts);
-         ++rawCertificateIt, ++certificateSpanIt)
-    {
-        *certificateSpanIt = ByteSpan(rawCertificateIt->data(), certificateSpanIt->size());
-    }
+    Cleanup();
+}
 
-    return CHIP_NO_ERROR;
+void FileAttestationTrustStore::Cleanup()
+{
+    mDerCerts.clear();
+    mIsInitialized = false;
 }
 
 CHIP_ERROR FileAttestationTrustStore::GetProductAttestationAuthorityCert(const ByteSpan & skid,
                                                                          MutableByteSpan & outPaaDerBuffer) const
 {
+    VerifyOrReturnError(!mDerCerts.empty(), CHIP_ERROR_CA_CERT_NOT_FOUND);
     VerifyOrReturnError(!skid.empty() && (skid.data() != nullptr), CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(skid.size() == Crypto::kSubjectKeyIdentifierLength, CHIP_ERROR_INVALID_ARGUMENT);
 
-    for (ByteSpan candidate : mDerCerts)
+    for (auto candidate : mDerCerts)
     {
         uint8_t skidBuf[Crypto::kSubjectKeyIdentifierLength] = { 0 };
         MutableByteSpan candidateSkidSpan{ skidBuf };
-        VerifyOrReturnError(CHIP_NO_ERROR == Crypto::ExtractSKIDFromX509Cert(candidate, candidateSkidSpan), CHIP_ERROR_INTERNAL);
+        VerifyOrReturnError(CHIP_NO_ERROR ==
+                                Crypto::ExtractSKIDFromX509Cert(ByteSpan{ candidate.data(), candidate.size() }, candidateSkidSpan),
+                            CHIP_ERROR_INTERNAL);
 
         if (skid.data_equal(candidateSkidSpan))
         {
             // Found a match
-            return CopySpanToMutableSpan(candidate, outPaaDerBuffer);
+            return CopySpanToMutableSpan(ByteSpan{ candidate.data(), candidate.size() }, outPaaDerBuffer);
         }
     }
 

src/credentials/attestation_verifier/FileAttestationTrustStore.h

@@ -29,15 +29,20 @@ class FileAttestationTrustStore : public AttestationTrustStore
 {
 public:
     FileAttestationTrustStore(const char * paaTrustStorePath);
+    ~FileAttestationTrustStore();
+
+    bool IsInitialized() { return mIsInitialized; }
 
     CHIP_ERROR GetProductAttestationAuthorityCert(const ByteSpan & skid, MutableByteSpan & outPaaDerBuffer) const override;
+    size_t size() const { return mDerCerts.size(); }
 
 protected:
-    std::vector<ByteSpan> mDerCerts;
-    std::vector<std::array<uint8_t, kMaxDERCertLength>> mDerCertsRaw;
+    std::vector<std::array<uint8_t, kMaxDERCertLength>> mDerCerts;
 
 private:
-    CHIP_ERROR AlignStartAddresses();
+    bool mIsInitialized = false;
+
+    void Cleanup();
 };
 
 } // namespace Credentials