diff --git a/src/credentials/CHIPCert.cpp b/src/credentials/CHIPCert.cpp index 8a651e78b7b21b..425d30a2d9a396 100644 --- a/src/credentials/CHIPCert.cpp +++ b/src/credentials/CHIPCert.cpp @@ -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; @@ -137,76 +134,21 @@ CHIP_ERROR ChipCertificateSet::LoadCert(const ByteSpan chipCert, BitFlags 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 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 caa47babae7375..69041f018d773d 100644 --- a/src/credentials/CHIPCert.h +++ b/src/credentials/CHIPCert.h @@ -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 decodeFlags = {}); /** * @brief Decode CHIP certificate. @@ -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 decodeFlags = {}); /** * @brief Decode CHIP Distinguished Name (DN). diff --git a/src/credentials/CHIPCertToX509.cpp b/src/credentials/CHIPCertToX509.cpp index 85e75147458d62..228aaf276796bf 100644 --- a/src/credentials/CHIPCertToX509.cpp +++ b/src/credentials/CHIPCertToX509.cpp @@ -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)); @@ -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 @@ -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; @@ -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; @@ -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 @@ -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 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 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 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 1a8da833c06173..ff79157815a79e 100644 --- a/src/credentials/tests/TestChipCert.cpp +++ b/src/credentials/tests/TestChipCert.cpp @@ -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; @@ -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), diff --git a/src/lib/asn1/ASN1.h b/src/lib/asn1/ASN1.h index fd7c51872ab076..1ba31661bca4ac 100644 --- a/src/lib/asn1/ASN1.h +++ b/src/lib/asn1/ASN1.h @@ -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); diff --git a/src/lib/asn1/ASN1Writer.cpp b/src/lib/asn1/ASN1Writer.cpp index 09b73d49823370..643d79038bca92 100644 --- a/src/lib/asn1/ASN1Writer.cpp +++ b/src/lib/asn1/ASN1Writer.cpp @@ -75,6 +75,8 @@ size_t ASN1Writer::GetLengthWritten() const 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::PutInteger(int64_t val) 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 @@ static uint8_t HighestBit(uint32_t v) CHIP_ERROR ASN1Writer::PutBitString(uint32_t val) { - uint8_t len; - - // Do nothing for a null writer. - VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR); + ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR); + uint8_t len; if (val == 0) len = 1; else if (val < 256) @@ -222,8 +221,7 @@ CHIP_ERROR ASN1Writer::PutBitString(uint32_t val) 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, const uint8_t * enco CHIP_ERROR ASN1Writer::PutBitString(uint8_t unusedBitCount, chip::TLV::TLVReader & tlvReader) { - ByteSpan encodedBits; - - // Do nothing for a null writer. - VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR); + ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR); + ByteSpan encodedBits; ReturnErrorOnFailure(tlvReader.Get(encodedBits)); VerifyOrReturnError(CanCastTo(encodedBits.size() + 1), ASN1_ERROR_LENGTH_OVERFLOW); @@ -257,6 +253,8 @@ CHIP_ERROR ASN1Writer::PutBitString(uint8_t unusedBitCount, chip::TLV::TLVReader 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::PutNull() 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::EndConstructedType() 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::EndEncapsulatedType() 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, co CHIP_ERROR ASN1Writer::PutValue(uint8_t cls, uint8_t tag, bool isConstructed, chip::TLV::TLVReader & tlvReader) { - ByteSpan val; - - // Do nothing for a null writer. - VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR); + ReturnErrorCodeIf(IsNullWriter(), CHIP_NO_ERROR); + ByteSpan val; ReturnErrorOnFailure(tlvReader.Get(val)); VerifyOrReturnError(CanCastTo(val.size()), ASN1_ERROR_LENGTH_OVERFLOW); @@ -358,12 +351,11 @@ CHIP_ERROR ASN1Writer::PutValue(uint8_t cls, uint8_t tag, bool isConstructed, ch 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::EncodeHead(uint8_t cls, uint8_t tag, bool isConstructed, 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);