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 3 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
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
4 changes: 1 addition & 3 deletions src/credentials/CHIPCertToX509.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,9 +574,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
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
47 changes: 21 additions & 26 deletions src/lib/core/TLVReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,56 +167,51 @@ class DLL_EXPORT TLVReader
* Advances the TLVReader object to the next TLV element to be read, asserting the tag of
* the new element.
*
* The Next(Tag expectedTag) method is a convenience method that has the
* same behavior as Next(), but also verifies that the tag of the new TLV element matches
* the supplied argument.
* This is a convenience method that combines the behavior of Next() and Expect().
*
* @param[in] expectedTag The expected tag for the next element.
* @retval #CHIP_NO_ERROR If the reader was successfully positioned on a new element
* matching the expected parameters.
* @retval other See return values of Next() and Expect().
*/
CHIP_ERROR Next(Tag expectedTag);

/* Checks that the TLV reader is position at an element with the expected tag.
ksperling-apple marked this conversation as resolved.
Show resolved Hide resolved
*
* @retval #CHIP_NO_ERROR If the reader was successfully positioned on a new element.
* @retval #CHIP_END_OF_TLV If no further elements are available.
* @retval #CHIP_NO_ERROR If the reader is positioned on the expected element.
* @retval #CHIP_ERROR_UNEXPECTED_TLV_ELEMENT
* If the tag associated with the new element does not match the
* value of the @p expectedTag argument.
* @retval #CHIP_ERROR_TLV_UNDERRUN If the underlying TLV encoding ended prematurely.
* @retval #CHIP_ERROR_INVALID_TLV_ELEMENT
* If the reader encountered an invalid or unsupported TLV
* element type.
* @retval #CHIP_ERROR_INVALID_TLV_TAG If the reader encountered a TLV tag in an invalid context.
* @retval other Other CHIP or platform error codes returned by the configured
* TLVBackingStore.
*
*/
CHIP_ERROR Next(Tag expectedTag);
CHIP_ERROR Expect(Tag expectedTag);

/**
* Advances the TLVReader object to the next TLV element to be read, asserting the type and tag of
* the new element.
*
* This is a convenience method that combines the behavior of Next() and Expect().
*
* @retval #CHIP_NO_ERROR If the reader was successfully positioned on a new element
* matching the expected parameters.
* @retval other See return values of Next() and Expect().
*/
CHIP_ERROR Next(TLVType expectedType, Tag expectedTag);

/**
* The Next(TLVType expectedType, Tag expectedTag) method is a convenience method that has the
ksperling-apple marked this conversation as resolved.
Show resolved Hide resolved
* same behavior as Next(), but also verifies that the type and tag of the new TLV element match
* the supplied arguments.
*
* @param[in] expectedType The expected data type for the next element.
* @param[in] expectedTag The expected tag for the next element.
*
* @retval #CHIP_NO_ERROR If the reader was successfully positioned on a new element.
* @retval #CHIP_END_OF_TLV If no further elements are available.
* @retval #CHIP_NO_ERROR If the reader is positioned on the expected element.
* @retval #CHIP_ERROR_WRONG_TLV_TYPE If the type of the new element does not match the value
* of the @p expectedType argument.
* @retval #CHIP_ERROR_UNEXPECTED_TLV_ELEMENT
* If the tag associated with the new element does not match the
* value of the @p expectedTag argument.
* @retval #CHIP_ERROR_TLV_UNDERRUN If the underlying TLV encoding ended prematurely.
* @retval #CHIP_ERROR_INVALID_TLV_ELEMENT
* If the reader encountered an invalid or unsupported TLV
* element type.
* @retval #CHIP_ERROR_INVALID_TLV_TAG If the reader encountered a TLV tag in an invalid context.
* @retval other Other CHIP or platform error codes returned by the configured
* TLVBackingStore.
*
*/
CHIP_ERROR Next(TLVType expectedType, Tag expectedTag);
CHIP_ERROR Expect(TLVType expectedType, Tag expectedTag);

/**
* Returns the type of the current TLV element.
Expand Down
Loading