Skip to content

Commit f2e74e5

Browse files
shubhamdpbzbarsky-applerestyled-commits
authored andcommitted
dac revocation: Default implementation to check if DAC chain is revoked (project-chip#33651)
* dac revocation: default implementation of CheckForRevokedDACChain * option to configure the revocation set file in chip-tool * Added few comments * restyle * add fstream to allow list of DefaultDeviceAttestationVerifier * Address comments Added an interface for device attestation revocation and the test implementation for the same. * error code if dac and pai both are revoked * unit tests * Update examples/chip-tool/commands/common/CredentialIssuerCommands.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Move setting of revocation delegate to default verifier * factor out getting of revocation delegate * Restyled by clang-format * address reviews * API to clear revocation set path, and minor cleanup and added a comment to explain the usage of --dac-revocation-set-path argument * Restyled by clang-format * add some details about json schema * Restyled by whitespace * Add the help text in the argument * Address review comments and added some TODOs --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Restyled.io <commits@restyled.io>
1 parent 28d6cd8 commit f2e74e5

14 files changed

+546
-13
lines changed

examples/chip-tool/BUILD.gn

+1
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ static_library("chip-tool-utils") {
109109
"${chip_root}/src/app/tests/suites/commands/interaction_model",
110110
"${chip_root}/src/controller/data_model",
111111
"${chip_root}/src/credentials:file_attestation_trust_store",
112+
"${chip_root}/src/credentials:test_dac_revocation_delegate",
112113
"${chip_root}/src/lib",
113114
"${chip_root}/src/lib/core:types",
114115
"${chip_root}/src/lib/support/jsontlv",

examples/chip-tool/commands/common/CHIPCommand.cpp

+21-2
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <commands/icd/ICDCommand.h>
2222
#include <controller/CHIPDeviceControllerFactory.h>
2323
#include <credentials/attestation_verifier/FileAttestationTrustStore.h>
24+
#include <credentials/attestation_verifier/TestDACRevocationDelegateImpl.h>
2425
#include <lib/core/CHIPConfig.h>
2526
#include <lib/core/CHIPVendorIdentifiers.hpp>
2627
#include <lib/support/CodeUtils.h>
@@ -48,7 +49,9 @@ constexpr chip::FabricId kIdentityOtherFabricId = 4;
4849
constexpr char kPAATrustStorePathVariable[] = "CHIPTOOL_PAA_TRUST_STORE_PATH";
4950
constexpr char kCDTrustStorePathVariable[] = "CHIPTOOL_CD_TRUST_STORE_PATH";
5051

51-
const chip::Credentials::AttestationTrustStore * CHIPCommand::sTrustStore = nullptr;
52+
const chip::Credentials::AttestationTrustStore * CHIPCommand::sTrustStore = nullptr;
53+
chip::Credentials::DeviceAttestationRevocationDelegate * CHIPCommand::sRevocationDelegate = nullptr;
54+
5255
chip::Credentials::GroupDataProviderImpl CHIPCommand::sGroupDataProvider{ kMaxGroupsPerFabric, kMaxGroupKeysPerFabric };
5356
// All fabrics share the same ICD client storage.
5457
chip::app::DefaultICDClientStorage CHIPCommand::sICDClientStorage;
@@ -87,6 +90,20 @@ CHIP_ERROR GetAttestationTrustStore(const char * paaTrustStorePath, const chip::
8790
return CHIP_NO_ERROR;
8891
}
8992

93+
CHIP_ERROR GetAttestationRevocationDelegate(const char * revocationSetPath,
94+
chip::Credentials::DeviceAttestationRevocationDelegate ** revocationDelegate)
95+
{
96+
if (revocationSetPath == nullptr)
97+
{
98+
return CHIP_NO_ERROR;
99+
}
100+
101+
static chip::Credentials::TestDACRevocationDelegateImpl testDacRevocationDelegate;
102+
ReturnErrorOnFailure(testDacRevocationDelegate.SetDeviceAttestationRevocationSetPath(revocationSetPath));
103+
*revocationDelegate = &testDacRevocationDelegate;
104+
return CHIP_NO_ERROR;
105+
}
106+
90107
} // namespace
91108

92109
CHIP_ERROR CHIPCommand::MaybeSetUpStack()
@@ -151,6 +168,8 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()
151168

152169
ReturnErrorOnFailure(GetAttestationTrustStore(mPaaTrustStorePath.ValueOr(nullptr), &sTrustStore));
153170

171+
ReturnLogErrorOnFailure(GetAttestationRevocationDelegate(mDacRevocationSetPath.ValueOr(nullptr), &sRevocationDelegate));
172+
154173
auto engine = chip::app::InteractionModelEngine::GetInstance();
155174
VerifyOrReturnError(engine != nullptr, CHIP_ERROR_INCORRECT_STATE);
156175
ReturnLogErrorOnFailure(ChipToolCheckInDelegate()->Init(&sICDClientStorage, engine));
@@ -450,7 +469,7 @@ CHIP_ERROR CHIPCommand::InitializeCommissioner(CommissionerIdentity & identity,
450469
std::unique_ptr<ChipDeviceCommissioner> commissioner = std::make_unique<ChipDeviceCommissioner>();
451470
chip::Controller::SetupParams commissionerParams;
452471

453-
ReturnLogErrorOnFailure(mCredIssuerCmds->SetupDeviceAttestation(commissionerParams, sTrustStore));
472+
ReturnLogErrorOnFailure(mCredIssuerCmds->SetupDeviceAttestation(commissionerParams, sTrustStore, sRevocationDelegate));
454473

455474
chip::Crypto::P256Keypair ephemeralKey;
456475

examples/chip-tool/commands/common/CHIPCommand.h

+9
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ class CHIPCommand : public Command
8686
AddArgument("only-allow-trusted-cd-keys", 0, 1, &mOnlyAllowTrustedCdKeys,
8787
"Only allow trusted CD verifying keys (disallow test keys). If not provided or 0 (\"false\"), untrusted CD "
8888
"verifying keys are allowed. If 1 (\"true\"), test keys are disallowed.");
89+
AddArgument("dac-revocation-set-path", &mDacRevocationSetPath,
90+
"Path to JSON file containing the device attestation revocation set. "
91+
"This argument caches the path to the revocation set. Once set, this will be used by all commands in "
92+
"interactive mode.");
8993
#if CHIP_CONFIG_TRANSPORT_TRACE_ENABLED
9094
AddArgument("trace_file", &mTraceFile);
9195
AddArgument("trace_log", 0, 1, &mTraceLog);
@@ -222,11 +226,16 @@ class CHIPCommand : public Command
222226
chip::Optional<char *> mCDTrustStorePath;
223227
chip::Optional<bool> mUseMaxSizedCerts;
224228
chip::Optional<bool> mOnlyAllowTrustedCdKeys;
229+
chip::Optional<char *> mDacRevocationSetPath;
225230

226231
// Cached trust store so commands other than the original startup command
227232
// can spin up commissioners as needed.
228233
static const chip::Credentials::AttestationTrustStore * sTrustStore;
229234

235+
// Cached DAC revocation delegate, this can be set using "--dac-revocation-set-path" argument
236+
// Once set this will be used by all commands.
237+
static chip::Credentials::DeviceAttestationRevocationDelegate * sRevocationDelegate;
238+
230239
static void RunQueuedCommand(intptr_t commandArg);
231240
typedef decltype(RunQueuedCommand) MatterWorkCallback;
232241
static void RunCommandCleanup(intptr_t commandArg);

examples/chip-tool/commands/common/CredentialIssuerCommands.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,13 @@ class CredentialIssuerCommands
5757
* Verifier.
5858
* @param[in] trustStore A pointer to the PAA trust store to use to find valid PAA roots.
5959
*
60+
* @param[in] revocationDelegate A pointer to the Device Attestation Revocation Delegate for checking revoked DACs and PAIs.
61+
*
6062
* @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error code.
6163
*/
6264
virtual CHIP_ERROR SetupDeviceAttestation(chip::Controller::SetupParams & setupParams,
63-
const chip::Credentials::AttestationTrustStore * trustStore) = 0;
65+
const chip::Credentials::AttestationTrustStore * trustStore,
66+
chip::Credentials::DeviceAttestationRevocationDelegate * revocationDelegate) = 0;
6467

6568
/**
6669
* @brief Add a list of additional non-default CD verifying keys (by certificate)

examples/chip-tool/commands/example/ExampleCredentialIssuerCommands.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,18 @@ class ExampleCredentialIssuerCommands : public CredentialIssuerCommands
3434
return mOpCredsIssuer.Initialize(storage);
3535
}
3636
CHIP_ERROR SetupDeviceAttestation(chip::Controller::SetupParams & setupParams,
37-
const chip::Credentials::AttestationTrustStore * trustStore) override
37+
const chip::Credentials::AttestationTrustStore * trustStore,
38+
chip::Credentials::DeviceAttestationRevocationDelegate * revocationDelegate) override
3839
{
3940
chip::Credentials::SetDeviceAttestationCredentialsProvider(chip::Credentials::Examples::GetExampleDACProvider());
4041

41-
mDacVerifier = chip::Credentials::GetDefaultDACVerifier(trustStore);
42+
mDacVerifier = chip::Credentials::GetDefaultDACVerifier(trustStore, revocationDelegate);
4243
setupParams.deviceAttestationVerifier = mDacVerifier;
4344
mDacVerifier->EnableCdTestKeySupport(mAllowTestCdSigningKey);
4445

4546
return CHIP_NO_ERROR;
4647
}
48+
4749
chip::Controller::OperationalCredentialsDelegate * GetCredentialIssuer() override { return &mOpCredsIssuer; }
4850
void SetCredentialIssuerCATValues(chip::CATValues cats) override { mOpCredsIssuer.SetCATValuesForNextNOCRequest(cats); }
4951
CHIP_ERROR GenerateControllerNOCChain(chip::NodeId nodeId, chip::FabricId fabricId, const chip::CATValues & cats,

scripts/tools/check_includes_config.py

+1
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@
139139

140140
'src/credentials/attestation_verifier/FileAttestationTrustStore.h': {'vector'},
141141
'src/credentials/attestation_verifier/FileAttestationTrustStore.cpp': {'string'},
142+
'src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp': {'fstream'},
142143

143144
'src/setup_payload/AdditionalDataPayload.h': {'string'},
144145
'src/setup_payload/AdditionalDataPayloadParser.cpp': {'vector', 'string'},

src/credentials/BUILD.gn

+15
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# limitations under the License.
1414

1515
import("//build_overrides/chip.gni")
16+
import("//build_overrides/jsoncpp.gni")
1617
import("//build_overrides/nlassert.gni")
1718
import("${chip_root}/src/crypto/crypto.gni")
1819
import("${chip_root}/src/lib/core/core.gni")
@@ -185,3 +186,17 @@ static_library("file_attestation_trust_store") {
185186
"${nlassert_root}:nlassert",
186187
]
187188
}
189+
190+
static_library("test_dac_revocation_delegate") {
191+
output_name = "libTestDACRevocationDelegate"
192+
193+
sources = [
194+
"attestation_verifier/TestDACRevocationDelegateImpl.cpp",
195+
"attestation_verifier/TestDACRevocationDelegateImpl.h",
196+
]
197+
198+
public_deps = [
199+
":credentials",
200+
jsoncpp_root,
201+
]
202+
}

src/credentials/attestation_verifier/DefaultDeviceAttestationVerifier.cpp

+11-7
Original file line numberDiff line numberDiff line change
@@ -610,11 +610,14 @@ CHIP_ERROR DefaultDACVerifier::VerifyNodeOperationalCSRInformation(const ByteSpa
610610
void DefaultDACVerifier::CheckForRevokedDACChain(const AttestationInfo & info,
611611
Callback::Callback<OnAttestationInformationVerification> * onCompletion)
612612
{
613-
AttestationVerificationResult attestationError = AttestationVerificationResult::kSuccess;
614-
615-
// TODO(#33124): Implement default version of CheckForRevokedDACChain
616-
617-
onCompletion->mCall(onCompletion->mContext, info, attestationError);
613+
if (mRevocationDelegate != nullptr)
614+
{
615+
mRevocationDelegate->CheckForRevokedDACChain(info, onCompletion);
616+
}
617+
else
618+
{
619+
onCompletion->mCall(onCompletion->mContext, info, AttestationVerificationResult::kSuccess);
620+
}
618621
}
619622

620623
bool CsaCdKeysTrustStore::IsCdTestKey(const ByteSpan & kid) const
@@ -693,9 +696,10 @@ const AttestationTrustStore * GetTestAttestationTrustStore()
693696
return &gTestAttestationTrustStore.get();
694697
}
695698

696-
DeviceAttestationVerifier * GetDefaultDACVerifier(const AttestationTrustStore * paaRootStore)
699+
DeviceAttestationVerifier * GetDefaultDACVerifier(const AttestationTrustStore * paaRootStore,
700+
DeviceAttestationRevocationDelegate * revocationDelegate)
697701
{
698-
static DefaultDACVerifier defaultDACVerifier{ paaRootStore };
702+
static DefaultDACVerifier defaultDACVerifier{ paaRootStore, revocationDelegate };
699703

700704
return &defaultDACVerifier;
701705
}

src/credentials/attestation_verifier/DefaultDeviceAttestationVerifier.h

+12-1
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ class DefaultDACVerifier : public DeviceAttestationVerifier
5959
public:
6060
DefaultDACVerifier(const AttestationTrustStore * paaRootStore) : mAttestationTrustStore(paaRootStore) {}
6161

62+
DefaultDACVerifier(const AttestationTrustStore * paaRootStore, DeviceAttestationRevocationDelegate * revocationDelegate) :
63+
mAttestationTrustStore(paaRootStore), mRevocationDelegate(revocationDelegate)
64+
{}
65+
6266
void VerifyAttestationInformation(const DeviceAttestationVerifier::AttestationInfo & info,
6367
Callback::Callback<OnAttestationInformationVerification> * onCompletion) override;
6468

@@ -79,11 +83,17 @@ class DefaultDACVerifier : public DeviceAttestationVerifier
7983

8084
CsaCdKeysTrustStore * GetCertificationDeclarationTrustStore() override { return &mCdKeysTrustStore; }
8185

86+
void SetRevocationDelegate(DeviceAttestationRevocationDelegate * revocationDelegate)
87+
{
88+
mRevocationDelegate = revocationDelegate;
89+
}
90+
8291
protected:
8392
DefaultDACVerifier() {}
8493

8594
CsaCdKeysTrustStore mCdKeysTrustStore;
8695
const AttestationTrustStore * mAttestationTrustStore;
96+
DeviceAttestationRevocationDelegate * mRevocationDelegate = nullptr;
8797
};
8898

8999
/**
@@ -112,7 +122,8 @@ const AttestationTrustStore * GetTestAttestationTrustStore();
112122
* process lifetime. In particular, after the first call it's not
113123
* possible to change which AttestationTrustStore is used by this verifier.
114124
*/
115-
DeviceAttestationVerifier * GetDefaultDACVerifier(const AttestationTrustStore * paaRootStore);
125+
DeviceAttestationVerifier * GetDefaultDACVerifier(const AttestationTrustStore * paaRootStore,
126+
DeviceAttestationRevocationDelegate * revocationDelegate = nullptr);
116127

117128
} // namespace Credentials
118129
} // namespace chip

src/credentials/attestation_verifier/DeviceAttestationVerifier.h

+23
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ enum class AttestationVerificationResult : uint16_t
4747
kPaiVendorIdMismatch = 205,
4848
kPaiAuthorityNotFound = 206,
4949
kPaiMissing = 207,
50+
kPaiAndDacRevoked = 208,
5051

5152
kDacExpired = 300,
5253
kDacSignatureInvalid = 301,
@@ -418,6 +419,28 @@ class DeviceAttestationVerifier
418419
bool mEnableCdTestKeySupport = true;
419420
};
420421

422+
/**
423+
* @brief Interface for checking the device attestation revocation status
424+
*
425+
*/
426+
class DeviceAttestationRevocationDelegate
427+
{
428+
public:
429+
DeviceAttestationRevocationDelegate() = default;
430+
virtual ~DeviceAttestationRevocationDelegate() = default;
431+
432+
/**
433+
* @brief Verify whether or not the given DAC chain is revoked.
434+
*
435+
* @param[in] info All of the information required to check for revoked DAC chain.
436+
* @param[in] onCompletion Callback handler to provide Attestation Information Verification result to the caller of
437+
* CheckForRevokedDACChain().
438+
*/
439+
virtual void
440+
CheckForRevokedDACChain(const DeviceAttestationVerifier::AttestationInfo & info,
441+
Callback::Callback<DeviceAttestationVerifier::OnAttestationInformationVerification> * onCompletion) = 0;
442+
};
443+
421444
/**
422445
* Instance getter for the global DeviceAttestationVerifier.
423446
*

0 commit comments

Comments
 (0)