Make command encoder/decoder work regardless of hardware endianness

[bzbarsky-apple]

Problem

The code that #1609 adds to src/app/chip-zcl-zpro/command-encoder/ copies non-single-byte ints to/from network buffers basically by using memcpy. This will not work if the sender and recipient hardware have different endianness.

Proposed Solution

Pick an endianness (little-endian, presumably, since that's what ZCL already does anyway as far as I can tell) and ensure that the wire format is that endianness, with byte-swapping during encode/decode as needed.

@bhaskar-apple @rwalker-apple

[jrhees-cae]

FYI - here are the changes I had made (rather rushed) to make the chip demo device run on a big-endian architecture:

diff --git a/src/app/plugin/codec-simple/codec-simple.c b/src/app/plugin/codec-simple/codec-simple.c
index 94230c8..c1488a6 100644
--- a/src/app/plugin/codec-simple/codec-simple.c
+++ b/src/app/plugin/codec-simple/codec-simple.c
@@ -26,6 +26,7 @@
 
 #include <memory.h>
 #include <stdint.h>
+#include <nlbyteorder.h>
 
 /**
  * @brief Starts the encoding process. if there is any kind of preamble of anything, this function is responsible for putting it
@@ -76,12 +77,26 @@ ChipZclStatus_t chipZclCodecEncode(ChipZclCodec_t * me, ChipZclType_t type, void
     switch (type)
     {
     case CHIP_ZCL_STRUCT_TYPE_INTEGER:
-        return put(me, ptr, ptrLen);
+        if (ptrLen == 2)
+        {
+            uint16_t val16 = nlByteOrderSwap16HostToLittle(*((uint16_t *)ptr));
+            return put(me, &val16, ptrLen);
+        }
+        else if (ptrLen == 4)
+        {
+            uint32_t val32 = nlByteOrderSwap32HostToLittle(*((uint32_t *)ptr));
+            return put(me, &val32, ptrLen);
+        }
+        else
+        {
+            return put(me, ptr, ptrLen);
+        }
 
     case CHIP_ZCL_STRUCT_TYPE_STRING:
         // In this simple case, length is encoded as the first 2 bytes.
         {
-            ChipZclStatus_t status = put(me, &ptrLen, 2);
+            uint16_t val16 = nlByteOrderSwap16HostToLittle(ptrLen);
+            ChipZclStatus_t status = put(me, &val16, 2);
 
             if (CHIP_ZCL_STATUS_SUCCESS != status)
             {
@@ -120,6 +135,7 @@ ChipZclStatus_t chipZclCodecDecodeStart(ChipZclCodec_t * me, ChipZclBuffer_t * b
  */
 ChipZclStatus_t chipZclCodecDecode(ChipZclCodec_t * me, ChipZclType_t type, void * ptr, uint16_t ptrLen, uint16_t * retLen)
 {
+    ChipZclStatus_t status;
     uint16_t dummy;
     if (NULL == retLen)
     {
@@ -130,13 +146,23 @@ ChipZclStatus_t chipZclCodecDecode(ChipZclCodec_t * me, ChipZclType_t type, void
     {
     case CHIP_ZCL_STRUCT_TYPE_INTEGER:
         *retLen = ptrLen;
-        return get(me, ptr, *retLen);
+        status = get(me, ptr, *retLen);
+        if (status == CHIP_ZCL_STATUS_SUCCESS)
+        {
+            if (ptrLen == 2)
+            {
+                *((uint16_t *)ptr) = nlByteOrderSwap16LittleToHost(*((uint16_t *)ptr));
+            }
+            else if (ptrLen == 4)
+            {
+                *((uint32_t *)ptr) = nlByteOrderSwap32LittleToHost(*((uint32_t *)ptr));
+            }
+        }
+        return status;
 
     case CHIP_ZCL_STRUCT_TYPE_STRING:
         // gotta read length first
         {
-            ChipZclStatus_t status;
-
             *retLen = 0;
             status  = get(me, retLen, 2);
 
@@ -144,6 +170,7 @@ ChipZclStatus_t chipZclCodecDecode(ChipZclCodec_t * me, ChipZclType_t type, void
             {
                 return status;
             }
+            *retLen = nlByteOrderSwap16LittleToHost(*retLen);
         }
         if (*retLen > ptrLen)
         {

[jrhees-cae]

diff --git a/src/transport/SecureSession.cpp b/src/transport/SecureSession.cpp
index ebb69f5..8a74516 100644
--- a/src/transport/SecureSession.cpp
+++ b/src/transport/SecureSession.cpp
@@ -24,6 +24,7 @@
 
 #include <core/CHIPEncoding.h>
 #include <crypto/CHIPCryptoPAL.h>
+#include <nlbyteorder.hpp>
 #include <support/CodeUtils.h>
 #include <transport/SecureSession.h>
 
@@ -98,7 +99,7 @@ CHIP_ERROR SecureSession::Encrypt(const unsigned char * input, size_t input_leng
 {
     CHIP_ERROR error = CHIP_NO_ERROR;
     uint64_t tag     = 0;
-    uint64_t IV      = SecureSession::GetIV(header);
+    uint64_t IV      = nlByteOrderSwap64HostToLittle(SecureSession::GetIV(header));
 
     VerifyOrExit(mKeyAvailable, error = CHIP_ERROR_INVALID_USE_OF_SESSION_KEY);
     VerifyOrExit(input != NULL, error = CHIP_ERROR_INVALID_ARGUMENT);
@@ -109,7 +110,7 @@ CHIP_ERROR SecureSession::Encrypt(const unsigned char * input, size_t input_leng
                             (unsigned char *) &tag, sizeof(tag));
     SuccessOrExit(error);
 
-    header.SetTag(tag);
+    header.SetTag(nlByteOrderSwap64LittleToHost(tag));
 
 exit:
     return error;
@@ -119,16 +120,18 @@ CHIP_ERROR SecureSession::Decrypt(const unsigned char * input, size_t input_leng
                                   const MessageHeader & header)
 {
     CHIP_ERROR error = CHIP_NO_ERROR;
-    uint64_t tag     = header.GetTag();
-    uint64_t IV      = SecureSession::GetIV(header);
+    uint64_t tag     = nlByteOrderSwap64HostToLittle(header.GetTag());
+    uint64_t IV      = nlByteOrderSwap64LittleToHost(SecureSession::GetIV(header));
 
     VerifyOrExit(mKeyAvailable, error = CHIP_ERROR_INVALID_USE_OF_SESSION_KEY);

[rwalker-apple]

@pan-apple ^^^ GetIV(), etc. on SecureSession should be doing that "wire to host" format stuff.

[pan-apple]

@pan-apple ^^^ GetIV(), etc. on SecureSession should be doing that "wire to host" format stuff.

Yes, it's using PutLE64, and PutLE32 to get it in the correct order.
Also, we should not be changing the byte ordering for Tag. It happens to fit in a uint64_t. But we are using it as a byte buffer, and not as a number. Changing the byte ordering will cause decrypt to fail.

I suspect the patch above is from an older code baseline, where we were still using an 8 byte IV. Now it's 12 bytes.

[rwalker-apple]

@pan-apple ^^^ GetIV(), etc. on SecureSession should be doing that "wire to host" format stuff.

Yes, it's using PutLE64, and PutLE32 to get it in the correct order.
Also, we should not be changing the byte ordering for Tag. It happens to fit in a uint64_t. But we are using it as a byte buffer, and not as a number. Changing the byte ordering will cause decrypt to fail.

I suspect the patch above is from an older code baseline, where we were still using an 8 byte IV. Now it's 12 bytes.

copy.