From 338556d808fdc7ea9fab73217964297088d5a130 Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Sat, 7 Dec 2024 11:53:03 +0530 Subject: [PATCH 1/3] da_revocation: option to load test data from the test without using file --- .../TestDACRevocationDelegateImpl.cpp | 55 ++++++++++++++----- .../TestDACRevocationDelegateImpl.h | 5 ++ .../TestDeviceAttestationCredentials.cpp | 49 ++++++++--------- 3 files changed, 69 insertions(+), 40 deletions(-) diff --git a/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp b/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp index 15804a61a3ead2..bbc73e1d58a757 100644 --- a/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp +++ b/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp @@ -50,12 +50,23 @@ CHIP_ERROR TestDACRevocationDelegateImpl::SetDeviceAttestationRevocationSetPath( return CHIP_NO_ERROR; } +CHIP_ERROR TestDACRevocationDelegateImpl::SetDeviceAttestationRevocationData(const std::string & jsonData) +{ + mRevocationData = jsonData; + return CHIP_NO_ERROR; +} + void TestDACRevocationDelegateImpl::ClearDeviceAttestationRevocationSetPath() { // clear the string_view mDeviceAttestationRevocationSetPath = mDeviceAttestationRevocationSetPath.substr(0, 0); } +void TestDACRevocationDelegateImpl::ClearDeviceAttestationRevocationData() +{ + mRevocationData.clear(); +} + // Check if issuer and AKID matches with the crl signer OR crl signer delegator's subject and SKID bool TestDACRevocationDelegateImpl::CrossValidateCert(const Json::Value & revokedSet, const std::string & akidHexStr, const std::string & issuerNameBase64Str) @@ -115,26 +126,42 @@ bool TestDACRevocationDelegateImpl::CrossValidateCert(const Json::Value & revoke bool TestDACRevocationDelegateImpl::IsEntryInRevocationSet(const std::string & akidHexStr, const std::string & issuerNameBase64Str, const std::string & serialNumberHexStr) { - std::ifstream file(mDeviceAttestationRevocationSetPath.c_str()); - if (!file.is_open()) - { - ChipLogError(NotSpecified, "Failed to open file: %s", mDeviceAttestationRevocationSetPath.c_str()); - return false; - } - - // Parse the JSON data incrementally Json::CharReaderBuilder readerBuilder; Json::Value jsonData; std::string errs; - bool parsingSuccessful = Json::parseFromStream(readerBuilder, file, &jsonData, &errs); + // Try direct data first, then fall back to file + std::istringstream jsonStream(!mRevocationData.empty() ? mRevocationData : "[]"); - // Close the file as it's no longer needed - file.close(); + if (!mRevocationData.empty()) + { + if (!Json::parseFromStream(readerBuilder, jsonStream, &jsonData, &errs)) + { + ChipLogError(NotSpecified, "Failed to parse JSON data: %s", errs.c_str()); + return false; + } + } + else if (!mDeviceAttestationRevocationSetPath.empty()) + { + std::ifstream file(mDeviceAttestationRevocationSetPath.c_str()); + if (!file.is_open()) + { + ChipLogError(NotSpecified, "Failed to open file: %s", mDeviceAttestationRevocationSetPath.c_str()); + return false; + } - if (!parsingSuccessful) + bool parsingSuccessful = Json::parseFromStream(readerBuilder, file, &jsonData, &errs); + file.close(); + + if (!parsingSuccessful) + { + ChipLogError(NotSpecified, "Failed to parse JSON from file: %s", errs.c_str()); + return false; + } + } + else { - ChipLogError(NotSpecified, "Failed to parse JSON: %s", errs.c_str()); + // No revocation data available return false; } @@ -278,7 +305,7 @@ void TestDACRevocationDelegateImpl::CheckForRevokedDACChain( { AttestationVerificationResult attestationError = AttestationVerificationResult::kSuccess; - if (mDeviceAttestationRevocationSetPath.empty()) + if (mDeviceAttestationRevocationSetPath.empty() && mRevocationData.empty()) { onCompletion->mCall(onCompletion->mContext, info, attestationError); return; diff --git a/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.h b/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.h index cea143603c341d..fc93ce4487e83a 100644 --- a/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.h +++ b/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.h @@ -52,6 +52,10 @@ class TestDACRevocationDelegateImpl : public DeviceAttestationRevocationDelegate // This can be used to skip the revocation check void ClearDeviceAttestationRevocationSetPath(); + // Set JSON data directly for unit test purposes. + CHIP_ERROR SetDeviceAttestationRevocationData(const std::string & jsonData); + void ClearDeviceAttestationRevocationData(); + private: enum class KeyIdType : uint8_t { @@ -83,6 +87,7 @@ class TestDACRevocationDelegateImpl : public DeviceAttestationRevocationDelegate bool IsCertificateRevoked(const ByteSpan & certDer); std::string mDeviceAttestationRevocationSetPath; + std::string mRevocationData; // Stores direct JSON data }; } // namespace Credentials diff --git a/src/credentials/tests/TestDeviceAttestationCredentials.cpp b/src/credentials/tests/TestDeviceAttestationCredentials.cpp index 8480fff4daaae4..f5bee29df50533 100644 --- a/src/credentials/tests/TestDeviceAttestationCredentials.cpp +++ b/src/credentials/tests/TestDeviceAttestationCredentials.cpp @@ -417,17 +417,6 @@ TEST_F(TestDeviceAttestationCredentials, TestAttestationTrustStore) } } -static void WriteTestRevokedData(const char * jsonData, const char * fileName) -{ - // TODO: Add option to load test data from the test without using file. #34588 - - // write data to /tmp/sample_revoked_set.json using fstream APIs - std::ofstream file; - file.open(fileName, std::ofstream::out | std::ofstream::trunc); - file << jsonData; - file.close(); -} - TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) { uint8_t attestationElementsTestVector[] = { 0 }; @@ -456,16 +445,14 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) TestDACRevocationDelegateImpl revocationDelegateImpl; - // Test without revocation set + // Test without revocation data revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); EXPECT_EQ(attestationResult, AttestationVerificationResult::kSuccess); - const char * tmpJsonFile = "/tmp/sample_revoked_set.json"; - revocationDelegateImpl.SetDeviceAttestationRevocationSetPath(tmpJsonFile); - // Test empty json - WriteTestRevokedData("", tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(""); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kSuccess); // Test DAC is revoked, crl signer is PAI itself @@ -478,8 +465,9 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) "revoked_serial_numbers": ["0C694F7F866067B2"] }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kDacRevoked); // Test PAI is revoked, crl signer is PAA itself @@ -492,8 +480,9 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) "revoked_serial_numbers": ["3E6CE6509AD840CD"] }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kPaiRevoked); // Test DAC and PAI both revoked, crl signers are PAI and PAA respectively @@ -513,7 +502,7 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) "revoked_serial_numbers": ["3E6CE6509AD840CD"] }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); EXPECT_EQ(attestationResult, AttestationVerificationResult::kPaiAndDacRevoked); @@ -523,6 +512,7 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) TestCerts::sTestCert_PAI_FFF2_8001_Cert, TestCerts::sTestCert_DAC_FFF2_8001_0008_Cert, ByteSpan(attestationNonceTestVector), static_cast(0xFFF2), 0x8001); revocationDelegateImpl.CheckForRevokedDACChain(FFF2_8001_info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kSuccess); // Test issuer does not match @@ -536,8 +526,9 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) "revoked_serial_numbers": ["0C694F7F866067B2"] }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kSuccess); // Test subject key ID does not match @@ -551,8 +542,9 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) "revoked_serial_numbers": ["0C694F7F866067B2"] }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kSuccess); // Test serial number does not match @@ -566,8 +558,9 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) "revoked_serial_numbers": ["3E6CE6509AD840CD1", "BC694F7F866067B1"] }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kSuccess); // Test starting serial number bytes match but not all, @@ -581,8 +574,9 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) "revoked_serial_numbers": ["0C694F7F866067B21234"] }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kSuccess); // Test DAC is revoked, and crl signer delegator is present @@ -597,8 +591,9 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) "revoked_serial_numbers": ["0C694F7F866067B2"] }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kDacRevoked); // Test with invalid crl signer cert missing begin and end cert markers @@ -612,8 +607,9 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kSuccess); // test with malformed crl signer certificate @@ -627,7 +623,8 @@ TEST_F(TestDeviceAttestationCredentials, TestDACRevocationDelegateImpl) }] )"; - WriteTestRevokedData(jsonData, tmpJsonFile); + revocationDelegateImpl.SetDeviceAttestationRevocationData(jsonData); revocationDelegateImpl.CheckForRevokedDACChain(info, &attestationInformationVerificationCallback); + revocationDelegateImpl.ClearDeviceAttestationRevocationData(); EXPECT_EQ(attestationResult, AttestationVerificationResult::kSuccess); } From b36f9235e2652cc0b43f11d19d1ac10d85838423 Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Tue, 10 Dec 2024 09:35:08 +0530 Subject: [PATCH 2/3] move the variable declaration inside scope --- .../attestation_verifier/TestDACRevocationDelegateImpl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp b/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp index bbc73e1d58a757..66891e04760169 100644 --- a/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp +++ b/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp @@ -131,10 +131,10 @@ bool TestDACRevocationDelegateImpl::IsEntryInRevocationSet(const std::string & a std::string errs; // Try direct data first, then fall back to file - std::istringstream jsonStream(!mRevocationData.empty() ? mRevocationData : "[]"); if (!mRevocationData.empty()) { + std::istringstream jsonStream(!mRevocationData.empty() ? mRevocationData : "[]"); if (!Json::parseFromStream(readerBuilder, jsonStream, &jsonData, &errs)) { ChipLogError(NotSpecified, "Failed to parse JSON data: %s", errs.c_str()); From 00e991e47300f86f5d588584cbb49a636cf42881 Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Fri, 13 Dec 2024 10:42:25 +0530 Subject: [PATCH 3/3] Move the variables whre they are used --- .../TestDACRevocationDelegateImpl.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp b/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp index 66891e04760169..7b3f0094c7249e 100644 --- a/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp +++ b/src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp @@ -126,16 +126,14 @@ bool TestDACRevocationDelegateImpl::CrossValidateCert(const Json::Value & revoke bool TestDACRevocationDelegateImpl::IsEntryInRevocationSet(const std::string & akidHexStr, const std::string & issuerNameBase64Str, const std::string & serialNumberHexStr) { - Json::CharReaderBuilder readerBuilder; Json::Value jsonData; - std::string errs; // Try direct data first, then fall back to file - if (!mRevocationData.empty()) { + std::string errs; std::istringstream jsonStream(!mRevocationData.empty() ? mRevocationData : "[]"); - if (!Json::parseFromStream(readerBuilder, jsonStream, &jsonData, &errs)) + if (!Json::parseFromStream(Json::CharReaderBuilder(), jsonStream, &jsonData, &errs)) { ChipLogError(NotSpecified, "Failed to parse JSON data: %s", errs.c_str()); return false; @@ -143,6 +141,7 @@ bool TestDACRevocationDelegateImpl::IsEntryInRevocationSet(const std::string & a } else if (!mDeviceAttestationRevocationSetPath.empty()) { + std::string errs; std::ifstream file(mDeviceAttestationRevocationSetPath.c_str()); if (!file.is_open()) { @@ -150,7 +149,7 @@ bool TestDACRevocationDelegateImpl::IsEntryInRevocationSet(const std::string & a return false; } - bool parsingSuccessful = Json::parseFromStream(readerBuilder, file, &jsonData, &errs); + bool parsingSuccessful = Json::parseFromStream(Json::CharReaderBuilder(), file, &jsonData, &errs); file.close(); if (!parsingSuccessful) @@ -161,6 +160,7 @@ bool TestDACRevocationDelegateImpl::IsEntryInRevocationSet(const std::string & a } else { + ChipLogDetail(NotSpecified, "No revocation data available"); // No revocation data available return false; }