Fix leftover from project-chip#8236 and make BufferReader/Writer supp…

…ort Span (project-chip#8408)

* Fix leftover from project-chip#8236 and make BufferReader/Writer support Span

- Fix leftover nits from @bzbarsky-apple's review of project-chip#8236
- In order to add span support cleanly, added Span support to
  Reader and BufferWriter, and fixed all necessary breakage.

Testing done: pass all unit tests and CASE cert tests

* Fix mbedTLS usage of EcdsaAsn1SignatureToRaw

* Apply review comments from @bzbarsky-apple

* Use some span reassign instead of out_size ref

* Restyled by clang-format

* Remove unnecessary nullptr checks handled by Span::size()

* Improve IntegerToDer test coverage

- Assign to output spans
- Use new span function for validity checks (`is_span_usable`)
- Replace an untested CHIPCert.cpp usage with tested version

* Add is_span_usable() tests

* Restyled by clang-format

* Grammar fix to kick CI

* Commit forgotten removal of obsolete ConvertIntegerRawToDER from CHIPCert.h

* Apply review comments from @mspang

* Fix clang

Co-authored-by: Restyled.io <commits@restyled.io>

src/app/decoder.cpp

@@ -53,7 +53,7 @@ uint16_t extractApsFrame(uint8_t * buffer, uint16_t buf_length, EmberApsFrame *
                                .ReadOctet(&outApsFrame->radius)
                                .StatusCode();
 
-    return err == CHIP_NO_ERROR ? reader.OctetsRead() : 0;
+    return err == CHIP_NO_ERROR ? static_cast<uint16_t>(reader.OctetsRead()) : 0;
 }
 
 void printApsFrame(EmberApsFrame * frame)

src/app/message-reader.h

@@ -51,7 +51,7 @@ class DataModelReader
      * we do less switching back and forth between DataModelReader and raw
      * buffers.
      */
-    uint16_t OctetsRead() const { return mReader.OctetsRead(); }
+    size_t OctetsRead() const { return mReader.OctetsRead(); }
 
     /**
      * The reader status.

src/app/util/attribute-list-byte-span.cpp

@@ -83,7 +83,7 @@ uint16_t GetByteSpanOffsetFromIndex(const uint8_t * buffer, uint16_t bufferLen,
         reader.Skip(entrySize);
     }
 
-    return reader.OctetsRead();
+    return static_cast<uint16_t>(reader.OctetsRead());
 }
 
 } // namespace List

src/ble/BtpEngine.cpp

@@ -292,7 +292,7 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
     data->SetDataLength(chip::min(data->DataLength(), mRxFragmentSize));
 
     // Now mark the bytes we consumed as consumed.
-    data->ConsumeHead(reader.OctetsRead());
+    data->ConsumeHead(static_cast<uint16_t>(reader.OctetsRead()));
 
     ChipLogDebugBtpEngine(Ble, ">>> BTP reassembler received data:");
     PrintBufDebug(data);
@@ -312,7 +312,7 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
 
         mRxState = kState_InProgress;
 
-        data->ConsumeHead(startReader.OctetsRead());
+        data->ConsumeHead(static_cast<uint16_t>(startReader.OctetsRead()));
 
         // Create a new buffer for use as the Rx re-assembly area.
         mRxBuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

src/credentials/CHIPCert.cpp

@@ -838,41 +838,6 @@ CHIP_ERROR ConvertIntegerDERToRaw(ByteSpan derInt, uint8_t * rawInt, const uint1
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR ConvertIntegerRawToDER(P256IntegerSpan rawInt, uint8_t * derInt, const uint16_t derIntBufSize, uint16_t & derIntLen)
-{
-    static_assert(rawInt.size() <= UINT16_MAX - 1, "P256 raw integer doesn't fit in a uint16_t");
-
-    VerifyOrReturnError(!rawInt.empty(), CHIP_ERROR_INVALID_ARGUMENT);
-    VerifyOrReturnError(derInt != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
-
-    const uint8_t * rawIntData = rawInt.data();
-    size_t rawIntLen           = rawInt.size();
-
-    while (*rawIntData == 0)
-    {
-        rawIntData++;
-        rawIntLen--;
-    }
-
-    if (*rawIntData & 0x80) /* Need Leading Zero */
-    {
-        VerifyOrReturnError(derIntBufSize >= rawIntLen + 1, CHIP_ERROR_BUFFER_TOO_SMALL);
-
-        *derInt++ = 0;
-        derIntLen = static_cast<uint16_t>(rawIntLen + 1);
-    }
-    else
-    {
-        VerifyOrReturnError(derIntBufSize >= rawIntLen, CHIP_ERROR_BUFFER_TOO_SMALL);
-
-        derIntLen = static_cast<uint16_t>(rawIntLen);
-    }
-
-    memcpy(derInt, rawIntData, rawIntLen);
-
-    return CHIP_NO_ERROR;
-}
-
 CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, uint8_t * derSig, const uint16_t derSigBufSize,
                                          uint16_t & derSigLen)
 {
@@ -892,22 +857,28 @@ CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, uint8_t
 CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, ASN1Writer & writer)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
-    uint8_t derInt[kP256_FE_Length + 1];
-    uint16_t derIntLen;
+    uint8_t derInt[kP256_FE_Length + kEmitDerIntegerWithoutTagOverhead];
 
     VerifyOrReturnError(!rawSig.empty(), CHIP_ERROR_INVALID_ARGUMENT);
 
     // Ecdsa-Sig-Value ::= SEQUENCE
     ASN1_START_SEQUENCE
     {
         // r INTEGER
-        ReturnErrorOnFailure(ConvertIntegerRawToDER(P256IntegerSpan(rawSig.data()), derInt, sizeof(derInt), derIntLen));
-        ReturnErrorOnFailure(writer.PutValue(kASN1TagClass_Universal, kASN1UniversalTag_Integer, false, derInt, derIntLen));
+        {
+            MutableByteSpan derIntSpan(derInt, sizeof(derInt));
+            ReturnErrorOnFailure(ConvertIntegerRawToDerWithoutTag(P256IntegerSpan(rawSig.data()), derIntSpan));
+            ReturnErrorOnFailure(writer.PutValue(kASN1TagClass_Universal, kASN1UniversalTag_Integer, false, derIntSpan.data(),
+                                                 static_cast<uint16_t>(derIntSpan.size())));
+        }
 
         // s INTEGER
-        ReturnErrorOnFailure(
-            ConvertIntegerRawToDER(P256IntegerSpan(rawSig.data() + kP256_FE_Length), derInt, sizeof(derInt), derIntLen));
-        ReturnErrorOnFailure(writer.PutValue(kASN1TagClass_Universal, kASN1UniversalTag_Integer, false, derInt, derIntLen));
+        {
+            MutableByteSpan derIntSpan(derInt, sizeof(derInt));
+            ReturnErrorOnFailure(ConvertIntegerRawToDerWithoutTag(P256IntegerSpan(rawSig.data() + kP256_FE_Length), derIntSpan));
+            ReturnErrorOnFailure(writer.PutValue(kASN1TagClass_Universal, kASN1UniversalTag_Integer, false, derIntSpan.data(),
+                                                 static_cast<uint16_t>(derIntSpan.size())));
+        }
     }
     ASN1_END_SEQUENCE;
 

src/credentials/CHIPCert.h

@@ -813,18 +813,6 @@ inline bool IsChipDNAttr(chip::ASN1::OID oid)
  */
 CHIP_ERROR ConvertIntegerDERToRaw(ByteSpan derInt, uint8_t * rawInt, const uint16_t rawIntLen);
 
-/**
- * @brief Convert a raw integer in big-endian form to an ASN.1 DER encoded integer.
- *
- * @param rawInt        P256 integer in raw form.
- * @param derInt        Buffer to store converted ASN.1 DER encoded integer.
- * @param derIntBufSize The size of the buffer to store ASN.1 DER encoded integer.
- * @param derIntLen     The length of the ASN.1 DER encoded integer.
- *
- * @retval  #CHIP_NO_ERROR  If the integer value was successfully converted.
- */
-CHIP_ERROR ConvertIntegerRawToDER(P256IntegerSpan rawInt, uint8_t * derInt, const uint16_t derIntBufSize, uint16_t & derIntLen);
-
 /**
  * @brief Convert a raw CHIP signature to an ASN.1 DER encoded signature structure.
  *