Add support for CertDecodeFlags to DecodeChipCert (#30013)
* 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
diff --git a/src/credentials/CHIPCert.cpp b/src/credentials/CHIPCert.cpp
index 8a651e7..425d30a 100644
--- a/src/credentials/CHIPCert.cpp
+++ b/src/credentials/CHIPCert.cpp
@@ -54,9 +54,6 @@
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;
@@ -137,76 +134,21 @@
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();
+ ReturnErrorOnFailure(DecodeChipCert(reader, cert, decodeFlags));
- // Must be positioned on the structure element representing the certificate.
- VerifyOrReturnError(reader.GetType() == kTLVType_Structure, CHIP_ERROR_INVALID_ARGUMENT);
+ // 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);
- cert.mCertificate = chipCert;
-
- {
- TLVType containerType;
-
- // Enter the certificate structure...
- ReturnErrorOnFailure(reader.EnterContainer(containerType));
-
- // 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++)
diff --git a/src/credentials/CHIPCert.h b/src/credentials/CHIPCert.h
index caa47ba..69041f0 100644
--- a/src/credentials/CHIPCert.h
+++ b/src/credentials/CHIPCert.h
@@ -458,7 +458,7 @@
*
* @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.
@@ -470,7 +470,8 @@
*
* @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).
diff --git a/src/credentials/CHIPCertToX509.cpp b/src/credentials/CHIPCertToX509.cpp
index 85e7514..228aaf2 100644
--- a/src/credentials/CHIPCertToX509.cpp
+++ b/src/credentials/CHIPCertToX509.cpp
@@ -454,7 +454,7 @@
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));
@@ -467,6 +467,9 @@
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
@@ -492,7 +495,7 @@
*
* @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;
@@ -550,7 +553,18 @@
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;
@@ -568,7 +582,7 @@
ASN1_START_SEQUENCE
{
// tbsCertificate TBSCertificate,
- ReturnErrorOnFailure(DecodeConvertTBSCert(reader, writer, certData));
+ ReturnErrorOnFailure(DecodeConvertTBSCert(reader, tbsWriter, certData));
// signatureAlgorithm AlgorithmIdentifier
// AlgorithmIdentifier ::= SEQUENCE
@@ -603,31 +617,56 @@
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)
diff --git a/src/credentials/tests/TestChipCert.cpp b/src/credentials/tests/TestChipCert.cpp
index 1a8da83..ff79157 100644
--- a/src/credentials/tests/TestChipCert.cpp
+++ b/src/credentials/tests/TestChipCert.cpp
@@ -1275,6 +1275,34 @@
}
}
+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;
@@ -2147,6 +2175,7 @@
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),
diff --git a/src/lib/asn1/ASN1.h b/src/lib/asn1/ASN1.h
index fd7c518..1ba3166 100644
--- a/src/lib/asn1/ASN1.h
+++ b/src/lib/asn1/ASN1.h
@@ -210,6 +210,8 @@
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);
diff --git a/src/lib/asn1/ASN1Writer.cpp b/src/lib/asn1/ASN1Writer.cpp
index 09b73d4..643d790 100644
--- a/src/lib/asn1/ASN1Writer.cpp
+++ b/src/lib/asn1/ASN1Writer.cpp
@@ -75,6 +75,8 @@
CHIP_ERROR ASN1Writer::PutInteger(int64_t val)
{
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
+
uint8_t encodedVal[sizeof(int64_t)];
uint8_t valStart, valLen;
@@ -95,8 +97,7 @@
CHIP_ERROR ASN1Writer::PutBoolean(bool val)
{
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
ReturnErrorOnFailure(EncodeHead(kASN1TagClass_Universal, kASN1UniversalTag_Boolean, false, 1));
@@ -172,11 +173,9 @@
CHIP_ERROR ASN1Writer::PutBitString(uint32_t val)
{
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
+
uint8_t len;
-
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
-
if (val == 0)
len = 1;
else if (val < 256)
@@ -222,8 +221,7 @@
CHIP_ERROR ASN1Writer::PutBitString(uint8_t unusedBitCount, const uint8_t * encodedBits, uint16_t encodedBitsLen)
{
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
ReturnErrorOnFailure(EncodeHead(kASN1TagClass_Universal, kASN1UniversalTag_BitString, false, encodedBitsLen + 1));
@@ -236,11 +234,9 @@
CHIP_ERROR ASN1Writer::PutBitString(uint8_t unusedBitCount, chip::TLV::TLVReader & tlvReader)
{
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
+
ByteSpan encodedBits;
-
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
-
ReturnErrorOnFailure(tlvReader.Get(encodedBits));
VerifyOrReturnError(CanCastTo<int32_t>(encodedBits.size() + 1), ASN1_ERROR_LENGTH_OVERFLOW);
@@ -257,6 +253,8 @@
CHIP_ERROR ASN1Writer::PutTime(const ASN1UniversalTime & val)
{
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
+
char buf[ASN1UniversalTime::kASN1TimeStringMaxLength];
MutableCharSpan bufSpan(buf);
uint8_t tag;
@@ -281,8 +279,7 @@
CHIP_ERROR ASN1Writer::PutConstructedType(const uint8_t * val, uint16_t valLen)
{
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
// Make sure we have enough space to write
VerifyOrReturnError((mWritePoint + valLen) <= mBufEnd, ASN1_ERROR_OVERFLOW);
@@ -304,8 +301,7 @@
CHIP_ERROR ASN1Writer::StartEncapsulatedType(uint8_t cls, uint8_t tag, bool bitStringEncoding)
{
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
ReturnErrorOnFailure(EncodeHead(cls, tag, false, kUnknownLength));
@@ -328,8 +324,7 @@
CHIP_ERROR ASN1Writer::PutValue(uint8_t cls, uint8_t tag, bool isConstructed, const uint8_t * val, uint16_t valLen)
{
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
ReturnErrorOnFailure(EncodeHead(cls, tag, isConstructed, valLen));
@@ -340,11 +335,9 @@
CHIP_ERROR ASN1Writer::PutValue(uint8_t cls, uint8_t tag, bool isConstructed, chip::TLV::TLVReader & tlvReader)
{
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
+
ByteSpan val;
-
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
-
ReturnErrorOnFailure(tlvReader.Get(val));
VerifyOrReturnError(CanCastTo<int32_t>(val.size()), ASN1_ERROR_LENGTH_OVERFLOW);
@@ -358,12 +351,11 @@
CHIP_ERROR ASN1Writer::EncodeHead(uint8_t cls, uint8_t tag, bool isConstructed, int32_t len)
{
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
+
uint8_t bytesForLen;
uint32_t totalLen;
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
-
// Only tags < 31 supported. The implication of this is that encoded tags are exactly 1 byte long.
VerifyOrReturnError(tag < 0x1F, ASN1_ERROR_UNSUPPORTED_ENCODING);
@@ -410,8 +402,7 @@
CHIP_ERROR ASN1Writer::WriteDeferredLength()
{
- // Do nothing for a null writer.
- VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
+ ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR);
VerifyOrReturnError(mDeferredLengthCount > 0, ASN1_ERROR_INVALID_STATE);
