From 14a51919c239f5ced8cf4b103efd3c3bbadbe871 Mon Sep 17 00:00:00 2001 From: Chris Letnick Date: Thu, 29 Aug 2024 16:21:10 -0700 Subject: [PATCH] FS - Check VendorID and ProductID when commissioning. (#35187) --------- Co-authored-by: Restyled.io --- .../commands/pairing/PairingCommand.cpp | 99 +++++++++++++++++-- .../commands/pairing/PairingCommand.h | 1 + 2 files changed, 92 insertions(+), 8 deletions(-) diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.cpp b/examples/fabric-admin/commands/pairing/PairingCommand.cpp index 9a7adfa3f910e5..55babddd19520b 100644 --- a/examples/fabric-admin/commands/pairing/PairingCommand.cpp +++ b/examples/fabric-admin/commands/pairing/PairingCommand.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include @@ -40,6 +41,27 @@ using namespace ::chip; using namespace ::chip::Controller; +namespace { + +CHIP_ERROR GetPayload(const char * setUpCode, SetupPayload & payload) +{ + bool isQRCode = strncmp(setUpCode, kQRCodePrefix, strlen(kQRCodePrefix)) == 0; + if (isQRCode) + { + ReturnErrorOnFailure(QRCodeSetupPayloadParser(setUpCode).populatePayload(payload)); + VerifyOrReturnError(payload.isValidQRCodePayload(), CHIP_ERROR_INVALID_ARGUMENT); + } + else + { + ReturnErrorOnFailure(ManualSetupPayloadParser(setUpCode).populatePayload(payload)); + VerifyOrReturnError(payload.isValidManualCode(), CHIP_ERROR_INVALID_ARGUMENT); + } + + return CHIP_NO_ERROR; +} + +} // namespace + CHIP_ERROR PairingCommand::RunCommand() { CurrentCommissioner().RegisterPairingDelegate(this); @@ -105,10 +127,7 @@ CommissioningParameters PairingCommand::GetCommissioningParameters() { auto params = CommissioningParameters(); params.SetSkipCommissioningComplete(mSkipCommissioningComplete.ValueOr(false)); - if (mBypassAttestationVerifier.ValueOr(false)) - { - params.SetDeviceAttestationDelegate(this); - } + params.SetDeviceAttestationDelegate(this); switch (mNetworkType) { @@ -564,18 +583,82 @@ void PairingCommand::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E chip::Optional PairingCommand::FailSafeExpiryTimeoutSecs() const { - // We don't need to set additional failsafe timeout as we don't ask the final user if he wants to continue + // No manual input, so do not need to extend. return chip::Optional(); } +bool PairingCommand::ShouldWaitAfterDeviceAttestation() +{ + // If there is a vendor ID and product ID, request OnDeviceAttestationCompleted(). + // Currently this is added in the case that the example is performing reverse commissioning, + // but it would be an improvement to store that explicitly. + // TODO: Issue #35297 - [Fabric Sync] Improve where we get VID and PID when validating CCTRL CommissionNode command + SetupPayload payload; + CHIP_ERROR err = GetPayload(mOnboardingPayload, payload); + return err == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0); +} + void PairingCommand::OnDeviceAttestationCompleted(chip::Controller::DeviceCommissioner * deviceCommissioner, chip::DeviceProxy * device, const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info, chip::Credentials::AttestationVerificationResult attestationResult) { - // Bypass attestation verification, continue with success - auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( - device, chip::Credentials::AttestationVerificationResult::kSuccess); + SetupPayload payload; + CHIP_ERROR parse_error = GetPayload(mOnboardingPayload, payload); + if (parse_error == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0)) + { + if (payload.vendorID == 0 || payload.productID == 0) + { + ChipLogProgress(NotSpecified, + "Failed validation: vendorID or productID must not be 0." + "Requested VID: %u, Requested PID: %u.", + payload.vendorID, payload.productID); + deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( + device, chip::Credentials::AttestationVerificationResult::kInvalidArgument); + return; + } + + if (payload.vendorID != info.BasicInformationVendorId() || payload.productID != info.BasicInformationProductId()) + { + ChipLogProgress(NotSpecified, + "Failed validation of vendorID or productID." + "Requested VID: %u, Requested PID: %u," + "Detected VID: %u, Detected PID %u.", + payload.vendorID, payload.productID, info.BasicInformationVendorId(), info.BasicInformationProductId()); + deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( + device, + payload.vendorID == info.BasicInformationVendorId() + ? chip::Credentials::AttestationVerificationResult::kDacProductIdMismatch + : chip::Credentials::AttestationVerificationResult::kDacVendorIdMismatch); + return; + } + + // NOTE: This will log errors even if the attestion was successful. + auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); + if (CHIP_NO_ERROR != err) + { + SetCommandExitStatus(err); + } + return; + } + + // OnDeviceAttestationCompleted() is called if ShouldWaitAfterDeviceAttestation() returns true + // or if there is an attestation error. The conditions for ShouldWaitAfterDeviceAttestation() have + // already been checked, so the call to OnDeviceAttestationCompleted() was an error. + if (mBypassAttestationVerifier.ValueOr(false)) + { + // Bypass attestation verification, continue with success + auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation( + device, chip::Credentials::AttestationVerificationResult::kSuccess); + if (CHIP_NO_ERROR != err) + { + SetCommandExitStatus(err); + } + return; + } + + // Don't bypass attestation, continue with error. + auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult); if (CHIP_NO_ERROR != err) { SetCommandExitStatus(err); diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.h b/examples/fabric-admin/commands/pairing/PairingCommand.h index 647f2c37fc78b2..293c369b25227e 100644 --- a/examples/fabric-admin/commands/pairing/PairingCommand.h +++ b/examples/fabric-admin/commands/pairing/PairingCommand.h @@ -221,6 +221,7 @@ class PairingCommand : public CHIPCommand, /////////// DeviceAttestationDelegate Interface ///////// chip::Optional FailSafeExpiryTimeoutSecs() const override; + bool ShouldWaitAfterDeviceAttestation() override; void OnDeviceAttestationCompleted(chip::Controller::DeviceCommissioner * deviceCommissioner, chip::DeviceProxy * device, const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info, chip::Credentials::AttestationVerificationResult attestationResult) override;