Skip to content

Commit

Permalink
Make CertType an enum class instead of an anonymous enum / uint8_t (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ksperling-apple authored and pull[bot] committed Nov 24, 2023
1 parent 4f158d9 commit 1403022
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 106 deletions.
4 changes: 2 additions & 2 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -542,9 +542,9 @@ class Server
case Credentials::CertificateValidityResult::kExpired:
case Credentials::CertificateValidityResult::kExpiredAtLastKnownGoodTime:
case Credentials::CertificateValidityResult::kTimeUnknown: {
uint8_t certType;
Credentials::CertType certType;
ReturnErrorOnFailure(cert->mSubjectDN.GetCertType(certType));
if (certType == Credentials::kCertType_Root)
if (certType == Credentials::CertType::kRoot)
{
return CHIP_NO_ERROR;
}
Expand Down
40 changes: 20 additions & 20 deletions src/credentials/CHIPCert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ CHIP_ERROR ChipCertificateSet::ValidateCert(const ChipCertificateData * cert, Va
{
CHIP_ERROR err = CHIP_NO_ERROR;
const ChipCertificateData * caCert = nullptr;
uint8_t certType;
CertType certType;

err = cert->mSubjectDN.GetCertType(certType);
SuccessOrExit(err);
Expand All @@ -328,7 +328,7 @@ CHIP_ERROR ChipCertificateSet::ValidateCert(const ChipCertificateData * cert, Va
err = CHIP_ERROR_CERT_USAGE_NOT_ALLOWED);

// Verify that the certificate type is set to Root or ICA.
VerifyOrExit(certType == kCertType_ICA || certType == kCertType_Root, err = CHIP_ERROR_WRONG_CERT_TYPE);
VerifyOrExit(certType == CertType::kICA || certType == CertType::kRoot, err = CHIP_ERROR_WRONG_CERT_TYPE);

// If a path length constraint was included, verify the cert depth vs. the specified constraint.
//
Expand Down Expand Up @@ -365,7 +365,7 @@ CHIP_ERROR ChipCertificateSet::ValidateCert(const ChipCertificateData * cert, Va
}

// If a required certificate type has been specified, verify it against the current certificate's type.
if (context.mRequiredCertType != kCertType_NotSpecified)
if (context.mRequiredCertType != CertType::kNotSpecified)
{
VerifyOrExit(certType == context.mRequiredCertType, err = CHIP_ERROR_WRONG_CERT_TYPE);
}
Expand Down Expand Up @@ -569,7 +569,7 @@ void ValidationContext::Reset()
mValidityPolicy = nullptr;
mRequiredKeyUsages.ClearAll();
mRequiredKeyPurposes.ClearAll();
mRequiredCertType = kCertType_NotSpecified;
mRequiredCertType = CertType::kNotSpecified;
}

bool ChipRDN::IsEqual(const ChipRDN & other) const
Expand Down Expand Up @@ -667,40 +667,40 @@ CHIP_ERROR ChipDN::AddAttribute(chip::ASN1::OID oid, CharSpan val, bool isPrinta
return CHIP_NO_ERROR;
}

CHIP_ERROR ChipDN::GetCertType(uint8_t & certType) const
CHIP_ERROR ChipDN::GetCertType(CertType & certType) const
{
uint8_t lCertType = kCertType_NotSpecified;
CertType lCertType = CertType::kNotSpecified;
bool fabricIdPresent = false;
bool catsPresent = false;
uint8_t rdnCount = RDNCount();

certType = kCertType_NotSpecified;
certType = CertType::kNotSpecified;

for (uint8_t i = 0; i < rdnCount; i++)
{
if (rdn[i].mAttrOID == kOID_AttributeType_MatterRCACId)
{
VerifyOrReturnError(lCertType == kCertType_NotSpecified, CHIP_ERROR_WRONG_CERT_DN);
VerifyOrReturnError(lCertType == CertType::kNotSpecified, CHIP_ERROR_WRONG_CERT_DN);

lCertType = kCertType_Root;
lCertType = CertType::kRoot;
}
else if (rdn[i].mAttrOID == kOID_AttributeType_MatterICACId)
{
VerifyOrReturnError(lCertType == kCertType_NotSpecified, CHIP_ERROR_WRONG_CERT_DN);
VerifyOrReturnError(lCertType == CertType::kNotSpecified, CHIP_ERROR_WRONG_CERT_DN);

lCertType = kCertType_ICA;
lCertType = CertType::kICA;
}
else if (rdn[i].mAttrOID == kOID_AttributeType_MatterNodeId)
{
VerifyOrReturnError(lCertType == kCertType_NotSpecified, CHIP_ERROR_WRONG_CERT_DN);
VerifyOrReturnError(lCertType == CertType::kNotSpecified, CHIP_ERROR_WRONG_CERT_DN);
VerifyOrReturnError(IsOperationalNodeId(rdn[i].mChipVal), CHIP_ERROR_WRONG_NODE_ID);
lCertType = kCertType_Node;
lCertType = CertType::kNode;
}
else if (rdn[i].mAttrOID == kOID_AttributeType_MatterFirmwareSigningId)
{
VerifyOrReturnError(lCertType == kCertType_NotSpecified, CHIP_ERROR_WRONG_CERT_DN);
VerifyOrReturnError(lCertType == CertType::kNotSpecified, CHIP_ERROR_WRONG_CERT_DN);

lCertType = kCertType_FirmwareSigning;
lCertType = CertType::kFirmwareSigning;
}
else if (rdn[i].mAttrOID == kOID_AttributeType_MatterFabricId)
{
Expand All @@ -717,7 +717,7 @@ CHIP_ERROR ChipDN::GetCertType(uint8_t & certType) const
}
}

if (lCertType == kCertType_Node)
if (lCertType == CertType::kNode)
{
VerifyOrReturnError(fabricIdPresent, CHIP_ERROR_WRONG_CERT_DN);
}
Expand Down Expand Up @@ -1151,7 +1151,7 @@ CHIP_ERROR ValidateChipRCAC(const ByteSpan & rcac)
ChipCertificateSet certSet;
ChipCertificateData certData;
ValidationContext validContext;
uint8_t certType;
CertType certType;

// Note that this function doesn't check RCAC NotBefore / NotAfter time validity.
// It is assumed that RCAC should be valid at the time of installation by definition.
Expand All @@ -1161,7 +1161,7 @@ CHIP_ERROR ValidateChipRCAC(const ByteSpan & rcac)
ReturnErrorOnFailure(certSet.LoadCert(rcac, CertDecodeFlags::kGenerateTBSHash));

ReturnErrorOnFailure(certData.mSubjectDN.GetCertType(certType));
VerifyOrReturnError(certType == kCertType_Root, CHIP_ERROR_WRONG_CERT_TYPE);
VerifyOrReturnError(certType == CertType::kRoot, CHIP_ERROR_WRONG_CERT_TYPE);

VerifyOrReturnError(certData.mSubjectDN.IsEqual(certData.mIssuerDN), CHIP_ERROR_WRONG_CERT_TYPE);

Expand Down Expand Up @@ -1337,10 +1337,10 @@ CHIP_ERROR ExtractCATsFromOpCert(const ByteSpan & opcert, CATValues & cats)
CHIP_ERROR ExtractCATsFromOpCert(const ChipCertificateData & opcert, CATValues & cats)
{
uint8_t catCount = 0;
uint8_t certType;
CertType certType;

ReturnErrorOnFailure(opcert.mSubjectDN.GetCertType(certType));
VerifyOrReturnError(certType == kCertType_Node, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(certType == CertType::kNode, CHIP_ERROR_INVALID_ARGUMENT);

const ChipDN & subjectDN = opcert.mSubjectDN;
for (uint8_t i = 0; i < subjectDN.RDNCount(); ++i)
Expand Down
22 changes: 11 additions & 11 deletions src/credentials/CHIPCert.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,17 @@ enum
*
* @note Cert type is an API data type only; it should never be sent over-the-wire.
*/
enum
enum class CertType : uint8_t
{
kCertType_NotSpecified = 0x00, /**< The certificate's type has not been specified. */
kCertType_Root = 0x01, /**< A CHIP Root certificate. */
kCertType_ICA = 0x02, /**< A CHIP Intermediate CA certificate. */
kCertType_Node = 0x03, /**< A CHIP node certificate. */
kCertType_FirmwareSigning = 0x04, /**< A CHIP firmware signing certificate. Note that CHIP doesn't
specify how firmware images are signed and implementation of
firmware image signing is manufacturer-specific. The CHIP
certificate format supports encoding of firmware signing
certificates if chosen by the manufacturer to use them. */
kNotSpecified = 0x00, /**< The certificate's type has not been specified. */
kRoot = 0x01, /**< A CHIP Root certificate. */
kICA = 0x02, /**< A CHIP Intermediate CA certificate. */
kNode = 0x03, /**< A CHIP node certificate. */
kFirmwareSigning = 0x04, /**< A CHIP firmware signing certificate. Note that CHIP doesn't
specify how firmware images are signed and implementation of
firmware image signing is manufacturer-specific. The CHIP
certificate format supports encoding of firmware signing
certificates if chosen by the manufacturer to use them. */
};

/** X.509 Certificate Key Purpose Flags
Expand Down Expand Up @@ -334,7 +334,7 @@ class ChipDN
*
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR GetCertType(uint8_t & certType) const;
CHIP_ERROR GetCertType(CertType & certType) const;

/**
* @brief Retrieve the ID of a CHIP certificate.
Expand Down
2 changes: 1 addition & 1 deletion src/credentials/CHIPCertificateSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ struct ValidationContext
validated certificate. */
BitFlags<KeyPurposeFlags> mRequiredKeyPurposes; /**< Extended Key usage extensions that should be present
in the validated certificate. */
uint8_t mRequiredCertType; /**< Required certificate type. */
CertType mRequiredCertType; /**< Required certificate type. */

CertificateValidityPolicy * mValidityPolicy =
nullptr; /**< Optional application policy to apply for certificate validity period evaluation. */
Expand Down
20 changes: 10 additions & 10 deletions src/credentials/GenerateChipX509Cert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,15 +326,15 @@ CHIP_ERROR EncodeTBSCert(const X509CertRequestParams & requestParams, const Cryp
const Crypto::P256PublicKey & issuerPubkey, ASN1Writer & writer)
{
CHIP_ERROR err = CHIP_NO_ERROR;
uint8_t certType;
CertType certType;
bool isCA;

VerifyOrReturnError(requestParams.SerialNumber >= 0, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(requestParams.ValidityEnd == kNullCertTime || requestParams.ValidityEnd >= requestParams.ValidityStart,
CHIP_ERROR_INVALID_ARGUMENT);

ReturnErrorOnFailure(requestParams.SubjectDN.GetCertType(certType));
isCA = (certType == kCertType_ICA || certType == kCertType_Root);
isCA = (certType == CertType::kICA || certType == CertType::kRoot);

ASN1_START_SEQUENCE
{
Expand Down Expand Up @@ -405,10 +405,10 @@ CHIP_ERROR NewChipX509Cert(const X509CertRequestParams & requestParams, const Cr
DLL_EXPORT CHIP_ERROR NewRootX509Cert(const X509CertRequestParams & requestParams, Crypto::P256Keypair & issuerKeypair,
MutableByteSpan & x509Cert)
{
uint8_t certType;
CertType certType;

ReturnErrorOnFailure(requestParams.SubjectDN.GetCertType(certType));
VerifyOrReturnError(certType == kCertType_Root, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(certType == CertType::kRoot, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(requestParams.SubjectDN.IsEqual(requestParams.IssuerDN), CHIP_ERROR_INVALID_ARGUMENT);

return NewChipX509Cert(requestParams, issuerKeypair.Pubkey(), issuerKeypair, x509Cert);
Expand All @@ -417,13 +417,13 @@ 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)
{
uint8_t certType;
CertType certType;

ReturnErrorOnFailure(requestParams.SubjectDN.GetCertType(certType));
VerifyOrReturnError(certType == kCertType_ICA, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(certType == CertType::kICA, CHIP_ERROR_INVALID_ARGUMENT);

ReturnErrorOnFailure(requestParams.IssuerDN.GetCertType(certType));
VerifyOrReturnError(certType == kCertType_Root, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(certType == CertType::kRoot, CHIP_ERROR_INVALID_ARGUMENT);

return NewChipX509Cert(requestParams, subjectPubkey, issuerKeypair, x509Cert);
}
Expand All @@ -432,13 +432,13 @@ DLL_EXPORT CHIP_ERROR NewNodeOperationalX509Cert(const X509CertRequestParams & r
const Crypto::P256PublicKey & subjectPubkey, Crypto::P256Keypair & issuerKeypair,
MutableByteSpan & x509Cert)
{
uint8_t certType;
CertType certType;

ReturnErrorOnFailure(requestParams.SubjectDN.GetCertType(certType));
VerifyOrReturnError(certType == kCertType_Node, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(certType == CertType::kNode, CHIP_ERROR_INVALID_ARGUMENT);

ReturnErrorOnFailure(requestParams.IssuerDN.GetCertType(certType));
VerifyOrReturnError(certType == kCertType_ICA || certType == kCertType_Root, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(certType == CertType::kICA || certType == CertType::kRoot, CHIP_ERROR_INVALID_ARGUMENT);

return NewChipX509Cert(requestParams, subjectPubkey, issuerKeypair, x509Cert);
}
Expand Down
48 changes: 22 additions & 26 deletions src/credentials/tests/TestChipCert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,13 +244,13 @@ static void TestChipCert_GetCertType_ErrorCases(nlTestSuite * inSuite, void * in

for (auto chipCert : gTestCert_GetCertType_ErrorCases)
{
uint8_t certType;
CertType certType;

err = certSet.LoadCert(chipCert, sNullDecodeFlag);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

err = certSet.GetCertSet()->mSubjectDN.GetCertType(certType);
NL_TEST_ASSERT(inSuite, err != CHIP_NO_ERROR || certType == kCertType_NotSpecified);
NL_TEST_ASSERT(inSuite, err != CHIP_NO_ERROR || certType == CertType::kNotSpecified);

certSet.Clear();
}
Expand Down Expand Up @@ -302,13 +302,13 @@ static void TestChipCert_ChipDN(nlTestSuite * inSuite, void * inContext)
const static CATValues noc_cats = { { 0xABCD0001, chip::kUndefinedCAT, chip::kUndefinedCAT } };

ChipDN chip_dn;
uint8_t certType = kCertType_FirmwareSigning; // Start with non-default value
CertType certType = CertType::kFirmwareSigning; // Start with non-default value

NL_TEST_ASSERT(inSuite, chip_dn.IsEmpty());
NL_TEST_ASSERT(inSuite, chip_dn.RDNCount() == 0);
NL_TEST_ASSERT(inSuite, chip_dn.GetCertType(certType) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, chip_dn.IsEmpty() == true);
NL_TEST_ASSERT(inSuite, certType == kCertType_NotSpecified);
NL_TEST_ASSERT(inSuite, certType == CertType::kNotSpecified);

NL_TEST_ASSERT(inSuite, chip_dn.AddAttribute_CommonName(CharSpan(noc_rdn, strlen(noc_rdn)), false) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, chip_dn.AddAttribute_MatterNodeId(0xAAAABBBBCCCCDDDD) == CHIP_NO_ERROR);
Expand All @@ -321,7 +321,7 @@ static void TestChipCert_ChipDN(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, chip_dn.RDNCount() == 5);

NL_TEST_ASSERT(inSuite, chip_dn.GetCertType(certType) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, certType == kCertType_Node);
NL_TEST_ASSERT(inSuite, certType == CertType::kNode);

uint64_t certId;
NL_TEST_ASSERT(inSuite, chip_dn.GetCertChipId(certId) == CHIP_NO_ERROR);
Expand All @@ -334,7 +334,7 @@ static void TestChipCert_ChipDN(nlTestSuite * inSuite, void * inContext)
chip_dn.Clear();
NL_TEST_ASSERT(inSuite, chip_dn.GetCertType(certType) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, chip_dn.IsEmpty() == true);
NL_TEST_ASSERT(inSuite, certType == kCertType_NotSpecified);
NL_TEST_ASSERT(inSuite, certType == CertType::kNotSpecified);

CATValues noc_cats2;
chip::CATValues::Serialized serializedCATs;
Expand Down Expand Up @@ -362,7 +362,7 @@ static void TestChipCert_CertValidation(nlTestSuite * inSuite, void * inContext)
{
int mSubjectCertIndex;
uint8_t mValidateFlags;
uint8_t mRequiredCertType;
CertType mRequiredCertType;
CHIP_ERROR mExpectedResult;
int mExpectedCertIndex;
int mExpectedTrustAnchorIndex;
Expand All @@ -375,13 +375,9 @@ static void TestChipCert_CertValidation(nlTestSuite * inSuite, void * inContext)
};

// Short-hand names to make the test cases table more concise.
enum
{
CTNS = kCertType_NotSpecified,
CTCA = kCertType_ICA,
CTNode = kCertType_Node,
CTFS = kCertType_FirmwareSigning,
};
const auto CTNS = CertType::kNotSpecified;
const auto CTCA = CertType::kICA;
const auto CTNode = CertType::kNode;

// clang-format off
static const ValidationTestCase sValidationTestCases[] = {
Expand Down Expand Up @@ -1196,28 +1192,28 @@ static void TestChipCert_CertType(nlTestSuite * inSuite, void * inContext)
struct TestCase
{
uint8_t Cert;
uint8_t ExpectedCertType;
CertType ExpectedCertType;
};

// clang-format off
static TestCase sTestCases[] = {
// Cert ExpectedCertType
// =============================================================
{ TestCert::kRoot01, kCertType_Root },
{ TestCert::kRoot02, kCertType_Root },
{ TestCert::kICA01, kCertType_ICA },
{ TestCert::kICA02, kCertType_ICA },
{ TestCert::kICA01_1, kCertType_ICA },
{ TestCert::kFWSign01, kCertType_FirmwareSigning },
{ TestCert::kNode01_01, kCertType_Node },
{ TestCert::kNode01_02, kCertType_Node },
{ TestCert::kNode02_01, kCertType_Node },
{ TestCert::kNode02_02, kCertType_Node },
{ TestCert::kRoot01, CertType::kRoot },
{ TestCert::kRoot02, CertType::kRoot },
{ TestCert::kICA01, CertType::kICA },
{ TestCert::kICA02, CertType::kICA },
{ TestCert::kICA01_1, CertType::kICA },
{ TestCert::kFWSign01, CertType::kFirmwareSigning },
{ TestCert::kNode01_01, CertType::kNode },
{ TestCert::kNode01_02, CertType::kNode },
{ TestCert::kNode02_01, CertType::kNode },
{ TestCert::kNode02_02, CertType::kNode },
};
// clang-format on
for (const auto & testCase : sTestCases)
{
uint8_t certType;
CertType certType;

err = certSet.Init(1);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
Expand Down
Loading

0 comments on commit 1403022

Please sign in to comment.