Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@pan-apple
pan-apple committed Apr 29, 2021
1 parent 786d1e6 commit 48094a0
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 82 deletions.
34 changes: 20 additions & 14 deletions src/credentials/CHIPCert.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,14 +658,20 @@ CHIP_ERROR GenerateSignedX509CertFromChipCert(const uint8_t * chipCert, uint32_t

struct X509CertRequestParams
{
int64_t mSerialNumber;
uint64_t mIssuer;
uint32_t mValidityStart;
uint32_t mValidityEnd;
bool mHasFabricID;
uint64_t mFabricID;
bool mHasNodeID;
uint64_t mNodeID;
int64_t SerialNumber;
uint64_t Issuer;
uint32_t ValidityStart;
uint32_t ValidityEnd;
bool HasFabricID;
uint64_t FabricID;
bool HasNodeID;
uint64_t NodeID;
};

enum CertificateIssuerLevel
{
kIssuerIsRootCA,
kIssuerIsIntermediateCA,
};

/**
Expand All @@ -679,7 +685,7 @@ struct X509CertRequestParams
*
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR NewRootX509Cert(X509CertRequestParams & requestParams, Crypto::P256Keypair & issuerKeypair, uint8_t * x509CertBuf,
CHIP_ERROR NewRootX509Cert(const X509CertRequestParams & requestParams, Crypto::P256Keypair & issuerKeypair, uint8_t * x509CertBuf,
uint32_t x509CertBufSize, uint32_t & x509CertLen);

/**
Expand All @@ -695,15 +701,15 @@ CHIP_ERROR NewRootX509Cert(X509CertRequestParams & requestParams, Crypto::P256Ke
*
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR NewICAX509Cert(X509CertRequestParams & requestParams, uint64_t subject, const Crypto::P256PublicKey & subjectPubkey,
Crypto::P256Keypair & issuerKeypair, uint8_t * x509CertBuf, uint32_t x509CertBufSize,
uint32_t & x509CertLen);
CHIP_ERROR NewICAX509Cert(const X509CertRequestParams & requestParams, uint64_t subject,
const Crypto::P256PublicKey & subjectPubkey, Crypto::P256Keypair & issuerKeypair, uint8_t * x509CertBuf,
uint32_t x509CertBufSize, uint32_t & x509CertLen);

/**
* @brief Generate a new X.509 DER encoded Node operational certificate
*
* @param requestParams Certificate request parameters.
* @param rootCASigner Indicates if the signer is a root CA or an intermediate CA
* @param issuerLevel Indicates if the issuer is a root CA or an intermediate CA
* @param subjectPubkey The public key of subject
* @param issuerKeypair The certificate signing key
* @param x509CertBuf Buffer to store signed certificate in X.509 DER format.
Expand All @@ -712,7 +718,7 @@ CHIP_ERROR NewICAX509Cert(X509CertRequestParams & requestParams, uint64_t subjec
*
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR NewNodeOperationalX509Cert(X509CertRequestParams & requestParams, bool rootCASigner,
CHIP_ERROR NewNodeOperationalX509Cert(const X509CertRequestParams & requestParams, CertificateIssuerLevel issuerLevel,
const Crypto::P256PublicKey & subjectPubkey, Crypto::P256Keypair & issuerKeypair,
uint8_t * x509CertBuf, uint32_t x509CertBufSize, uint32_t & x509CertLen);

Expand Down
94 changes: 52 additions & 42 deletions src/credentials/GenerateChipX509Cert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@ namespace Credentials {
using namespace chip::ASN1;
using namespace chip::Protocols;

struct ChipDNParams
{
OID AttrOID;
uint64_t Value;
};

enum IsCACert
{
kCACert,
kNotCACert,
};

// For CHIP 64-bit DN attribute the string representation is 16 uppercase hex characters.
static constexpr uint8_t kChipDNAttributeLen = 16;

static CHIP_ERROR EncodeSubjectPublicKeyInfo(const Crypto::P256PublicKey & pubkey, ASN1Writer & writer)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand Down Expand Up @@ -141,7 +156,7 @@ static CHIP_ERROR EncodeKeyUsageExtension(uint16_t keyUsageBits, ASN1Writer & wr
return err;
}

static CHIP_ERROR EncodeIsCAExtension(bool isCA, ASN1Writer & writer)
static CHIP_ERROR EncodeIsCAExtension(IsCACert isCA, ASN1Writer & writer)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand All @@ -159,10 +174,10 @@ static CHIP_ERROR EncodeIsCAExtension(bool isCA, ASN1Writer & writer)
ASN1_START_SEQUENCE
{
// cA BOOLEAN
if (isCA)
if (isCA == kCACert)
{
// Encode the boolean only if isCA is true
ASN1_ENCODE_BOOLEAN(isCA);
ASN1_ENCODE_BOOLEAN(true);
}
}
ASN1_END_SEQUENCE;
Expand All @@ -177,7 +192,7 @@ static CHIP_ERROR EncodeIsCAExtension(bool isCA, ASN1Writer & writer)

static CHIP_ERROR EncodeCASpecificExtensions(ASN1Writer & writer)
{
ReturnErrorOnFailure(EncodeIsCAExtension(true, writer));
ReturnErrorOnFailure(EncodeIsCAExtension(kCACert, writer));

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

Expand All @@ -193,7 +208,7 @@ static CHIP_ERROR EncodeNOCSpecificExtensions(ASN1Writer & writer)
uint16_t keyUsageBits =
static_cast<uint16_t>(KeyUsageFlags::kDigitalSignature) | static_cast<uint16_t>(KeyUsageFlags::kKeyEncipherment);

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

ASN1_START_SEQUENCE
Expand Down Expand Up @@ -251,12 +266,6 @@ static CHIP_ERROR EncodeExtensions(bool isCA, const Crypto::P256PublicKey & SKI,
return err;
}

struct ChipDNParams
{
OID mAttrOID;
uint64_t mValue;
};

static CHIP_ERROR EncodeChipDNs(ChipDNParams * params, uint8_t numParams, ASN1Writer & writer)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand All @@ -267,12 +276,12 @@ static CHIP_ERROR EncodeChipDNs(ChipDNParams * params, uint8_t numParams, ASN1Wr
{
ASN1_START_SET
{
uint8_t chipAttrStr[17];
snprintf(reinterpret_cast<char *>(chipAttrStr), sizeof(chipAttrStr), "%016" PRIX64, params[i].mValue);
uint8_t chipAttrStr[kChipDNAttributeLen + 1];
snprintf(reinterpret_cast<char *>(chipAttrStr), sizeof(chipAttrStr), "%016" PRIX64, params[i].Value);

ASN1_START_SEQUENCE
{
ASN1_ENCODE_OBJECT_ID(params[i].mAttrOID);
ASN1_ENCODE_OBJECT_ID(params[i].AttrOID);
ReturnErrorOnFailure(writer.PutString(kASN1UniversalTag_UTF8String, Uint8::to_const_char(chipAttrStr), 16));
}
ASN1_END_SEQUENCE;
Expand Down Expand Up @@ -331,7 +340,7 @@ static CHIP_ERROR EncodeChipECDSASignature(Crypto::P256ECDSASignature & signatur
return err;
}

CHIP_ERROR EncodeTBSCert(X509CertRequestParams & requestParams, bool rootCASigner, uint64_t subject,
CHIP_ERROR EncodeTBSCert(const X509CertRequestParams & requestParams, CertificateIssuerLevel issuerLevel, uint64_t subject,
const Crypto::P256PublicKey & subjectPubkey, const Crypto::P256PublicKey & issuerPubkey,
ASN1Writer & writer)
{
Expand All @@ -350,45 +359,45 @@ CHIP_ERROR EncodeTBSCert(X509CertRequestParams & requestParams, bool rootCASigne
}
ASN1_END_CONSTRUCTED;

ReturnErrorOnFailure(writer.PutInteger(requestParams.mSerialNumber));
ReturnErrorOnFailure(writer.PutInteger(requestParams.SerialNumber));

ASN1_START_SEQUENCE { ASN1_ENCODE_OBJECT_ID(kOID_SigAlgo_ECDSAWithSHA256); }
ASN1_END_SEQUENCE;

// Issuer OID depends on if cert is being signed by the root CA
if (rootCASigner)
if (issuerLevel == kIssuerIsRootCA)
{
dnParams[0].mAttrOID = chip::ASN1::kOID_AttributeType_ChipRootId;
dnParams[0].AttrOID = chip::ASN1::kOID_AttributeType_ChipRootId;
}
else
{
dnParams[0].mAttrOID = chip::ASN1::kOID_AttributeType_ChipICAId;
dnParams[0].AttrOID = chip::ASN1::kOID_AttributeType_ChipICAId;
}
dnParams[0].mValue = requestParams.mIssuer;
dnParams[0].Value = requestParams.Issuer;

if (requestParams.mHasFabricID)
if (requestParams.HasFabricID)
{
dnParams[1].mAttrOID = chip::ASN1::kOID_AttributeType_ChipFabricId;
dnParams[1].mValue = requestParams.mFabricID;
numDNs = 2;
dnParams[1].AttrOID = chip::ASN1::kOID_AttributeType_ChipFabricId;
dnParams[1].Value = requestParams.FabricID;
numDNs = 2;
}
ReturnErrorOnFailure(EncodeChipDNs(dnParams, numDNs, writer));

// validity Validity,
ReturnErrorOnFailure(EncodeValidity(requestParams.mValidityStart, requestParams.mValidityEnd, writer));
ReturnErrorOnFailure(EncodeValidity(requestParams.ValidityStart, requestParams.ValidityEnd, writer));

// subject Name
if (requestParams.mHasNodeID)
if (requestParams.HasNodeID)
{
dnParams[0].mAttrOID = chip::ASN1::kOID_AttributeType_ChipNodeId;
dnParams[0].AttrOID = chip::ASN1::kOID_AttributeType_ChipNodeId;

isCA = false;
}
else if (subjectPubkey != issuerPubkey)
{
dnParams[0].mAttrOID = chip::ASN1::kOID_AttributeType_ChipICAId;
dnParams[0].AttrOID = chip::ASN1::kOID_AttributeType_ChipICAId;
}
dnParams[0].mValue = subject;
dnParams[0].Value = subject;

ReturnErrorOnFailure(EncodeChipDNs(dnParams, numDNs, writer));

Expand All @@ -403,15 +412,15 @@ CHIP_ERROR EncodeTBSCert(X509CertRequestParams & requestParams, bool rootCASigne
return err;
}

static CHIP_ERROR NewChipX509Cert(X509CertRequestParams & requestParams, bool rootCASigner, uint64_t subject,
static CHIP_ERROR NewChipX509Cert(const X509CertRequestParams & requestParams, CertificateIssuerLevel issuerLevel, uint64_t subject,
const Crypto::P256PublicKey & subjectPubkey, Crypto::P256Keypair & issuerKeypair,
uint8_t * x509CertBuf, uint32_t x509CertBufSize, uint32_t & x509CertLen)
{
CHIP_ERROR err = CHIP_NO_ERROR;
ASN1Writer writer;
writer.Init(x509CertBuf, x509CertBufSize);

ReturnErrorOnFailure(EncodeTBSCert(requestParams, rootCASigner, subject, subjectPubkey, issuerKeypair.Pubkey(), writer));
ReturnErrorOnFailure(EncodeTBSCert(requestParams, issuerLevel, subject, subjectPubkey, issuerKeypair.Pubkey(), writer));
writer.Finalize();

Crypto::P256ECDSASignature signature;
Expand All @@ -421,7 +430,7 @@ static CHIP_ERROR NewChipX509Cert(X509CertRequestParams & requestParams, bool ro

ASN1_START_SEQUENCE
{
ReturnErrorOnFailure(EncodeTBSCert(requestParams, rootCASigner, subject, subjectPubkey, issuerKeypair.Pubkey(), writer));
ReturnErrorOnFailure(EncodeTBSCert(requestParams, issuerLevel, subject, subjectPubkey, issuerKeypair.Pubkey(), writer));
SuccessOrExit(err);

ASN1_START_SEQUENCE { ASN1_ENCODE_OBJECT_ID(kOID_SigAlgo_ECDSAWithSHA256); }
Expand All @@ -439,29 +448,30 @@ static CHIP_ERROR NewChipX509Cert(X509CertRequestParams & requestParams, bool ro
return err;
}

DLL_EXPORT CHIP_ERROR NewRootX509Cert(X509CertRequestParams & requestParams, Crypto::P256Keypair & issuerKeypair,
DLL_EXPORT CHIP_ERROR NewRootX509Cert(const X509CertRequestParams & requestParams, Crypto::P256Keypair & issuerKeypair,
uint8_t * x509CertBuf, uint32_t x509CertBufSize, uint32_t & x509CertLen)
{
ReturnErrorCodeIf(requestParams.mHasNodeID, CHIP_ERROR_INVALID_ARGUMENT);
return NewChipX509Cert(requestParams, true, requestParams.mIssuer, issuerKeypair.Pubkey(), issuerKeypair, x509CertBuf,
ReturnErrorCodeIf(requestParams.HasNodeID, CHIP_ERROR_INVALID_ARGUMENT);
return NewChipX509Cert(requestParams, kIssuerIsRootCA, requestParams.Issuer, issuerKeypair.Pubkey(), issuerKeypair, x509CertBuf,
x509CertBufSize, x509CertLen);
}

DLL_EXPORT CHIP_ERROR NewICAX509Cert(X509CertRequestParams & requestParams, uint64_t subject,
DLL_EXPORT CHIP_ERROR NewICAX509Cert(const X509CertRequestParams & requestParams, uint64_t subject,
const Crypto::P256PublicKey & subjectPubkey, Crypto::P256Keypair & issuerKeypair,
uint8_t * x509CertBuf, uint32_t x509CertBufSize, uint32_t & x509CertLen)
{
ReturnErrorCodeIf(requestParams.mHasNodeID, CHIP_ERROR_INVALID_ARGUMENT);
return NewChipX509Cert(requestParams, true, subject, subjectPubkey, issuerKeypair, x509CertBuf, x509CertBufSize, x509CertLen);
ReturnErrorCodeIf(requestParams.HasNodeID, CHIP_ERROR_INVALID_ARGUMENT);
return NewChipX509Cert(requestParams, kIssuerIsRootCA, subject, subjectPubkey, issuerKeypair, x509CertBuf, x509CertBufSize,
x509CertLen);
}

DLL_EXPORT CHIP_ERROR NewNodeOperationalX509Cert(X509CertRequestParams & requestParams, bool rootCASigner,
DLL_EXPORT CHIP_ERROR NewNodeOperationalX509Cert(const X509CertRequestParams & requestParams, CertificateIssuerLevel issuerLevel,
const Crypto::P256PublicKey & subjectPubkey, Crypto::P256Keypair & issuerKeypair,
uint8_t * x509CertBuf, uint32_t x509CertBufSize, uint32_t & x509CertLen)
{
VerifyOrReturnError(requestParams.mHasNodeID, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(requestParams.mHasFabricID, CHIP_ERROR_INVALID_ARGUMENT);
return NewChipX509Cert(requestParams, rootCASigner, requestParams.mNodeID, subjectPubkey, issuerKeypair, x509CertBuf,
VerifyOrReturnError(requestParams.HasNodeID, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(requestParams.HasFabricID, CHIP_ERROR_INVALID_ARGUMENT);
return NewChipX509Cert(requestParams, issuerLevel, requestParams.NodeID, subjectPubkey, issuerKeypair, x509CertBuf,
x509CertBufSize, x509CertLen);
}

Expand Down
26 changes: 13 additions & 13 deletions src/credentials/tests/TestChipCert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ static void TestChipCert_GenerateRootCert(nlTestSuite * inSuite, void * inContex
NL_TEST_ASSERT(inSuite, DecodeChipCert(outCertBuf, outCertLen, certData) == CHIP_NO_ERROR);

// Test that root cert cannot be provided a node ID
root_params.mHasNodeID = true;
root_params.HasNodeID = true;
NL_TEST_ASSERT(inSuite,
NewRootX509Cert(root_params, keypair, signed_cert, sizeof(signed_cert), signed_len) ==
CHIP_ERROR_INVALID_ARGUMENT);
Expand Down Expand Up @@ -720,7 +720,7 @@ static void TestChipCert_GenerateICACert(nlTestSuite * inSuite, void * inContext
NL_TEST_ASSERT(inSuite, DecodeChipCert(outCertBuf, outCertLen, certData) == CHIP_NO_ERROR);

// Test that ICA cert cannot be provided a node ID
ica_params.mHasNodeID = true;
ica_params.HasNodeID = true;
NL_TEST_ASSERT(inSuite,
NewICAX509Cert(ica_params, 4321, ica_keypair.Pubkey(), keypair, signed_cert, sizeof(signed_cert), signed_len) ==
CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -745,26 +745,26 @@ static void TestChipCert_GenerateNOCRoot(nlTestSuite * inSuite, void * inContext
NL_TEST_ASSERT(inSuite, noc_keypair.Initialize() == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite,
NewNodeOperationalX509Cert(noc_params, true, noc_keypair.Pubkey(), keypair, signed_cert, sizeof(signed_cert),
signed_len) == CHIP_NO_ERROR);
NewNodeOperationalX509Cert(noc_params, kIssuerIsRootCA, noc_keypair.Pubkey(), keypair, signed_cert,
sizeof(signed_cert), signed_len) == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite,
ConvertX509CertToChipCert(signed_cert, signed_len, outCertBuf, sizeof(outCertBuf), outCertLen) == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite, DecodeChipCert(outCertBuf, outCertLen, certData) == CHIP_NO_ERROR);

// Test that NOC cert must be provided a node ID
noc_params.mHasNodeID = false;
noc_params.HasNodeID = false;
NL_TEST_ASSERT(inSuite,
NewNodeOperationalX509Cert(noc_params, true, noc_keypair.Pubkey(), keypair, signed_cert, sizeof(signed_cert),
signed_len) == CHIP_ERROR_INVALID_ARGUMENT);
NewNodeOperationalX509Cert(noc_params, kIssuerIsRootCA, noc_keypair.Pubkey(), keypair, signed_cert,
sizeof(signed_cert), signed_len) == CHIP_ERROR_INVALID_ARGUMENT);

// Test that NOC cert must be provided a fabric ID
noc_params.mHasNodeID = true;
noc_params.mHasFabricID = false;
noc_params.HasNodeID = true;
noc_params.HasFabricID = false;
NL_TEST_ASSERT(inSuite,
NewNodeOperationalX509Cert(noc_params, true, noc_keypair.Pubkey(), keypair, signed_cert, sizeof(signed_cert),
signed_len) == CHIP_ERROR_INVALID_ARGUMENT);
NewNodeOperationalX509Cert(noc_params, kIssuerIsRootCA, noc_keypair.Pubkey(), keypair, signed_cert,
sizeof(signed_cert), signed_len) == CHIP_ERROR_INVALID_ARGUMENT);
}

static void TestChipCert_GenerateNOCICA(nlTestSuite * inSuite, void * inContext)
Expand All @@ -786,8 +786,8 @@ static void TestChipCert_GenerateNOCICA(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, noc_keypair.Initialize() == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite,
NewNodeOperationalX509Cert(noc_params, false, noc_keypair.Pubkey(), keypair, signed_cert, sizeof(signed_cert),
signed_len) == CHIP_NO_ERROR);
NewNodeOperationalX509Cert(noc_params, kIssuerIsIntermediateCA, noc_keypair.Pubkey(), keypair, signed_cert,
sizeof(signed_cert), signed_len) == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite,
ConvertX509CertToChipCert(signed_cert, signed_len, outCertBuf, sizeof(outCertBuf), outCertLen) == CHIP_NO_ERROR);
Expand Down
8 changes: 2 additions & 6 deletions src/crypto/CHIPCryptoPALOpenSSL.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,16 +297,12 @@ CHIP_ERROR Hash_SHA256(const uint8_t * data, const size_t data_length, uint8_t *

CHIP_ERROR Hash_SHA1(const uint8_t * data, const size_t data_length, uint8_t * out_buffer)
{
CHIP_ERROR error = CHIP_NO_ERROR;

// zero data length hash is supported.

VerifyOrExit(out_buffer != nullptr, error = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(out_buffer != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

SHA1(data, data_length, Uint8::to_uchar(out_buffer));

exit:
return error;
return CHIP_NO_ERROR;
}

Hash_SHA256_stream::Hash_SHA256_stream() {}
Expand Down
Loading

0 comments on commit 48094a0

Please sign in to comment.