Skip to content

Commit

Permalink
Use Default Critical (Non-Critical) Marks for CHIP Cert Extensions. (#…
Browse files Browse the repository at this point in the history 
…4710)

* Use Default Critical (Non-Critical) Marks for CHIP Cert Extensions.

The following certificate extensions, when present, MUST be marked as critical:
  - basic constraints extension
  - key usage extension
  - extended key usage extension

The following certificate extensions, when present, MUST be marked as non-critical:
  - subject key identifier extension
  - authority key identifier extension

These rules should always be enforced when processing X.509 representation of the certificate.
In the CHIP certificate representation the "critical" field is omitted.

Ticket: #4708

* Fixed typo: extention --> extension

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@emargolis @bzbarsky-apple
emargolis and bzbarsky-apple authored Feb 10, 2021
1 parent 1b6a96b commit ac22f1b
Show file tree
Hide file tree
Showing 4 changed files with 396 additions and 450 deletions.
49 changes: 15 additions & 34 deletions src/credentials/CHIPCert.h
100755 → 100644
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
namespace chip {
namespace Credentials {

const uint32_t kKeyIdentifierLength = 20;
const uint32_t kChipIdUTF8Length = 16;

/** Data Element Tags for the CHIP Certificate
*/
enum
Expand All @@ -61,43 +64,21 @@ enum
// ---- Tags identifying certificate extensions (tag numbers 128 - 255) ----
kCertificateExtensionTagsStart = 128,
kTag_BasicConstraints = 128, /**< [ structure ] Identifies whether the subject of the certificate is a CA. */
kTag_KeyUsage = 129, /**< [ structure ] Purpose of the key contained in the certificate. */
kTag_ExtendedKeyUsage = 130, /**< [ structure ] Purposes for which the certified public key may be used. */
kTag_SubjectKeyIdentifier = 131, /**< [ structure ] Information about the certificate's public key. */
kTag_AuthorityKeyIdentifier = 132, /**< [ structure ] Information about the public key used to sign the certificate. */
kCertificateExtensionTagsEnd = 255,
kTag_KeyUsage = 129, /**< [ unsigned int ] Bits identifying key usage, per RFC5280. */
kTag_ExtendedKeyUsage = 130, /**< [ array ] Array of enumerated values giving the purposes for which
the public key can be used, per RFC5280. */
kTag_SubjectKeyIdentifier = 131, /**< [ byte string ] Identifier of the certificate's public key. */
kTag_AuthorityKeyIdentifier = 132, /**< [ byte string ] Identifier of the public key used to sign the certificate. */
kCertificateExtensionTagsEnd = 255,

// ---- Context-specific Tags for ECDSASignature Structure ----
kTag_ECDSASignature_r = 1, /**< [ byte string ] ECDSA r value, in ASN.1 integer encoding. */
kTag_ECDSASignature_s = 2, /**< [ byte string ] ECDSA s value, in ASN.1 integer encoding. */

// ---- Context-specific Tags for AuthorityKeyIdentifier Structure ----
kTag_AuthorityKeyIdentifier_Critical = 1, /**< [ boolean ] True if the AuthorityKeyIdentifier extension is critical.
Otherwise absent. */
kTag_AuthorityKeyIdentifier_KeyIdentifier = 2, /**< [ byte string ] Unique identifier of the issuer's public key,
per RFC5280. */

// ---- Context-specific Tags for SubjectKeyIdentifier Structure ----
kTag_SubjectKeyIdentifier_Critical = 1, /**< [ boolean ] True if the SubjectKeyIdentifier extension is critical.
Otherwise absent. */
kTag_SubjectKeyIdentifier_KeyIdentifier = 2, /**< [ byte string ] Unique identifier for certificate's public key,
per RFC5280. */

// ---- Context-specific Tags for KeyUsage Structure ----
kTag_KeyUsage_Critical = 1, /**< [ boolean ] True if the KeyUsage extension is critical. Otherwise absent. */
kTag_KeyUsage_KeyUsage = 2, /**< [ unsigned int ] Integer containing key usage bits, per to RFC5280. */

// ---- Context-specific Tags for BasicConstraints Structure ----
kTag_BasicConstraints_Critical = 1, /**< [ boolean ] True if the BasicConstraints extension is critical.
Otherwise absent. */
kTag_BasicConstraints_IsCA = 2, /**< [ boolean ] True if the certificate can be used to verify certificate
kTag_BasicConstraints_IsCA = 1, /**< [ boolean ] True if the certificate can be used to verify certificate
signatures. */
kTag_BasicConstraints_PathLenConstraint = 3, /**< [ unsigned int ] Maximum number of subordinate intermediate certificates. */

// ---- Context-specific Tags for ExtendedKeyUsage Structure ----
kTag_ExtendedKeyUsage_Critical = 1, /**< [ boolean ] True if the ExtendedKeyUsage extension is critical. Otherwise absent. */
kTag_ExtendedKeyUsage_KeyPurposes = 2, /**< [ array ] Array of enumerated values giving the purposes for which the public key
can be used. */
kTag_BasicConstraints_PathLenConstraint = 2, /**< [ unsigned int ] Maximum number of subordinate intermediate certificates. */
};

/** Identifies the purpose or application of certificate
Expand Down Expand Up @@ -161,10 +142,10 @@ enum class KeyUsageFlags : uint16_t
enum class CertFlags : uint16_t
{
kExtPresent_BasicConstraints = 0x0001, /**< Basic constraints extension is present in the certificate. */
kExtPresent_KeyUsage = 0x0002, /**< Key usage extention is present in the certificate. */
kExtPresent_ExtendedKeyUsage = 0x0004, /**< Extended key usage extention is present in the certificate. */
kExtPresent_SubjectKeyId = 0x0008, /**< Subject key identifier extention is present in the certificate. */
kExtPresent_AuthKeyId = 0x0010, /**< Authority key identifier extention is present in the certificate. */
kExtPresent_KeyUsage = 0x0002, /**< Key usage extension is present in the certificate. */
kExtPresent_ExtendedKeyUsage = 0x0004, /**< Extended key usage extension is present in the certificate. */
kExtPresent_SubjectKeyId = 0x0008, /**< Subject key identifier extension is present in the certificate. */
kExtPresent_AuthKeyId = 0x0010, /**< Authority key identifier extension is present in the certificate. */
kPathLenConstraintPresent = 0x0020, /**< Path length constraint is present in the certificate. */
kIsCA = 0x0040, /**< Indicates that certificate is a CA certificate. */
kIsTrustAnchor = 0x0080, /**< Indicates that certificate is a trust anchor. */
Expand Down
151 changes: 53 additions & 98 deletions src/credentials/CHIPCertFromX509.cpp
100755 → 100644
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ static ASN1_ERROR ParseChipIdAttribute(ASN1Reader & reader, uint64_t & chipIdOut
ASN1_ERROR err = ASN1_NO_ERROR;
const uint8_t * value = nullptr;

static constexpr uint32_t kChipIdUTF8Length = 16;

VerifyOrExit(reader.GetValueLen() == kChipIdUTF8Length, err = ASN1_ERROR_INVALID_ENCODING);

value = reader.GetValue();
Expand Down Expand Up @@ -204,38 +202,6 @@ static CHIP_ERROR ConvertValidity(ASN1Reader & reader, TLVWriter & writer)
return err;
}

static CHIP_ERROR ConvertAuthorityKeyIdentifierExtension(ASN1Reader & reader, TLVWriter & writer)
{
CHIP_ERROR err;

// AuthorityKeyIdentifier ::= SEQUENCE
ASN1_PARSE_ENTER_SEQUENCE
{
err = reader.Next();

// keyIdentifier [0] IMPLICIT KeyIdentifier OPTIONAL,
// KeyIdentifier ::= OCTET STRING
if (err == ASN1_NO_ERROR && reader.GetClass() == kASN1TagClass_ContextSpecific && reader.GetTag() == 0)
{
VerifyOrExit(reader.IsConstructed() == false, err = ASN1_ERROR_INVALID_ENCODING);

err = writer.PutBytes(ContextTag(kTag_AuthorityKeyIdentifier_KeyIdentifier), reader.GetValue(), reader.GetValueLen());
SuccessOrExit(err);

err = reader.Next();
}

if (err != ASN1_END)
{
SuccessOrExit(err);
}
}
ASN1_EXIT_SEQUENCE;

exit:
return err;
}

static CHIP_ERROR ConvertSubjectPublicKeyInfo(ASN1Reader & reader, TLVWriter & writer)
{
CHIP_ERROR err;
Expand Down Expand Up @@ -308,7 +274,7 @@ static CHIP_ERROR ConvertSubjectPublicKeyInfo(ASN1Reader & reader, TLVWriter & w
static CHIP_ERROR ConvertExtension(ASN1Reader & reader, TLVWriter & writer)
{
CHIP_ERROR err;
TLVType outerContainer, outerContainer2;
TLVType outerContainer;
OID extensionOID;
bool critical = false;

Expand All @@ -318,25 +284,16 @@ static CHIP_ERROR ConvertExtension(ASN1Reader & reader, TLVWriter & writer)
// extnID OBJECT IDENTIFIER,
ASN1_PARSE_OBJECT_ID(extensionOID);

if (extensionOID == kOID_Unknown)
{
ExitNow(err = ASN1_ERROR_UNSUPPORTED_ENCODING);
}
if (GetOIDCategory(extensionOID) != kOIDCategory_Extension)
{
ExitNow(err = ASN1_ERROR_INVALID_ENCODING);
}
VerifyOrExit(extensionOID != kOID_Unknown, err = ASN1_ERROR_UNSUPPORTED_ENCODING);
VerifyOrExit(GetOIDCategory(extensionOID) == kOIDCategory_Extension, err = ASN1_ERROR_INVALID_ENCODING);

// critical BOOLEAN DEFAULT FALSE,
ASN1_PARSE_ANY;
if (reader.GetClass() == kASN1TagClass_Universal && reader.GetTag() == kASN1UniversalTag_Boolean)
{
ASN1_GET_BOOLEAN(critical);

if (!critical)
{
ExitNow(err = ASN1_ERROR_INVALID_ENCODING);
}
VerifyOrExit(critical, err = ASN1_ERROR_INVALID_ENCODING);

ASN1_PARSE_ANY;
}
Expand All @@ -349,49 +306,53 @@ static CHIP_ERROR ConvertExtension(ASN1Reader & reader, TLVWriter & writer)
{
if (extensionOID == kOID_Extension_AuthorityKeyIdentifier)
{
err = writer.StartContainer(ContextTag(kTag_AuthorityKeyIdentifier), kTLVType_Structure, outerContainer);
SuccessOrExit(err);
// This extension MUST be marked as non-critical.
VerifyOrExit(!critical, err = ASN1_ERROR_INVALID_ENCODING);

if (critical)
// AuthorityKeyIdentifier ::= SEQUENCE
ASN1_PARSE_ENTER_SEQUENCE
{
err = writer.PutBoolean(ContextTag(kTag_AuthorityKeyIdentifier_Critical), critical);
err = reader.Next();
VerifyOrExit(err == ASN1_NO_ERROR, err = ASN1_ERROR_INVALID_ENCODING);

// keyIdentifier [0] IMPLICIT KeyIdentifier,
// KeyIdentifier ::= OCTET STRING
VerifyOrExit(reader.GetClass() == kASN1TagClass_ContextSpecific && reader.GetTag() == 0,
err = ASN1_ERROR_INVALID_ENCODING);

VerifyOrExit(reader.IsConstructed() == false, err = ASN1_ERROR_INVALID_ENCODING);
VerifyOrExit(reader.GetValueLen() == kKeyIdentifierLength, err = ASN1_ERROR_INVALID_ENCODING);

err = writer.PutBytes(ContextTag(kTag_AuthorityKeyIdentifier), reader.GetValue(), reader.GetValueLen());
SuccessOrExit(err);
}

err = ConvertAuthorityKeyIdentifierExtension(reader, writer);
SuccessOrExit(err);
err = reader.Next();
VerifyOrExit(err == ASN1_END, err = ASN1_ERROR_INVALID_ENCODING);
}
ASN1_EXIT_SEQUENCE;
}
else if (extensionOID == kOID_Extension_SubjectKeyIdentifier)
{
// This extension MUST be marked as non-critical.
VerifyOrExit(!critical, err = ASN1_ERROR_INVALID_ENCODING);

// SubjectKeyIdentifier ::= KeyIdentifier
// KeyIdentifier ::= OCTET STRING
ASN1_PARSE_ELEMENT(kASN1TagClass_Universal, kASN1UniversalTag_OctetString);

err = writer.StartContainer(ContextTag(kTag_SubjectKeyIdentifier), kTLVType_Structure, outerContainer);
SuccessOrExit(err);

if (critical)
{
err = writer.PutBoolean(ContextTag(kTag_SubjectKeyIdentifier_Critical), critical);
SuccessOrExit(err);
}
VerifyOrExit(reader.GetValueLen() == kKeyIdentifierLength, err = ASN1_ERROR_INVALID_ENCODING);

err = writer.PutBytes(ContextTag(kTag_SubjectKeyIdentifier_KeyIdentifier), reader.GetValue(), reader.GetValueLen());
err = writer.PutBytes(ContextTag(kTag_SubjectKeyIdentifier), reader.GetValue(), reader.GetValueLen());
SuccessOrExit(err);
}
else if (extensionOID == kOID_Extension_KeyUsage)
{
// This extension MUST be marked as critical.
VerifyOrExit(critical, err = ASN1_ERROR_INVALID_ENCODING);

// KeyUsage ::= BIT STRING
ASN1_PARSE_ELEMENT(kASN1TagClass_Universal, kASN1UniversalTag_BitString);

err = writer.StartContainer(ContextTag(kTag_KeyUsage), kTLVType_Structure, outerContainer);
SuccessOrExit(err);

if (critical)
{
err = writer.PutBoolean(ContextTag(kTag_KeyUsage_Critical), critical);
SuccessOrExit(err);
}

uint32_t keyUsageBits;
err = reader.GetBitString(keyUsageBits);
SuccessOrExit(err);
Expand All @@ -406,11 +367,14 @@ static CHIP_ERROR ConvertExtension(ASN1Reader & reader, TLVWriter & writer)
KeyUsageFlags::kCRLSign, KeyUsageFlags::kEncipherOnly, KeyUsageFlags::kEncipherOnly),
err = ASN1_ERROR_INVALID_ENCODING);

err = writer.Put(ContextTag(kTag_KeyUsage_KeyUsage), keyUsageBits);
err = writer.Put(ContextTag(kTag_KeyUsage), keyUsageBits);
SuccessOrExit(err);
}
else if (extensionOID == kOID_Extension_BasicConstraints)
{
// This extension MUST be marked as critical.
VerifyOrExit(critical, err = ASN1_ERROR_INVALID_ENCODING);

// BasicConstraints ::= SEQUENCE
ASN1_PARSE_ENTER_SEQUENCE
{
Expand All @@ -424,7 +388,7 @@ static CHIP_ERROR ConvertExtension(ASN1Reader & reader, TLVWriter & writer)
{
ASN1_GET_BOOLEAN(isCA);

VerifyOrExit(isCA == true, err = ASN1_ERROR_INVALID_ENCODING);
VerifyOrExit(isCA, err = ASN1_ERROR_INVALID_ENCODING);

err = reader.Next();
}
Expand All @@ -437,44 +401,36 @@ static CHIP_ERROR ConvertExtension(ASN1Reader & reader, TLVWriter & writer)

VerifyOrExit(pathLenConstraint <= UINT8_MAX, err = ASN1_ERROR_INVALID_ENCODING);
VerifyOrExit(pathLenConstraint >= 0, err = ASN1_ERROR_INVALID_ENCODING);

// pathLenConstraint is present only when cA is TRUE
VerifyOrExit(isCA, err = ASN1_ERROR_INVALID_ENCODING);
}

err = writer.StartContainer(ContextTag(kTag_BasicConstraints), kTLVType_Structure, outerContainer);
SuccessOrExit(err);

if (critical)
{
err = writer.PutBoolean(ContextTag(kTag_BasicConstraints_Critical), critical);
SuccessOrExit(err);
}

if (isCA)
{
err = writer.PutBoolean(ContextTag(kTag_BasicConstraints_IsCA), isCA);
SuccessOrExit(err);
}
// Set also when cA is FALSE
err = writer.PutBoolean(ContextTag(kTag_BasicConstraints_IsCA), isCA);
SuccessOrExit(err);

if (pathLenConstraint != -1)
{
err = writer.Put(ContextTag(kTag_BasicConstraints_PathLenConstraint),
static_cast<uint8_t>(pathLenConstraint));
SuccessOrExit(err);
}

err = writer.EndContainer(outerContainer);
SuccessOrExit(err);
}
ASN1_EXIT_SEQUENCE;
}
else if (extensionOID == kOID_Extension_ExtendedKeyUsage)
{
err = writer.StartContainer(ContextTag(kTag_ExtendedKeyUsage), kTLVType_Structure, outerContainer);
SuccessOrExit(err);
// This extension MUST be marked as critical.
VerifyOrExit(critical, err = ASN1_ERROR_INVALID_ENCODING);

if (critical)
{
err = writer.PutBoolean(ContextTag(kTag_ExtendedKeyUsage_Critical), critical);
SuccessOrExit(err);
}

err = writer.StartContainer(ContextTag(kTag_ExtendedKeyUsage_KeyPurposes), kTLVType_Array, outerContainer2);
err = writer.StartContainer(ContextTag(kTag_ExtendedKeyUsage), kTLVType_Array, outerContainer);
SuccessOrExit(err);

// ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId
Expand All @@ -499,14 +455,13 @@ static CHIP_ERROR ConvertExtension(ASN1Reader & reader, TLVWriter & writer)
}
ASN1_EXIT_SEQUENCE;

err = writer.EndContainer(outerContainer2);
err = writer.EndContainer(outerContainer);
SuccessOrExit(err);
}
else
{
ExitNow(err = ASN1_ERROR_UNSUPPORTED_ENCODING);

err = writer.EndContainer(outerContainer);
SuccessOrExit(err);
}
}
ASN1_EXIT_ENCAPSULATED;
}
Expand Down
Loading

0 comments on commit ac22f1b

Please sign in to comment.