Skip to content

Commit 58bbfa8

Browse files
Fix loading of CD signing certs in chip-tool.
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
1 parent fa9bfbd commit 58bbfa8

File tree

3 files changed

+42
-11
lines changed

3 files changed

+42
-11
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()
166166
cdTrustStorePath = getenv(kCDTrustStorePathVariable);
167167
}
168168

169-
auto additionalCdCerts = chip::Credentials::LoadAllX509DerCerts(cdTrustStorePath);
169+
auto additionalCdCerts =
170+
chip::Credentials::LoadAllX509DerCerts(cdTrustStorePath, chip::Credentials::CertificateValidationMode::kPublicKeyOnly);
170171
if (cdTrustStorePath != nullptr && additionalCdCerts.size() == 0)
171172
{
172173
ChipLogError(chipTool, "Warning: no CD signing certs found in path: %s, only defaults will be used", cdTrustStorePath);

src/credentials/attestation_verifier/FileAttestationTrustStore.cpp

+26-8
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ FileAttestationTrustStore::FileAttestationTrustStore(const char * paaTrustStoreP
5353
mIsInitialized = true;
5454
}
5555

56-
std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePath)
56+
std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePath, CertificateValidationMode validationMode)
5757
{
5858
std::vector<std::vector<uint8_t>> certs;
5959
if (trustStorePath == nullptr)
@@ -89,21 +89,39 @@ std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePat
8989
if ((certificateLength > 0) && (certificateLength <= kMaxDERCertLength))
9090
{
9191
certificate.resize(certificateLength);
92-
// Only accumulate certificate if it has a subject key ID extension
93-
{
94-
uint8_t kidBuf[Crypto::kSubjectKeyIdentifierLength] = { 0 };
95-
MutableByteSpan kidSpan{ kidBuf };
96-
ByteSpan certSpan{ certificate.data(), certificate.size() };
92+
ByteSpan certSpan{ certificate.data(), certificate.size() };
9793

94+
// Only accumulate certificate if it passes validation.
95+
bool isValid = false;
96+
switch (validationMode)
97+
{
98+
case CertificateValidationMode::kPAA: {
9899
if (CHIP_NO_ERROR != VerifyAttestationCertificateFormat(certSpan, Crypto::AttestationCertType::kPAA))
99100
{
100-
continue;
101+
break;
101102
}
102103

104+
uint8_t kidBuf[Crypto::kSubjectKeyIdentifierLength] = { 0 };
105+
MutableByteSpan kidSpan{ kidBuf };
103106
if (CHIP_NO_ERROR == Crypto::ExtractSKIDFromX509Cert(certSpan, kidSpan))
104107
{
105-
certs.push_back(certificate);
108+
isValid = true;
106109
}
110+
break;
111+
}
112+
case CertificateValidationMode::kPublicKeyOnly: {
113+
Crypto::P256PublicKey publicKey;
114+
if (CHIP_NO_ERROR == Crypto::ExtractPubkeyFromX509Cert(certSpan, publicKey))
115+
{
116+
isValid = true;
117+
}
118+
break;
119+
}
120+
}
121+
122+
if (isValid)
123+
{
124+
certs.push_back(certificate);
107125
}
108126
}
109127
fclose(file);

src/credentials/attestation_verifier/FileAttestationTrustStore.h

+14-2
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,29 @@
2525
namespace chip {
2626
namespace Credentials {
2727

28+
enum class CertificateValidationMode
29+
{
30+
// Validate that the certificate is a valid PAA certificate.
31+
kPAA,
32+
// Validate just that the certificate has a public key we can extract
33+
// (e.g. it's a CD signing certificate).
34+
kPublicKeyOnly,
35+
};
36+
2837
/**
2938
* @brief Load all X.509 DER certificates in a given path.
3039
*
31-
* Silently ignores non-X.509 files and X.509 files without a subject key identifier.
40+
* Silently ignores non-X.509 files and X.509 files that fail validation as
41+
* determined by the provided validation mode.
3242
*
3343
* Returns an empty vector if no files are found or unrecoverable errors arise.
3444
*
3545
* @param trustStorePath - path from where to search for certificates.
46+
* @param validationMode - how the certificate files should be validated.
3647
* @return a vector of certificate DER data
3748
*/
38-
std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePath);
49+
std::vector<std::vector<uint8_t>> LoadAllX509DerCerts(const char * trustStorePath,
50+
CertificateValidationMode validationMode = CertificateValidationMode::kPAA);
3951

4052
class FileAttestationTrustStore : public AttestationTrustStore
4153
{

0 commit comments

Comments
 (0)