Skip to content

Commit

Permalink
Small ChipCert refactoring in preparation for more work on PDC (#30163)
Browse files Browse the repository at this point in the history 
* TLVReader: Factor Expect() helper out of Next(...)

* ChipCert: Make issuerKeypair parameters const

* ChipCert: Factor out EncodeExtKeyUsageExtension helper

... and tidy up the API of some other helpers a little to make them easier to re-use.

* Fix doc comments as per review

* Condense a few more checks into an Expect()
  • Loading branch information
@ksperling-apple @pull
ksperling-apple authored and pull[bot] committed Dec 5, 2023
1 parent b0b2241 commit 2800011
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 125 deletions.
3 changes: 1 addition & 2 deletions src/controller/ExampleOperationalCredentialsIssuer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,7 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan
ReturnErrorOnFailure(reader.Next());
}

VerifyOrReturnError(reader.GetType() == kTLVType_Structure, CHIP_ERROR_WRONG_TLV_TYPE);
VerifyOrReturnError(reader.GetTag() == AnonymousTag(), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
ReturnErrorOnFailure(reader.Expect(kTLVType_Structure, AnonymousTag()));

TLVType containerType;
ReturnErrorOnFailure(reader.EnterContainer(containerType));
Expand Down
6 changes: 2 additions & 4 deletions src/controller/java/AndroidOperationalCredentialsIssuer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,7 @@ CHIP_ERROR AndroidOperationalCredentialsIssuer::CallbackGenerateNOCChain(const B
ReturnErrorOnFailure(reader.Next());
}

VerifyOrReturnError(reader.GetType() == kTLVType_Structure, CHIP_ERROR_WRONG_TLV_TYPE);
VerifyOrReturnError(reader.GetTag() == AnonymousTag(), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
ReturnErrorOnFailure(reader.Expect(kTLVType_Structure, AnonymousTag()));

TLVType containerType;
ReturnErrorOnFailure(reader.EnterContainer(containerType));
Expand Down Expand Up @@ -335,8 +334,7 @@ CHIP_ERROR AndroidOperationalCredentialsIssuer::LocalGenerateNOCChain(const Byte
ReturnErrorOnFailure(reader.Next());
}

VerifyOrReturnError(reader.GetType() == kTLVType_Structure, CHIP_ERROR_WRONG_TLV_TYPE);
VerifyOrReturnError(reader.GetTag() == AnonymousTag(), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
ReturnErrorOnFailure(reader.Expect(kTLVType_Structure, AnonymousTag()));

TLVType containerType;
ReturnErrorOnFailure(reader.EnterContainer(containerType));
Expand Down
6 changes: 3 additions & 3 deletions src/credentials/CHIPCert.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ struct X509CertRequestParams
*
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR NewRootX509Cert(const X509CertRequestParams & requestParams, Crypto::P256Keypair & issuerKeypair,
CHIP_ERROR NewRootX509Cert(const X509CertRequestParams & requestParams, const Crypto::P256Keypair & issuerKeypair,
MutableByteSpan & x509Cert);

/**
Expand All @@ -583,7 +583,7 @@ CHIP_ERROR NewRootX509Cert(const X509CertRequestParams & requestParams, Crypto::
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR NewICAX509Cert(const X509CertRequestParams & requestParams, const Crypto::P256PublicKey & subjectPubkey,
Crypto::P256Keypair & issuerKeypair, MutableByteSpan & x509Cert);
const Crypto::P256Keypair & issuerKeypair, MutableByteSpan & x509Cert);

/**
* @brief Generate a new X.509 DER encoded Node operational certificate
Expand All @@ -596,7 +596,7 @@ CHIP_ERROR NewICAX509Cert(const X509CertRequestParams & requestParams, const Cry
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR NewNodeOperationalX509Cert(const X509CertRequestParams & requestParams, const Crypto::P256PublicKey & subjectPubkey,
Crypto::P256Keypair & issuerKeypair, MutableByteSpan & x509Cert);
const Crypto::P256Keypair & issuerKeypair, MutableByteSpan & x509Cert);

/**
* @brief
Expand Down
27 changes: 7 additions & 20 deletions src/credentials/CHIPCertToX509.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,7 @@ static CHIP_ERROR DecodeConvertAuthorityKeyIdentifierExtension(TLVReader & reade
{
// keyIdentifier [0] IMPLICIT KeyIdentifier
// KeyIdentifier ::= OCTET STRING
VerifyOrReturnError(reader.GetType() == kTLVType_ByteString, CHIP_ERROR_WRONG_TLV_TYPE);
VerifyOrReturnError(reader.GetTag() == ContextTag(kTag_AuthorityKeyIdentifier), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);

ReturnErrorOnFailure(reader.Expect(kTLVType_ByteString, ContextTag(kTag_AuthorityKeyIdentifier)));
ReturnErrorOnFailure(reader.Get(certData.mAuthKeyId));

static_assert(CertificateKeyId().size() <= UINT16_MAX, "Authority key id size doesn't fit in a uint16_t");
Expand All @@ -177,9 +175,7 @@ static CHIP_ERROR DecodeConvertSubjectKeyIdentifierExtension(TLVReader & reader,

// SubjectKeyIdentifier ::= KeyIdentifier
// KeyIdentifier ::= OCTET STRING
VerifyOrReturnError(reader.GetType() == kTLVType_ByteString, CHIP_ERROR_WRONG_TLV_TYPE);
VerifyOrReturnError(reader.GetTag() == ContextTag(kTag_SubjectKeyIdentifier), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);

ReturnErrorOnFailure(reader.Expect(kTLVType_ByteString, ContextTag(kTag_SubjectKeyIdentifier)));
ReturnErrorOnFailure(reader.Get(certData.mSubjectKeyId));

static_assert(CertificateKeyId().size() <= UINT16_MAX, "Subject key id size doesn't fit in a uint16_t");
Expand All @@ -198,8 +194,7 @@ static CHIP_ERROR DecodeConvertKeyUsageExtension(TLVReader & reader, ASN1Writer
certData.mCertFlags.Set(CertFlags::kExtPresent_KeyUsage);

// KeyUsage ::= BIT STRING
VerifyOrReturnError(reader.GetTag() == ContextTag(kTag_KeyUsage), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);

ReturnErrorOnFailure(reader.Expect(ContextTag(kTag_KeyUsage)));
ReturnErrorOnFailure(reader.Get(keyUsageBits));

{
Expand Down Expand Up @@ -229,9 +224,7 @@ static CHIP_ERROR DecodeConvertBasicConstraintsExtension(TLVReader & reader, ASN
// BasicConstraints ::= SEQUENCE
ASN1_START_SEQUENCE
{
VerifyOrReturnError(reader.GetTag() == ContextTag(kTag_BasicConstraints), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
VerifyOrReturnError(reader.GetType() == kTLVType_Structure, CHIP_ERROR_WRONG_TLV_TYPE);

ReturnErrorOnFailure(reader.Expect(kTLVType_Structure, ContextTag(kTag_BasicConstraints)));
ReturnErrorOnFailure(reader.EnterContainer(outerContainer));

// cA BOOLEAN DEFAULT FALSE
Expand Down Expand Up @@ -282,9 +275,7 @@ static CHIP_ERROR DecodeConvertExtendedKeyUsageExtension(TLVReader & reader, ASN
// ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId
ASN1_START_SEQUENCE
{
VerifyOrReturnError(reader.GetTag() == ContextTag(kTag_ExtendedKeyUsage), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
VerifyOrReturnError(reader.GetType() == kTLVType_Array, CHIP_ERROR_WRONG_TLV_TYPE);

ReturnErrorOnFailure(reader.Expect(kTLVType_Array, ContextTag(kTag_ExtendedKeyUsage)));
ReturnErrorOnFailure(reader.EnterContainer(outerContainer));

while ((err = reader.Next(AnonymousTag())) == CHIP_NO_ERROR)
Expand Down Expand Up @@ -312,9 +303,7 @@ static CHIP_ERROR DecodeConvertFutureExtension(TLVReader & tlvReader, ASN1Writer
ByteSpan extensionSequence;
ASN1Reader reader;

VerifyOrReturnError(tlvReader.GetTag() == ContextTag(kTag_FutureExtension), CHIP_ERROR_INVALID_TLV_TAG);
VerifyOrReturnError(tlvReader.GetType() == kTLVType_ByteString, CHIP_ERROR_WRONG_TLV_TYPE);

ReturnErrorOnFailure(tlvReader.Expect(kTLVType_ByteString, ContextTag(kTag_FutureExtension)));
ReturnErrorOnFailure(tlvReader.Get(extensionSequence));

reader.Init(extensionSequence);
Expand Down Expand Up @@ -574,9 +563,7 @@ static CHIP_ERROR DecodeConvertCert(TLVReader & reader, ASN1Writer & writer, ASN
{
ReturnErrorOnFailure(reader.Next());
}
VerifyOrReturnError(reader.GetType() == kTLVType_Structure, CHIP_ERROR_WRONG_TLV_TYPE);
VerifyOrReturnError(reader.GetTag() == AnonymousTag(), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);

ReturnErrorOnFailure(reader.Expect(kTLVType_Structure, AnonymousTag()));
ReturnErrorOnFailure(reader.EnterContainer(containerType));

// Certificate ::= SEQUENCE
Expand Down
98 changes: 44 additions & 54 deletions src/credentials/GenerateChipX509Cert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#endif

#include <algorithm>
#include <initializer_list>
#include <inttypes.h>
#include <stddef.h>

Expand Down Expand Up @@ -81,9 +82,7 @@ CHIP_ERROR EncodeAuthorityKeyIdentifierExtension(const Crypto::P256PublicKey & p

ASN1_START_SEQUENCE
{
OID extensionOID = GetOID(kOIDCategory_Extension, static_cast<uint8_t>(kTag_AuthorityKeyIdentifier));

ASN1_ENCODE_OBJECT_ID(extensionOID);
ASN1_ENCODE_OBJECT_ID(kOID_Extension_AuthorityKeyIdentifier);

ASN1_START_OCTET_STRING_ENCAPSULATED
{
Expand Down Expand Up @@ -111,9 +110,7 @@ CHIP_ERROR EncodeSubjectKeyIdentifierExtension(const Crypto::P256PublicKey & pub

ASN1_START_SEQUENCE
{
OID extensionOID = GetOID(kOIDCategory_Extension, static_cast<uint8_t>(kTag_SubjectKeyIdentifier));

ASN1_ENCODE_OBJECT_ID(extensionOID);
ASN1_ENCODE_OBJECT_ID(kOID_Extension_SubjectKeyIdentifier);

ASN1_START_OCTET_STRING_ENCAPSULATED
{
Expand All @@ -130,21 +127,46 @@ CHIP_ERROR EncodeSubjectKeyIdentifierExtension(const Crypto::P256PublicKey & pub
return err;
}

CHIP_ERROR EncodeKeyUsageExtension(uint16_t keyUsageBits, ASN1Writer & writer)
CHIP_ERROR EncodeExtKeyUsageExtension(std::initializer_list<OID> keyPurposeOIDs, ASN1Writer & writer)
{
CHIP_ERROR err = CHIP_NO_ERROR;

ASN1_START_SEQUENCE
{
OID extensionOID = GetOID(kOIDCategory_Extension, static_cast<uint8_t>(kTag_KeyUsage));
ASN1_ENCODE_OBJECT_ID(kOID_Extension_ExtendedKeyUsage);

// ExtKeyUsage extension MUST be marked as critical.
ASN1_ENCODE_BOOLEAN(true);
ASN1_START_OCTET_STRING_ENCAPSULATED
{
ASN1_START_SEQUENCE
{
for (auto && oid : keyPurposeOIDs)
{
ASN1_ENCODE_OBJECT_ID(oid);
}
}
ASN1_END_SEQUENCE;
}
ASN1_END_ENCAPSULATED;
}
ASN1_END_SEQUENCE;

exit:
return err;
}

ASN1_ENCODE_OBJECT_ID(extensionOID);
CHIP_ERROR EncodeKeyUsageExtension(BitFlags<KeyUsageFlags> keyUsageFlags, ASN1Writer & writer)
{
CHIP_ERROR err = CHIP_NO_ERROR;
ASN1_START_SEQUENCE
{
ASN1_ENCODE_OBJECT_ID(kOID_Extension_KeyUsage);

// KeyUsage extension MUST be marked as critical.
ASN1_ENCODE_BOOLEAN(true);
ASN1_START_OCTET_STRING_ENCAPSULATED
{
ASN1_ENCODE_BIT_STRING(keyUsageBits);
ASN1_ENCODE_BIT_STRING(keyUsageFlags.Raw());
}
ASN1_END_ENCAPSULATED;
}
Expand All @@ -157,12 +179,9 @@ CHIP_ERROR EncodeKeyUsageExtension(uint16_t keyUsageBits, ASN1Writer & writer)
CHIP_ERROR EncodeIsCAExtension(IsCACert isCA, ASN1Writer & writer)
{
CHIP_ERROR err = CHIP_NO_ERROR;

ASN1_START_SEQUENCE
{
OID extensionOID = GetOID(kOIDCategory_Extension, static_cast<uint8_t>(kTag_BasicConstraints));

ASN1_ENCODE_OBJECT_ID(extensionOID);
ASN1_ENCODE_OBJECT_ID(kOID_Extension_BasicConstraints);

// BasicConstraints extension MUST be marked as critical.
ASN1_ENCODE_BOOLEAN(true);
Expand Down Expand Up @@ -191,46 +210,17 @@ CHIP_ERROR EncodeIsCAExtension(IsCACert isCA, ASN1Writer & writer)
CHIP_ERROR EncodeCASpecificExtensions(ASN1Writer & writer)
{
ReturnErrorOnFailure(EncodeIsCAExtension(kCACert, writer));

uint16_t keyUsageBits = static_cast<uint16_t>(KeyUsageFlags::kKeyCertSign) | static_cast<uint16_t>(KeyUsageFlags::kCRLSign);

ReturnErrorOnFailure(EncodeKeyUsageExtension(keyUsageBits, writer));

ReturnErrorOnFailure(
EncodeKeyUsageExtension(BitFlags<KeyUsageFlags>(KeyUsageFlags::kKeyCertSign, KeyUsageFlags::kCRLSign), writer));
return CHIP_NO_ERROR;
}

CHIP_ERROR EncodeNOCSpecificExtensions(ASN1Writer & writer)
{
CHIP_ERROR err = CHIP_NO_ERROR;

uint16_t keyUsageBits = static_cast<uint16_t>(KeyUsageFlags::kDigitalSignature);

ReturnErrorOnFailure(EncodeIsCAExtension(kNotCACert, writer));
ReturnErrorOnFailure(EncodeKeyUsageExtension(keyUsageBits, writer));

ASN1_START_SEQUENCE
{
OID extensionOID = GetOID(kOIDCategory_Extension, static_cast<uint8_t>(kTag_ExtendedKeyUsage));

ASN1_ENCODE_OBJECT_ID(extensionOID);

// ExtKeyUsage extension MUST be marked as critical.
ASN1_ENCODE_BOOLEAN(true);
ASN1_START_OCTET_STRING_ENCAPSULATED
{
ASN1_START_SEQUENCE
{
ASN1_ENCODE_OBJECT_ID(kOID_KeyPurpose_ClientAuth);
ASN1_ENCODE_OBJECT_ID(kOID_KeyPurpose_ServerAuth);
}
ASN1_END_SEQUENCE;
}
ASN1_END_ENCAPSULATED;
}
ASN1_END_SEQUENCE;

exit:
return err;
ReturnErrorOnFailure(EncodeKeyUsageExtension(KeyUsageFlags::kDigitalSignature, writer));
ReturnErrorOnFailure(EncodeExtKeyUsageExtension({ kOID_KeyPurpose_ClientAuth, kOID_KeyPurpose_ServerAuth }, writer));
return CHIP_NO_ERROR;
}

CHIP_ERROR EncodeFutureExtension(const Optional<FutureExtension> & futureExt, ASN1Writer & writer)
Expand Down Expand Up @@ -378,7 +368,7 @@ CHIP_ERROR EncodeTBSCert(const X509CertRequestParams & requestParams, const Cryp
}

CHIP_ERROR NewChipX509Cert(const X509CertRequestParams & requestParams, const Crypto::P256PublicKey & subjectPubkey,
Crypto::P256Keypair & issuerKeypair, MutableByteSpan & x509Cert)
const Crypto::P256Keypair & issuerKeypair, MutableByteSpan & x509Cert)
{
CHIP_ERROR err = CHIP_NO_ERROR;
ASN1Writer writer;
Expand Down Expand Up @@ -411,7 +401,7 @@ CHIP_ERROR NewChipX509Cert(const X509CertRequestParams & requestParams, const Cr
return err;
}

DLL_EXPORT CHIP_ERROR NewRootX509Cert(const X509CertRequestParams & requestParams, Crypto::P256Keypair & issuerKeypair,
DLL_EXPORT CHIP_ERROR NewRootX509Cert(const X509CertRequestParams & requestParams, const Crypto::P256Keypair & issuerKeypair,
MutableByteSpan & x509Cert)
{
CertType certType;
Expand All @@ -424,7 +414,7 @@ DLL_EXPORT CHIP_ERROR NewRootX509Cert(const X509CertRequestParams & requestParam
}

DLL_EXPORT CHIP_ERROR NewICAX509Cert(const X509CertRequestParams & requestParams, const Crypto::P256PublicKey & subjectPubkey,
Crypto::P256Keypair & issuerKeypair, MutableByteSpan & x509Cert)
const Crypto::P256Keypair & issuerKeypair, MutableByteSpan & x509Cert)
{
CertType certType;

Expand All @@ -438,8 +428,8 @@ DLL_EXPORT CHIP_ERROR NewICAX509Cert(const X509CertRequestParams & requestParams
}

DLL_EXPORT CHIP_ERROR NewNodeOperationalX509Cert(const X509CertRequestParams & requestParams,
const Crypto::P256PublicKey & subjectPubkey, Crypto::P256Keypair & issuerKeypair,
MutableByteSpan & x509Cert)
const Crypto::P256PublicKey & subjectPubkey,
const Crypto::P256Keypair & issuerKeypair, MutableByteSpan & x509Cert)
{
CertType certType;

Expand Down
3 changes: 1 addition & 2 deletions src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,7 @@
ReturnErrorOnFailure(reader.Next());
}

VerifyOrReturnError(reader.GetType() == kTLVType_Structure, CHIP_ERROR_WRONG_TLV_TYPE);
VerifyOrReturnError(reader.GetTag() == AnonymousTag(), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
ReturnErrorOnFailure(reader.Expect(kTLVType_Structure, AnonymousTag()));

TLVType containerType;
ReturnErrorOnFailure(reader.EnterContainer(containerType));
Expand Down
2 changes: 1 addition & 1 deletion src/lib/core/OTAImageHeader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ CHIP_ERROR OTAImageHeaderParser::DecodeTlv(OTAImageHeader & header)
ReturnErrorOnFailure(tlvReader.Next());
}

VerifyOrReturnError(tlvReader.GetTag() == TLV::ContextTag(Tag::kImageDigestType), CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
ReturnErrorOnFailure(tlvReader.Expect(TLV::ContextTag(Tag::kImageDigestType)));
ReturnErrorOnFailure(tlvReader.Get(header.mImageDigestType));
ReturnErrorOnFailure(tlvReader.Next(TLV::ContextTag(Tag::kImageDigest)));
ReturnErrorOnFailure(tlvReader.Get(header.mImageDigest));
Expand Down
27 changes: 17 additions & 10 deletions src/lib/core/TLVReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,23 +583,30 @@ CHIP_ERROR TLVReader::Next()
return CHIP_NO_ERROR;
}

CHIP_ERROR TLVReader::Expect(Tag expectedTag)
{
VerifyOrReturnError(mElemTag == expectedTag, CHIP_ERROR_UNEXPECTED_TLV_ELEMENT);
return CHIP_NO_ERROR;
}

CHIP_ERROR TLVReader::Next(Tag expectedTag)
{
CHIP_ERROR err = Next();
if (err != CHIP_NO_ERROR)
return err;
if (mElemTag != expectedTag)
return CHIP_ERROR_UNEXPECTED_TLV_ELEMENT;
ReturnErrorOnFailure(Next());
ReturnErrorOnFailure(Expect(expectedTag));
return CHIP_NO_ERROR;
}

CHIP_ERROR TLVReader::Expect(TLVType expectedType, Tag expectedTag)
{
ReturnErrorOnFailure(Expect(expectedTag));
VerifyOrReturnError(GetType() == expectedType, CHIP_ERROR_WRONG_TLV_TYPE);
return CHIP_NO_ERROR;
}

CHIP_ERROR TLVReader::Next(TLVType expectedType, Tag expectedTag)
{
CHIP_ERROR err = Next(expectedTag);
if (err != CHIP_NO_ERROR)
return err;
if (GetType() != expectedType)
return CHIP_ERROR_WRONG_TLV_TYPE;
ReturnErrorOnFailure(Next());
ReturnErrorOnFailure(Expect(expectedType, expectedTag));
return CHIP_NO_ERROR;
}

Expand Down
Loading

0 comments on commit 2800011

Please sign in to comment.