Skip to content

Commit

Permalink
address some 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 e554b51 commit 18ccf49
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 21 deletions.
26 changes: 5 additions & 21 deletions src/credentials/GenerateChipX509Cert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ enum IsCACert
kNotCACert,
};

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

static CHIP_ERROR EncodeSubjectPublicKeyInfo(const Crypto::P256PublicKey & pubkey, ASN1Writer & writer)
{
Expand Down Expand Up @@ -94,7 +93,7 @@ static CHIP_ERROR EncodeAuthorityKeyIdentifierExtension(const Crypto::P256Public
{
ASN1_START_SEQUENCE
{
uint8_t keyid[20];
uint8_t keyid[kSHA1_Hash_Langth];
ReturnErrorOnFailure(Crypto::Hash_SHA1(pubkey, pubkey.Length(), keyid));

ReturnErrorOnFailure(
Expand Down Expand Up @@ -122,7 +121,7 @@ static CHIP_ERROR EncodeSubjectKeyIdentifierExtension(const Crypto::P256PublicKe

ASN1_START_OCTET_STRING_ENCAPSULATED
{
uint8_t keyid[20];
uint8_t keyid[kSHA1_Hash_Langth];
ReturnErrorOnFailure(Crypto::Hash_SHA1(pubkey, pubkey.Length(), keyid));

ReturnErrorOnFailure(writer.PutOctetString(keyid, static_cast<uint8_t>(sizeof(keyid))));
Expand Down Expand Up @@ -276,7 +275,7 @@ static CHIP_ERROR EncodeChipDNs(ChipDNParams * params, uint8_t numParams, ASN1Wr
{
ASN1_START_SET
{
uint8_t chipAttrStr[kChipDNAttributeLen + 1];
uint8_t chipAttrStr[kChip64bitAttrUTF8Length + 1];
snprintf(reinterpret_cast<char *>(chipAttrStr), sizeof(chipAttrStr), "%016" PRIX64, params[i].Value);

ASN1_START_SEQUENCE
Expand Down Expand Up @@ -318,22 +317,7 @@ static CHIP_ERROR EncodeChipECDSASignature(Crypto::P256ECDSASignature & signatur
{
CHIP_ERROR err = CHIP_NO_ERROR;

ASN1_START_BIT_STRING_ENCAPSULATED
{
ASN1_START_SEQUENCE
{
const uint8_t * signatureBuf = signature;
uint16_t rLength = (uint16_t) signature.Length() / 2;

// r INTEGER
ReturnErrorOnFailure(writer.PutValue(kASN1TagClass_Universal, kASN1UniversalTag_Integer, false, signatureBuf, rLength));

// s INTEGER
ReturnErrorOnFailure(
writer.PutValue(kASN1TagClass_Universal, kASN1UniversalTag_Integer, false, &signatureBuf[rLength], rLength));
}
ASN1_END_SEQUENCE;
}
ASN1_START_BIT_STRING_ENCAPSULATED { writer.PutRaw(signature, (uint16_t) signature.Length()); }
ASN1_END_ENCAPSULATED;

exit:
Expand Down
78 changes: 78 additions & 0 deletions src/credentials/tests/TestChipCert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,83 @@ static void TestChipCert_GenerateNOCICA(nlTestSuite * inSuite, void * inContext)
NL_TEST_ASSERT(inSuite, DecodeChipCert(outCertBuf, outCertLen, certData) == CHIP_NO_ERROR);
}

static void TestChipCert_VerifyGeneratedCerts(nlTestSuite * inSuite, void * inContext)
{
// Generate a new keypair for cert signing
P256Keypair keypair;
NL_TEST_ASSERT(inSuite, keypair.Initialize() == CHIP_NO_ERROR);

uint8_t root_cert[kTestCertBufSize];
uint32_t root_len = 0;

X509CertRequestParams root_params = { 1234, 0xabcdabcd, 9876, 98790000, true, 0x8888, false, 0 };
NL_TEST_ASSERT(inSuite, NewRootX509Cert(root_params, keypair, root_cert, sizeof(root_cert), root_len) == CHIP_NO_ERROR);

uint8_t ica_cert[kTestCertBufSize];
uint32_t ica_len = 0;

X509CertRequestParams ica_params = { 1234, 0xabcdabcd, 9876, 98790000, true, 0x8888, false, 0 };
P256Keypair ica_keypair;
NL_TEST_ASSERT(inSuite, ica_keypair.Initialize() == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite,
NewICAX509Cert(ica_params, 0xaabbccdd, ica_keypair.Pubkey(), keypair, ica_cert, sizeof(ica_cert), ica_len) ==
CHIP_NO_ERROR);

uint8_t noc_cert[kTestCertBufSize];
uint32_t noc_len = 0;

X509CertRequestParams noc_params = { 1234, 0xaabbccdd, 9876, 98790000, true, 0x8888, true, 0x1234 };
P256Keypair noc_keypair;
NL_TEST_ASSERT(inSuite, noc_keypair.Initialize() == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite,
NewNodeOperationalX509Cert(noc_params, kIssuerIsIntermediateCA, noc_keypair.Pubkey(), ica_keypair, noc_cert,
sizeof(noc_cert), noc_len) == CHIP_NO_ERROR);

ChipCertificateSet certSet;
NL_TEST_ASSERT(inSuite, certSet.Init(3, kTestCertBufSize * 3) == CHIP_NO_ERROR);

uint8_t rootCertBuf[kTestCertBufSize];
uint8_t icaCertBuf[kTestCertBufSize];
uint8_t nocCertBuf[kTestCertBufSize];
uint32_t outCertLen;

NL_TEST_ASSERT(inSuite,
ConvertX509CertToChipCert(root_cert, root_len, rootCertBuf, sizeof(rootCertBuf), outCertLen) == CHIP_NO_ERROR);
NL_TEST_ASSERT(
inSuite,
certSet.LoadCert(rootCertBuf, outCertLen,
BitFlags<CertDecodeFlags>(CertDecodeFlags::kIsTrustAnchor).Set(CertDecodeFlags::kGenerateTBSHash)) ==
CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite,
ConvertX509CertToChipCert(ica_cert, ica_len, icaCertBuf, sizeof(icaCertBuf), outCertLen) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite,
certSet.LoadCert(icaCertBuf, outCertLen, BitFlags<CertDecodeFlags>(CertDecodeFlags::kGenerateTBSHash)) ==
CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite,
ConvertX509CertToChipCert(noc_cert, noc_len, nocCertBuf, sizeof(nocCertBuf), outCertLen) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite,
certSet.LoadCert(nocCertBuf, outCertLen, BitFlags<CertDecodeFlags>(CertDecodeFlags::kGenerateTBSHash)) ==
CHIP_NO_ERROR);

ValidationContext validContext;

validContext.Reset();
NL_TEST_ASSERT(inSuite, SetEffectiveTime(validContext, 2022, 1, 1) == CHIP_NO_ERROR);
validContext.mRequiredKeyUsages.Set(KeyUsageFlags::kDigitalSignature);
validContext.mRequiredKeyPurposes.Set(KeyPurposeFlags::kServerAuth);

// Locate the subject DN and key id that will be used as input the FindValidCert() method.
const ChipDN & subjectDN = certSet.GetCertSet()[2].mSubjectDN;
const CertificateKeyId & subjectKeyId = certSet.GetCertSet()[2].mSubjectKeyId;

ChipCertificateData * resultCert = nullptr;
NL_TEST_ASSERT(inSuite, certSet.FindValidCert(subjectDN, subjectKeyId, validContext, resultCert) == CHIP_NO_ERROR);
}

/**
* Set up the test suite.
*/
Expand Down Expand Up @@ -835,6 +912,7 @@ static const nlTest sTests[] = {
NL_TEST_DEF("Test CHIP Generate ICA Certificate", TestChipCert_GenerateICACert),
NL_TEST_DEF("Test CHIP Generate NOC using Root", TestChipCert_GenerateNOCRoot),
NL_TEST_DEF("Test CHIP Generate NOC using ICA", TestChipCert_GenerateNOCICA),
NL_TEST_DEF("Test CHIP Verify Generated Cert Chain", TestChipCert_VerifyGeneratedCerts),
NL_TEST_SENTINEL()
};
// clang-format on
Expand Down
1 change: 1 addition & 0 deletions src/lib/asn1/ASN1.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ class DLL_EXPORT ASN1Writer
ASN1_ERROR EndEncapsulatedType(void);
ASN1_ERROR PutValue(uint8_t cls, uint32_t tag, bool isConstructed, const uint8_t * val, uint16_t valLen);
ASN1_ERROR PutValue(uint8_t cls, uint32_t tag, bool isConstructed, chip::TLV::TLVReader & val);
ASN1_ERROR PutRaw(const uint8_t * val, uint16_t valLen);

private:
uint8_t * mBuf;
Expand Down
11 changes: 11 additions & 0 deletions src/lib/asn1/ASN1Writer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,17 @@ ASN1_ERROR ASN1Writer::EndEncapsulatedType()
return WriteDeferredLength();
}

ASN1_ERROR ASN1Writer::PutRaw(const uint8_t * val, uint16_t valLen)
{
// Do nothing for a null writer.
VerifyOrReturnError(mBuf != nullptr, ASN1_NO_ERROR);

memmove(mWritePoint, val, valLen);
mWritePoint += valLen;

return ASN1_NO_ERROR;
}

ASN1_ERROR ASN1Writer::PutValue(uint8_t cls, uint32_t tag, bool isConstructed, const uint8_t * val, uint16_t valLen)
{
// Do nothing for a null writer.
Expand Down

0 comments on commit 18ccf49

Please sign in to comment.