From 17968601dfd871b8b0dfa895fec2f2b158a616c7 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 1 Jul 2022 22:01:34 -0400 Subject: [PATCH] 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 https://github.com/project-chip/connectedhomeip/issues/20226 --- .../chip-tool/commands/common/Command.cpp | 2 +- .../payload/SetupPayloadGenerateCommand.cpp | 128 +++++++++++++++++- .../payload/SetupPayloadGenerateCommand.h | 11 +- .../QRCodeSetupPayloadGenerator.cpp | 48 +++++++ .../QRCodeSetupPayloadGenerator.h | 44 ++++-- src/setup_payload/SetupPayload.h | 10 +- src/setup_payload/tests/TestQRCodeTLV.cpp | 18 +++ 7 files changed, 241 insertions(+), 20 deletions(-) diff --git a/examples/chip-tool/commands/common/Command.cpp b/examples/chip-tool/commands/common/Command.cpp index c8ee1850a729e1..59812076cfde75 100644 --- a/examples/chip-tool/commands/common/Command.cpp +++ b/examples/chip-tool/commands/common/Command.cpp @@ -355,7 +355,7 @@ bool Command::InitArgument(size_t argIndex, char * argValue) isValidArgument = HandleNullableOptional(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 diff --git a/examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.cpp b/examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.cpp index a9f18be49cd1e6..74abe2a0d58d16 100644 --- a/examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.cpp +++ b/examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.cpp @@ -17,6 +17,7 @@ */ #include "SetupPayloadGenerateCommand.h" +#include #include #include #include @@ -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(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); diff --git a/examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.h b/examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.h index 9e0d13c7cef3b2..31a16e1d9940a5 100644 --- a/examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.h +++ b/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 mVersion; chip::Optional mVendorId; chip::Optional mProductId; - chip::Optional mPayload; + chip::Optional mExistingPayload; chip::Optional mCommissioningMode; chip::Optional 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 mRendezvous; + chip::Optional mTLVBytes; }; class SetupPayloadGenerateManualCodeCommand : public SetupPayloadGenerateCommand diff --git a/src/setup_payload/QRCodeSetupPayloadGenerator.cpp b/src/setup_payload/QRCodeSetupPayloadGenerator.cpp index 976e443afa2c89..207a16a3e96c14 100644 --- a/src/setup_payload/QRCodeSetupPayloadGenerator.cpp +++ b/src/setup_payload/QRCodeSetupPayloadGenerator.cpp @@ -31,6 +31,8 @@ #include #include #include +#include +#include #include #include @@ -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(estimate), CHIP_ERROR_NO_MEMORY); + + Platform::ScopedMemoryBuffer buf; + VerifyOrReturnError(buf.Alloc(estimate), CHIP_ERROR_NO_MEMORY); + + return payloadBase38Representation(base38Representation, buf.Get(), static_cast(estimate)); +} + CHIP_ERROR QRCodeSetupPayloadGenerator::payloadBase38Representation(std::string & base38Representation, uint8_t * tlvDataStart, uint32_t tlvDataStartSize) { diff --git a/src/setup_payload/QRCodeSetupPayloadGenerator.h b/src/setup_payload/QRCodeSetupPayloadGenerator.h index 7d79e67cede8fc..d4a151a8c5717e 100644 --- a/src/setup_payload/QRCodeSetupPayloadGenerator.h +++ b/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. diff --git a/src/setup_payload/SetupPayload.h b/src/setup_payload/SetupPayload.h index 34473e95dc8a9c..868c71067f4cf6 100644 --- a/src/setup_payload/SetupPayload.h +++ b/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 **/ diff --git a/src/setup_payload/tests/TestQRCodeTLV.cpp b/src/setup_payload/tests/TestQRCodeTLV.cpp index 14afbf8a88fec8..739a074184b0ca 100644 --- a/src/setup_payload/tests/TestQRCodeTLV.cpp +++ b/src/setup_payload/tests/TestQRCodeTLV.cpp @@ -82,6 +82,12 @@ void TestSimpleWrite(nlTestSuite * inSuite, void * inContext) string result; CHIP_ERROR err = generator.payloadBase38Representation(result); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + string result2; + err = generator.payloadBase38RepresentationWithAutoTLVBuffer(result2); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + NL_TEST_ASSERT(inSuite, result == result2); } void TestSimpleRead(nlTestSuite * inSuite, void * inContext) @@ -158,6 +164,12 @@ void TestOptionalDataWriteSerial(nlTestSuite * inSuite, void * inContext) uint8_t optionalInfo[kDefaultBufferSizeInBytes]; err = generator.payloadBase38Representation(result, optionalInfo, sizeof(optionalInfo)); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + string result2; + err = generator.payloadBase38RepresentationWithAutoTLVBuffer(result2); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + NL_TEST_ASSERT(inSuite, result == result2); } void TestOptionalDataWrite(nlTestSuite * inSuite, void * inContext) @@ -169,6 +181,12 @@ void TestOptionalDataWrite(nlTestSuite * inSuite, void * inContext) uint8_t optionalInfo[kDefaultBufferSizeInBytes]; CHIP_ERROR err = generator.payloadBase38Representation(result, optionalInfo, sizeof(optionalInfo)); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + string result2; + err = generator.payloadBase38RepresentationWithAutoTLVBuffer(result2); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + NL_TEST_ASSERT(inSuite, result == result2); } void TestOptionalDataReadSerial(nlTestSuite * inSuite, void * inContext)