Skip to content

Commit

Permalink
Issue 22318 - commissioner attestation delegate should be able to ove…
Browse files Browse the repository at this point in the history 
…rride success
  • Loading branch information
@jtung-apple
jtung-apple committed Aug 31, 2022
1 parent f77eed5 commit ea82080
Show file tree
Hide file tree
Showing 16 changed files with 303 additions and 56 deletions.
66 changes: 47 additions & 19 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -796,8 +796,8 @@ CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId)
}

CHIP_ERROR
DeviceCommissioner::ContinueCommissioningAfterDeviceAttestationFailure(DeviceProxy * device,
Credentials::AttestationVerificationResult attestationResult)
DeviceCommissioner::ContinueCommissioningAfterDeviceAttestation(DeviceProxy * device,
Credentials::AttestationVerificationResult attestationResult)
{
MATTER_TRACE_EVENT_SCOPE("continueCommissioningDevice", "DeviceCommissioner");
if (device == nullptr || device != mDeviceBeingCommissioned)
Expand Down Expand Up @@ -993,7 +993,8 @@ void DeviceCommissioner::OnAttestationResponse(void * context,
commissioner->CommissioningStageComplete(CHIP_NO_ERROR, report);
}

void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * context, AttestationVerificationResult result)
void DeviceCommissioner::OnDeviceAttestationInformationVerification(
void * context, const Credentials::DeviceAttestationVerifier::AttestationInfo & info, AttestationVerificationResult result)
{
MATTER_TRACE_EVENT_SCOPE("OnDeviceAttestationInformationVerification", "DeviceCommissioner");
DeviceCommissioner * commissioner = reinterpret_cast<DeviceCommissioner *>(context);
Expand All @@ -1004,6 +1005,9 @@ void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * conte
return;
}

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

if (result != AttestationVerificationResult::kSuccess)
{
CommissioningDelegate::CommissioningReport report;
Expand All @@ -1024,14 +1028,11 @@ void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * conte
// Go look at AttestationVerificationResult enum in src/credentials/attestation_verifier/DeviceAttestationVerifier.h to
// understand the errors.

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);
commissioner->ExtendArmFailSafeForDeviceAttestation(info, result);
}
else
{
Expand All @@ -1040,15 +1041,22 @@ void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * conte
}
else
{
ChipLogProgress(Controller, "Successfully validated 'Attestation Information' command received from the device.");
commissioner->CommissioningStageComplete(CHIP_NO_ERROR);
if (deviceAttestationDelegate && deviceAttestationDelegate->ShouldWaitAfterDeviceAttestation())
{
commissioner->ExtendArmFailSafeForDeviceAttestation(info, result);
}
else
{
ChipLogProgress(Controller, "Successfully validated 'Attestation Information' command received from the device.");
commissioner->CommissioningStageComplete(CHIP_NO_ERROR);
}
}
}

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

if (!commissioner->mDeviceBeingCommissioned)
Expand All @@ -1061,8 +1069,9 @@ void DeviceCommissioner::OnArmFailSafeExtendedForFailedDeviceAttestation(
if (deviceAttestationDelegate)
{
ChipLogProgress(Controller, "Device attestation failed, delegating error handling to client");
deviceAttestationDelegate->OnDeviceAttestationFailed(commissioner, commissioner->mDeviceBeingCommissioned,
commissioner->mAttestationResult);
deviceAttestationDelegate->OnDeviceAttestationCompleted(commissioner, commissioner->mDeviceBeingCommissioned,
*commissioner->mAttestationDeviceInfo,
commissioner->mAttestationResult);
}
else
{
Expand All @@ -1073,7 +1082,7 @@ void DeviceCommissioner::OnArmFailSafeExtendedForFailedDeviceAttestation(
}
}

void DeviceCommissioner::OnFailedToExtendedArmFailSafeFailedDeviceAttestation(void * context, CHIP_ERROR error)
void DeviceCommissioner::OnFailedToExtendedArmFailSafeDeviceAttestation(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);
Expand All @@ -1083,13 +1092,29 @@ void DeviceCommissioner::OnFailedToExtendedArmFailSafeFailedDeviceAttestation(vo
commissioner->CommissioningStageComplete(CHIP_ERROR_INTERNAL, report);
}

void DeviceCommissioner::ExtendArmFailSafeForFailedDeviceAttestation(AttestationVerificationResult result)
void DeviceCommissioner::ClearAttestationDeviceInfo()
{
if (mAttestationDeviceInfo != nullptr)
{
chip::Platform::Delete(mAttestationDeviceInfo);
mAttestationDeviceInfo = nullptr;
}
}

void DeviceCommissioner::ExtendArmFailSafeForDeviceAttestation(const Credentials::DeviceAttestationVerifier::AttestationInfo & info,
Credentials::AttestationVerificationResult result)
{
mAttestationResult = result;

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

if (deviceAttestationDelegate->ShouldWaitAfterDeviceAttestation())
{
mAttestationDeviceInfo = chip::Platform::New<Credentials::DeviceAttestationVerifier::AttestationDeviceInfo>(info);
}

auto expiryLengthSeconds = deviceAttestationDelegate->FailSafeExpiryTimeoutSecs();
if (expiryLengthSeconds.HasValue())
{
GeneralCommissioning::Commands::ArmFailSafe::Type request;
Expand All @@ -1098,16 +1123,16 @@ void DeviceCommissioner::ExtendArmFailSafeForFailedDeviceAttestation(Attestation
ChipLogProgress(Controller, "Changing fail-safe timer to %u seconds to handle DA failure", request.expiryLengthSeconds);
// Per spec, anything we do with the fail-safe armed must not time out
// in less than kMinimumCommissioningStepTimeout.
SendCommand<GeneralCommissioningCluster>(mDeviceBeingCommissioned, request, OnArmFailSafeExtendedForFailedDeviceAttestation,
OnFailedToExtendedArmFailSafeFailedDeviceAttestation,
SendCommand<GeneralCommissioningCluster>(mDeviceBeingCommissioned, request, OnArmFailSafeExtendedForDeviceAttestation,
OnFailedToExtendedArmFailSafeDeviceAttestation,
MakeOptional(kMinimumCommissioningStepTimeout));
}
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);
OnArmFailSafeExtendedForDeviceAttestation(this, data);
}
}

Expand Down Expand Up @@ -1474,6 +1499,9 @@ void OnBasicFailure(void * context, CHIP_ERROR error)
void DeviceCommissioner::CleanupCommissioning(DeviceProxy * proxy, NodeId nodeId, const CompletionStatus & completionStatus)
{
commissioningCompletionStatus = completionStatus;

ClearAttestationDeviceInfo();

if (completionStatus.err == CHIP_NO_ERROR)
{

Expand Down
48 changes: 34 additions & 14 deletions src/controller/CHIPDeviceController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,15 +487,14 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
/**
* @brief
* This function instructs the commissioner to proceed to the next stage of commissioning after
* attestation failure is reported to an installed attestation delegate.
* attestation 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);
ContinueCommissioningAfterDeviceAttestation(DeviceProxy * device, Credentials::AttestationVerificationResult attestationResult);

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

Expand Down Expand Up @@ -565,7 +564,10 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
* @brief
* This function returns the current CommissioningStage for this commissioner.
*/
CommissioningStage GetCommissioningStage() { return mCommissioningStage; }
CommissioningStage GetCommissioningStage()
{
return mCommissioningStage;
}

#if CONFIG_NETWORK_LAYER_BLE
#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
Expand Down Expand Up @@ -607,7 +609,10 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
* Returns the max number of commissionable nodes this commissioner can track mdns information for.
* @return int The max number of commissionable nodes supported
*/
int GetMaxCommissionableNodesSupported() { return kMaxCommissionableNodes; }
int GetMaxCommissionableNodesSupported()
{
return kMaxCommissionableNodes;
}

#if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY // make this commissioner discoverable
/**
Expand All @@ -626,7 +631,10 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
* Return the UDC Server instance
*
*/
UserDirectedCommissioningServer * GetUserDirectedCommissioningServer() { return mUdcServer; }
UserDirectedCommissioningServer * GetUserDirectedCommissioningServer()
{
return mUdcServer;
}
#endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY

/**
Expand All @@ -638,8 +646,14 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
*/
void OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData) override;

void RegisterPairingDelegate(DevicePairingDelegate * pairingDelegate) { mPairingDelegate = pairingDelegate; }
DevicePairingDelegate * GetPairingDelegate() const { return mPairingDelegate; }
void RegisterPairingDelegate(DevicePairingDelegate * pairingDelegate)
{
mPairingDelegate = pairingDelegate;
}
DevicePairingDelegate * GetPairingDelegate() const
{
return mPairingDelegate;
}

// ClusterStateCache::Callback impl
void OnDone(app::ReadClient *) override;
Expand Down Expand Up @@ -715,7 +729,9 @@ 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);
void ClearAttestationDeviceInfo();
void ExtendArmFailSafeForDeviceAttestation(const Credentials::DeviceAttestationVerifier::AttestationInfo & info,
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 @@ -754,7 +770,9 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
static void OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
static void OnDeviceConnectionFailureFn(void * context, const ScopedNodeId & peerId, CHIP_ERROR error);

static void OnDeviceAttestationInformationVerification(void * context, Credentials::AttestationVerificationResult result);
static void OnDeviceAttestationInformationVerification(void * context,
const Credentials::DeviceAttestationVerifier::AttestationInfo & info,
Credentials::AttestationVerificationResult result);

static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
const ByteSpan & rcac, Optional<AesCcm128KeySpan> ipk, Optional<NodeId> adminSubject);
Expand All @@ -779,9 +797,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(
static void OnArmFailSafeExtendedForDeviceAttestation(
void * context, const chip::app::Clusters::GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data);
static void OnFailedToExtendedArmFailSafeFailedDeviceAttestation(void * context, CHIP_ERROR error);
static void OnFailedToExtendedArmFailSafeDeviceAttestation(void * context, CHIP_ERROR error);

/**
* @brief
Expand Down Expand Up @@ -860,7 +878,8 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
chip::Callback::Callback<OnDeviceConnected> mOnDeviceConnectedCallback;
chip::Callback::Callback<OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;

chip::Callback::Callback<Credentials::OnAttestationInformationVerification> mDeviceAttestationInformationVerificationCallback;
chip::Callback::Callback<Credentials::DeviceAttestationVerifier::OnAttestationInformationVerification>
mDeviceAttestationInformationVerificationCallback;

chip::Callback::Callback<OnNOCChainGeneration> mDeviceNOCChainCallback;
SetUpCodePairer mSetUpCodePairer;
Expand All @@ -874,7 +893,8 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
Platform::UniquePtr<app::ClusterStateCache> mAttributeCache;
Platform::UniquePtr<app::ReadClient> mReadClient;
Credentials::AttestationVerificationResult mAttestationResult;
Credentials::DeviceAttestationVerifier * mDeviceAttestationVerifier = nullptr;
Credentials::DeviceAttestationVerifier::AttestationDeviceInfo * mAttestationDeviceInfo = nullptr;
Credentials::DeviceAttestationVerifier * mDeviceAttestationVerifier = nullptr;
};

} // namespace Controller
Expand Down
1 change: 1 addition & 0 deletions src/controller/CommissioningDelegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ class CommissioningParameters
CompletionStatus completionStatus;
Credentials::DeviceAttestationDelegate * mDeviceAttestationDelegate =
nullptr; // Delegate to handle device attestation failures during commissioning
Optional<bool> mShouldWaitPostDeviceAttestation;
Optional<bool> mAttemptWiFiNetworkScan;
Optional<bool> mAttemptThreadNetworkScan; // This automatically gets set to false when a ThreadOperationalDataset is set
Optional<bool> mSkipCommissioningComplete;
Expand Down
2 changes: 1 addition & 1 deletion src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ void PartialDACVerifier::VerifyAttestationInformation(const DeviceAttestationVer
}

exit:
onCompletion->mCall(onCompletion->mContext, attestationError);
onCompletion->mCall(onCompletion->mContext, info, attestationError);
}

} // namespace Credentials
Expand Down
2 changes: 1 addition & 1 deletion src/credentials/attestation_verifier/DefaultDeviceAttestationVerifier.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ void DefaultDACVerifier::VerifyAttestationInformation(const DeviceAttestationVer
}

exit:
onCompletion->mCall(onCompletion->mContext, attestationError);
onCompletion->mCall(onCompletion->mContext, info, attestationError);
}

AttestationVerificationResult DefaultDACVerifier::ValidateCertificationDeclarationSignature(const ByteSpan & cmsEnvelopeBuffer,
Expand Down
21 changes: 18 additions & 3 deletions src/credentials/attestation_verifier/DeviceAttestationDelegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,29 @@ class DeviceAttestationDelegate
/**
* @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.
* handling the failure has the option to continue commissioning or fail the operation.
*
* If ShouldWaitAfterDeviceAttestation returns false, then in the case attestationResult is successful, the
* commissioner would finish commissioning the device after OnDeviceAttestationCompleted returns.
*
* If ShouldWaitAfterDeviceAttestation returns true, then the commissioner will always wait for a
* ContinueCommissioningAfterDeviceAttestation call after calling OnDeviceAttestationCompleted.
*
* @param deviceCommissioner The commissioner object that is commissioning the device
* @param device The proxy represent the device being commissioned
* @param info The structure holding device infor for additional verification by the application
* @param attestationResult The failure code for the device attestation validation operation
*/
virtual void OnDeviceAttestationFailed(Controller::DeviceCommissioner * deviceCommissioner, DeviceProxy * device,
AttestationVerificationResult attestationResult) = 0;
virtual void OnDeviceAttestationCompleted(Controller::DeviceCommissioner * deviceCommissioner, DeviceProxy * device,
const DeviceAttestationVerifier::AttestationDeviceInfo & info,
AttestationVerificationResult attestationResult) = 0;

/**
* @brief
* Override this method to return whether the attestation delegate wants the commissioner to wait for a
* ContinueCommissioningAfterDeviceAttestation call in the case attestationResult is successful.
*/
virtual bool ShouldWaitAfterDeviceAttestation() { return false; };
};

} // namespace Credentials
Expand Down
Loading

0 comments on commit ea82080

Please sign in to comment.