Skip to content

Commit

Permalink
Do not load duplicate certs in ChipCertificateSet (#7115)
Browse files Browse the repository at this point in the history 
* Do not load duplicate certs in ChipCertificateSet

* fix build issues

* address review comments
  • Loading branch information
@pan-apple
pan-apple authored May 27, 2021
1 parent 7a172f3 commit c2cc13d
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 33 deletions.
76 changes: 43 additions & 33 deletions src/credentials/CHIPCert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,84 +164,78 @@ CHIP_ERROR ChipCertificateSet::LoadCert(const uint8_t * chipCert, uint32_t chipC

CHIP_ERROR ChipCertificateSet::LoadCert(TLVReader & reader, BitFlags<CertDecodeFlags> decodeFlags)
{
CHIP_ERROR err;
ASN1Writer writer; // ASN1Writer is used to encode TBS portion of the certificate for the purpose of signature
// validation, which should be performed on the TBS data encoded in ASN.1 DER form.
ChipCertificateData * cert = nullptr;
ChipCertificateData cert;
cert.Clear();

// Must be positioned on the structure element representing the certificate.
VerifyOrExit(reader.GetType() == kTLVType_Structure, err = CHIP_ERROR_INVALID_ARGUMENT);

// Verify we have room for the new certificate.
VerifyOrExit(mCertCount < mMaxCerts, err = CHIP_ERROR_NO_MEMORY);

cert = new (&mCerts[mCertCount]) ChipCertificateData();
VerifyOrReturnError(reader.GetType() == kTLVType_Structure, CHIP_ERROR_INVALID_ARGUMENT);

{
TLVType containerType;

// Enter the certificate structure...
err = reader.EnterContainer(containerType);
SuccessOrExit(err);
ReturnErrorOnFailure(reader.EnterContainer(containerType));

// Initialize an ASN1Writer and convert the TBS (to-be-signed) portion of the certificate to ASN.1 DER
// encoding. At the same time, parse various components within the certificate and set the corresponding
// fields in the CertificateData object.
writer.Init(mDecodeBuf, mDecodeBufSize);
err = DecodeConvertTBSCert(reader, writer, *cert);
SuccessOrExit(err);
ReturnErrorOnFailure(DecodeConvertTBSCert(reader, writer, cert));

// Verify the cert has both the Subject Key Id and Authority Key Id extensions present.
// Only certs with both these extensions are supported for the purposes of certificate validation.
VerifyOrExit(cert->mCertFlags.HasAll(CertFlags::kExtPresent_SubjectKeyId, CertFlags::kExtPresent_AuthKeyId),
err = CHIP_ERROR_UNSUPPORTED_CERT_FORMAT);
VerifyOrReturnError(cert.mCertFlags.HasAll(CertFlags::kExtPresent_SubjectKeyId, CertFlags::kExtPresent_AuthKeyId),
CHIP_ERROR_UNSUPPORTED_CERT_FORMAT);

// Verify the cert was signed with ECDSA-SHA256. This is the only signature algorithm currently supported.
VerifyOrExit(cert->mSigAlgoOID == kOID_SigAlgo_ECDSAWithSHA256, err = CHIP_ERROR_UNSUPPORTED_SIGNATURE_TYPE);
VerifyOrReturnError(cert.mSigAlgoOID == kOID_SigAlgo_ECDSAWithSHA256, CHIP_ERROR_UNSUPPORTED_SIGNATURE_TYPE);

// If requested, generate the hash of the TBS portion of the certificate...
if (decodeFlags.Has(CertDecodeFlags::kGenerateTBSHash))
{
// Finish writing the ASN.1 DER encoding of the TBS certificate.
err = writer.Finalize();
SuccessOrExit(err);
ReturnErrorOnFailure(writer.Finalize());

// Generate a SHA hash of the encoded TBS certificate.
chip::Crypto::Hash_SHA256(mDecodeBuf, writer.GetLengthWritten(), cert->mTBSHash);
chip::Crypto::Hash_SHA256(mDecodeBuf, writer.GetLengthWritten(), cert.mTBSHash);

cert->mCertFlags.Set(CertFlags::kTBSHashPresent);
cert.mCertFlags.Set(CertFlags::kTBSHashPresent);
}

// Decode the certificate's signature...
err = DecodeECDSASignature(reader, *cert);
SuccessOrExit(err);
ReturnErrorOnFailure(DecodeECDSASignature(reader, cert));

// Verify no more elements in the certificate.
err = reader.VerifyEndOfContainer();
SuccessOrExit(err);
ReturnErrorOnFailure(reader.VerifyEndOfContainer());

err = reader.ExitContainer(containerType);
SuccessOrExit(err);
ReturnErrorOnFailure(reader.ExitContainer(containerType));
}

// If requested by the caller, mark the certificate as trusted.
if (decodeFlags.Has(CertDecodeFlags::kIsTrustAnchor))
{
cert->mCertFlags.Set(CertFlags::kIsTrustAnchor);
cert.mCertFlags.Set(CertFlags::kIsTrustAnchor);
}

mCertCount++;

exit:
if (err != CHIP_NO_ERROR)
// Check if this cert matches any currently loaded certificates
for (uint32_t i = 0; i < mCertCount; i++)
{
if (cert != nullptr)
if (cert.IsEqual(mCerts[i]))
{
cert->~ChipCertificateData();
// This cert is already loaded. Let's skip adding this cert.
return CHIP_NO_ERROR;
}
}

return err;
// Verify we have room for the new certificate.
VerifyOrReturnError(mCertCount < mMaxCerts, CHIP_ERROR_NO_MEMORY);

new (&mCerts[mCertCount]) ChipCertificateData(cert);
mCertCount++;

return CHIP_NO_ERROR;
}

CHIP_ERROR ChipCertificateSet::LoadCerts(const uint8_t * chipCerts, uint32_t chipCertsLen, BitFlags<CertDecodeFlags> decodeFlags)
Expand Down Expand Up @@ -618,6 +612,22 @@ void ChipCertificateData::Clear()
memset(mTBSHash, 0, sizeof(mTBSHash));
}

bool ChipCertificateData::IsEqual(const ChipCertificateData & other) const
{
// TODO - Add an operator== on BitFlags class.
return mSubjectDN.IsEqual(other.mSubjectDN) && mIssuerDN.IsEqual(other.mIssuerDN) &&
mSubjectKeyId.IsEqual(other.mSubjectKeyId) && mAuthKeyId.IsEqual(other.mAuthKeyId) &&
(mNotBeforeTime == other.mNotBeforeTime) && (mNotAfterTime == other.mNotAfterTime) &&
(mPublicKeyLen == other.mPublicKeyLen) && (memcmp(mPublicKey, other.mPublicKey, mPublicKeyLen) == 0) &&
(mPubKeyCurveOID == other.mPubKeyCurveOID) && (mPubKeyAlgoOID == other.mPubKeyAlgoOID) &&
(mSigAlgoOID == other.mSigAlgoOID) && (mCertFlags.Raw() == other.mCertFlags.Raw()) &&
(mKeyUsageFlags.Raw() == other.mKeyUsageFlags.Raw()) && (mKeyPurposeFlags.Raw() == other.mKeyPurposeFlags.Raw()) &&
(mPathLenConstraint == other.mPathLenConstraint) && (mSignature.RLen == other.mSignature.RLen) &&
(memcmp(mSignature.R, other.mSignature.R, mSignature.RLen) == 0) && (mSignature.SLen == other.mSignature.SLen) &&
(memcmp(mSignature.S, other.mSignature.S, mSignature.SLen) == 0) &&
(memcmp(mTBSHash, other.mTBSHash, sizeof(mTBSHash)) == 0);
}

void ValidationContext::Reset()
{
mEffectiveTime = 0;
Expand Down
1 change: 1 addition & 0 deletions src/credentials/CHIPCert.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ struct ChipCertificateData
~ChipCertificateData();

void Clear();
bool IsEqual(const ChipCertificateData & other) const;

ChipDN mSubjectDN; /**< Certificate Subject DN. */
ChipDN mIssuerDN; /**< Certificate Issuer DN. */
Expand Down
33 changes: 33 additions & 0 deletions src/credentials/tests/TestChipCert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,38 @@ static void TestChipCert_CertType(nlTestSuite * inSuite, void * inContext)
}
}

static void TestChipCert_LoadDuplicateCerts(nlTestSuite * inSuite, void * inContext)
{
CHIP_ERROR err;
ChipCertificateSet certSet;
ValidationContext validContext;

certSet.Init(kStandardCertsCount, kTestCertBufSize);

// Let's load two distinct certificates, and make sure cert count is 2
err = LoadTestCert(certSet, TestCert::kRoot01, sNullLoadFlag, sTrustAnchorFlag);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

err = LoadTestCert(certSet, TestCert::kICA01, sNullLoadFlag, sGenTBSHashFlag);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, certSet.GetCertCount() == 2);

// Let's load a previously loaded cert and make sure cert count is still 2
err = LoadTestCert(certSet, TestCert::kRoot01, sNullLoadFlag, sTrustAnchorFlag);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, certSet.GetCertCount() == 2);

// Let's load the other previously loaded cert and make sure cert count is still 2
err = LoadTestCert(certSet, TestCert::kICA01, sNullLoadFlag, sGenTBSHashFlag);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, certSet.GetCertCount() == 2);

// Let's load a new cert and make sure cert count updates to 3
err = LoadTestCert(certSet, TestCert::kNode01_01, sNullLoadFlag, sGenTBSHashFlag);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, certSet.GetCertCount() == 3);
}

static void TestChipCert_GenerateRootCert(nlTestSuite * inSuite, void * inContext)
{
// Generate a new keypair for cert signing
Expand Down Expand Up @@ -931,6 +963,7 @@ static const nlTest sTests[] = {
NL_TEST_DEF("Test CHIP Certificate Validation time", TestChipCert_CertValidTime),
NL_TEST_DEF("Test CHIP Certificate Usage", TestChipCert_CertUsage),
NL_TEST_DEF("Test CHIP Certificate Type", TestChipCert_CertType),
NL_TEST_DEF("Test Loading Duplicate Certificates", TestChipCert_LoadDuplicateCerts),
NL_TEST_DEF("Test CHIP Generate Root Certificate", TestChipCert_GenerateRootCert),
NL_TEST_DEF("Test CHIP Generate Root Certificate with Fabric", TestChipCert_GenerateRootFabCert),
NL_TEST_DEF("Test CHIP Generate ICA Certificate", TestChipCert_GenerateICACert),
Expand Down

0 comments on commit c2cc13d

Please sign in to comment.