Skip to content

Commit

Permalink
Add support for CertDecodeFlags to DecodeChipCert (#30013)
Browse files Browse the repository at this point in the history 
* Add support for CertDecodeFlags to DecodeChipCert

This moves support for generating the TBSHash during decoding from
ChipCertificateSet::LoadCert to DecodeChipCert. LoadCert can now delegate
to DecodeChipCert and only contains a few extra checks to retain existing
behavior.

* Add ASN1Writer.IsNullWriter() and use it internally

* Avoid conversion work in DecodeConvertECDSASignature if using a null writer

The original LoadCert avoided this work by calling DecodeECDSASignature
directly, so this brings the refactored version back to baseline.

* Add test for DecodeChipCert with different options
  • Loading branch information
@ksperling-apple @pull
ksperling-apple authored and pull[bot] committed Feb 2, 2024
1 parent ab0542e commit 9060036
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 107 deletions.
72 changes: 7 additions & 65 deletions src/credentials/CHIPCert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ using namespace chip::TLV;
using namespace chip::Protocols;
using namespace chip::Crypto;

extern CHIP_ERROR DecodeConvertTBSCert(TLVReader & reader, ASN1Writer & writer, ChipCertificateData & certData);
extern CHIP_ERROR DecodeECDSASignature(TLVReader & reader, ChipCertificateData & certData);

ChipCertificateSet::ChipCertificateSet()
{
mCerts = nullptr;
Expand Down Expand Up @@ -137,76 +134,21 @@ CHIP_ERROR ChipCertificateSet::LoadCert(const ByteSpan chipCert, BitFlags<CertDe
TLVReader reader;

reader.Init(chipCert);

ReturnErrorOnFailure(reader.Next(kTLVType_Structure, AnonymousTag()));

return LoadCert(reader, decodeFlags, chipCert);
}

CHIP_ERROR ChipCertificateSet::LoadCert(TLVReader & reader, BitFlags<CertDecodeFlags> decodeFlags, ByteSpan chipCert)
{
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;
cert.Clear();

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

cert.mCertificate = chipCert;

{
TLVType containerType;
ReturnErrorOnFailure(DecodeChipCert(reader, cert, decodeFlags));

// Enter the certificate structure...
ReturnErrorOnFailure(reader.EnterContainer(containerType));
// 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.
VerifyOrReturnError(cert.mCertFlags.HasAll(CertFlags::kExtPresent_SubjectKeyId, CertFlags::kExtPresent_AuthKeyId),
CHIP_ERROR_UNSUPPORTED_CERT_FORMAT);

// If requested to generate the TBSHash.
if (decodeFlags.Has(CertDecodeFlags::kGenerateTBSHash))
{
chip::Platform::ScopedMemoryBuffer<uint8_t> asn1TBSBuf;
ReturnErrorCodeIf(!asn1TBSBuf.Alloc(kMaxCHIPCertDecodeBufLength), CHIP_ERROR_NO_MEMORY);

// 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(asn1TBSBuf.Get(), kMaxCHIPCertDecodeBufLength);
ReturnErrorOnFailure(DecodeConvertTBSCert(reader, writer, cert));

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

cert.mCertFlags.Set(CertFlags::kTBSHashPresent);
}
else
{
// Initialize an ASN1Writer as a NullWriter.
writer.InitNullWriter();
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.
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.
VerifyOrReturnError(cert.mSigAlgoOID == kOID_SigAlgo_ECDSAWithSHA256, CHIP_ERROR_UNSUPPORTED_SIGNATURE_TYPE);

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

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

ReturnErrorOnFailure(reader.ExitContainer(containerType));
}

// If requested by the caller, mark the certificate as trusted.
if (decodeFlags.Has(CertDecodeFlags::kIsTrustAnchor))
{
cert.mCertFlags.Set(CertFlags::kIsTrustAnchor);
}
// Verify the cert was signed with ECDSA-SHA256. This is the only signature algorithm currently supported.
VerifyOrReturnError(cert.mSigAlgoOID == kOID_SigAlgo_ECDSAWithSHA256, CHIP_ERROR_UNSUPPORTED_SIGNATURE_TYPE);

// Check if this cert matches any currently loaded certificates
for (uint32_t i = 0; i < mCertCount; i++)
Expand Down
5 changes: 3 additions & 2 deletions src/credentials/CHIPCert.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ struct ChipCertificateData
*
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR DecodeChipCert(const ByteSpan chipCert, ChipCertificateData & certData);
CHIP_ERROR DecodeChipCert(const ByteSpan chipCert, ChipCertificateData & certData, BitFlags<CertDecodeFlags> decodeFlags = {});

/**
* @brief Decode CHIP certificate.
Expand All @@ -470,7 +470,8 @@ CHIP_ERROR DecodeChipCert(const ByteSpan chipCert, ChipCertificateData & certDat
*
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR DecodeChipCert(chip::TLV::TLVReader & reader, ChipCertificateData & certData);
CHIP_ERROR DecodeChipCert(chip::TLV::TLVReader & reader, ChipCertificateData & certData,
BitFlags<CertDecodeFlags> decodeFlags = {});

/**
* @brief Decode CHIP Distinguished Name (DN).
Expand Down
65 changes: 52 additions & 13 deletions src/credentials/CHIPCertToX509.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ static CHIP_ERROR DecodeConvertExtensions(TLVReader & reader, ASN1Writer & write
return err;
}

CHIP_ERROR DecodeECDSASignature(TLVReader & reader, ChipCertificateData & certData)
static CHIP_ERROR DecodeECDSASignature(TLVReader & reader, ChipCertificateData & certData)
{
ReturnErrorOnFailure(reader.Next(kTLVType_ByteString, ContextTag(kTag_ECDSASignature)));
ReturnErrorOnFailure(reader.Get(certData.mSignature));
Expand All @@ -467,6 +467,9 @@ static CHIP_ERROR DecodeConvertECDSASignature(TLVReader & reader, ASN1Writer & w

ReturnErrorOnFailure(DecodeECDSASignature(reader, certData));

// Converting the signature is a bit of work, so explicitly check if we have a null writer
ReturnErrorCodeIf(writer.IsNullWriter(), CHIP_NO_ERROR);

// signatureValue BIT STRING
// Per RFC3279, the ECDSA signature value is encoded in DER encapsulated in the signatureValue BIT STRING.
ASN1_START_BIT_STRING_ENCAPSULATED
Expand All @@ -492,7 +495,7 @@ static CHIP_ERROR DecodeConvertECDSASignature(TLVReader & reader, ASN1Writer & w
*
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR DecodeConvertTBSCert(TLVReader & reader, ASN1Writer & writer, ChipCertificateData & certData)
static CHIP_ERROR DecodeConvertTBSCert(TLVReader & reader, ASN1Writer & writer, ChipCertificateData & certData)
{
CHIP_ERROR err;

Expand Down Expand Up @@ -550,7 +553,18 @@ CHIP_ERROR DecodeConvertTBSCert(TLVReader & reader, ASN1Writer & writer, ChipCer
return err;
}

static CHIP_ERROR DecodeConvertCert(TLVReader & reader, ASN1Writer & writer, ChipCertificateData & certData)
/**
* Decode a CHIP TLV certificate and convert it to X.509 DER form.
*
* This helper function takes separate ASN1Writers for the whole Certificate
* and the TBSCertificate, to allow the caller to control which part (if any)
* to capture.
*
* If `writer` is NOT a null writer, then `tbsWriter` MUST be a reference
* to the same writer, otherwise the overall Certificate written will not be
* valid.
*/
static CHIP_ERROR DecodeConvertCert(TLVReader & reader, ASN1Writer & writer, ASN1Writer & tbsWriter, ChipCertificateData & certData)
{
CHIP_ERROR err;
TLVType containerType;
Expand All @@ -568,7 +582,7 @@ static CHIP_ERROR DecodeConvertCert(TLVReader & reader, ASN1Writer & writer, Chi
ASN1_START_SEQUENCE
{
// tbsCertificate TBSCertificate,
ReturnErrorOnFailure(DecodeConvertTBSCert(reader, writer, certData));
ReturnErrorOnFailure(DecodeConvertTBSCert(reader, tbsWriter, certData));

// signatureAlgorithm AlgorithmIdentifier
// AlgorithmIdentifier ::= SEQUENCE
Expand Down Expand Up @@ -603,31 +617,56 @@ DLL_EXPORT CHIP_ERROR ConvertChipCertToX509Cert(const ByteSpan chipCert, Mutable

certData.Clear();

ReturnErrorOnFailure(DecodeConvertCert(reader, writer, certData));
ReturnErrorOnFailure(DecodeConvertCert(reader, writer, writer, certData));

x509Cert.reduce_size(writer.GetLengthWritten());

return CHIP_NO_ERROR;
}

CHIP_ERROR DecodeChipCert(const ByteSpan chipCert, ChipCertificateData & certData)
CHIP_ERROR DecodeChipCert(const ByteSpan chipCert, ChipCertificateData & certData, BitFlags<CertDecodeFlags> decodeFlags)
{
TLVReader reader;

reader.Init(chipCert);

return DecodeChipCert(reader, certData);
return DecodeChipCert(reader, certData, decodeFlags);
}

CHIP_ERROR DecodeChipCert(TLVReader & reader, ChipCertificateData & certData)
CHIP_ERROR DecodeChipCert(TLVReader & reader, ChipCertificateData & certData, BitFlags<CertDecodeFlags> decodeFlags)
{
ASN1Writer writer;

writer.InitNullWriter();
ASN1Writer nullWriter;
nullWriter.InitNullWriter();

certData.Clear();

return DecodeConvertCert(reader, writer, certData);
if (decodeFlags.Has(CertDecodeFlags::kGenerateTBSHash))
{
// Create a buffer and writer to capture the TBS (to-be-signed) portion of the certificate
// when we decode (and convert) the certificate, so we can hash it to create the TBSHash.
chip::Platform::ScopedMemoryBuffer<uint8_t> asn1TBSBuf;
VerifyOrReturnError(asn1TBSBuf.Alloc(kMaxCHIPCertDecodeBufLength), CHIP_ERROR_NO_MEMORY);
ASN1Writer tbsWriter;
tbsWriter.Init(asn1TBSBuf.Get(), kMaxCHIPCertDecodeBufLength);

ReturnErrorOnFailure(DecodeConvertCert(reader, nullWriter, tbsWriter, certData));

// Hash the encoded TBS certificate. Only SHA256 is supported.
VerifyOrReturnError(certData.mSigAlgoOID == kOID_SigAlgo_ECDSAWithSHA256, CHIP_ERROR_UNSUPPORTED_SIGNATURE_TYPE);
chip::Crypto::Hash_SHA256(asn1TBSBuf.Get(), tbsWriter.GetLengthWritten(), certData.mTBSHash);
certData.mCertFlags.Set(CertFlags::kTBSHashPresent);
}
else
{
ReturnErrorOnFailure(DecodeConvertCert(reader, nullWriter, nullWriter, certData));
}

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

return CHIP_NO_ERROR;
}

CHIP_ERROR DecodeChipDN(TLVReader & reader, ChipDN & dn)
Expand Down
29 changes: 29 additions & 0 deletions src/credentials/tests/TestChipCert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,34 @@ static void TestChipCert_CertId(nlTestSuite * inSuite, void * inContext)
}
}

static void TestChipCert_DecodingOptions(nlTestSuite * inSuite, void * inContext)
{
CHIP_ERROR err;
ByteSpan cert;
ChipCertificateData certData;

err = GetTestCert(TestCert::kRoot01, sNullLoadFlag, cert);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

// Decode with default (null) options
err = DecodeChipCert(cert, certData);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, !certData.mCertFlags.Has(CertFlags::kIsTrustAnchor));

// Decode as trust anchor
err = DecodeChipCert(cert, certData, CertDecodeFlags::kIsTrustAnchor);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, certData.mCertFlags.Has(CertFlags::kIsTrustAnchor));

// Decode with TBS Hash calculation
err = DecodeChipCert(cert, certData, CertDecodeFlags::kGenerateTBSHash);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, certData.mCertFlags.Has(CertFlags::kTBSHashPresent));
// When the TBS hash is available signature verification should work
err = VerifyCertSignature(certData, certData); // test cert is a self-signed root
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
}

static void TestChipCert_LoadDuplicateCerts(nlTestSuite * inSuite, void * inContext)
{
CHIP_ERROR err;
Expand Down Expand Up @@ -2147,6 +2175,7 @@ static const nlTest sTests[] = {
NL_TEST_DEF("Test CHIP Certificate Usage", TestChipCert_CertUsage),
NL_TEST_DEF("Test CHIP Certificate Type", TestChipCert_CertType),
NL_TEST_DEF("Test CHIP Certificate ID", TestChipCert_CertId),
NL_TEST_DEF("Test CHIP Certificate Decoding Options", TestChipCert_DecodingOptions),
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),
Expand Down
2 changes: 2 additions & 0 deletions src/lib/asn1/ASN1.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ class DLL_EXPORT ASN1Writer
void InitNullWriter(void);
size_t GetLengthWritten(void) const;

bool IsNullWriter() const { return mBuf == nullptr; }

CHIP_ERROR PutInteger(int64_t val);
CHIP_ERROR PutBoolean(bool val);
CHIP_ERROR PutObjectId(const uint8_t * val, uint16_t valLen);
Expand Down
Loading

0 comments on commit 9060036

Please sign in to comment.