FS - Check VendorID and ProductID when commissioning. (#35187)
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.cpp b/examples/fabric-admin/commands/pairing/PairingCommand.cpp
index 9a7adfa..55babdd 100644
--- a/examples/fabric-admin/commands/pairing/PairingCommand.cpp
+++ b/examples/fabric-admin/commands/pairing/PairingCommand.cpp
@@ -30,6 +30,7 @@
#include <protocols/secure_channel/PASESession.h>
#include <setup_payload/ManualSetupPayloadParser.h>
#include <setup_payload/QRCodeSetupPayloadParser.h>
+#include <setup_payload/SetupPayload.h>
#include <string>
@@ -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 @@
{
auto params = CommissioningParameters();
params.SetSkipCommissioningComplete(mSkipCommissioningComplete.ValueOr(false));
- if (mBypassAttestationVerifier.ValueOr(false))
- {
- params.SetDeviceAttestationDelegate(this);
- }
+ params.SetDeviceAttestationDelegate(this);
switch (mNetworkType)
{
@@ -564,18 +583,82 @@
chip::Optional<uint16_t> 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<uint16_t>();
}
+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 647f2c3..293c369 100644
--- a/examples/fabric-admin/commands/pairing/PairingCommand.h
+++ b/examples/fabric-admin/commands/pairing/PairingCommand.h
@@ -221,6 +221,7 @@
/////////// DeviceAttestationDelegate Interface /////////
chip::Optional<uint16_t> 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;
