Allow chip-tool to generate onboarding payloads with extra TLV. (#20231)

Specific changes:

1) Clarify the naming of some of the chip-tool payload arguments and add
   documentation.
2) Fix chip-tool handling of the existing-payload argument so that we error out
   on invalid existing payloads instead of silently pressing on.
3) Add a way to pass in the TLV-encoded extra bytes to be added to a payload.
   Unfortunately, only tags that our SetupPayload knows about right now are
   supported.
4) Add a function on QRCodeSetupPayloadGenerator that allows generating a code
   without having to guess at how much space the TLV will take up.
5) Add tests for that new function.

Fixes #20226

examples/chip-tool/commands/common/Command.cpp

@@ -355,7 +355,7 @@ bool Command::InitArgument(size_t argIndex, char * argValue)
         isValidArgument = HandleNullableOptional<chip::ByteSpan>(arg, argValue, [&](auto * value) {
             // We support two ways to pass an octet string argument.  If it happens
             // to be all-ASCII, you can just pass it in.  Otherwise you can pass in
-            // 0x followed by the hex-encoded bytes.
+            // "hex:" followed by the hex-encoded bytes.
             size_t argLen                     = strlen(argValue);
             static constexpr char hexPrefix[] = "hex:";
             constexpr size_t prefixLen        = ArraySize(hexPrefix) - 1; // Don't count the null

examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.cpp

@@ -17,6 +17,7 @@
  */
 
 #include "SetupPayloadGenerateCommand.h"
+#include <lib/core/CHIPTLV.h>
 #include <setup_payload/ManualSetupPayloadGenerator.h>
 #include <setup_payload/ManualSetupPayloadParser.h>
 #include <setup_payload/QRCodeSetupPayloadGenerator.h>
@@ -62,9 +63,14 @@ CHIP_ERROR SetupPayloadGenerateQRCodeCommand::Run()
 {
     SetupPayload payload;
 
-    if (mPayload.HasValue())
+    if (mExistingPayload.HasValue())
     {
-        QRCodeSetupPayloadParser(mPayload.Value()).populatePayload(payload);
+        CHIP_ERROR err = QRCodeSetupPayloadParser(mExistingPayload.Value()).populatePayload(payload);
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(chipTool, "Invalid existing payload: %" CHIP_ERROR_FORMAT, err.Format());
+            return err;
+        }
     }
 
     ConfigurePayload(payload);
@@ -74,23 +80,135 @@ CHIP_ERROR SetupPayloadGenerateQRCodeCommand::Run()
         payload.rendezvousInformation.SetRaw(mRendezvous.Value());
     }
 
+    if (mTLVBytes.HasValue())
+    {
+        CHIP_ERROR err = PopulatePayloadTLVFromBytes(payload, mTLVBytes.Value());
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(chipTool, "Unable to populate payload TLV: %" CHIP_ERROR_FORMAT, err.Format());
+            return err;
+        }
+    }
+
     QRCodeSetupPayloadGenerator generator(payload);
     generator.SetAllowInvalidPayload(mAllowInvalidPayload.ValueOr(false));
 
     std::string code;
-    ReturnErrorOnFailure(generator.payloadBase38Representation(code));
+    ReturnErrorOnFailure(generator.payloadBase38RepresentationWithAutoTLVBuffer(code));
     ChipLogProgress(chipTool, "QR Code: %s", code.c_str());
 
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR SetupPayloadGenerateQRCodeCommand::PopulatePayloadTLVFromBytes(SetupPayload & payload, const ByteSpan & tlvBytes)
+{
+    // First clear out all the existing TVL bits from the payload.  Ignore
+    // errors here, because we don't care if those bits are not present.
+    payload.removeSerialNumber();
+
+    auto existingVendorData = payload.getAllOptionalVendorData();
+    for (auto & data : existingVendorData)
+    {
+        payload.removeOptionalVendorData(data.tag);
+    }
+
+    if (tlvBytes.empty())
+    {
+        // Used to just clear out the existing TLV.
+        return CHIP_NO_ERROR;
+    }
+
+    TLV::TLVReader reader;
+    reader.Init(tlvBytes);
+
+    // Data is a TLV structure.
+    ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag()));
+
+    TLV::TLVType outerType;
+    ReturnErrorOnFailure(reader.EnterContainer(outerType));
+
+    CHIP_ERROR err;
+    while ((err = reader.Next()) == CHIP_NO_ERROR)
+    {
+        TLV::Tag tag = reader.GetTag();
+        if (!TLV::IsContextTag(tag))
+        {
+            ChipLogError(chipTool, "Unexpected non-context TLV tag.");
+            return CHIP_ERROR_INVALID_TLV_TAG;
+        }
+
+        uint8_t tagNum = static_cast<uint8_t>(TLV::TagNumFromTag(tag));
+        if (tagNum < 0x80)
+        {
+            // Matter-common tag.
+            if (tagNum != kSerialNumberTag)
+            {
+                ChipLogError(chipTool, "No support yet for Matter-common tags other than serial number");
+                return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+            }
+
+            // Serial number can be a string or an unsigned integer.
+            if (reader.GetType() == TLV::kTLVType_UTF8String)
+            {
+                CharSpan data;
+                ReturnErrorOnFailure(reader.Get(data));
+                ReturnErrorOnFailure(payload.addSerialNumber(std::string(data.data(), data.size())));
+                continue;
+            }
+
+            if (reader.GetType() == TLV::kTLVType_UnsignedInteger)
+            {
+                uint32_t value;
+                ReturnErrorOnFailure(reader.Get(value));
+                ReturnErrorOnFailure(payload.addSerialNumber(value));
+                continue;
+            }
+
+            ChipLogError(chipTool, "Unexpected type for serial number: %d", to_underlying(reader.GetType()));
+            return CHIP_ERROR_WRONG_TLV_TYPE;
+        }
+
+        // Vendor tag.  We support strings and signed integers.
+        if (reader.GetType() == TLV::kTLVType_UTF8String)
+        {
+            CharSpan data;
+            ReturnErrorOnFailure(reader.Get(data));
+            ReturnErrorOnFailure(payload.addOptionalVendorData(tagNum, std::string(data.data(), data.size())));
+            continue;
+        }
+
+        if (reader.GetType() == TLV::kTLVType_SignedInteger)
+        {
+            int32_t value;
+            ReturnErrorOnFailure(reader.Get(value));
+            ReturnErrorOnFailure(payload.addOptionalVendorData(tagNum, value));
+            continue;
+        }
+
+        ChipLogError(chipTool, "Unexpected type for vendor data: %d", to_underlying(reader.GetType()));
+        return CHIP_ERROR_WRONG_TLV_TYPE;
+    }
+
+    VerifyOrReturnError(err == CHIP_END_OF_TLV, err);
+
+    ReturnErrorOnFailure(reader.ExitContainer(outerType));
+    ReturnErrorOnFailure(reader.VerifyEndOfContainer());
+
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR SetupPayloadGenerateManualCodeCommand::Run()
 {
     SetupPayload payload;
 
-    if (mPayload.HasValue())
+    if (mExistingPayload.HasValue())
     {
-        ManualSetupPayloadParser(mPayload.Value()).populatePayload(payload);
+        CHIP_ERROR err = ManualSetupPayloadParser(mExistingPayload.Value()).populatePayload(payload);
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(chipTool, "Invalid existing payload: %" CHIP_ERROR_FORMAT, err.Format());
+            return err;
+        }
     }
 
     ConfigurePayload(payload);

examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.h

@@ -26,7 +26,7 @@ class SetupPayloadGenerateCommand : public Command
 public:
     SetupPayloadGenerateCommand(const char * name) : Command(name)
     {
-        AddArgument("payload", &mPayload);
+        AddArgument("existing-payload", &mExistingPayload, "An existing setup payload to modify based on the other arguments.");
         AddArgument("discriminator", 0, UINT16_MAX, &mDiscriminator);
         AddArgument("setup-pin-code", 0, UINT32_MAX, &mSetUpPINCode);
         AddArgument("version", 0, UINT8_MAX, &mVersion);
@@ -44,7 +44,7 @@ class SetupPayloadGenerateCommand : public Command
     chip::Optional<uint8_t> mVersion;
     chip::Optional<uint16_t> mVendorId;
     chip::Optional<uint16_t> mProductId;
-    chip::Optional<char *> mPayload;
+    chip::Optional<char *> mExistingPayload;
     chip::Optional<uint8_t> mCommissioningMode;
     chip::Optional<bool> mAllowInvalidPayload;
 };
@@ -55,11 +55,18 @@ class SetupPayloadGenerateQRCodeCommand : public SetupPayloadGenerateCommand
     SetupPayloadGenerateQRCodeCommand() : SetupPayloadGenerateCommand("generate-qrcode")
     {
         AddArgument("rendezvous", 0, UINT8_MAX, &mRendezvous);
+        AddArgument(
+            "tlvBytes", &mTLVBytes,
+            "Pre-encoded TLV for the optional part of the payload.  A nonempty value should be passed as \"hex:\" followed by the "
+            "bytes in hex encoding.  Passing an empty string to override the TLV in an existing payload is allowed.");
     }
     CHIP_ERROR Run() override;
 
 private:
+    static CHIP_ERROR PopulatePayloadTLVFromBytes(chip::SetupPayload & payload, const chip::ByteSpan & tlvBytes);
+
     chip::Optional<uint8_t> mRendezvous;
+    chip::Optional<chip::ByteSpan> mTLVBytes;
 };
 
 class SetupPayloadGenerateManualCodeCommand : public SetupPayloadGenerateCommand

src/setup_payload/QRCodeSetupPayloadGenerator.cpp

@@ -31,6 +31,8 @@
 #include <lib/core/CHIPTLVDebug.hpp>
 #include <lib/core/CHIPTLVUtilities.hpp>
 #include <lib/support/CodeUtils.h>
+#include <lib/support/SafeInt.h>
+#include <lib/support/ScopedBuffer.h>
 #include <protocols/Protocols.h>
 
 #include <stdlib.h>
@@ -210,6 +212,52 @@ CHIP_ERROR QRCodeSetupPayloadGenerator::payloadBase38Representation(std::string
     return payloadBase38Representation(base38Representation, nullptr, 0);
 }
 
+CHIP_ERROR QRCodeSetupPayloadGenerator::payloadBase38RepresentationWithAutoTLVBuffer(std::string & base38Representation)
+{
+    // Estimate the size of the needed buffer.
+    size_t estimate = 0;
+
+    auto dataItemSizeEstimate = [](const OptionalQRCodeInfo & item) {
+        // Each data item needs a control byte and a context tag.
+        size_t size = 2;
+
+        if (item.type == optionalQRCodeInfoTypeString)
+        {
+            // We'll need to encode the string length and then the string data.
+            // Length is at most 8 bytes.
+            size += 8;
+            size += item.data.size();
+        }
+        else
+        {
+            // Integer.  Assume it might need up to 8 bytes, for simplicity.
+            size += 8;
+        }
+        return size;
+    };
+
+    auto vendorData = mPayload.getAllOptionalVendorData();
+    for (auto & data : vendorData)
+    {
+        estimate += dataItemSizeEstimate(data);
+    }
+
+    auto extensionData = mPayload.getAllOptionalExtensionData();
+    for (auto & data : extensionData)
+    {
+        estimate += dataItemSizeEstimate(data);
+    }
+
+    estimate = TLV::EstimateStructOverhead(estimate);
+
+    VerifyOrReturnError(CanCastTo<uint32_t>(estimate), CHIP_ERROR_NO_MEMORY);
+
+    Platform::ScopedMemoryBuffer<uint8_t> buf;
+    VerifyOrReturnError(buf.Alloc(estimate), CHIP_ERROR_NO_MEMORY);
+
+    return payloadBase38Representation(base38Representation, buf.Get(), static_cast<uint32_t>(estimate));
+}
+
 CHIP_ERROR QRCodeSetupPayloadGenerator::payloadBase38Representation(std::string & base38Representation, uint8_t * tlvDataStart,
                                                                     uint32_t tlvDataStartSize)
 {

src/setup_payload/QRCodeSetupPayloadGenerator.h

@@ -45,7 +45,10 @@ class QRCodeSetupPayloadGenerator
 
     /**
      * This function is called to encode the binary data of a payload to a
-     * base38 null-terminated string using CHIP TLV encoding scheme.
+     * base38 null-terminated string.
+     *
+     * If the payload has any optional data that needs to be TLV encoded, this
+     * function will fail.
      *
      * @param[out] base38Representation
      *                  The string to copy the base38 to.
@@ -60,10 +63,29 @@ class QRCodeSetupPayloadGenerator
 
     /**
      * This function is called to encode the binary data of a payload to a
-     * base38 null-terminated string. Callers must pass a buffer of at least
-     * chip::kTotalPayloadDataInBytes or more if there is any serialNumber or
-     * any other optional data. The buffer should be big enough to hold the
-     * TLV encoded value of the payload. If not an error will be throw.
+     * base38 null-terminated string.
+     *
+     * If the payload has any optional data that needs to be TLV encoded, this
+     * function will allocate a scratch heap buffer to hold the TLV data while
+     * encoding.
+     *
+     * @param[out] base38Representation
+     *                  The string to copy the base38 to.
+     *
+     * @retval #CHIP_NO_ERROR if the method succeeded.
+     * @retval #CHIP_ERROR_INVALID_ARGUMENT if the payload is invalid.
+     * @retval other Other CHIP or platform-specific error codes indicating
+     *               that an error occurred preventing the function from
+     *               producing the requested string.
+     */
+    CHIP_ERROR payloadBase38RepresentationWithAutoTLVBuffer(std::string & base38Representation);
+
+    /**
+     * This function is called to encode the binary data of a payload to a
+     * base38 null-terminated string, using the caller-provided buffer as
+     * temporary scratch space for optional data that needs to be TLV-encoded.
+     * If that buffer is not big enough to hold the TLV-encoded part of the
+     * payload, this function will fail.
      *
      * @param[out] base38Representation
      *                  The string to copy the base38 to.
@@ -83,9 +105,9 @@ class QRCodeSetupPayloadGenerator
     CHIP_ERROR payloadBase38Representation(std::string & base38Representation, uint8_t * tlvDataStart, uint32_t tlvDataStartSize);
 
     /**
-     * This function disable internal checks about the validity of the generated payload.
-     * It allows using the generator to generates invalid payloads.
-     * Defaults is false.
+     * This function disables internal checks about the validity of the generated payload.
+     * It allows using the generator to generate invalid payloads.
+     * Default is false.
      */
     void SetAllowInvalidPayload(bool allow) { mAllowInvalidPayload = allow; }
 
@@ -112,7 +134,11 @@ class QRCodeBasicSetupPayloadGenerator
      * This function is called to encode the binary data of a payload to a
      * base38 null-terminated string.
      *
-     * The resulting size of the out_buf span will be the size of data written and not including the null terminator.
+     * The resulting size of the out_buf span will be the size of data written
+     * and not including the null terminator.
+     *
+     * This function will fail if the payload has any optional data requiring
+     * TLV encoding.
      *
      * @param[out] outBuffer
      *                  The buffer to copy the base38 to.

src/setup_payload/SetupPayload.h

@@ -65,8 +65,12 @@ const int kManualSetupCodeChunk3CharLength = 4;
 const int kManualSetupVendorIdCharLength   = 5;
 const int kManualSetupProductIdCharLength  = 5;
 
-// Spec 5.1.4.2 CHIP-Common Reserved Tag (kTag_SerialNumber)
-const uint8_t kSerialNumberTag = 0;
+// Spec 5.1.4.2 CHIP-Common Reserved Tags
+constexpr uint8_t kSerialNumberTag         = 0x00;
+constexpr uint8_t kPBKDFIterationsTag      = 0x01;
+constexpr uint8_t kBPKFSaltTag             = 0x02;
+constexpr uint8_t kNumberOFDevicesTag      = 0x03;
+constexpr uint8_t kCommissioningTimeoutTag = 0x04;
 
 // clang-format off
 const int kTotalPayloadDataSizeInBits =
@@ -172,7 +176,7 @@ class SetupPayload : public PayloadContents
 
 public:
     /** @brief A function to add an optional vendor data
-     * @param tag 7 bit [0-127] tag number
+     * @param tag tag number in the [0x80-0xFF] range
      * @param data String representation of data to add
      * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
      **/