Implement separate Produce and Consume SetupPayload validation modes (#33285)
* Implement separate Produce and Consume SetupPayload validation modes
In Consume mode, unknown rendevouz flags are allowed.
* Simplify the validation code by using VerifyOrReturnValue
setUpPINCode range is checked in IsValidSetupPIN() via CheckPayloadCommonConstraints()
so no separate check is needed.
* Fix comment spelling.
---------
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
diff --git a/src/setup_payload/SetupPayload.cpp b/src/setup_payload/SetupPayload.cpp
index 0634563..1ef4795 100644
--- a/src/setup_payload/SetupPayload.cpp
+++ b/src/setup_payload/SetupPayload.cpp
@@ -36,60 +36,43 @@
// Check the Setup Payload for validity
//
// `vendor_id` and `product_id` are allowed all of uint16_t
-bool PayloadContents::isValidQRCodePayload() const
+bool PayloadContents::isValidQRCodePayload(ValidationMode mode) const
{
// 3-bit value specifying the QR code payload version.
- if (version >= 1 << kVersionFieldLengthInBits)
- {
- return false;
- }
+ VerifyOrReturnValue(version < (1 << kVersionFieldLengthInBits), false);
- if (static_cast<uint8_t>(commissioningFlow) > static_cast<uint8_t>((1 << kCommissioningFlowFieldLengthInBits) - 1))
- {
- return false;
- }
+ VerifyOrReturnValue(static_cast<uint8_t>(commissioningFlow) < (1 << kCommissioningFlowFieldLengthInBits), false);
// Device Commissioning Flow
+ // Even in ValidationMode::kConsume we can only handle modes that we understand.
// 0: Standard commissioning flow: such a device, when uncommissioned, always enters commissioning mode upon power-up, subject
// to the rules in [ref_Announcement_Commencement]. 1: User-intent commissioning flow: user action required to enter
// commissioning mode. 2: Custom commissioning flow: interaction with a vendor-specified means is needed before commissioning.
// 3: Reserved
- if (commissioningFlow != CommissioningFlow::kStandard && commissioningFlow != CommissioningFlow::kUserActionRequired &&
- commissioningFlow != CommissioningFlow::kCustom)
- {
- return false;
- }
-
- chip::RendezvousInformationFlags allvalid(RendezvousInformationFlag::kBLE, RendezvousInformationFlag::kOnNetwork,
- RendezvousInformationFlag::kSoftAP);
- if (!rendezvousInformation.HasValue() || !rendezvousInformation.Value().HasOnly(allvalid))
- {
- return false;
- }
+ VerifyOrReturnValue(commissioningFlow == CommissioningFlow::kStandard ||
+ commissioningFlow == CommissioningFlow::kUserActionRequired ||
+ commissioningFlow == CommissioningFlow::kCustom,
+ false);
// General discriminator validity is enforced by the SetupDiscriminator class, but it can't be short for QR a code.
- if (discriminator.IsShortDiscriminator())
- {
- return false;
- }
+ VerifyOrReturnValue(!discriminator.IsShortDiscriminator(), false);
- if (setUpPINCode >= 1 << kSetupPINCodeFieldLengthInBits)
+ // RendevouzInformation must be present for a QR code.
+ VerifyOrReturnValue(rendezvousInformation.HasValue(), false);
+ if (mode == ValidationMode::kProduce)
{
- return false;
+ chip::RendezvousInformationFlags valid(RendezvousInformationFlag::kBLE, RendezvousInformationFlag::kOnNetwork,
+ RendezvousInformationFlag::kSoftAP);
+ VerifyOrReturnValue(rendezvousInformation.Value().HasOnly(valid), false);
}
return CheckPayloadCommonConstraints();
}
-bool PayloadContents::isValidManualCode() const
+bool PayloadContents::isValidManualCode(ValidationMode mode) const
{
- // Discriminator validity is enforced by the SetupDiscriminator class.
-
- if (setUpPINCode >= 1 << kSetupPINCodeFieldLengthInBits)
- {
- return false;
- }
-
+ // No additional constraints apply to Manual Pairing Codes.
+ // (If the payload has a long discriminator it will be converted automatically.)
return CheckPayloadCommonConstraints();
}
@@ -109,31 +92,22 @@
bool PayloadContents::CheckPayloadCommonConstraints() const
{
- // A version not equal to 0 would be invalid for v1 and would indicate new format (e.g. version 2)
- if (version != 0)
- {
- return false;
- }
+ // Validation rules in this method apply to all validation modes.
- if (!IsValidSetupPIN(setUpPINCode))
- {
- return false;
- }
+ // Even in ValidationMode::kConsume we don't understand how to handle any payload version other than 0.
+ VerifyOrReturnValue(version == 0, false);
+
+ VerifyOrReturnValue(IsValidSetupPIN(setUpPINCode), false);
// VendorID must be unspecified (0) or in valid range expected.
- if (!IsVendorIdValidOperationally(vendorID) && (vendorID != VendorId::Unspecified))
- {
- return false;
- }
+ VerifyOrReturnValue((vendorID == VendorId::Unspecified) || IsVendorIdValidOperationally(vendorID), false);
// A value of 0x0000 SHALL NOT be assigned to a product since Product ID = 0x0000 is used for these specific cases:
// * To announce an anonymized Product ID as part of device discovery
// * To indicate an OTA software update file applies to multiple Product IDs equally.
// * To avoid confusion when presenting the Onboarding Payload for ECM with multiple nodes
- if (productID == 0 && vendorID != VendorId::Unspecified)
- {
- return false;
- }
+ // In these special cases the vendorID must be 0 (Unspecified)
+ VerifyOrReturnValue(productID != 0 || vendorID == VendorId::Unspecified, false);
return true;
}
diff --git a/src/setup_payload/SetupPayload.h b/src/setup_payload/SetupPayload.h
index 6e81e0e..cc71547 100644
--- a/src/setup_payload/SetupPayload.h
+++ b/src/setup_payload/SetupPayload.h
@@ -76,6 +76,7 @@
inline constexpr uint32_t kSetupPINCodeMaximumValue = 99999998;
inline constexpr uint32_t kSetupPINCodeUndefinedValue = 0;
+static_assert(kSetupPINCodeMaximumValue < (1 << kSetupPINCodeFieldLengthInBits));
// clang-format off
const int kTotalPayloadDataSizeInBits =
@@ -128,8 +129,20 @@
SetupDiscriminator discriminator;
uint32_t setUpPINCode = 0;
- bool isValidQRCodePayload() const;
- bool isValidManualCode() const;
+ enum class ValidationMode : uint8_t
+ {
+ kProduce, ///< Only flags or values allowed by the current spec version are allowed.
+ /// Producers of a Setup Payload should use this mode to ensure the
+ // payload is valid according to the current spec version.
+ kConsume, ///< Flags or values that are reserved for future use, or were allowed in
+ /// a previous spec version may be present. Consumers of a Setup Payload
+ /// should use this mode to ensure they are forward and backwards
+ /// compatible with payloads from older or newer Matter devices.
+ };
+
+ bool isValidQRCodePayload(ValidationMode mode = ValidationMode::kProduce) const;
+ bool isValidManualCode(ValidationMode mode = ValidationMode::kProduce) const;
+
bool operator==(const PayloadContents & input) const;
static bool IsValidSetupPIN(uint32_t setupPIN);
diff --git a/src/setup_payload/tests/TestQRCode.cpp b/src/setup_payload/tests/TestQRCode.cpp
index 6ca349c..6482b9a 100644
--- a/src/setup_payload/tests/TestQRCode.cpp
+++ b/src/setup_payload/tests/TestQRCode.cpp
@@ -307,6 +307,13 @@
invalid.SetRaw(static_cast<uint8_t>(invalid.Raw() + 1));
test_payload.rendezvousInformation.SetValue(invalid);
EXPECT_EQ(test_payload.isValidQRCodePayload(), false);
+ // When validating in Consume mode, unknown rendezvous flags are OK.
+ EXPECT_TRUE(test_payload.isValidQRCodePayload(PayloadContents::ValidationMode::kConsume));
+ test_payload.rendezvousInformation.SetValue(RendezvousInformationFlags(0xff));
+ EXPECT_TRUE(test_payload.isValidQRCodePayload(PayloadContents::ValidationMode::kConsume));
+ // Rendezvous information is still required even in Consume mode.
+ test_payload.rendezvousInformation.ClearValue();
+ EXPECT_FALSE(test_payload.isValidQRCodePayload(PayloadContents::ValidationMode::kConsume));
// test invalid setup PIN
test_payload = payload;
