Skip to content

Commit

Permalink
Added mechanism to override device attestation failure based on clien…
Browse files Browse the repository at this point in the history
…t/user (#17028)

* Added mechanism to override device attestation failure based on client/user input

Fixes #16681

Change overview

Added delegates that can be optionally set by the client of the SDK.
When the device attestation delegate is set and when a DA failure is
encountered during commissioning, the delegate is invoked letting the
client decide on the behavior to either ignore the error and continue
commissioning or fail the commissioning. The arm failsafe timer can also
be adjusted by the client to handle any delays due to user input.

Testing

Flashed an M5 with the changes. Used the iOS CHIPTool with the changes
and verified no regression in current commissioning process when the
delegate is either setup or not.
Modified the kTestPaaRoots array in DefaultDeviceAttestationVerifier.cpp
to be empty causing DA failure. Verified the dialog is presented to the
user and that commissioning succeeds with error is ignored and fails
when the user declines to proceed.

* Restyled by whitespace

* Restyled by clang-format

* Restyled by gn

* Fixed linker error for chip-tool-darwin

* Restyled by gn

* Changed CHIPDeviceAttestationDelegate delegate call to match Obj-C API conventions

* Restyled by clang-format

* Switched to passing DeviceProxy* in DA delegate methods

* Restyled by clang-format

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

* Restyled by whitespace

* Restyled by clang-format

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
3 people authored and pull[bot] committed Sep 21, 2023
1 parent 0de567a commit 5570062
Show file tree
Hide file tree
Showing 17 changed files with 503 additions and 3 deletions.
5 changes: 5 additions & 0 deletions src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam
return CHIP_NO_ERROR;
}

const CommissioningParameters & AutoCommissioner::GetCommissioningParameters() const
{
return mParams;
}

CommissioningStage AutoCommissioner::GetNextCommissioningStage(CommissioningStage currentStage, CHIP_ERROR & lastErr)
{
auto nextStage = GetNextCommissioningStageInternal(currentStage, lastErr);
Expand Down
1 change: 1 addition & 0 deletions src/controller/AutoCommissioner.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class AutoCommissioner : public CommissioningDelegate
AutoCommissioner();
~AutoCommissioner() override;
CHIP_ERROR SetCommissioningParameters(const CommissioningParameters & params) override;
const CommissioningParameters & GetCommissioningParameters() const override;
void SetOperationalCredentialsDelegate(OperationalCredentialsDelegate * operationalCredentialsDelegate) override;

CHIP_ERROR StartCommissioning(DeviceCommissioner * commissioner, CommissioneeDeviceProxy * proxy) override;
Expand Down
134 changes: 131 additions & 3 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,56 @@ CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId)
return CHIP_NO_ERROR;
}

CHIP_ERROR
DeviceCommissioner::ContinueCommissioningAfterDeviceAttestationFailure(DeviceProxy * device,
Credentials::AttestationVerificationResult attestationResult)
{
MATTER_TRACE_EVENT_SCOPE("continueCommissioningDevice", "DeviceCommissioner");
if (device == nullptr || device != mDeviceBeingCommissioned)
{
ChipLogError(Controller, "Invalid device for commissioning %p", device);
return CHIP_ERROR_INCORRECT_STATE;
}
CommissioneeDeviceProxy * commissioneeDevice = FindCommissioneeDevice(device->GetDeviceId());
if (commissioneeDevice == nullptr || !commissioneeDevice->IsSecureConnected() || commissioneeDevice != mDeviceBeingCommissioned)
{
ChipLogError(Controller, "Invalid device for commissioning after attestation failure: 0x" ChipLogFormatX64,
ChipLogValueX64(commissioneeDevice->GetDeviceId()));
return CHIP_ERROR_INCORRECT_STATE;
}

if (mCommissioningStage != CommissioningStage::kAttestationVerification)
{
ChipLogError(Controller, "Commissioning is not attestation verification phase");
return CHIP_ERROR_INCORRECT_STATE;
}

if (mDefaultCommissioner == nullptr)
{
ChipLogError(Controller, "No default commissioner is specified");
return CHIP_ERROR_INCORRECT_STATE;
}

ChipLogProgress(Controller, "Continuing commissioning after attestation failure for device ID 0x" ChipLogFormatX64,
ChipLogValueX64(commissioneeDevice->GetDeviceId()));

if (attestationResult != AttestationVerificationResult::kSuccess)
{
ChipLogError(Controller, "Client selected error: %u for failed 'Attestation Information' for device",
to_underlying(attestationResult));

CommissioningDelegate::CommissioningReport report;
report.Set<AdditionalErrorInfo>(attestationResult);
CommissioningStageComplete(CHIP_ERROR_INTERNAL, report);
}
else
{
ChipLogProgress(Controller, "Overriding attestation failure per client and continuing commissioning");
CommissioningStageComplete(CHIP_NO_ERROR);
}
return CHIP_NO_ERROR;
}

CHIP_ERROR DeviceCommissioner::GetAttestationChallenge(ByteSpan & attestationChallenge)
{
Optional<SessionHandle> secureSessionHandle;
Expand Down Expand Up @@ -992,12 +1042,89 @@ void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * conte
static_cast<uint16_t>(result));
// Go look at AttestationVerificationResult enum in src/credentials/attestation_verifier/DeviceAttestationVerifier.h to
// understand the errors.
commissioner->CommissioningStageComplete(CHIP_ERROR_INTERNAL, report);

auto & params = commissioner->mDefaultCommissioner->GetCommissioningParameters();
Credentials::DeviceAttestationDelegate * deviceAttestationDelegate = params.GetDeviceAttestationDelegate();

// If a device attestation status delegate is installed, delegate handling of failure to the client and let them
// decide on whether to proceed further or not.
if (deviceAttestationDelegate)
{
commissioner->ExtendArmFailSafeForFailedDeviceAttestation(result);
}
else
{
commissioner->CommissioningStageComplete(CHIP_ERROR_INTERNAL, report);
}
}
else
{
ChipLogProgress(Controller, "Successfully validated 'Attestation Information' command received from the device.");
commissioner->CommissioningStageComplete(CHIP_NO_ERROR);
}
}

void DeviceCommissioner::OnArmFailSafeExtendedForFailedDeviceAttestation(
void * context, const GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data)
{
// If this function starts using "data", need to fix ExtendArmFailSafeForFailedDeviceAttestation accordingly.
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);

if (!commissioner->mDeviceBeingCommissioned)
{
return;
}

ChipLogProgress(Controller, "Successfully validated 'Attestation Information' command received from the device.");
commissioner->CommissioningStageComplete(CHIP_NO_ERROR);
auto & params = commissioner->mDefaultCommissioner->GetCommissioningParameters();
Credentials::DeviceAttestationDelegate * deviceAttestationDelegate = params.GetDeviceAttestationDelegate();
if (deviceAttestationDelegate)
{
ChipLogProgress(Controller, "Device attestation failed, delegating error handling to client");
deviceAttestationDelegate->OnDeviceAttestationFailed(commissioner, commissioner->mDeviceBeingCommissioned,
commissioner->mAttestationResult);
}
else
{
ChipLogProgress(Controller, "Device attestation failed and no delegate set, failing commissioning");
CommissioningDelegate::CommissioningReport report;
report.Set<AdditionalErrorInfo>(commissioner->mAttestationResult);
commissioner->CommissioningStageComplete(CHIP_ERROR_INTERNAL, report);
}
}

void DeviceCommissioner::OnFailedToExtendedArmFailSafeFailedDeviceAttestation(void * context, CHIP_ERROR error)
{
ChipLogProgress(Controller, "Failed to extend fail-safe timer to handle attestation failure %s", chip::ErrorStr(error));
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);

CommissioningDelegate::CommissioningReport report;
report.Set<AdditionalErrorInfo>(commissioner->mAttestationResult);
commissioner->CommissioningStageComplete(CHIP_ERROR_INTERNAL, report);
}

void DeviceCommissioner::ExtendArmFailSafeForFailedDeviceAttestation(AttestationVerificationResult result)
{
mAttestationResult = result;

auto & params = mDefaultCommissioner->GetCommissioningParameters();
Credentials::DeviceAttestationDelegate * deviceAttestationDelegate = params.GetDeviceAttestationDelegate();
auto expiryLengthSeconds = deviceAttestationDelegate->FailSafeExpiryTimeoutSecs();
if (expiryLengthSeconds.HasValue())
{
GeneralCommissioning::Commands::ArmFailSafe::Type request;
request.expiryLengthSeconds = expiryLengthSeconds.Value();
request.breadcrumb = mCommissioningStage;
ChipLogProgress(Controller, "Changing fail-safe timer to %u seconds to handle DA failure", request.expiryLengthSeconds);
SendCommand<GeneralCommissioningCluster>(mDeviceBeingCommissioned, request, OnArmFailSafeExtendedForFailedDeviceAttestation,
OnFailedToExtendedArmFailSafeFailedDeviceAttestation);
}
else
{
ChipLogProgress(Controller, "Proceeding without changing fail-safe timer value as delegate has not set it");
// Callee does not use data argument.
const GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType data;
OnArmFailSafeExtendedForFailedDeviceAttestation(this, data);
}
}

CHIP_ERROR DeviceCommissioner::ValidateAttestationInfo(const Credentials::DeviceAttestationVerifier::AttestationInfo & info)
Expand Down Expand Up @@ -1684,6 +1811,7 @@ void DeviceCommissioner::OnDone()
report.Set<ReadCommissioningInfo>(info);
CommissioningStageComplete(return_err, report);
}

void DeviceCommissioner::OnArmFailSafe(void * context,
const GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data)
{
Expand Down
19 changes: 19 additions & 0 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include <controller/OperationalCredentialsDelegate.h>
#include <controller/SetUpCodePairer.h>
#include <credentials/FabricTable.h>
#include <credentials/attestation_verifier/DeviceAttestationDelegate.h>
#include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/CHIPCore.h>
Expand Down Expand Up @@ -414,6 +415,19 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
CHIP_ERROR Commission(NodeId remoteDeviceId, CommissioningParameters & params);
CHIP_ERROR Commission(NodeId remoteDeviceId);

/**
* @brief
* This function instructs the commissioner to proceed to the next stage of commissioning after
* attestation failure is reported to an installed attestation delegate.
*
* @param[in] device The device being commissioned.
* @param[in] attestationResult The attestation result to use instead of whatever the device
* attestation verifier came up with. May be a success or an error result.
*/
CHIP_ERROR
ContinueCommissioningAfterDeviceAttestationFailure(DeviceProxy * device,
Credentials::AttestationVerificationResult attestationResult);

CHIP_ERROR GetDeviceBeingCommissioned(NodeId deviceId, CommissioneeDeviceProxy ** device);

/**
Expand Down Expand Up @@ -615,6 +629,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
/* Callback when the previously sent CSR request results in failure */
static void OnCSRFailureResponse(void * context, CHIP_ERROR error);

void ExtendArmFailSafeForFailedDeviceAttestation(Credentials::AttestationVerificationResult result);
static void OnCertificateChainFailureResponse(void * context, CHIP_ERROR error);
static void OnCertificateChainResponse(
void * context, const app::Clusters::OperationalCredentials::Commands::CertificateChainResponse::DecodableType & response);
Expand Down Expand Up @@ -674,6 +689,9 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
const app::Clusters::GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data);
static void OnDisarmFailsafeFailure(void * context, CHIP_ERROR error);
void DisarmDone();
static void OnArmFailSafeExtendedForFailedDeviceAttestation(
void * context, const chip::app::Clusters::GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data);
static void OnFailedToExtendedArmFailSafeFailedDeviceAttestation(void * context, CHIP_ERROR error);

/**
* @brief
Expand Down Expand Up @@ -767,6 +785,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,

Platform::UniquePtr<app::AttributeCache> mAttributeCache;
Platform::UniquePtr<app::ReadClient> mReadClient;
Credentials::AttestationVerificationResult mAttestationResult;
};

} // namespace Controller
Expand Down
12 changes: 12 additions & 0 deletions src/controller/CommissioningDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#pragma once
#include <app/OperationalDeviceProxy.h>
#include <controller/CommissioneeDeviceProxy.h>
#include <credentials/attestation_verifier/DeviceAttestationDelegate.h>
#include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
#include <lib/support/Variant.h>

Expand Down Expand Up @@ -323,6 +324,14 @@ class CommissioningParameters
}
void SetCompletionStatus(const CompletionStatus & status) { completionStatus = status; }

CommissioningParameters & SetDeviceAttestationDelegate(Credentials::DeviceAttestationDelegate * deviceAttestationDelegate)
{
mDeviceAttestationDelegate = deviceAttestationDelegate;
return *this;
}

Credentials::DeviceAttestationDelegate * GetDeviceAttestationDelegate() const { return mDeviceAttestationDelegate; }

private:
// Items that can be set by the commissioner
Optional<uint16_t> mFailsafeTimerSeconds;
Expand All @@ -347,6 +356,8 @@ class CommissioningParameters
Optional<app::Clusters::GeneralCommissioning::RegulatoryLocationType> mDefaultRegulatoryLocation;
Optional<app::Clusters::GeneralCommissioning::RegulatoryLocationType> mLocationCapability;
CompletionStatus completionStatus;
Credentials::DeviceAttestationDelegate * mDeviceAttestationDelegate =
nullptr; // Delegate to handle device attestation failures during commissioning
};

struct RequestedCertificate
Expand Down Expand Up @@ -460,6 +471,7 @@ class CommissioningDelegate
CommissioningStage stageCompleted;
};
virtual CHIP_ERROR SetCommissioningParameters(const CommissioningParameters & params) = 0;
virtual const CommissioningParameters & GetCommissioningParameters() const = 0;
virtual void SetOperationalCredentialsDelegate(OperationalCredentialsDelegate * operationalCredentialsDelegate) = 0;
virtual CHIP_ERROR StartCommissioning(DeviceCommissioner * commissioner, CommissioneeDeviceProxy * proxy) = 0;
virtual CHIP_ERROR CommissioningStepFinished(CHIP_ERROR err, CommissioningReport report) = 0;
Expand Down
2 changes: 2 additions & 0 deletions src/credentials/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ static_library("credentials") {
"GenerateChipX509Cert.cpp",
"GroupDataProvider.h",
"GroupDataProviderImpl.cpp",
"attestation_verifier/DeviceAttestationDelegate.h",
"attestation_verifier/DeviceAttestationVerifier.cpp",
"attestation_verifier/DeviceAttestationVerifier.h",
"examples/DeviceAttestationCredsExample.cpp",
Expand Down Expand Up @@ -85,6 +86,7 @@ static_library("default_attestation_verifier") {
sources = [
"attestation_verifier/DefaultDeviceAttestationVerifier.cpp",
"attestation_verifier/DefaultDeviceAttestationVerifier.h",
"attestation_verifier/DeviceAttestationDelegate.h",
]

if (chip_device_platform == "esp32" || chip_device_platform == "nrfconnect") {
Expand Down
62 changes: 62 additions & 0 deletions src/credentials/attestation_verifier/DeviceAttestationDelegate.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
*
* Copyright (c) 2021 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
#include <lib/core/Optional.h>

namespace chip {

class DeviceProxy;

namespace Controller {
class DeviceCommissioner;
} // namespace Controller

namespace Credentials {

/// Callbacks for CHIP device attestation status
class DeviceAttestationDelegate
{
public:
virtual ~DeviceAttestationDelegate() {}

/**
* @brief
* If valid, value to set for the fail-safe timer before the delegate's OnDeviceAttestionFailed is invoked.
*
* @return Optional value for the fail-safe timer in seconds.
*/
virtual Optional<uint16_t> FailSafeExpiryTimeoutSecs() const = 0;

/**
* @brief
* This method is invoked when device attestation fails for a device that is being commissioned. The client
* handling the failure has the option to continue commissionning or fail the operation.
*
* @param deviceCommissioner The commissioner object that is commissioning the device
* @param device The proxy represent the device being commissioned
* @param attestationResult The failure code for the device attestation validation operation
*/
virtual void OnDeviceAttestationFailed(Controller::DeviceCommissioner * deviceCommissioner, DeviceProxy * device,
AttestationVerificationResult attestationResult) = 0;
};

} // namespace Credentials
} // namespace chip
Loading

0 comments on commit 5570062

Please sign in to comment.