From e9320c75ed0b34681deadac5a3ed3a0e2d16632f Mon Sep 17 00:00:00 2001 From: Vijay Selvaraj Date: Fri, 18 Mar 2022 14:52:27 -0400 Subject: [PATCH] Implemented PR #16124 review comments --- .../fetch-development-paa-certs-from-dcl.py | 25 ++++++++- .../chip-tool/commands/common/CHIPCommand.cpp | 23 ++++++-- scripts/tests/chiptest/test_definition.py | 4 +- .../FileAttestationTrustStore.cpp | 55 ++++++++++--------- .../FileAttestationTrustStore.h | 11 +++- 5 files changed, 83 insertions(+), 35 deletions(-) diff --git a/credentials/development/fetch-development-paa-certs-from-dcl.py b/credentials/development/fetch-development-paa-certs-from-dcl.py index 368aa7317df6a6..7ac67b275c220f 100644 --- a/credentials/development/fetch-development-paa-certs-from-dcl.py +++ b/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) diff --git a/examples/chip-tool/commands/common/CHIPCommand.cpp b/examples/chip-tool/commands/common/CHIPCommand.cpp index fa30d3bde282ea..45e8a85d2bce06 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.cpp +++ b/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(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)); diff --git a/scripts/tests/chiptest/test_definition.py b/scripts/tests/chiptest/test_definition.py index 63f1cdcc1af61f..a5300778976a32 100644 --- a/scripts/tests/chiptest/test_definition.py +++ b/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,7 +240,8 @@ 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( diff --git a/src/credentials/attestation_verifier/FileAttestationTrustStore.cpp b/src/credentials/attestation_verifier/FileAttestationTrustStore.cpp index 39219b2b6e6c81..2f990f989a182f 100644 --- a/src/credentials/attestation_verifier/FileAttestationTrustStore.cpp +++ b/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 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); } } diff --git a/src/credentials/attestation_verifier/FileAttestationTrustStore.h b/src/credentials/attestation_verifier/FileAttestationTrustStore.h index 1aa2ac146e7e0c..090be169367c63 100644 --- a/src/credentials/attestation_verifier/FileAttestationTrustStore.h +++ b/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 mDerCerts; - std::vector> mDerCertsRaw; + std::vector> mDerCerts; private: - CHIP_ERROR AlignStartAddresses(); + bool mIsInitialized = false; + + void Cleanup(); }; } // namespace Credentials