From 18030376ecaa4856cd7db98f033e5c143412078b Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 15 Jul 2021 01:58:05 -0400 Subject: [PATCH] Add utilities for extracting a PeerId from an opcert, and use them in CASE. (#8345) This sets up the PeerNodeId for our CASE session's PeerConnectionState correctly. Without doing this, when we try to expire existing connections to the peer node id at the end of CASE setup (which would normally drop stale CASE sessions to the same node, PASE sessions, etc), we fail to do that, because we try to do it for kUndefinedNodeId instead of the right node id. --- src/credentials/CHIPCert.cpp | 51 ++++++++++++ src/credentials/CHIPCert.h | 32 +++++++ src/credentials/tests/TestChipCert.cpp | 87 ++++++++++++++++++++ src/protocols/secure_channel/CASESession.cpp | 6 ++ 4 files changed, 176 insertions(+) diff --git a/src/credentials/CHIPCert.cpp b/src/credentials/CHIPCert.cpp index 1769d387a7804f..0057e413381781 100644 --- a/src/credentials/CHIPCert.cpp +++ b/src/credentials/CHIPCert.cpp @@ -933,5 +933,56 @@ CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, ASN1Writ return err; } +CHIP_ERROR ExtractPeerIdFromOpCert(const ChipCertificateData & opcert, PeerId * peerId) +{ + // Since we assume the cert is pre-validated, we are going to assume that + // its subject in fact has both a node id and a fabric id. + PeerId id; + bool foundNodeId = false; + bool foundFabricId = false; + const ChipDN & subjectDN = opcert.mSubjectDN; + for (uint8_t i = 0; i < subjectDN.RDNCount(); ++i) + { + const auto & rdn = subjectDN.rdn[i]; + if (rdn.mAttrOID == ASN1::kOID_AttributeType_ChipNodeId) + { + id.SetNodeId(rdn.mChipVal); + foundNodeId = true; + } + else if (rdn.mAttrOID == ASN1::kOID_AttributeType_ChipFabricId) + { + id.SetFabricId(rdn.mChipVal); + foundFabricId = true; + } + } + if (!foundNodeId || !foundFabricId) + { + return CHIP_ERROR_INVALID_ARGUMENT; + } + + *peerId = id; + return CHIP_NO_ERROR; +} + +CHIP_ERROR ExtractPeerIdFromOpCert(const ByteSpan & opcert, PeerId * peerId) +{ + ChipCertificateSet certSet; + + ReturnErrorOnFailure(certSet.Init(1, kMaxCHIPCertDecodeBufLength)); + + ReturnErrorOnFailure(certSet.LoadCert(opcert.data(), static_cast(opcert.size()), BitFlags())); + + return ExtractPeerIdFromOpCert(certSet.GetCertSet()[0], peerId); +} + +CHIP_ERROR ExtractPeerIdFromOpCertArray(const ByteSpan & opcertarray, PeerId * peerId) +{ + ByteSpan noc; + ByteSpan icac; + ReturnErrorOnFailure(ExtractCertsFromCertArray(opcertarray, noc, icac)); + + return ExtractPeerIdFromOpCert(noc, peerId); +} + } // namespace Credentials } // namespace chip diff --git a/src/credentials/CHIPCert.h b/src/credentials/CHIPCert.h index 7f231fe87360b5..42d4dc97f6e2af 100644 --- a/src/credentials/CHIPCert.h +++ b/src/credentials/CHIPCert.h @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -867,5 +868,36 @@ CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, ASN1::AS */ CHIP_ERROR ConvertECDSASignatureDERToRaw(ASN1::ASN1Reader & reader, chip::TLV::TLVWriter & writer, uint64_t tag); +/** + * Extract a PeerId from an operational certificate that has already been + * parsed. + * + * @return CHIP_ERROR_INVALID_ARGUMENT if the passed-in cert does not have at + * least one NodeId RDN and one FabricId RDN in the Subject DN. No other + * validation (e.g. checkign that there is exactly one RDN of each type) is + * performed. + */ +CHIP_ERROR ExtractPeerIdFromOpCert(const ChipCertificateData & opcert, PeerId * peerId); + +/** + * Extract a PeerId from an operational certificate in ByteSpan TLV-encoded + * form. This does not perform any sort of validation on the certificate + * structure other than parsing it. + * + * Can return any error that can be returned from parsing the cert or from the + * ChipCertificateData* version of ExtractPeerIdFromOpCert. + */ +CHIP_ERROR ExtractPeerIdFromOpCert(const ByteSpan & opcert, PeerId * peerId); + +/** + * Extract a PeerId from an operational certificate array in ByteSpan + * TLV-encoded form. This does not perform any sort of validation on the + * certificate structure other than parsing it. + * + * Can return any error that can be returned from parsing the array or from the + * ChipCertificateData* version of ExtractPeerIdFromOpCert. + */ +CHIP_ERROR ExtractPeerIdFromOpCertArray(const ByteSpan & opcertarray, PeerId * peerId); + } // namespace Credentials } // namespace chip diff --git a/src/credentials/tests/TestChipCert.cpp b/src/credentials/tests/TestChipCert.cpp index 89cdad88cde16c..d15cfe2221b312 100644 --- a/src/credentials/tests/TestChipCert.cpp +++ b/src/credentials/tests/TestChipCert.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -1328,6 +1329,91 @@ static void TestChipCert_ChipArrayToChipCertsNoICA(nlTestSuite * inSuite, void * NL_TEST_ASSERT(inSuite, certSet.FindValidCert(subjectDN, subjectKeyId, validContext, resultCert) == CHIP_NO_ERROR); } +static void TestChipCert_ExtractPeerId(nlTestSuite * inSuite, void * inContext) +{ + struct TestCase + { + uint8_t Cert; + uint8_t ICACert; + uint64_t ExpectedNodeId; + uint64_t ExpectedFabricId; + }; + + // clang-format off + static constexpr TestCase sTestCases[] = { + // Cert ICA ExpectedNodeId ExpectedFabricId + // ============================================================= + { TestCert::kNode01_01, TestCert::kICA01, 0xDEDEDEDE00010001, 0xFAB000000000001D }, + { TestCert::kNode01_02, TestCert::kNone, 0xDEDEDEDE00010002, 0xFAB000000000001D }, + { TestCert::kNode02_01, TestCert::kICA02, 0xDEDEDEDE00020001, 0xFAB000000000001D }, + { TestCert::kNode02_02, TestCert::kICA02, 0xDEDEDEDE00020002, 0xFAB000000000001D }, + { TestCert::kNode02_03, TestCert::kICA02, 0xDEDEDEDE00020003, 0xFAB000000000001D }, + { TestCert::kNode02_04, TestCert::kICA02, 0xDEDEDEDE00020004, 0xFAB000000000001D }, + { TestCert::kNode02_05, TestCert::kICA02, 0xDEDEDEDE00020005, 0xFAB000000000001D }, + { TestCert::kNode02_06, TestCert::kICA02, 0xDEDEDEDE00020006, 0xFAB000000000001D }, + { TestCert::kNode02_07, TestCert::kICA02, 0xDEDEDEDE00020007, 0xFAB000000000001D }, + }; + // clang-format on + + // Test extraction from the raw ByteSpan form. + for (auto & testCase : sTestCases) + { + ByteSpan cert; + CHIP_ERROR err = GetTestCert(testCase.Cert, sNullLoadFlag, cert); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + PeerId peerId; + err = ExtractPeerIdFromOpCert(cert, &peerId); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, peerId.GetNodeId() == testCase.ExpectedNodeId); + NL_TEST_ASSERT(inSuite, peerId.GetFabricId() == testCase.ExpectedFabricId); + } + + // Test extraction from the parsed form. + ChipCertificateSet certSet; + for (auto & testCase : sTestCases) + { + CHIP_ERROR err = certSet.Init(1, kMaxCHIPCertDecodeBufLength); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + err = LoadTestCert(certSet, testCase.Cert, sNullLoadFlag, sNullDecodeFlag); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + PeerId peerId; + err = ExtractPeerIdFromOpCert(certSet.GetCertSet()[0], &peerId); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, peerId.GetNodeId() == testCase.ExpectedNodeId); + NL_TEST_ASSERT(inSuite, peerId.GetFabricId() == testCase.ExpectedFabricId); + certSet.Release(); + } + + // Test extraction from cert array form. + for (auto & testCase : sTestCases) + { + ByteSpan cert; + CHIP_ERROR err = GetTestCert(testCase.Cert, sDerFormFlag, cert); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + ByteSpan icaCert; + if (testCase.ICACert != TestCert::kNone) + { + err = GetTestCert(testCase.ICACert, sDerFormFlag, icaCert); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + } + + uint8_t certArray[kMaxCHIPCertLength * 2]; + MutableByteSpan certs(certArray); + err = ConvertX509CertsToChipCertArray(cert, icaCert, certs); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + PeerId peerId; + err = ExtractPeerIdFromOpCertArray(certs, &peerId); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, peerId.GetNodeId() == testCase.ExpectedNodeId); + NL_TEST_ASSERT(inSuite, peerId.GetFabricId() == testCase.ExpectedFabricId); + } +} + /** * Set up the test suite. */ @@ -1376,6 +1462,7 @@ static const nlTest sTests[] = { NL_TEST_DEF("Test CHIP Certificates X509 to CHIP Array Conversion Error Scenarios", TestChipCert_X509ToChipArrayErrorScenarios), NL_TEST_DEF("Test CHIP Array to Chip Certificates Conversion", TestChipCert_ChipArrayToChipCerts), NL_TEST_DEF("Test No ICA CHIP Array to Chip Certificates Conversion", TestChipCert_ChipArrayToChipCertsNoICA), + NL_TEST_DEF("Test extracting PeerId from node certificate", TestChipCert_ExtractPeerId), NL_TEST_SENTINEL() }; // clang-format on diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 55dbfed81295cf..5d84bf464d81bf 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -1216,6 +1216,12 @@ CHIP_ERROR CASESession::Validate_and_RetrieveResponderID(const uint8_t * respond const CertificateKeyId & subjectKeyId = certSet.GetCertSet()[0].mSubjectKeyId; ReturnErrorOnFailure(mOpCredSet->FindValidCert(mTrustedRootId, subjectDN, subjectKeyId, mValidContext, resultCert)); + + // Now that we have verified that this is a valid cert, try to get the + // peer's operational identity from it. + PeerId peerId; + ReturnErrorOnFailure(ExtractPeerIdFromOpCert(certSet.GetCertSet()[0], &peerId)); + mConnectionState.SetPeerNodeId(peerId.GetNodeId()); } return CHIP_NO_ERROR;