Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dac revocation: Default implementation to check if DAC chain is revoked #33651

Merged
merged 22 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
e66e762
dac revocation: default implementation of CheckForRevokedDACChain
shubhamdp May 29, 2024
7bfe98c
option to configure the revocation set file in chip-tool
shubhamdp May 29, 2024
20a0e0b
Added few comments
shubhamdp May 29, 2024
c4dd7d5
restyle
shubhamdp May 29, 2024
bdaf0ad
add fstream to allow list of DefaultDeviceAttestationVerifier
shubhamdp May 29, 2024
981e03a
Address comments
shubhamdp Jun 24, 2024
87dee75
error code if dac and pai both are revoked
shubhamdp Jun 25, 2024
6eab648
unit tests
shubhamdp Jun 25, 2024
49c57c6
Update examples/chip-tool/commands/common/CredentialIssuerCommands.h
shubhamdp Jul 4, 2024
32b2967
Move setting of revocation delegate to default verifier
shubhamdp Jul 5, 2024
5b271ff
factor out getting of revocation delegate
shubhamdp Jul 9, 2024
d9a6bbe
Restyled by clang-format
restyled-commits Jul 9, 2024
c0f4ba6
Merge branch 'master' into dac_revoke_def_impl
shubhamdp Jul 9, 2024
d0f80af
address reviews
shubhamdp Jul 22, 2024
5c48abb
API to clear revocation set path, and minor cleanup and added a comment
shubhamdp Jul 22, 2024
dff75bb
Merge branch 'master' into dac_revoke_def_impl
shubhamdp Jul 22, 2024
24b0499
Restyled by clang-format
restyled-commits Jul 22, 2024
6214262
add some details about json schema
shubhamdp Jul 24, 2024
608ffba
Restyled by whitespace
restyled-commits Jul 24, 2024
f9fad81
Merge branch 'master' into dac_revoke_def_impl
shubhamdp Jul 24, 2024
97c0935
Add the help text in the argument
shubhamdp Jul 25, 2024
a9d8e1d
Address review comments and added some TODOs
shubhamdp Jul 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/chip-tool/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ static_library("chip-tool-utils") {
"${chip_root}/src/app/tests/suites/commands/interaction_model",
"${chip_root}/src/controller/data_model",
"${chip_root}/src/credentials:file_attestation_trust_store",
"${chip_root}/src/credentials:test_dac_revocation_delegate",
"${chip_root}/src/lib",
"${chip_root}/src/lib/core:types",
"${chip_root}/src/lib/support/jsontlv",
Expand Down
23 changes: 21 additions & 2 deletions examples/chip-tool/commands/common/CHIPCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <commands/icd/ICDCommand.h>
#include <controller/CHIPDeviceControllerFactory.h>
#include <credentials/attestation_verifier/FileAttestationTrustStore.h>
#include <credentials/attestation_verifier/TestDACRevocationDelegateImpl.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/CHIPVendorIdentifiers.hpp>
#include <lib/support/CodeUtils.h>
Expand Down Expand Up @@ -48,7 +49,9 @@ constexpr chip::FabricId kIdentityOtherFabricId = 4;
constexpr char kPAATrustStorePathVariable[] = "CHIPTOOL_PAA_TRUST_STORE_PATH";
constexpr char kCDTrustStorePathVariable[] = "CHIPTOOL_CD_TRUST_STORE_PATH";

const chip::Credentials::AttestationTrustStore * CHIPCommand::sTrustStore = nullptr;
const chip::Credentials::AttestationTrustStore * CHIPCommand::sTrustStore = nullptr;
chip::Credentials::DeviceAttestationRevocationDelegate * CHIPCommand::sRevocationDelegate = nullptr;

chip::Credentials::GroupDataProviderImpl CHIPCommand::sGroupDataProvider{ kMaxGroupsPerFabric, kMaxGroupKeysPerFabric };
// All fabrics share the same ICD client storage.
chip::app::DefaultICDClientStorage CHIPCommand::sICDClientStorage;
Expand Down Expand Up @@ -87,6 +90,20 @@ CHIP_ERROR GetAttestationTrustStore(const char * paaTrustStorePath, const chip::
return CHIP_NO_ERROR;
}

CHIP_ERROR GetAttestationRevocationDelegate(const char * revocationSetPath,
chip::Credentials::DeviceAttestationRevocationDelegate ** revocationDelegate)
{
if (revocationSetPath == nullptr)
{
return CHIP_NO_ERROR;
}

static chip::Credentials::TestDACRevocationDelegateImpl testDacRevocationDelegate;
ReturnErrorOnFailure(testDacRevocationDelegate.SetDeviceAttestationRevocationSetPath(revocationSetPath));
*revocationDelegate = &testDacRevocationDelegate;
return CHIP_NO_ERROR;
}

} // namespace

CHIP_ERROR CHIPCommand::MaybeSetUpStack()
Expand Down Expand Up @@ -151,6 +168,8 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()

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

ReturnLogErrorOnFailure(GetAttestationRevocationDelegate(mDacRevocationSetPath.ValueOr(nullptr), &sRevocationDelegate));

auto engine = chip::app::InteractionModelEngine::GetInstance();
VerifyOrReturnError(engine != nullptr, CHIP_ERROR_INCORRECT_STATE);
ReturnLogErrorOnFailure(ChipToolCheckInDelegate()->Init(&sICDClientStorage, engine));
Expand Down Expand Up @@ -450,7 +469,7 @@ CHIP_ERROR CHIPCommand::InitializeCommissioner(CommissionerIdentity & identity,
std::unique_ptr<ChipDeviceCommissioner> commissioner = std::make_unique<ChipDeviceCommissioner>();
chip::Controller::SetupParams commissionerParams;

ReturnLogErrorOnFailure(mCredIssuerCmds->SetupDeviceAttestation(commissionerParams, sTrustStore));
ReturnLogErrorOnFailure(mCredIssuerCmds->SetupDeviceAttestation(commissionerParams, sTrustStore, sRevocationDelegate));

chip::Crypto::P256Keypair ephemeralKey;

Expand Down
9 changes: 9 additions & 0 deletions examples/chip-tool/commands/common/CHIPCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ class CHIPCommand : public Command
AddArgument("only-allow-trusted-cd-keys", 0, 1, &mOnlyAllowTrustedCdKeys,
"Only allow trusted CD verifying keys (disallow test keys). If not provided or 0 (\"false\"), untrusted CD "
"verifying keys are allowed. If 1 (\"true\"), test keys are disallowed.");
AddArgument("dac-revocation-set-path", &mDacRevocationSetPath,
"Path to JSON file containing the device attestation revocation set. "
"This argument caches the path to the revocation set. Once set, this will be used by all commands in "
"interactive mode.");
#if CHIP_CONFIG_TRANSPORT_TRACE_ENABLED
AddArgument("trace_file", &mTraceFile);
AddArgument("trace_log", 0, 1, &mTraceLog);
Expand Down Expand Up @@ -222,11 +226,16 @@ class CHIPCommand : public Command
chip::Optional<char *> mCDTrustStorePath;
chip::Optional<bool> mUseMaxSizedCerts;
chip::Optional<bool> mOnlyAllowTrustedCdKeys;
chip::Optional<char *> mDacRevocationSetPath;

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

// Cached DAC revocation delegate, this can be set using "--dac-revocation-set-path" argument
// Once set this will be used by all commands.
static chip::Credentials::DeviceAttestationRevocationDelegate * sRevocationDelegate;

static void RunQueuedCommand(intptr_t commandArg);
typedef decltype(RunQueuedCommand) MatterWorkCallback;
static void RunCommandCleanup(intptr_t commandArg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,13 @@ class CredentialIssuerCommands
* Verifier.
* @param[in] trustStore A pointer to the PAA trust store to use to find valid PAA roots.
*
* @param[in] revocationDelegate A pointer to the Device Attestation Revocation Delegate for checking revoked DACs and PAIs.
*
* @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error code.
*/
virtual CHIP_ERROR SetupDeviceAttestation(chip::Controller::SetupParams & setupParams,
const chip::Credentials::AttestationTrustStore * trustStore) = 0;
const chip::Credentials::AttestationTrustStore * trustStore,
chip::Credentials::DeviceAttestationRevocationDelegate * revocationDelegate) = 0;

/**
* @brief Add a list of additional non-default CD verifying keys (by certificate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,18 @@ class ExampleCredentialIssuerCommands : public CredentialIssuerCommands
return mOpCredsIssuer.Initialize(storage);
}
CHIP_ERROR SetupDeviceAttestation(chip::Controller::SetupParams & setupParams,
const chip::Credentials::AttestationTrustStore * trustStore) override
const chip::Credentials::AttestationTrustStore * trustStore,
chip::Credentials::DeviceAttestationRevocationDelegate * revocationDelegate) override
{
chip::Credentials::SetDeviceAttestationCredentialsProvider(chip::Credentials::Examples::GetExampleDACProvider());

mDacVerifier = chip::Credentials::GetDefaultDACVerifier(trustStore);
mDacVerifier = chip::Credentials::GetDefaultDACVerifier(trustStore, revocationDelegate);
setupParams.deviceAttestationVerifier = mDacVerifier;
mDacVerifier->EnableCdTestKeySupport(mAllowTestCdSigningKey);

return CHIP_NO_ERROR;
}

chip::Controller::OperationalCredentialsDelegate * GetCredentialIssuer() override { return &mOpCredsIssuer; }
void SetCredentialIssuerCATValues(chip::CATValues cats) override { mOpCredsIssuer.SetCATValuesForNextNOCRequest(cats); }
CHIP_ERROR GenerateControllerNOCChain(chip::NodeId nodeId, chip::FabricId fabricId, const chip::CATValues & cats,
Expand Down
1 change: 1 addition & 0 deletions scripts/tools/check_includes_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@

'src/credentials/attestation_verifier/FileAttestationTrustStore.h': {'vector'},
'src/credentials/attestation_verifier/FileAttestationTrustStore.cpp': {'string'},
'src/credentials/attestation_verifier/TestDACRevocationDelegateImpl.cpp': {'fstream'},

'src/setup_payload/AdditionalDataPayload.h': {'string'},
'src/setup_payload/AdditionalDataPayloadParser.cpp': {'vector', 'string'},
Expand Down
15 changes: 15 additions & 0 deletions src/credentials/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

import("//build_overrides/chip.gni")
import("//build_overrides/jsoncpp.gni")
import("//build_overrides/nlassert.gni")
import("${chip_root}/src/crypto/crypto.gni")
import("${chip_root}/src/lib/core/core.gni")
Expand Down Expand Up @@ -185,3 +186,17 @@ static_library("file_attestation_trust_store") {
"${nlassert_root}:nlassert",
]
}

static_library("test_dac_revocation_delegate") {
output_name = "libTestDACRevocationDelegate"

sources = [
"attestation_verifier/TestDACRevocationDelegateImpl.cpp",
"attestation_verifier/TestDACRevocationDelegateImpl.h",
]

public_deps = [
":credentials",
jsoncpp_root,
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -610,11 +610,14 @@ CHIP_ERROR DefaultDACVerifier::VerifyNodeOperationalCSRInformation(const ByteSpa
void DefaultDACVerifier::CheckForRevokedDACChain(const AttestationInfo & info,
Callback::Callback<OnAttestationInformationVerification> * onCompletion)
{
AttestationVerificationResult attestationError = AttestationVerificationResult::kSuccess;

// TODO(#33124): Implement default version of CheckForRevokedDACChain

onCompletion->mCall(onCompletion->mContext, info, attestationError);
if (mRevocationDelegate != nullptr)
{
mRevocationDelegate->CheckForRevokedDACChain(info, onCompletion);
}
else
{
onCompletion->mCall(onCompletion->mContext, info, AttestationVerificationResult::kSuccess);
}
}

bool CsaCdKeysTrustStore::IsCdTestKey(const ByteSpan & kid) const
Expand Down Expand Up @@ -693,9 +696,10 @@ const AttestationTrustStore * GetTestAttestationTrustStore()
return &gTestAttestationTrustStore.get();
}

DeviceAttestationVerifier * GetDefaultDACVerifier(const AttestationTrustStore * paaRootStore)
DeviceAttestationVerifier * GetDefaultDACVerifier(const AttestationTrustStore * paaRootStore,
DeviceAttestationRevocationDelegate * revocationDelegate)
{
static DefaultDACVerifier defaultDACVerifier{ paaRootStore };
static DefaultDACVerifier defaultDACVerifier{ paaRootStore, revocationDelegate };

return &defaultDACVerifier;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ class DefaultDACVerifier : public DeviceAttestationVerifier
public:
DefaultDACVerifier(const AttestationTrustStore * paaRootStore) : mAttestationTrustStore(paaRootStore) {}

DefaultDACVerifier(const AttestationTrustStore * paaRootStore, DeviceAttestationRevocationDelegate * revocationDelegate) :
mAttestationTrustStore(paaRootStore), mRevocationDelegate(revocationDelegate)
{}

void VerifyAttestationInformation(const DeviceAttestationVerifier::AttestationInfo & info,
Callback::Callback<OnAttestationInformationVerification> * onCompletion) override;

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

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

void SetRevocationDelegate(DeviceAttestationRevocationDelegate * revocationDelegate)
{
mRevocationDelegate = revocationDelegate;
}

protected:
DefaultDACVerifier() {}

CsaCdKeysTrustStore mCdKeysTrustStore;
const AttestationTrustStore * mAttestationTrustStore;
DeviceAttestationRevocationDelegate * mRevocationDelegate = nullptr;
};

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

} // namespace Credentials
} // namespace chip
23 changes: 23 additions & 0 deletions src/credentials/attestation_verifier/DeviceAttestationVerifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ enum class AttestationVerificationResult : uint16_t
kPaiVendorIdMismatch = 205,
kPaiAuthorityNotFound = 206,
kPaiMissing = 207,
kPaiAndDacRevoked = 208,

kDacExpired = 300,
kDacSignatureInvalid = 301,
Expand Down Expand Up @@ -418,6 +419,28 @@ class DeviceAttestationVerifier
bool mEnableCdTestKeySupport = true;
};

/**
* @brief Interface for checking the device attestation revocation status
*
*/
class DeviceAttestationRevocationDelegate
{
public:
DeviceAttestationRevocationDelegate() = default;
virtual ~DeviceAttestationRevocationDelegate() = default;

/**
* @brief Verify whether or not the given DAC chain is revoked.
*
* @param[in] info All of the information required to check for revoked DAC chain.
* @param[in] onCompletion Callback handler to provide Attestation Information Verification result to the caller of
* CheckForRevokedDACChain().
*/
virtual void
CheckForRevokedDACChain(const DeviceAttestationVerifier::AttestationInfo & info,
Callback::Callback<DeviceAttestationVerifier::OnAttestationInformationVerification> * onCompletion) = 0;
};

/**
* Instance getter for the global DeviceAttestationVerifier.
*
Expand Down
Loading
Loading