Rotating device id representation in TLV Additional Data needs to be …

…aligned with the spec (project-chip#9455) * RotatingId: version0 * RotatingId: version1 * RotatingId: version0 * RotatingId: version1 * Fix Darwin host build (project-chip#3990) #### Problem Some conversions to use PacketBufferHandle (project-chip#3909) broke Darwin builds, which aren't currently run in CI. #### Summary of Changes Fix src/platform/Darwin/BleConnectionDelegateImpl.mm to match the API change in project-chip#3909. * Add '-Wextra' to compiler flags (project-chip#3902) * Implement Level Control Cluster (project-chip#3806) * New seekbar in Android CHIPTool * Sample usage in lighting-app/nrfconnect Signed-off-by: Markus Becker <markus.becker@tridonic.com> * Fix Rendezvous over BLE after recent changes (project-chip#4012) PR project-chip#3704 introduced a change that the BLE transport in RendezvousSession is only initialized when PeerAddress in RendezvousParams is of type BLE. However, PeerAddress isn't initialized anywhere. As a result Rendezvous over BLE stopped working between Android CHIPTool and accessories. Btw, remove an assert related to the storage delegate as it seems an optional member of the device controller. * [thread] fix invalid configuration of active dataset (project-chip#4008) * Fix data loss or crash in TCPEndPoint with LwIP (project-chip#4022) * Fix data loss or crash in TCPEndPoint with LwIP #### Problem Under the configuration CHIP_SYSTEM_CONFIG_USE_LWIP, in some cases where the data size exceeds the TCP window size, TCPEndPoint can either die or lose data when accounting of un-acked data falls out of sync. #### Summary of Changes Imported fix from Weave: * This change removes separate accounting of the unsent data position and replaces it with simple counting of sent-but-not-acked data and a skip-loop at the start of DriveSending(). Fixes project-chip#4013 - Data loss or crash in TCPEndPoint with LwIP * Restyled by clang-format Co-authored-by: Restyled.io <commits@restyled.io> * Update lighting-app gen/ folder with ZAP generated content (project-chip#4010) * Fix segmentation fault error in response echo message (project-chip#3984) * Add back Android default build coverage & fix the build (project-chip#3966) mDNS doesn't build with no device layer. Remove it from the build and add back the coverage that would catch this that was lost in project-chip#3340. * Update all-clusters-app gen/ folder with ZAP generated content (project-chip#3963) * Move src/inet/tests to auto-test-driver generation (project-chip#3997) * Rename TestUtils to UnitTestRegistration. (project-chip#4021) Looking to remove usage of 'Utils' unless we have really no choice. 'UnitTestRegistration' seems clearer in what it does compared to 'TestUtils'. * Update src/lib/core/tests to auto-test-driver generation (project-chip#3991) * Cleanup zap chip-helper.js (project-chip#3973) * Cleanup zap chip-helper.js * Restyled by clang-format Co-authored-by: Restyled.io <commits@restyled.io> * Move src/transport/tests to auto-test-driver generation (project-chip#3999) * Move src/transport/tests to auto-test-driver generation * Add relevant libraries (and more test-capable libs) to nrf. * Refactor inet test helpers (to not include actual inet tests), try to make qemu allow better linkage but still failed for transport tests so disabled for now * Added more tests on esp32 qemu * Restyle fixes * Fix cast errors in InetCommon * Disable raw tests from zephyr: somehow they fail running out of endpoints * Disable DNS test on zephyr * Remove inet endpoint test from zephyr * Remove inet endpoint test from zephyr - fix again * Modify gitignore * Restyle fixes * Use CHIPDeviceController instead of CHIPDeviceController_deprecated (project-chip#3979) * Implement the missing part of Exchange Header in Transport layer (project-chip#4017) * Implement the missing part of Exchange Header in Transport layer * Revert comment 'if' back to 'iff'("if and only if") * Remove duplicated send flag defines and put ExchangeMgr/ExchangeConte… (project-chip#3994) * Remove duplicated send flag defines and put ExchangeMgr/ExchangeContext under the same namespace as CRMP * Rename kSendFlag_Default to kSendFlag_None * Move src/lib/asn1/tests and src/ble/tests to auto-test-driver generation (project-chip#3998) * Move src/lib/asn1/tests and src/ble/tests to auto-test-driver generation * Remove one more unused file * Attempt to enable asn1 and ble tests in esp32 - see if they pass or not * Fix merge error * Update include header for ASN1 test * Include ASN1 in libCHIP * Some conversions to use PacketBufferHandle (project-chip#4011) * Some conversions to use PacketBufferHandle #### Problem Code should use `PacketBufferHandle` rather than `PacketBuffer *`. #### Summary of Changes - Converts remaining receive path in //src/inet and //src/transport. - Converts most of //src/ble. - Introduces Handle versions of the `AddToEnd`/`DetachTail` pair. Part of issue project-chip#2707 - Figure out a way to express PacketBuffer ownership in the type system * Restyled by clang-format * review * revive BtpEngine::Clear[TR]xPacket() * simplify conditional * (void) message.DetachTail() → message.FreeHead() * remove ExchangeContext::kSendFlag_RetainBuffer * missed pBuf.IsNull() * DetachHead() → PopTail() * typos Co-authored-by: Restyled.io <commits@restyled.io> * Move src/system/tests to auto-test-driver generation (project-chip#4000) * Move src/system/tests to auto-test-driver generation * Remove a TCP/IP init call that was killing qemu * Remove explicit "all" target from root build file (project-chip#3967) The "all" target exists implicitly and contains everything. We don't need to specify one, and it's misleading to do so specifying deps has no effect. * Make src/setup_payload compile with -Werror=conversion (project-chip#4032) * Add SSID and password to chip-tool pairing (project-chip#4054) * Get temperature-measurement and all-clusters-app to use examples/common/chip-app-server (project-chip#4039) #### Problem PR project-chip#3704 introduced a change where a `PeerAddress` is now required in order to start `RendezvousSession`. Sadly the multiple code paths bootstrapping `RendezvousSession` has not been updated. PR project-chip#4012 add a fix for some of the `examples/` but not for the `all-clusters-app` nor the `temperature-measurement-app`. To avoid such situation, this PR merge `examples/common/chip-app-server` and the custom code from `all-clusters-app` and `temperature-measurement-app`. One of the more discutable change of this PR (imo) is the code that moves the custom `Echo` mechanism from the `all-clusters-app` to `chip-app-server`. I was hoping to get rid of it before doing this change but the `all-clusters-app` and the `temperature-measurement-app` are broken since project-chip#3704 and this PR should fix that. Also I have a PR (mostly) ready once project-chip#3979 lands to get rid of this `Echo` specific code and replace it by a manufacturer specific `ping` command. #### Summary of Changes * Remove `EchoServer.cpp`, `RendezvousDeviceDelegate.cpp` and `include/RendezvousDeviceDelegate.h` from `all-clusters-app` * Remove `ResponseServer.cpp`, `RendezvousDeviceDelegate.cpp` and `include/RendezvousDeviceDelegate.h` from `temperature-measurement-app` * Introduce `chip-app-server/include/AppDelegate.h` in order to keep the behavior the `all-clusters-app` that turns on/off leds on different events. Maybe it should be converted to some types of `ChipDeviceEvent` or `CHIPCallback` at some point. * Fix `chip-app-server` to accomodate for the specifics of `all-clusters-app` * Add all Thread ULA addresses to the lwip interface (project-chip#4053) ULA prefixes will used for CHIP network so we need to add all these addresses to the interface. * Remove src/lib/message. (project-chip#4055) * Remove src/lib/message. Updated messaging implementation lives in src/messaging and the src/lib/message is not currently compiled or linked in. This saves us from looking at this code when some refactoring is needed (e.g. the SystemPacketBuffer changes). * Remove one more unused file * [ChipTool] Add Payload Parse Command (project-chip#3696) * [ChipTool] Add Payload Parse Command * [ChipTool] Add Payload Parse Command * [ChipTool] Restyle issues resolved * Restyled by whitespace * Resolve build errors caused by Command.h schema change Co-authored-by: lijayaku <lijayaku@amazon.com> Co-authored-by: Restyled.io <commits@restyled.io> * rename ParseCommand to QRCodeParseCommand * adding AdditionalDataParseCommand version 0 * Adding parsing logic + logging * adding another parsing method * Basic Parsing is DONE * fixing memory issue * removing some logs * removing more logs * minor update * Add RotatingDeviceId to DNS-SD * Revert "Merge pull request #4 from hnnajh/rotating-id-test" This reverts commit 0235d05, reversing changes made to 3e1a4b9. * Storing RI in Octet String + Adding Binary format for BLE * Fixing rotating id parser + adding unittests * restyling * refactoring rotating id unit tests * Added more unit tests for Rotating Device Id * updated styling * refactor RI tests * Added RI Unittest + more validation * applying restyling * Fix CI * update styling * Fix CI * Update Styling * Fix CI * Restyling * Fixing nits * Using MutableByteSpan in RI generation * Fixing nits Co-authored-by: Kevin Schoedel <67607049+kpschoedel@users.noreply.github.com> Co-authored-by: Vivien Nicolas <vnicolas@apple.com> Co-authored-by: Markus Becker <Markus.Becker@tridonic.com> Co-authored-by: Damian Królik <66667989+Damian-Nordic@users.noreply.github.com> Co-authored-by: Łukasz Duda <lukasz.duda@nordicsemi.no> Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Yufeng Wang <44623591+yufengwangca@users.noreply.github.com> Co-authored-by: Michael Spang <spang@google.com> Co-authored-by: Andrei Litvin <andrei@andy314.com> Co-authored-by: jepenven-silabs <67962328+jepenven-silabs@users.noreply.github.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Jiacheng Guo <gjc@google.com> Co-authored-by: Liju Jayakumar <26148162+lijujayakumar@users.noreply.github.com> Co-authored-by: lijayaku <lijayaku@amazon.com>
mleisner · Sep 17, 2021 · c535a6c · c535a6c
1 parent 1a12da7
commit c535a6c
Show file tree

Hide file tree

Showing 8 changed files with 383 additions and 48 deletions.
diff --git a/examples/chip-tool/commands/payload/AdditionalDataParseCommand.cpp b/examples/chip-tool/commands/payload/AdditionalDataParseCommand.cpp
@@ -17,6 +17,7 @@
  */
 
 #include "AdditionalDataParseCommand.h"
+#include <math.h>
 #include <setup_payload/AdditionalDataPayload.h>
 #include <setup_payload/AdditionalDataPayloadParser.h>
 #include <string>
@@ -26,22 +27,20 @@ using namespace ::chip::SetupPayloadData;
 
 CHIP_ERROR AdditionalDataParseCommand::Run()
 {
-    std::vector<uint8_t> payloadData;
     AdditionalDataPayload resultPayload;
     CHIP_ERROR err = CHIP_NO_ERROR;
-    std::string payloadString(mPayload);
 
-    // Decode input payload
-    size_t len = payloadString.length();
-
-    for (size_t i = 0; i < len; i += 2)
+    if (strlen(mPayload) % 2 != 0)
     {
-        auto str  = payloadString.substr(i, 2);
-        uint8_t x = (uint8_t) stoi(str, 0, 16);
-        payloadData.push_back(x);
+        return CHIP_ERROR_INVALID_STRING_LENGTH;
     }
+    size_t additionalDataPayloadBytesLength = strlen(mPayload) / 2;
+    std::unique_ptr<uint8_t[]> additionalDataPayloadBytes(new uint8_t[additionalDataPayloadBytesLength]);
+
+    size_t bufferSize =
+        chip::Encoding::HexToBytes(mPayload, strlen(mPayload), additionalDataPayloadBytes.get(), additionalDataPayloadBytesLength);
 
-    err = AdditionalDataPayloadParser(payloadData.data(), (uint32_t) payloadData.size()).populatePayload(resultPayload);
+    err = AdditionalDataPayloadParser(additionalDataPayloadBytes.get(), bufferSize).populatePayload(resultPayload);
     SuccessOrExit(err);
 
     ChipLogProgress(chipTool, "AdditionalDataParseCommand, RotatingDeviceId=%s", resultPayload.rotatingDeviceId.c_str());

diff --git a/src/app/server/Mdns.cpp b/src/app/server/Mdns.cpp
@@ -29,7 +29,9 @@
 #include <platform/ConfigurationManager.h>
 #include <platform/KeyValueStoreManager.h>
 #include <protocols/secure_channel/PASESession.h>
+#if CHIP_ENABLE_ROTATING_DEVICE_ID
 #include <setup_payload/AdditionalDataPayloadGenerator.h>
+#endif
 #include <system/TimeSource.h>
 #include <transport/FabricTable.h>
 
@@ -475,9 +477,9 @@ CHIP_ERROR MdnsServer::GenerateRotatingDeviceId(char rotatingDeviceIdHexBuffer[]
     ReturnErrorOnFailure(
         chip::DeviceLayer::ConfigurationMgr().GetSerialNumber(serialNumber, sizeof(serialNumber), serialNumberSize));
     ReturnErrorOnFailure(chip::DeviceLayer::ConfigurationMgr().GetLifetimeCounter(lifetimeCounter));
-    return AdditionalDataPayloadGenerator().generateRotatingDeviceId(lifetimeCounter, serialNumber, serialNumberSize,
-                                                                     rotatingDeviceIdHexBuffer, rotatingDeviceIdHexBufferSize,
-                                                                     rotatingDeviceIdValueOutputSize);
+    return AdditionalDataPayloadGenerator().generateRotatingDeviceIdAsHexString(
+        lifetimeCounter, serialNumber, serialNumberSize, rotatingDeviceIdHexBuffer, rotatingDeviceIdHexBufferSize,
+        rotatingDeviceIdValueOutputSize);
 }
 #endif
 

diff --git a/src/setup_payload/AdditionalDataPayloadGenerator.cpp b/src/setup_payload/AdditionalDataPayloadGenerator.cpp
@@ -51,8 +51,6 @@ AdditionalDataPayloadGenerator::generateAdditionalDataPayload(uint16_t lifetimeC
 {
     System::PacketBufferTLVWriter writer;
     TLVWriter innerWriter;
-    char rotatingDeviceIdBuffer[RotatingDeviceId::kHexMaxLength];
-    size_t rotatingDeviceIdBufferSize = 0;
 
     // Initialize TLVWriter
     writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
@@ -61,34 +59,36 @@ AdditionalDataPayloadGenerator::generateAdditionalDataPayload(uint16_t lifetimeC
 
     if (additionalDataFields.Has(AdditionalDataFields::RotatingDeviceId))
     {
-        // Generating Device Rotating Id
-        ReturnErrorOnFailure(generateRotatingDeviceId(lifetimeCounter, serialNumberBuffer, serialNumberBufferSize,
-                                                      rotatingDeviceIdBuffer, ArraySize(rotatingDeviceIdBuffer),
-                                                      rotatingDeviceIdBufferSize));
+        uint8_t rotatingDeviceIdInternalBuffer[RotatingDeviceId::kMaxLength];
+        MutableByteSpan rotatingDeviceIdBuffer = MutableByteSpan(rotatingDeviceIdInternalBuffer);
 
+        // Generating Device Rotating Id
+        ReturnErrorOnFailure(generateRotatingDeviceIdAsBinary(lifetimeCounter, serialNumberBuffer, serialNumberBufferSize,
+                                                              rotatingDeviceIdBuffer));
         // Adding the rotating device id to the TLV data
-        ReturnErrorOnFailure(innerWriter.PutString(ContextTag(kRotatingDeviceIdTag), rotatingDeviceIdBuffer,
-                                                   static_cast<uint32_t>(rotatingDeviceIdBufferSize)));
+        ReturnErrorOnFailure(innerWriter.PutBytes(ContextTag(kRotatingDeviceIdTag),
+                                                  rotatingDeviceIdBuffer.data(),
+                                                  static_cast<uint32_t>(rotatingDeviceIdBuffer.size())));
     }
 
     ReturnErrorOnFailure(writer.CloseContainer(innerWriter));
 
     return writer.Finalize(&bufferHandle);
 }
 
-CHIP_ERROR AdditionalDataPayloadGenerator::generateRotatingDeviceId(uint16_t lifetimeCounter, const char * serialNumberBuffer,
-                                                                    size_t serialNumberBufferSize, char rotatingDeviceIdBuffer[],
-                                                                    size_t rotatingDeviceIdBufferSize,
-                                                                    size_t & rotatingDeviceIdValueOutputSize)
+CHIP_ERROR AdditionalDataPayloadGenerator::generateRotatingDeviceIdAsBinary(
+    uint16_t lifetimeCounter, const char * serialNumberBuffer, size_t serialNumberBufferSize, MutableByteSpan & rotatingDeviceIdBuffer)
 {
-    uint8_t outputBuffer[RotatingDeviceId::kMaxLength];
     uint8_t hashOutputBuffer[kSHA256_Hash_Length];
-    BufferWriter outputBufferWriter(outputBuffer, sizeof(outputBuffer));
+    BufferWriter outputBufferWriter(rotatingDeviceIdBuffer);
     uint8_t lifetimeCounterBuffer[2];
 
-    Put16(lifetimeCounterBuffer, lifetimeCounter);
+    if (serialNumberBuffer == nullptr)
+    {
+        return CHIP_ERROR_INVALID_ARGUMENT;
+    }
 
-    VerifyOrReturnError(rotatingDeviceIdBufferSize >= RotatingDeviceId::kHexMaxLength, CHIP_ERROR_BUFFER_TOO_SMALL);
+    Put16(lifetimeCounterBuffer, lifetimeCounter);
 
     // Computing the Rotating Device Id
     // RDI = Lifetime_Counter + SuffixBytes(SHA256(Serial_Number + Lifetime_Counter), 16)
@@ -103,11 +103,23 @@ CHIP_ERROR AdditionalDataPayloadGenerator::generateRotatingDeviceId(uint16_t lif
     outputBufferWriter.Put16(lifetimeCounter);
     outputBufferWriter.Put(&hashOutputBuffer[kSHA256_Hash_Length - RotatingDeviceId::kHashSuffixLength],
                            RotatingDeviceId::kHashSuffixLength);
+    VerifyOrReturnError(outputBufferWriter.Fit(), CHIP_ERROR_BUFFER_TOO_SMALL);
+    rotatingDeviceIdBuffer.reduce_size(outputBufferWriter.Needed());
+    return CHIP_NO_ERROR;
+}
 
-    ReturnErrorOnFailure(
-        BytesToUppercaseHexString(outputBuffer, outputBufferWriter.Needed(), rotatingDeviceIdBuffer, rotatingDeviceIdBufferSize));
-    rotatingDeviceIdValueOutputSize = outputBufferWriter.Needed() * 2;
-    ChipLogDetail(DeviceLayer, "rotatingDeviceId: %s", rotatingDeviceIdBuffer);
+CHIP_ERROR AdditionalDataPayloadGenerator::generateRotatingDeviceIdAsHexString(
+    uint16_t lifetimeCounter, const char * serialNumberBuffer, size_t serialNumberBufferSize, char * rotatingDeviceIdBuffer,
+    size_t rotatingDeviceIdBufferSize, size_t & rotatingDeviceIdValueOutputSize)
+{
+    uint8_t rotatingDeviceIdInternalBuffer[RotatingDeviceId::kMaxLength];
+    MutableByteSpan rotatingDeviceIdBufferTemp = MutableByteSpan(rotatingDeviceIdInternalBuffer);
+    ReturnErrorOnFailure(generateRotatingDeviceIdAsBinary(lifetimeCounter, serialNumberBuffer, serialNumberBufferSize,
+                                                          rotatingDeviceIdBufferTemp));
 
+    VerifyOrReturnError(rotatingDeviceIdBufferSize >= RotatingDeviceId::kHexMaxLength, CHIP_ERROR_BUFFER_TOO_SMALL);
+    ReturnErrorOnFailure(BytesToUppercaseHexString(rotatingDeviceIdBufferTemp.data(), rotatingDeviceIdBufferTemp.size(),
+                                                   rotatingDeviceIdBuffer, rotatingDeviceIdBufferSize));
+    rotatingDeviceIdValueOutputSize = rotatingDeviceIdBufferTemp.size() * 2;
     return CHIP_NO_ERROR;
 }
diff --git a/src/setup_payload/AdditionalDataPayloadGenerator.h b/src/setup_payload/AdditionalDataPayloadGenerator.h
@@ -72,23 +72,37 @@ class AdditionalDataPayloadGenerator
     CHIP_ERROR generateAdditionalDataPayload(uint16_t lifetimeCounter, const char * serialNumberBuffer,
                                              size_t serialNumberBufferSize, chip::System::PacketBufferHandle & bufferHandle,
                                              BitFlags<AdditionalDataFields> additionalDataFields);
-    // Generate Device Rotating ID
     /**
-     * Generate additional data payload (i.e. TLV encoded).
+     * Generate Rotating Device ID in Binary Format
+     *
+     * @param lifetimeCounter lifetime counter
+     * @param serialNumberBuffer null-terminated serial number buffer
+     * @param serialNumberBufferSize size of the serial number buffer supplied.
+     * @param rotatingDeviceIdBuffer rotating device id mutable byte span, it will be resized to the actual size used upon successful generation
+     *
+     * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise.
+     *
+     */
+    CHIP_ERROR generateRotatingDeviceIdAsBinary(uint16_t lifetimeCounter, const char * serialNumberBuffer,
+                                                size_t serialNumberBufferSize, MutableByteSpan & rotatingDeviceIdBuffer);
+
+    /**
+     * Generate Device Rotating ID in String Format
      *
      * @param lifetimeCounter lifetime counter
      * @param serialNumberBuffer null-terminated serial number buffer
      * @param serialNumberBufferSize size of the serial number buffer supplied.
      * @param rotatingDeviceIdBuffer rotating device id buffer
      * @param rotatingDeviceIdBufferSize the current size of the supplied buffer
      * @param rotatingDeviceIdValueOutputSize the number of chars making up the actual value of the returned rotating device id
+     * excluding the null terminator
      *
      * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise.
      *
      */
-    CHIP_ERROR generateRotatingDeviceId(uint16_t lifetimeCounter, const char * serialNumberBuffer, size_t serialNumberBufferSize,
-                                        char * rotatingDeviceIdBuffer, size_t rotatingDeviceIdBufferSize,
-                                        size_t & rotatingDeviceIdValueOutputSize);
+    CHIP_ERROR generateRotatingDeviceIdAsHexString(uint16_t lifetimeCounter, const char * serialNumberBuffer,
+                                                   size_t serialNumberBufferSize, char * rotatingDeviceIdBuffer,
+                                                   size_t rotatingDeviceIdBufferSize, size_t & rotatingDeviceIdValueOutputSize);
 };
 
 } // namespace chip
diff --git a/src/setup_payload/AdditionalDataPayloadParser.cpp b/src/setup_payload/AdditionalDataPayloadParser.cpp
@@ -30,6 +30,7 @@
 #include <lib/core/CHIPError.h>
 #include <lib/core/CHIPTLVData.hpp>
 #include <lib/core/CHIPTLVUtilities.hpp>
+#include <lib/support/BytesToHex.h>
 #include <lib/support/CodeUtils.h>
 #include <protocols/Protocols.h>
 #include <setup_payload/AdditionalDataPayloadGenerator.h>
@@ -46,16 +47,23 @@ CHIP_ERROR AdditionalDataPayloadParser::populatePayload(SetupPayloadData::Additi
 
     // Open the container
     ReturnErrorOnFailure(reader.OpenContainer(innerReader));
+    if (innerReader.Next(TLV::kTLVType_ByteString, TLV::ContextTag(SetupPayloadData::kRotatingDeviceIdTag)) == CHIP_NO_ERROR)
+    {
+        // Get the value of the rotating device id
+        ByteSpan rotatingDeviceId;
+        ReturnErrorOnFailure(innerReader.GetByteView(rotatingDeviceId));
 
-    ReturnErrorOnFailure(innerReader.Next(TLV::kTLVType_UTF8String, TLV::ContextTag(SetupPayloadData::kRotatingDeviceIdTag)));
+        VerifyOrReturnError(rotatingDeviceId.size() == RotatingDeviceId::kMaxLength, CHIP_ERROR_INVALID_STRING_LENGTH);
+        char rotatingDeviceIdBufferTemp[RotatingDeviceId::kHexMaxLength];
 
-    // Get the value of the rotating device id
-    Span<const char> rotatingDeviceId;
-    ReturnErrorOnFailure(innerReader.GetStringView(rotatingDeviceId));
-
-    // This test uses <, not <=, because kHexMaxLength includes the null-terminator.
-    VerifyOrReturnError(rotatingDeviceId.size() < RotatingDeviceId::kHexMaxLength, CHIP_ERROR_INVALID_STRING_LENGTH);
-    outPayload.rotatingDeviceId = std::string(rotatingDeviceId.data(), rotatingDeviceId.size());
+        ReturnErrorOnFailure(Encoding::BytesToUppercaseHexString(rotatingDeviceId.data(), rotatingDeviceId.size(),
+                                                                 rotatingDeviceIdBufferTemp, RotatingDeviceId::kHexMaxLength));
+        outPayload.rotatingDeviceId = std::string(rotatingDeviceIdBufferTemp, RotatingDeviceId::kHexMaxLength);
+    }
+    else
+    {
+        outPayload.rotatingDeviceId = "";
+    }
 
     // Verify the end of the container
     ReturnErrorOnFailure(reader.VerifyEndOfContainer());

diff --git a/src/setup_payload/AdditionalDataPayloadParser.h b/src/setup_payload/AdditionalDataPayloadParser.h
@@ -38,7 +38,7 @@ class AdditionalDataPayloadParser
 {
 private:
     const uint8_t * mPayloadBufferData;
-    const uint32_t mPayloadBufferLength;
+    const size_t mPayloadBufferLength;
 
 public:
     /**
@@ -49,7 +49,7 @@ class AdditionalDataPayloadParser
      *                                     it needs to outlive the lifetime of this parse.
      * @param[in] payloadBufferLength      The buffer data length for the additional data payload.
      */
-    AdditionalDataPayloadParser(const uint8_t * payloadBufferData, const uint32_t payloadBufferLength) :
+    AdditionalDataPayloadParser(const uint8_t * payloadBufferData, const size_t payloadBufferLength) :
         mPayloadBufferData(payloadBufferData), mPayloadBufferLength(payloadBufferLength)
     {}
 

diff --git a/src/setup_payload/tests/BUILD.gn b/src/setup_payload/tests/BUILD.gn
@@ -22,6 +22,7 @@ chip_test_suite("tests") {
   output_name = "libSetupPayloadTests"
 
   test_sources = [
+    "TestAdditionalDataPayload.cpp",
     "TestManualCode.cpp",
     "TestQRCode.cpp",
     "TestQRCodeTLV.cpp",