Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small ChipCert refactoring in preparation for more work on PDC #30163

Merged
merged 5 commits into from
Nov 2, 2023
Merged

Small ChipCert refactoring in preparation for more work on PDC #30163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ad38d0e
TLVReader: Factor Expect() helper out of Next(...)
ksperling-apple Oct 29, 2023
510e10c
ChipCert: Make issuerKeypair parameters const
ksperling-apple Nov 1, 2023
f61524f
ChipCert: Factor out EncodeExtKeyUsageExtension helper
ksperling-apple Nov 2, 2023
d68b777
Fix doc comments as per review
ksperling-apple Nov 2, 2023
efdf342
Condense a few more checks into an Expect()
ksperling-apple Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/controller/ExampleOperationalCredentialsIssuer.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break users that derived P256Keypair in a way that it internally mutates state with any of the methods, which was allowed before. It would have been best to leave this alone.

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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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)
ksperling-apple marked this conversation as resolved.
Show resolved Hide resolved
{
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
Loading