Implement separate Produce and Consume SetupPayload validation modes (project-chip#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>

Author: ksperling-apple

src/setup_payload/SetupPayload.cpp

@@ -36,60 +36,43 @@ namespace chip {
 // 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::IsValidSetupPIN(uint32_t setupPIN)
 
 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;
 }

src/setup_payload/SetupPayload.h

@@ -76,6 +76,7 @@ inline constexpr uint8_t kCommissioningTimeoutTag = 0x04;
 
 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 @@ struct PayloadContents
     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);

src/setup_payload/tests/TestQRCode.cpp

@@ -307,6 +307,13 @@ TEST(TestQRCode, TestSetupPayloadVerify)
     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;