diff --git a/src/app/decoder.cpp b/src/app/decoder.cpp index 3fd6d414ce52ff..5f713842634691 100644 --- a/src/app/decoder.cpp +++ b/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(reader.OctetsRead()) : 0; } void printApsFrame(EmberApsFrame * frame) diff --git a/src/app/message-reader.h b/src/app/message-reader.h index 5f95c21ddb58dc..844d5883e41367 100644 --- a/src/app/message-reader.h +++ b/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. diff --git a/src/app/util/attribute-list-byte-span.cpp b/src/app/util/attribute-list-byte-span.cpp index 0bde62a70fd2ef..1249f27ed40201 100644 --- a/src/app/util/attribute-list-byte-span.cpp +++ b/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(reader.OctetsRead()); } } // namespace List diff --git a/src/ble/BtpEngine.cpp b/src/ble/BtpEngine.cpp index 921d4e2b4106c9..2c3b970c903728 100644 --- a/src/ble/BtpEngine.cpp +++ b/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(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(startReader.OctetsRead())); // Create a new buffer for use as the Rx re-assembly area. mRxBuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize); diff --git a/src/credentials/CHIPCert.cpp b/src/credentials/CHIPCert.cpp index 70dc24dd6cb20a..779863375e45b6 100644 --- a/src/credentials/CHIPCert.cpp +++ b/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(rawIntLen + 1); - } - else - { - VerifyOrReturnError(derIntBufSize >= rawIntLen, CHIP_ERROR_BUFFER_TOO_SMALL); - - derIntLen = static_cast(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,8 +857,7 @@ 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); @@ -901,13 +865,20 @@ CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, ASN1Writ 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(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(derIntSpan.size()))); + } } ASN1_END_SEQUENCE; diff --git a/src/credentials/CHIPCert.h b/src/credentials/CHIPCert.h index 0fb638afeab5bd..294d153a5185f7 100644 --- a/src/credentials/CHIPCert.h +++ b/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. * diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index c7203140d0f5e6..3542a7f1dd2010 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -25,7 +25,10 @@ #include #include #include +#include +using chip::ByteSpan; +using chip::MutableByteSpan; using chip::Encoding::BufferWriter; using chip::Encoding::LittleEndian::Reader; @@ -35,6 +38,12 @@ constexpr uint8_t kIntegerTag = 0x02u; constexpr uint8_t kSeqTag = 0x30u; constexpr size_t kMinSequenceOverhead = 1 /* tag */ + 1 /* length */ + 1 /* actual data or second length byte*/; +/** + * @brief Utility to read a length field after a tag in a DER-encoded stream. + * @param[in] reader Reader instance from which the input will be read + * @param[out] length Length of the following element read from the stream + * @return CHIP_ERROR_INVALID_ARGUMENT or CHIP_ERROR_BUFFER_TOO_SMALL on error, CHIP_NO_ERROR otherwise + */ CHIP_ERROR ReadDerLength(Reader & reader, uint8_t & length) { length = 0; @@ -64,7 +73,14 @@ CHIP_ERROR ReadDerLength(Reader & reader, uint8_t & length) } } -CHIP_ERROR ReadDerUnsignedIntegerIntoRaw(Reader & reader, uint8_t * raw_integer_out, size_t raw_integer_length) +/** + * @brief Utility to convert DER-encoded INTEGER into a raw integer buffer in big-endian order + * with leading zeroes if the output buffer is larger than needed. + * @param[in] reader Reader instance from which the input will be read + * @param[out] raw_integer_out Buffer to receive the DER-encoded integer + * @return CHIP_ERROR_INVALID_ARGUMENT or CHIP_ERROR_BUFFER_TOO_SMALL on error, CHIP_NO_ERROR otherwise + */ +CHIP_ERROR ReadDerUnsignedIntegerIntoRaw(Reader & reader, MutableByteSpan raw_integer_out) { uint8_t cur_byte = 0; @@ -78,13 +94,13 @@ CHIP_ERROR ReadDerUnsignedIntegerIntoRaw(Reader & reader, uint8_t * raw_integer_ ReturnErrorOnFailure(ReadDerLength(reader, integer_len)); // Clear the destination buffer, so we can blit the unsigned value into place - memset(raw_integer_out, 0, raw_integer_length); + memset(raw_integer_out.data(), 0, raw_integer_out.size()); // Check for pseudo-zero to mark unsigned value // This means we have too large an integer (should be at most 1 byte too large), it's invalid - ReturnErrorCodeIf(integer_len > (raw_integer_length + 1), CHIP_ERROR_INVALID_ARGUMENT); + ReturnErrorCodeIf(integer_len > (raw_integer_out.size() + 1), CHIP_ERROR_INVALID_ARGUMENT); - if (integer_len == (raw_integer_length + 1u)) + if (integer_len == (raw_integer_out.size() + 1u)) { // Means we had a 0x00 byte stuffed due to MSB being high in original integer ReturnErrorOnFailure(reader.Read8(&cur_byte).StatusCode()); @@ -97,20 +113,20 @@ CHIP_ERROR ReadDerUnsignedIntegerIntoRaw(Reader & reader, uint8_t * raw_integer_ // We now have the rest of the tag that is a "minimal length" unsigned integer. // Blit it at the correct offset, since the order we use is MSB first for // both ASN.1 and EC curve raw points. - size_t offset = raw_integer_length - integer_len; - return reader.ReadBytes(&raw_integer_out[offset], integer_len).StatusCode(); + size_t offset = raw_integer_out.size() - integer_len; + return reader.ReadBytes(raw_integer_out.data() + offset, integer_len).StatusCode(); } -size_t EmitDerIntegerFromRaw(const uint8_t * raw_integer, size_t raw_integer_length_bytes, uint8_t * out_der_integer, - size_t out_der_integer_size) +CHIP_ERROR ConvertIntegerRawToDerInternal(const ByteSpan & raw_integer, MutableByteSpan & out_der_integer, + bool include_tag_and_length) { - if ((raw_integer == nullptr) || (raw_integer_length_bytes == 0)) + if (!IsSpanUsable(raw_integer) || !IsSpanUsable(out_der_integer)) { - return 0; + return CHIP_ERROR_INVALID_ARGUMENT; } - Reader reader(raw_integer, static_cast(raw_integer_length_bytes)); - BufferWriter writer(out_der_integer, static_cast(out_der_integer_size)); + Reader reader(raw_integer); + BufferWriter writer(out_der_integer); bool needs_leading_zero_byte = false; @@ -127,32 +143,35 @@ size_t EmitDerIntegerFromRaw(const uint8_t * raw_integer, size_t raw_integer_len needs_leading_zero_byte = true; } - // The + 1 is to account for the last consummed byte of the loop to skip leading zeros + // The + 1 is to account for the last consumed byte of the loop to skip leading zeros size_t length = reader.Remaining() + 1 + (needs_leading_zero_byte ? 1 : 0); if (length > 127) { // We do not support length over more than 1 bytes. - return 0; + return CHIP_ERROR_INVALID_ARGUMENT; } - // Put INTEGER tag - writer.Put(kIntegerTag); + if (include_tag_and_length) + { + // Put INTEGER tag + writer.Put(kIntegerTag); - // Put length over 1 byte (i.e. MSB clear) - writer.Put(static_cast(length)); + // Put length over 1 byte (i.e. MSB clear) + writer.Put(static_cast(length)); + } - // If leading zero or no more bytes remaining, must ensure we start we at least a zero byte + // If leading zero or no more bytes remaining, must ensure we start with at least a zero byte if (needs_leading_zero_byte) { writer.Put(static_cast(0u)); } - // Put first consummed byte from last read iteration of leading zero suppression + // Put first consumed byte from last read iteration of leading zero suppression writer.Put(cur_byte); // Fill the rest from the input in order - while ((reader.Remaining() > 0) && (reader.Read8(&cur_byte).StatusCode() == CHIP_NO_ERROR)) + while (reader.Read8(&cur_byte).StatusCode() == CHIP_NO_ERROR) { // Emit all other bytes as-is writer.Put(cur_byte); @@ -161,11 +180,12 @@ size_t EmitDerIntegerFromRaw(const uint8_t * raw_integer, size_t raw_integer_len size_t actually_written = 0; if (!writer.Fit(actually_written)) { - // Somehow it was too big, return 0 for error. - return 0; + return CHIP_ERROR_BUFFER_TOO_SMALL; } - return actually_written; + out_der_integer = out_der_integer.SubSpan(0, actually_written); + + return CHIP_NO_ERROR; } } // namespace @@ -473,41 +493,53 @@ CHIP_ERROR Spake2p_P256_SHA256_HKDF_HMAC::KDF(const uint8_t * ikm, const size_t return CHIP_NO_ERROR; } -CHIP_ERROR EcdsaRawSignatureToAsn1(size_t fe_length_bytes, const uint8_t * raw_sig, size_t raw_sig_length, uint8_t * out_asn1_sig, - size_t out_asn1_sig_length, size_t & out_asn1_sig_actual_length) +CHIP_ERROR ConvertIntegerRawToDerWithoutTag(const ByteSpan & raw_integer, MutableByteSpan & out_der_integer) +{ + return ConvertIntegerRawToDerInternal(raw_integer, out_der_integer, /* include_tag_and_length = */ false); +} + +CHIP_ERROR ConvertIntegerRawToDer(const ByteSpan & raw_integer, MutableByteSpan & out_der_integer) +{ + return ConvertIntegerRawToDerInternal(raw_integer, out_der_integer, /* include_tag_and_length = */ true); +} + +CHIP_ERROR EcdsaRawSignatureToAsn1(size_t fe_length_bytes, const ByteSpan & raw_sig, MutableByteSpan & out_asn1_sig) { VerifyOrReturnError(fe_length_bytes > 0, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(raw_sig != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(raw_sig_length == (2u * fe_length_bytes), CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(out_asn1_sig != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(out_asn1_sig_length >= (raw_sig_length + kMax_ECDSA_X9Dot62_Asn1_Overhead), CHIP_ERROR_BUFFER_TOO_SMALL); + VerifyOrReturnError(raw_sig.size() == (2u * fe_length_bytes), CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(out_asn1_sig.size() >= (raw_sig.size() + kMax_ECDSA_X9Dot62_Asn1_Overhead), CHIP_ERROR_BUFFER_TOO_SMALL); // Write both R an S integers past the overhead, we will shift them back later if we only needed 2 size bytes. - uint8_t * cursor = &out_asn1_sig[kMinSequenceOverhead]; - size_t remaining = out_asn1_sig_length - kMinSequenceOverhead; + uint8_t * cursor = out_asn1_sig.data() + kMinSequenceOverhead; + size_t remaining = out_asn1_sig.size() - kMinSequenceOverhead; size_t integers_length = 0; // Write R (first `fe_length_bytes` block of raw signature) - size_t integer_length = EmitDerIntegerFromRaw(&raw_sig[0], fe_length_bytes, cursor, remaining); - VerifyOrReturnError(integer_length != 0, CHIP_ERROR_INTERNAL); - VerifyOrReturnError(integer_length < remaining, CHIP_ERROR_INTERNAL); - remaining -= integer_length; - integers_length += integer_length; - cursor += integer_length; + { + MutableByteSpan out_der_integer(cursor, remaining); + ReturnErrorOnFailure(ConvertIntegerRawToDer(raw_sig.SubSpan(0, fe_length_bytes), out_der_integer)); + VerifyOrReturnError(out_der_integer.size() <= remaining, CHIP_ERROR_INTERNAL); + + integers_length += out_der_integer.size(); + remaining -= out_der_integer.size(); + cursor += out_der_integer.size(); + } // Write S (second `fe_length_bytes` block of raw signature) - integer_length = EmitDerIntegerFromRaw(&raw_sig[fe_length_bytes], fe_length_bytes, cursor, remaining); - VerifyOrReturnError(integer_length != 0, CHIP_ERROR_INTERNAL); - VerifyOrReturnError(integer_length <= remaining, CHIP_ERROR_INTERNAL); - integers_length += integer_length; + { + MutableByteSpan out_der_integer(cursor, remaining); + ReturnErrorOnFailure(ConvertIntegerRawToDer(raw_sig.SubSpan(fe_length_bytes, fe_length_bytes), out_der_integer)); + VerifyOrReturnError(out_der_integer.size() <= remaining, CHIP_ERROR_INTERNAL); + integers_length += out_der_integer.size(); + } // We only support outputs that would use 1 or 2 bytes of DER length after the SEQUENCE tag VerifyOrReturnError(integers_length <= UINT8_MAX, CHIP_ERROR_INVALID_ARGUMENT); // We now know the length of both variable sized integers in the sequence, so we // can write the tag and length. - BufferWriter writer(out_asn1_sig, static_cast(out_asn1_sig_length)); + BufferWriter writer(out_asn1_sig); // Put SEQUENCE tag writer.Put(kSeqTag); @@ -528,28 +560,24 @@ CHIP_ERROR EcdsaRawSignatureToAsn1(size_t fe_length_bytes, const uint8_t * raw_s // Put the contents of the integers previously serialized in the buffer. // The writer.Put is memmove-safe, so the shifting will happen from the read // of the same buffer where the write is taking place. - writer.Put(&out_asn1_sig[kMinSequenceOverhead], integers_length); + writer.Put(out_asn1_sig.data() + kMinSequenceOverhead, integers_length); size_t actually_written = 0; VerifyOrReturnError(writer.Fit(actually_written), CHIP_ERROR_BUFFER_TOO_SMALL); - out_asn1_sig_actual_length = actually_written; + out_asn1_sig = out_asn1_sig.SubSpan(0, actually_written); return CHIP_NO_ERROR; } -CHIP_ERROR EcdsaAsn1SignatureToRaw(size_t fe_length_bytes, const uint8_t * asn1_sig, size_t asn1_sig_length, uint8_t * out_raw_sig, - size_t out_raw_sig_length) +CHIP_ERROR EcdsaAsn1SignatureToRaw(size_t fe_length_bytes, const ByteSpan & asn1_sig, MutableByteSpan & out_raw_sig) { VerifyOrReturnError(fe_length_bytes > 0, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(asn1_sig != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - - VerifyOrReturnError(asn1_sig_length > kMinSequenceOverhead, CHIP_ERROR_BUFFER_TOO_SMALL); + VerifyOrReturnError(asn1_sig.size() > kMinSequenceOverhead, CHIP_ERROR_BUFFER_TOO_SMALL); // Output raw signature is both of which are of fe_length_bytes (see SEC1). - VerifyOrReturnError(out_raw_sig_length >= (2u * fe_length_bytes), CHIP_ERROR_BUFFER_TOO_SMALL); - VerifyOrReturnError(out_raw_sig != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(out_raw_sig.size() >= (2u * fe_length_bytes), CHIP_ERROR_BUFFER_TOO_SMALL); - chip::Encoding::LittleEndian::Reader reader(asn1_sig, static_cast(asn1_sig_length)); + Reader reader(asn1_sig); // Make sure we have a starting Sequence uint8_t tag = 0; @@ -564,15 +592,17 @@ CHIP_ERROR EcdsaAsn1SignatureToRaw(size_t fe_length_bytes, const uint8_t * asn1_ VerifyOrReturnError(tag_len == reader.Remaining(), CHIP_ERROR_INVALID_ARGUMENT); // Can now clear raw signature integers r,s one by one - uint8_t * raw_cursor = out_raw_sig; + uint8_t * raw_cursor = out_raw_sig.data(); // Read R - ReturnErrorOnFailure(ReadDerUnsignedIntegerIntoRaw(reader, raw_cursor, fe_length_bytes)); + ReturnErrorOnFailure(ReadDerUnsignedIntegerIntoRaw(reader, MutableByteSpan{ raw_cursor, fe_length_bytes })); raw_cursor += fe_length_bytes; // Read S - ReturnErrorOnFailure(ReadDerUnsignedIntegerIntoRaw(reader, raw_cursor, fe_length_bytes)); + ReturnErrorOnFailure(ReadDerUnsignedIntegerIntoRaw(reader, MutableByteSpan{ raw_cursor, fe_length_bytes })); + + out_raw_sig = out_raw_sig.SubSpan(0, (2u * fe_length_bytes)); return CHIP_NO_ERROR; } diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index 2d9052871bf848..1a4f8c74d326e8 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -69,6 +69,9 @@ constexpr size_t kMAX_Spake2p_Context_Size = 1024; constexpr size_t kMAX_Hash_SHA256_Context_Size = 296; constexpr size_t kMAX_P256Keypair_Context_Size = 512; +constexpr size_t kEmitDerIntegerWithoutTagOverhead = 1; // 1 sign stuffer +constexpr size_t kEmitDerIntegerOverhead = 3; // Tag + Length byte + 1 sign stuffer + /* * Overhead to encode a raw ECDSA signature in X9.62 format in ASN.1 DER * @@ -88,9 +91,9 @@ constexpr size_t kMAX_P256Keypair_Context_Size = 512; constexpr size_t kMax_ECDSA_X9Dot62_Asn1_Overhead = 9; constexpr size_t kMax_ECDSA_Signature_Length_Der = kMax_ECDSA_Signature_Length + kMax_ECDSA_X9Dot62_Asn1_Overhead; -nlSTATIC_ASSERT_PRINT(kMax_ECDH_Secret_Length >= kP256_FE_Length, "ECDH shared secret is too short for crypto suite"); -nlSTATIC_ASSERT_PRINT(kMax_ECDSA_Signature_Length >= kP256_ECDSA_Signature_Length_Raw, - "ECDSA signature buffer length is too short for crypto suite"); +static_assert(kMax_ECDH_Secret_Length >= kP256_FE_Length, "ECDH shared secret is too short for crypto suite"); +static_assert(kMax_ECDSA_Signature_Length >= kP256_ECDSA_Signature_Length_Raw, + "ECDSA signature buffer length is too short for crypto suite"); /** * Spake2+ parameters for P256 @@ -362,15 +365,12 @@ class P256Keypair : public ECPKeypair concatenated - * @param[in] raw_sig_length Raw signature length (MUST be 2*`fe_length_bytes` long) - * @param[out] out_asn1_sig ASN.1 DER signature format output buffer - * @param[in] out_asn1_sig_length ASN.1 DER signature format output buffer length. Must have space for at least - * kMax_ECDSA_X9Dot62_Asn1_Overhead. - * @param[out] out_asn1_sig_actual_length Final computed size of signature. + * @param[out] out_asn1_sig ASN.1 DER signature format output buffer. Size must have space for at least + * kMax_ECDSA_X9Dot62_Asn1_Overhead. On CHIP_NO_ERROR, the out_asn1_sig buffer will be re-assigned + * to have the correct size based on variable-length output. * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise */ -CHIP_ERROR EcdsaRawSignatureToAsn1(size_t fe_length_bytes, const uint8_t * raw_sig, size_t raw_sig_length, uint8_t * out_asn1_sig, - size_t out_asn1_sig_length, size_t & out_asn1_sig_actual_length); +CHIP_ERROR EcdsaRawSignatureToAsn1(size_t fe_length_bytes, const ByteSpan & raw_sig, MutableByteSpan & out_asn1_sig); /** * @brief Convert an ASN.1 DER signature (per X9.62) as used by TLS libraries to SEC1 raw format @@ -383,14 +383,34 @@ CHIP_ERROR EcdsaRawSignatureToAsn1(size_t fe_length_bytes, const uint8_t * raw_s * * @param[in] fe_length_bytes Field Element length in bytes (e.g. 32 for P256 curve) * @param[in] asn1_sig ASN.1 DER signature input - * @param[in] asn1_sig_length ASN.1 DER signature length - * @param[out] out_raw_sig Raw signature of concatenated format output buffer - * @param[in] out_raw_sig_length Raw signature output buffer length. Must be at least >= `2 * fe_length_bytes` + * @param[out] out_raw_sig Raw signature of concatenated format output buffer. Size must be at + * least >= `2 * fe_length_bytes`. On CHIP_NO_ERROR, the out_asn1_sig buffer will be re-assigned + * to have the correct size based on variable-length output. * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise */ +CHIP_ERROR EcdsaAsn1SignatureToRaw(size_t fe_length_bytes, const ByteSpan & asn1_sig, MutableByteSpan & out_raw_sig); + +/** + * @brief Utility to emit a DER-encoded INTEGER given a raw unsigned large integer + * in big-endian order. The `out_der_integer` span is updated to reflect the final + * variable length, including tag and length, and must have at least `kEmitDerIntegerOverhead` + * extra space in addition to the `raw_integer.size()`. + * @param[in] raw_integer Bytes of a large unsigned integer in big-endian, possibly including leading zeroes + * @param[out] out_der_integer Buffer to receive the DER-encoded integer + * @return Returns CHIP_ERROR_INVALID_ARGUMENT or CHIP_ERROR_BUFFER_TOO_SMALL on error, CHIP_NO_ERROR otherwise. + */ +CHIP_ERROR ConvertIntegerRawToDer(const ByteSpan & raw_integer, MutableByteSpan & out_der_integer); -CHIP_ERROR EcdsaAsn1SignatureToRaw(size_t fe_length_bytes, const uint8_t * asn1_sig, size_t asn1_sig_length, uint8_t * out_raw_sig, - size_t out_raw_sig_length); +/** + * @brief Utility to emit a DER-encoded INTEGER given a raw unsigned large integer + * in big-endian order. The `out_der_integer` span is updated to reflect the final + * variable length, excluding tag and length, and must have at least `kEmitDerIntegerWithoutTagOverhead` + * extra space in addition to the `raw_integer.size()`. + * @param[in] raw_integer Bytes of a large unsigned integer in big-endian, possibly including leading zeroes + * @param[out] out_der_integer Buffer to receive the DER-encoded integer + * @return Returns CHIP_ERROR_INVALID_ARGUMENT or CHIP_ERROR_BUFFER_TOO_SMALL on error, CHIP_NO_ERROR otherwise. + */ +CHIP_ERROR ConvertIntegerRawToDerWithoutTag(const ByteSpan & raw_integer, MutableByteSpan & out_der_integer); /** * @brief A function that implements AES-CCM encryption @@ -567,7 +587,7 @@ class HMAC_sha /** * @brief A cryptographically secure random number generator based on NIST SP800-90A - * @param out_buffer Buffer to write random bytes into + * @param out_buffer Buffer into which to write random bytes * @param out_length Number of random bytes to generate * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise **/ diff --git a/src/crypto/CHIPCryptoPALmbedTLS.cpp b/src/crypto/CHIPCryptoPALmbedTLS.cpp index 8261411baddc6c..538ea58ad9ec08 100644 --- a/src/crypto/CHIPCryptoPALmbedTLS.cpp +++ b/src/crypto/CHIPCryptoPALmbedTLS.cpp @@ -798,6 +798,7 @@ CHIP_ERROR VerifyCertificateSigningRequest(const uint8_t * csr_buf, size_t csr_l mbedtls_ecp_keypair * keypair = nullptr; P256ECDSASignature signature; + MutableByteSpan out_raw_sig_span(signature.Bytes(), signature.Capacity()); mbedtls_x509_csr csr; mbedtls_x509_csr_init(&csr); @@ -818,9 +819,10 @@ CHIP_ERROR VerifyCertificateSigningRequest(const uint8_t * csr_buf, size_t csr_l VerifyOrExit(pubkey_size == pubkey.Length(), error = CHIP_ERROR_INTERNAL); // Convert DER signature to raw signature - error = EcdsaAsn1SignatureToRaw(kP256_FE_Length, csr.sig.p, csr.sig.len, signature.Bytes(), signature.Capacity()); + error = EcdsaAsn1SignatureToRaw(kP256_FE_Length, ByteSpan{ csr.sig.p, csr.sig.len }, out_raw_sig_span); VerifyOrExit(error == CHIP_NO_ERROR, error = CHIP_ERROR_INVALID_ARGUMENT); - signature.SetLength(kP256_FE_Length * 2); + VerifyOrExit(out_raw_sig_span.size() == (kP256_FE_Length * 2), error = CHIP_ERROR_INTERNAL); + signature.SetLength(out_raw_sig_span.size()); // Verify the signature using the public key error = pubkey.ECDSA_validate_msg_signature(csr.cri.p, csr.cri.len, signature); diff --git a/src/crypto/tests/BUILD.gn b/src/crypto/tests/BUILD.gn index 22291641aa8d98..97e97eef1f82e4 100644 --- a/src/crypto/tests/BUILD.gn +++ b/src/crypto/tests/BUILD.gn @@ -32,6 +32,7 @@ chip_test_suite("tests") { "Hash_SHA256_test_vectors.h", "PBKDF2_SHA256_test_vectors.h", "PBKDF2_SHA256_test_vectors.h", + "RawIntegerToDer_test_vectors.h", "SPAKE2P_FE_MUL_test_vectors.h", "SPAKE2P_FE_RW_test_vectors.h", "SPAKE2P_HMAC_test_vectors.h", diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index 1e4a81a642d116..21fcf6e89ade41 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -26,6 +26,7 @@ #include "Hash_SHA256_test_vectors.h" #include "PBKDF2_SHA256_test_vectors.h" +#include "RawIntegerToDer_test_vectors.h" #include "SPAKE2P_FE_MUL_test_vectors.h" #include "SPAKE2P_FE_RW_test_vectors.h" #include "SPAKE2P_HMAC_test_vectors.h" @@ -619,26 +620,110 @@ static void TestAsn1Conversions(nlTestSuite * inSuite, void * inContext) out_raw_sig.Calloc(out_raw_sig_allocated_size); NL_TEST_ASSERT(inSuite, out_raw_sig); - chip::Platform::ScopedMemoryBuffer out_asn1_sig; - size_t out_asn1_sig_allocated_size = (vector->fe_length_bytes * 2) + kMax_ECDSA_X9Dot62_Asn1_Overhead; - out_asn1_sig.Calloc(out_asn1_sig_allocated_size); - NL_TEST_ASSERT(inSuite, out_asn1_sig); + chip::Platform::ScopedMemoryBuffer out_der_sig; + size_t out_der_sig_allocated_size = (vector->fe_length_bytes * 2) + kMax_ECDSA_X9Dot62_Asn1_Overhead; + out_der_sig.Calloc(out_der_sig_allocated_size); + NL_TEST_ASSERT(inSuite, out_der_sig); // Test converstion from ASN.1 ER to raw - CHIP_ERROR status = EcdsaAsn1SignatureToRaw(vector->fe_length_bytes, vector->der_version, vector->der_version_length, - out_raw_sig.Get(), out_raw_sig_allocated_size); + MutableByteSpan out_raw_sig_span(out_raw_sig.Get(), out_raw_sig_allocated_size); + + CHIP_ERROR status = EcdsaAsn1SignatureToRaw(vector->fe_length_bytes, + ByteSpan{ vector->der_version, vector->der_version_length }, out_raw_sig_span); NL_TEST_ASSERT(inSuite, status == CHIP_NO_ERROR); - NL_TEST_ASSERT(inSuite, (memcmp(out_raw_sig.Get(), vector->raw_version, vector->raw_version_length) == 0)); + NL_TEST_ASSERT(inSuite, out_raw_sig_span.size() == vector->raw_version_length); + NL_TEST_ASSERT(inSuite, (memcmp(out_raw_sig_span.data(), vector->raw_version, vector->raw_version_length) == 0)); // Test conversion from raw to ASN.1 DER - size_t der_size = 0; - status = EcdsaRawSignatureToAsn1(vector->fe_length_bytes, vector->raw_version, vector->raw_version_length, - out_asn1_sig.Get(), out_asn1_sig_allocated_size, der_size); - + MutableByteSpan out_der_sig_span(out_der_sig.Get(), out_der_sig_allocated_size); + status = EcdsaRawSignatureToAsn1(vector->fe_length_bytes, ByteSpan{ vector->raw_version, vector->raw_version_length }, + out_der_sig_span); NL_TEST_ASSERT(inSuite, status == CHIP_NO_ERROR); - NL_TEST_ASSERT(inSuite, der_size <= out_asn1_sig_allocated_size); - NL_TEST_ASSERT(inSuite, der_size == vector->der_version_length); - NL_TEST_ASSERT(inSuite, (memcmp(out_asn1_sig.Get(), vector->der_version, vector->der_version_length) == 0)); + NL_TEST_ASSERT(inSuite, out_der_sig_span.size() <= out_der_sig_allocated_size); + NL_TEST_ASSERT(inSuite, out_der_sig_span.size() == vector->der_version_length); + NL_TEST_ASSERT(inSuite, (memcmp(out_der_sig_span.data(), vector->der_version, vector->der_version_length) == 0)); + } +} + +static void TestRawIntegerToDerValidCases(nlTestSuite * inSuite, void * inContext) +{ + int numOfTestCases = ArraySize(kRawIntegerToDerVectors); + + for (int testIdx = 0; testIdx < numOfTestCases; testIdx++) + { + RawIntegerToDerVector v = kRawIntegerToDerVectors[testIdx]; + + // Cover case with tag/length + { + chip::Platform::ScopedMemoryBuffer out_der_buffer; + out_der_buffer.Alloc(v.expected_size); + NL_TEST_ASSERT(inSuite, out_der_buffer); + + MutableByteSpan out_der_integer(out_der_buffer.Get(), v.expected_size); + CHIP_ERROR status = ConvertIntegerRawToDer(ByteSpan{ v.candidate, v.candidate_size }, out_der_integer); + NL_TEST_ASSERT(inSuite, status == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, out_der_integer.size() == v.expected_size); + NL_TEST_ASSERT(inSuite, out_der_integer.data_equal(ByteSpan(v.expected, v.expected_size))); + + // Cover case of buffer too small + MutableByteSpan out_der_integer_too_small(out_der_buffer.Get(), v.expected_size - 1); + status = ConvertIntegerRawToDer(ByteSpan{ v.candidate, v.candidate_size }, out_der_integer_too_small); + NL_TEST_ASSERT(inSuite, status == CHIP_ERROR_BUFFER_TOO_SMALL); + } + + // Cover case without tag/length + { + chip::Platform::ScopedMemoryBuffer out_der_buffer; + out_der_buffer.Alloc(v.expected_without_tag_size); + NL_TEST_ASSERT(inSuite, out_der_buffer); + + MutableByteSpan out_der_integer(out_der_buffer.Get(), v.expected_without_tag_size); + CHIP_ERROR status = ConvertIntegerRawToDerWithoutTag(ByteSpan{ v.candidate, v.candidate_size }, out_der_integer); + + NL_TEST_ASSERT(inSuite, status == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, out_der_integer.size() == v.expected_without_tag_size); + NL_TEST_ASSERT(inSuite, out_der_integer.data_equal(ByteSpan(v.expected_without_tag, v.expected_without_tag_size))); + } + } +} + +static void TestRawIntegerToDerInvalidCases(nlTestSuite * inSuite, void * inContext) +{ + // Cover case of invalid buffers + uint8_t placeholder[10] = { 0 }; + MutableByteSpan good_out_buffer(placeholder, sizeof(placeholder)); + ByteSpan good_buffer(placeholder, sizeof(placeholder)); + + MutableByteSpan bad_out_buffer_nullptr(nullptr, sizeof(placeholder)); + MutableByteSpan bad_out_buffer_empty(placeholder, 0); + + ByteSpan bad_buffer_nullptr(nullptr, sizeof(placeholder)); + ByteSpan bad_buffer_empty(placeholder, 0); + + struct ErrorCase + { + const ByteSpan & input; + MutableByteSpan & output; + CHIP_ERROR expected_status; + }; + + const ErrorCase error_cases[] = { + { .input = good_buffer, .output = bad_out_buffer_nullptr, .expected_status = CHIP_ERROR_INVALID_ARGUMENT }, + { .input = good_buffer, .output = bad_out_buffer_empty, .expected_status = CHIP_ERROR_INVALID_ARGUMENT }, + { .input = bad_buffer_nullptr, .output = good_out_buffer, .expected_status = CHIP_ERROR_INVALID_ARGUMENT }, + { .input = bad_buffer_empty, .output = good_out_buffer, .expected_status = CHIP_ERROR_INVALID_ARGUMENT } + }; + + int case_idx = 0; + for (const ErrorCase & v : error_cases) + { + CHIP_ERROR status = ConvertIntegerRawToDerWithoutTag(v.input, v.output); + if (status != v.expected_status) + { + ChipLogError(Crypto, "Failed TestRawIntegerToDerInvalidCases sub-case %d", case_idx); + NL_TEST_ASSERT(inSuite, v.expected_status == status); + } + ++case_idx; } } @@ -1591,6 +1676,8 @@ static const nlTest sTests[] = { NL_TEST_DEF("Test decrypting AES-CCM-256 invalid IV", TestAES_CCM_256DecryptInvalidIVLen), NL_TEST_DEF("Test decrypting AES-CCM-256 invalid vectors", TestAES_CCM_256DecryptInvalidTestVectors), NL_TEST_DEF("Test ASN.1 signature conversion routines", TestAsn1Conversions), + NL_TEST_DEF("Test Integer to ASN.1 DER conversion", TestRawIntegerToDerValidCases), + NL_TEST_DEF("Test Integer to ASN.1 DER conversion error cases", TestRawIntegerToDerInvalidCases), NL_TEST_DEF("Test ECDSA signing and validation message using SHA256", TestECDSA_Signing_SHA256_Msg), NL_TEST_DEF("Test ECDSA signing and validation SHA256 Hash", TestECDSA_Signing_SHA256_Hash), NL_TEST_DEF("Test ECDSA signature validation fail - Different msg", TestECDSA_ValidationFailsDifferentMessage), diff --git a/src/crypto/tests/RawIntegerToDer_test_vectors.h b/src/crypto/tests/RawIntegerToDer_test_vectors.h new file mode 100644 index 00000000000000..ed496b88fc42fe --- /dev/null +++ b/src/crypto/tests/RawIntegerToDer_test_vectors.h @@ -0,0 +1,210 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file - This file contains integer to ASN.1 DER conversion test vector + * + */ + +#pragma once + +#include +#include + +struct RawIntegerToDerVector +{ + const uint8_t * candidate; + const size_t candidate_size; + const uint8_t * expected; + const size_t expected_size; + const uint8_t * expected_without_tag; + const size_t expected_without_tag_size; +}; + +// MSB set, no leading zeros +const uint8_t kRawToDerMsbSetNoLeadingZeroes_Candidate[5] = { + 0x80, 0x01, 0x02, 0x03, 0x04, +}; + +const uint8_t kRawToDerMsbSetNoLeadingZeroes_Expected[8] = { + 0x02, 0x06, 0x00, 0x80, 0x01, 0x02, 0x03, 0x04, +}; + +const uint8_t kRawToDerMsbSetNoLeadingZeroes_Expected_WithoutTag[6] = { + 0x00, 0x80, 0x01, 0x02, 0x03, 0x04, +}; + +const RawIntegerToDerVector kRawIntegerToDerVector1 = { + .candidate = &kRawToDerMsbSetNoLeadingZeroes_Candidate[0], + .candidate_size = sizeof(kRawToDerMsbSetNoLeadingZeroes_Candidate), + .expected = &kRawToDerMsbSetNoLeadingZeroes_Expected[0], + .expected_size = sizeof(kRawToDerMsbSetNoLeadingZeroes_Expected), + .expected_without_tag = &kRawToDerMsbSetNoLeadingZeroes_Expected_WithoutTag[0], + .expected_without_tag_size = sizeof(kRawToDerMsbSetNoLeadingZeroes_Expected_WithoutTag), +}; + +// MSB clear, no leading zeros +const uint8_t kRawToDerMsbClearNoLeadingZeroes_Candidate[5] = { + 0x40, 0x01, 0x02, 0x03, 0x04, +}; + +const uint8_t kRawToDerMsbClearNoLeadingZeroes_Expected[7] = { + 0x02, 0x05, 0x40, 0x01, 0x02, 0x03, 0x04, +}; + +const uint8_t kRawToDerMsbClearNoLeadingZeroes_Expected_WithoutTag[5] = { + 0x40, 0x01, 0x02, 0x03, 0x04, +}; + +const RawIntegerToDerVector kRawIntegerToDerVector2 = { + .candidate = &kRawToDerMsbClearNoLeadingZeroes_Candidate[0], + .candidate_size = sizeof(kRawToDerMsbClearNoLeadingZeroes_Candidate), + .expected = &kRawToDerMsbClearNoLeadingZeroes_Expected[0], + .expected_size = sizeof(kRawToDerMsbClearNoLeadingZeroes_Expected), + .expected_without_tag = &kRawToDerMsbClearNoLeadingZeroes_Expected_WithoutTag[0], + .expected_without_tag_size = sizeof(kRawToDerMsbClearNoLeadingZeroes_Expected_WithoutTag), +}; + +// Three leading zeroes +const uint8_t kRawToDerThreeLeadingZeroes_Candidate[7] = { + 0x00, 0x00, 0x00, 0x01, 0x02, 0x03, 0x04, +}; + +const uint8_t kRawToDerThreeLeadingZeroes_Expected[6] = { + 0x02, 0x04, 0x01, 0x02, 0x03, 0x04, +}; + +const uint8_t kRawToDerThreeLeadingZeroes_Expected_WithoutTag[4] = { + 0x01, + 0x02, + 0x03, + 0x04, +}; + +const RawIntegerToDerVector kRawIntegerToDerVector3 = { + .candidate = &kRawToDerThreeLeadingZeroes_Candidate[0], + .candidate_size = sizeof(kRawToDerThreeLeadingZeroes_Candidate), + .expected = &kRawToDerThreeLeadingZeroes_Expected[0], + .expected_size = sizeof(kRawToDerThreeLeadingZeroes_Expected), + .expected_without_tag = &kRawToDerThreeLeadingZeroes_Expected_WithoutTag[0], + .expected_without_tag_size = sizeof(kRawToDerThreeLeadingZeroes_Expected_WithoutTag), +}; + +// Literal zero +const uint8_t kRawToDerLiteralZero_Candidate[1] = { + 0x00, +}; + +const uint8_t kRawToDerLiteralZero_Expected[3] = { + 0x02, + 0x01, + 0x00, +}; + +const uint8_t kRawToDerLiteralZero_Expected_WithoutTag[1] = { + 0x00, +}; + +const RawIntegerToDerVector kRawIntegerToDerVector4 = { + .candidate = &kRawToDerLiteralZero_Candidate[0], + .candidate_size = sizeof(kRawToDerLiteralZero_Candidate), + .expected = &kRawToDerLiteralZero_Expected[0], + .expected_size = sizeof(kRawToDerLiteralZero_Expected), + .expected_without_tag = &kRawToDerLiteralZero_Expected_WithoutTag[0], + .expected_without_tag_size = sizeof(kRawToDerLiteralZero_Expected_WithoutTag), +}; + +// Only leading zeroes +const uint8_t kRawToDerOnlyLeadingZeroes_Candidate[4] = { + 0x00, + 0x00, + 0x00, + 0x00, +}; + +const uint8_t kRawToDerOnlyLeadingZeroes_Expected[3] = { + 0x02, + 0x01, + 0x00, +}; + +const uint8_t kRawToDerOnlyLeadingZeroes_Expected_WithoutTag[1] = { + 0x00, +}; + +const RawIntegerToDerVector kRawIntegerToDerVector5 = { + .candidate = &kRawToDerOnlyLeadingZeroes_Candidate[0], + .candidate_size = sizeof(kRawToDerOnlyLeadingZeroes_Candidate), + .expected = &kRawToDerOnlyLeadingZeroes_Expected[0], + .expected_size = sizeof(kRawToDerOnlyLeadingZeroes_Expected), + .expected_without_tag = &kRawToDerOnlyLeadingZeroes_Expected_WithoutTag[0], + .expected_without_tag_size = sizeof(kRawToDerOnlyLeadingZeroes_Expected_WithoutTag), +}; + +// Only one byte, non-zero, MSB set +const uint8_t kRawToDerMsbSetOneByte_Candidate[1] = { + 0xff, +}; + +const uint8_t kRawToDerMsbSetOneByte_Expected[4] = { + 0x02, + 0x02, + 0x00, + 0xff, +}; + +const uint8_t kRawToDerMsbSetOneByte_Expected_WithoutTag[2] = { + 0x00, + 0xff, +}; + +const RawIntegerToDerVector kRawIntegerToDerVector6 = { + .candidate = &kRawToDerMsbSetOneByte_Candidate[0], + .candidate_size = sizeof(kRawToDerMsbSetOneByte_Candidate), + .expected = &kRawToDerMsbSetOneByte_Expected[0], + .expected_size = sizeof(kRawToDerMsbSetOneByte_Expected), + .expected_without_tag = &kRawToDerMsbSetOneByte_Expected_WithoutTag[0], + .expected_without_tag_size = sizeof(kRawToDerMsbSetOneByte_Expected_WithoutTag), +}; + +// Only one byte, non-zero, MSB clear +const uint8_t kRawToDerMsbClearOneByte_Candidate[1] = { + 0x7f, +}; + +const uint8_t kRawToDerMsbClearOneByte_Expected[3] = { + 0x02, + 0x01, + 0x7f, +}; + +const uint8_t kRawToDerMsbClearOneByte_Expected_WithoutTag[1] = { + 0x7f, +}; + +const RawIntegerToDerVector kRawIntegerToDerVector7 = { + .candidate = &kRawToDerMsbClearOneByte_Candidate[0], + .candidate_size = sizeof(kRawToDerMsbClearOneByte_Candidate), + .expected = &kRawToDerMsbClearOneByte_Expected[0], + .expected_size = sizeof(kRawToDerMsbClearOneByte_Expected), + .expected_without_tag = &kRawToDerMsbClearOneByte_Expected_WithoutTag[0], + .expected_without_tag_size = sizeof(kRawToDerMsbClearOneByte_Expected_WithoutTag), +}; + +const RawIntegerToDerVector kRawIntegerToDerVectors[] = { kRawIntegerToDerVector1, kRawIntegerToDerVector2, kRawIntegerToDerVector3, + kRawIntegerToDerVector4, kRawIntegerToDerVector5, kRawIntegerToDerVector6, + kRawIntegerToDerVector7 }; diff --git a/src/lib/support/BufferReader.h b/src/lib/support/BufferReader.h index 9a9eb7dc2b5afb..bbf21f529d8fb6 100644 --- a/src/lib/support/BufferReader.h +++ b/src/lib/support/BufferReader.h @@ -26,6 +26,7 @@ #include #include #include +#include #include namespace chip { @@ -41,32 +42,43 @@ class Reader { public: /** - * Create a data model reader from a given buffer and length. + * Create a buffer reader from a given buffer and length. * - * @param buffer The octet buffer to read from. The caller must ensure + * @param buffer The octet buffer from which to read. The caller must ensure * (most simply by allocating the reader on the stack) that - * the buffer outlives the reader. The buffer is allowed to - * be null if buf_len is 0. + * the buffer outlives the reader. If `buffer` is nullptr, + * length is automatically overridden to zero, to avoid accesses. * @param buf_len The number of octets in the buffer. */ - Reader(const uint8_t * buffer, uint16_t buf_len) : mBufStart(buffer), mReadPtr(buffer), mAvailable(buf_len) {} + Reader(const uint8_t * buffer, size_t buf_len) : mBufStart(buffer), mReadPtr(buffer), mAvailable(buf_len) + { + if (mBufStart == nullptr) + { + mAvailable = 0; + } + } + + /** + * Create a buffer reader from a given byte span. + * + * @param buffer The octet buffer byte span from which to read. The caller must ensure + * that the buffer outlives the reader. The buffer's ByteSpan .data() pointer + * is is nullptr, length is automatically overridden to zero, to avoid accesses. + */ + Reader(const ByteSpan & buffer) : Reader(buffer.data(), buffer.size()) {} /** - * Number of octets we have read so far. This might be able to go away once - * we do less switching back and forth between DataModelReader and raw - * buffers. + * Number of octets we have read so far. */ - uint16_t OctetsRead() const { return static_cast(mReadPtr - mBufStart); } + size_t OctetsRead() const { return static_cast(mReadPtr - mBufStart); } /** - * Number of octets we have remaining to read. Can be useful for logging. + * Number of octets we have remaining to read. */ - uint16_t Remaining() const { return mAvailable; } + size_t Remaining() const { return mAvailable; } /** * Test whether we have at least the given number of octets left to read. - * This takes a size_t, not uint16_t, to make life a bit simpler for - * consumers and avoid casting. */ bool HasAtLeast(size_t octets) const { return octets <= Remaining(); } @@ -179,11 +191,11 @@ class Reader * remaining, the Reader will advance to the end of the buffer * without entering a failed-status state. */ - Reader & Skip(uint16_t len) + Reader & Skip(size_t len) { len = ::chip::min(len, mAvailable); mReadPtr += len; - mAvailable = static_cast(mAvailable - len); + mAvailable = static_cast(mAvailable - len); return *this; } @@ -201,7 +213,7 @@ class Reader /** * The number of octets we can still read starting at mReadPtr. */ - uint16_t mAvailable; + size_t mAvailable; /** * Our current status. diff --git a/src/lib/support/BufferWriter.h b/src/lib/support/BufferWriter.h index f0ce2e18674cb6..b608d54ceda69c 100644 --- a/src/lib/support/BufferWriter.h +++ b/src/lib/support/BufferWriter.h @@ -21,20 +21,32 @@ #include #include +#include + namespace chip { namespace Encoding { class BufferWriter { public: - BufferWriter(uint8_t * buf, size_t len) : mBuf(buf), mSize(len), mNeeded(0) {} + BufferWriter(uint8_t * buf, size_t len) : mBuf(buf), mSize(len), mNeeded(0) + { + if (buf == nullptr) + { + mSize = 0; + } + } + BufferWriter(MutableByteSpan buf) : BufferWriter(buf.data(), buf.size()) {} BufferWriter(const BufferWriter & other) = default; BufferWriter & operator=(const BufferWriter & other) = default; /// Append a null terminated string, exclude the null terminator BufferWriter & Put(const char * s); - /// Raw append a buffer, regardless of endianess + /// Raw append a buffer, regardless of endianess. + /// This is memmove-safe: if `buf` points to the underlying buffer, where output + /// will be written, and the overlap is legal for a memmove to have worked properly, + /// then this method will properly copy data. BufferWriter & Put(const void * buf, size_t len); /// Append a single byte @@ -98,6 +110,7 @@ class EndianBufferWriterBase : public BufferWriter protected: EndianBufferWriterBase(uint8_t * buf, size_t len) : BufferWriter(buf, len) {} + EndianBufferWriterBase(MutableByteSpan buf) : BufferWriter(buf.data(), buf.size()) {} EndianBufferWriterBase(const EndianBufferWriterBase & other) = default; EndianBufferWriterBase & operator=(const EndianBufferWriterBase & other) = default; }; @@ -108,6 +121,7 @@ class BufferWriter : public EndianBufferWriterBase { public: BufferWriter(uint8_t * buf, size_t len) : EndianBufferWriterBase(buf, len) {} + BufferWriter(MutableByteSpan buf) : EndianBufferWriterBase(buf) {} BufferWriter(const BufferWriter & other) = default; BufferWriter & operator=(const BufferWriter & other) = default; BufferWriter & EndianPut(uint64_t x, size_t size); @@ -121,6 +135,7 @@ class BufferWriter : public EndianBufferWriterBase { public: BufferWriter(uint8_t * buf, size_t len) : EndianBufferWriterBase(buf, len) {} + BufferWriter(MutableByteSpan buf) : EndianBufferWriterBase(buf) {} BufferWriter(const BufferWriter & other) = default; BufferWriter & operator=(const BufferWriter & other) = default; BufferWriter & EndianPut(uint64_t x, size_t size); diff --git a/src/lib/support/Span.h b/src/lib/support/Span.h index 4a4e259162b2a0..680557b88f4c32 100644 --- a/src/lib/support/Span.h +++ b/src/lib/support/Span.h @@ -173,6 +173,28 @@ inline bool Span::data_equal(const FixedSpan & other) const return (size() == other.size()) && (empty() || (memcmp(data(), other.data(), size() * sizeof(T)) == 0)); } +/** + * @brief Returns true if the `span` could be used to access some data, + * false otherwise. + * @param[in] span The Span to validate. + */ +template +inline bool IsSpanUsable(const Span & span) +{ + return (span.data() != nullptr) && (span.size() > 0); +} + +/** + * @brief Returns true if the `span` could be used to access some data, + * false otherwise. + * @param[in] span The FixedSpan to validate. + */ +template +inline bool IsSpanUsable(const FixedSpan & span) +{ + return (span.data() != nullptr); +} + using ByteSpan = Span; using MutableByteSpan = Span; template diff --git a/src/lib/support/tests/TestBufferReader.cpp b/src/lib/support/tests/TestBufferReader.cpp index 9f290cb276f9e4..a1802d24cbeca1 100644 --- a/src/lib/support/tests/TestBufferReader.cpp +++ b/src/lib/support/tests/TestBufferReader.cpp @@ -38,9 +38,13 @@ struct TestReader : public Reader TestReader() : Reader(test_buffer, std::extent::value) {} }; -static void TestBufferReader_Basic(nlTestSuite * inSuite, void * inContext) +struct TestSpanReader : public Reader +{ + TestSpanReader() : Reader(ByteSpan{ test_buffer, std::extent::value }) {} +}; + +static void TestBufferReader_BasicImpl(nlTestSuite * inSuite, void * inContext, Reader & reader) { - TestReader reader; uint8_t first; uint16_t second; uint32_t third; @@ -68,6 +72,20 @@ static void TestBufferReader_Basic(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, err != CHIP_NO_ERROR); } +static void TestBufferReader_Basic(nlTestSuite * inSuite, void * inContext) +{ + TestReader reader; + + TestBufferReader_BasicImpl(inSuite, inContext, reader); +} + +static void TestBufferReader_BasicSpan(nlTestSuite * inSuite, void * inContext) +{ + TestSpanReader reader; + + TestBufferReader_BasicImpl(inSuite, inContext, reader); +} + static void TestBufferReader_Saturation(nlTestSuite * inSuite, void * inContext) { TestReader reader; @@ -102,7 +120,7 @@ static void TestBufferReader_Skip(nlTestSuite * inSuite, void * inContext) CHIP_ERROR err = reader.Skip(firstSkipLen).Read8(&temp).StatusCode(); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, temp == test_buffer[firstSkipLen]); - NL_TEST_ASSERT(inSuite, reader.OctetsRead() == firstSkipLen + 1); + NL_TEST_ASSERT(inSuite, reader.OctetsRead() == (firstSkipLen + 1u)); // Verify Skip() called with a length larger than available buffer space jumps to the end. err = reader.Skip(sizeof(test_buffer)).StatusCode(); @@ -119,8 +137,9 @@ static void TestBufferReader_Skip(nlTestSuite * inSuite, void * inContext) /** * Test Suite. It lists all the test functions. */ -static const nlTest sTests[] = { NL_TEST_DEF_FN(TestBufferReader_Basic), NL_TEST_DEF_FN(TestBufferReader_Saturation), - NL_TEST_DEF_FN(TestBufferReader_Skip), NL_TEST_SENTINEL() }; +static const nlTest sTests[] = { NL_TEST_DEF_FN(TestBufferReader_Basic), NL_TEST_DEF_FN(TestBufferReader_BasicSpan), + NL_TEST_DEF_FN(TestBufferReader_Saturation), NL_TEST_DEF_FN(TestBufferReader_Skip), + NL_TEST_SENTINEL() }; int TestBufferReader(void) { diff --git a/src/lib/support/tests/TestBufferWriter.cpp b/src/lib/support/tests/TestBufferWriter.cpp index 8d86f09a3a5f9d..531c9a98b958b9 100644 --- a/src/lib/support/tests/TestBufferWriter.cpp +++ b/src/lib/support/tests/TestBufferWriter.cpp @@ -77,6 +77,29 @@ class BWTest : public Base using namespace chip::Encoding; +void TestSpanVersusRegular(nlTestSuite * inSuite, void * inContext) +{ + uint8_t buf_regular[5] = { 0, 0, 0, 0, 0 }; + uint8_t buf_span[5] = { 0, 0, 0, 0, 0 }; + uint8_t all_zeroes[5] = { 0, 0, 0, 0, 0 }; + uint8_t final_expected[5] = { 1, 2, 3, 4, 0 }; + + BufferWriter regular_writer(buf_regular, sizeof(buf_regular)); + BufferWriter span_writer(chip::MutableByteSpan{ buf_span }); + + NL_TEST_ASSERT(inSuite, regular_writer.Available() == sizeof(buf_regular)); + NL_TEST_ASSERT(inSuite, span_writer.Available() == sizeof(buf_span)); + + NL_TEST_ASSERT(inSuite, 0 == memcmp(buf_regular, all_zeroes, sizeof(all_zeroes))); + NL_TEST_ASSERT(inSuite, 0 == memcmp(buf_span, all_zeroes, sizeof(all_zeroes))); + + NL_TEST_ASSERT(inSuite, regular_writer.Put(1).Put(2).Put(3).Put(4).Fit()); + NL_TEST_ASSERT(inSuite, span_writer.Put(1).Put(2).Put(3).Put(4).Fit()); + + NL_TEST_ASSERT(inSuite, 0 == memcmp(buf_regular, final_expected, sizeof(final_expected))); + NL_TEST_ASSERT(inSuite, 0 == memcmp(buf_span, final_expected, sizeof(final_expected))); +} + void TestStringWrite(nlTestSuite * inSuite, void * inContext) { { @@ -203,11 +226,12 @@ void TestPutBigEndian(nlTestSuite * inSuite, void * inContext) } const nlTest sTests[] = { - NL_TEST_DEF("TestStringWrite", TestStringWrite), // - NL_TEST_DEF("TestBufferWrite", TestBufferWrite), // - NL_TEST_DEF("TestPutLittleEndian", TestPutLittleEndian), // - NL_TEST_DEF("TestPutBigEndian", TestPutBigEndian), // - NL_TEST_SENTINEL() // + NL_TEST_DEF("TestSpanVersusRegular", TestSpanVersusRegular), // + NL_TEST_DEF("TestStringWrite", TestStringWrite), // + NL_TEST_DEF("TestBufferWrite", TestBufferWrite), // + NL_TEST_DEF("TestPutLittleEndian", TestPutLittleEndian), // + NL_TEST_DEF("TestPutBigEndian", TestPutBigEndian), // + NL_TEST_SENTINEL() // }; } // namespace diff --git a/src/lib/support/tests/TestSpan.cpp b/src/lib/support/tests/TestSpan.cpp index 2589a70a777a87..efe771986792cb 100644 --- a/src/lib/support/tests/TestSpan.cpp +++ b/src/lib/support/tests/TestSpan.cpp @@ -38,6 +38,7 @@ static void TestByteSpan(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, s0.size() == 0); NL_TEST_ASSERT(inSuite, s0.empty()); NL_TEST_ASSERT(inSuite, s0.data_equal(s0)); + NL_TEST_ASSERT(inSuite, IsSpanUsable(s0) == false); ByteSpan s1(arr, 2); NL_TEST_ASSERT(inSuite, s1.data() == arr); @@ -45,6 +46,7 @@ static void TestByteSpan(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !s1.empty()); NL_TEST_ASSERT(inSuite, s1.data_equal(s1)); NL_TEST_ASSERT(inSuite, !s1.data_equal(s0)); + NL_TEST_ASSERT(inSuite, IsSpanUsable(s1) == true); ByteSpan s2(arr); NL_TEST_ASSERT(inSuite, s2.data() == arr); @@ -53,6 +55,7 @@ static void TestByteSpan(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !s2.empty()); NL_TEST_ASSERT(inSuite, s2.data_equal(s2)); NL_TEST_ASSERT(inSuite, !s2.data_equal(s1)); + NL_TEST_ASSERT(inSuite, IsSpanUsable(s2) == true); ByteSpan s3 = s2; NL_TEST_ASSERT(inSuite, s3.data() == arr); @@ -60,6 +63,7 @@ static void TestByteSpan(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, s3.data()[2] == 3); NL_TEST_ASSERT(inSuite, !s3.empty()); NL_TEST_ASSERT(inSuite, s3.data_equal(s2)); + NL_TEST_ASSERT(inSuite, IsSpanUsable(s3) == true); uint8_t arr2[] = { 3, 2, 1 }; ByteSpan s4(arr2); @@ -88,6 +92,7 @@ static void TestMutableByteSpan(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, s0.size() == 0); NL_TEST_ASSERT(inSuite, s0.empty()); NL_TEST_ASSERT(inSuite, s0.data_equal(s0)); + NL_TEST_ASSERT(inSuite, IsSpanUsable(s0) == false); MutableByteSpan s1(arr, 2); NL_TEST_ASSERT(inSuite, s1.data() == arr); @@ -95,6 +100,7 @@ static void TestMutableByteSpan(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !s1.empty()); NL_TEST_ASSERT(inSuite, s1.data_equal(s1)); NL_TEST_ASSERT(inSuite, !s1.data_equal(s0)); + NL_TEST_ASSERT(inSuite, IsSpanUsable(s1) == true); MutableByteSpan s2(arr); NL_TEST_ASSERT(inSuite, s2.data() == arr); @@ -154,12 +160,14 @@ static void TestFixedByteSpan(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, s0.size() == 3); NL_TEST_ASSERT(inSuite, s0.empty()); NL_TEST_ASSERT(inSuite, s0.data_equal(s0)); + NL_TEST_ASSERT(inSuite, IsSpanUsable(s0) == false); FixedByteSpan<2> s1(arr); NL_TEST_ASSERT(inSuite, s1.data() == arr); NL_TEST_ASSERT(inSuite, s1.size() == 2); NL_TEST_ASSERT(inSuite, !s1.empty()); NL_TEST_ASSERT(inSuite, s1.data_equal(s1)); + NL_TEST_ASSERT(inSuite, IsSpanUsable(s1) == true); FixedByteSpan<3> s2(arr); NL_TEST_ASSERT(inSuite, s2.data() == arr); diff --git a/src/protocols/bdx/BdxMessages.h b/src/protocols/bdx/BdxMessages.h index e65fd8c394b70f..6c04ae00339471 100644 --- a/src/protocols/bdx/BdxMessages.h +++ b/src/protocols/bdx/BdxMessages.h @@ -150,7 +150,7 @@ struct TransferInit : public BdxMessage const uint8_t * FileDesignator = nullptr; uint16_t FileDesLength = 0; ///< Length of file designator string (not including null-terminator) const uint8_t * Metadata = nullptr; - uint16_t MetadataLength = 0; + size_t MetadataLength = 0; // Retain ownership of the packet buffer so that the FileDesignator and Metadata pointers remain valid. System::PacketBufferHandle Buffer; @@ -184,7 +184,7 @@ struct SendAccept : public BdxMessage // WARNING: there is no guarantee at any point that this pointer will point to valid memory. The Buffer field should be used to // hold a reference to the PacketBuffer containing the data in order to ensure the data is not freed. const uint8_t * Metadata = nullptr; - uint16_t MetadataLength = 0; + size_t MetadataLength = 0; // Retain ownership of the packet buffer so that the FileDesignator and Metadata pointers remain valid. System::PacketBufferHandle Buffer; @@ -218,7 +218,7 @@ struct ReceiveAccept : public BdxMessage // WARNING: there is no guarantee at any point that this pointer will point to valid memory. The Buffer field should be used to // hold a reference to the PacketBuffer containing the data in order to ensure the data is not freed. const uint8_t * Metadata = nullptr; - uint16_t MetadataLength = 0; + size_t MetadataLength = 0; // Retain ownership of the packet buffer so that the FileDesignator and Metadata pointers remain valid. System::PacketBufferHandle Buffer; @@ -267,7 +267,7 @@ struct DataBlock : public BdxMessage // WARNING: there is no guarantee at any point that this pointer will point to valid memory. The Buffer field should be used to // hold a reference to the PacketBuffer containing the data in order to ensure the data is not freed. const uint8_t * Data = nullptr; - uint16_t DataLength = 0; + size_t DataLength = 0; // Retain ownership of the packet buffer so that the FileDesignator and Metadata pointers remain valid. System::PacketBufferHandle Buffer; diff --git a/src/protocols/bdx/BdxTransferSession.h b/src/protocols/bdx/BdxTransferSession.h index 0e2b4a9bd5309f..e37e463dc60211 100644 --- a/src/protocols/bdx/BdxTransferSession.h +++ b/src/protocols/bdx/BdxTransferSession.h @@ -52,7 +52,7 @@ class DLL_EXPORT TransferSession // Additional metadata (optional, TLV format) const uint8_t * Metadata = nullptr; - uint16_t MetadataLength = 0; + size_t MetadataLength = 0; }; struct TransferAcceptData @@ -65,7 +65,7 @@ class DLL_EXPORT TransferSession // Additional metadata (optional, TLV format) const uint8_t * Metadata = nullptr; - uint16_t MetadataLength = 0; + size_t MetadataLength = 0; }; struct StatusReportData @@ -76,7 +76,7 @@ class DLL_EXPORT TransferSession struct BlockData { const uint8_t * Data = nullptr; - uint16_t Length = 0; + size_t Length = 0; bool IsEof = false; }; @@ -332,7 +332,7 @@ class DLL_EXPORT TransferSession BlockData mBlockEventData; MessageTypeData mMsgTypeData; - uint32_t mNumBytesProcessed = 0; + size_t mNumBytesProcessed = 0; uint32_t mLastBlockNum = 0; uint32_t mNextBlockNum = 0; diff --git a/src/protocols/bdx/tests/TestBdxTransferSession.cpp b/src/protocols/bdx/tests/TestBdxTransferSession.cpp index 07f6d0a50c58e8..27d31dd3ffb78c 100644 --- a/src/protocols/bdx/tests/TestBdxTransferSession.cpp +++ b/src/protocols/bdx/tests/TestBdxTransferSession.cpp @@ -49,11 +49,11 @@ CHIP_ERROR WriteChipTLVString(uint8_t * buf, uint32_t bufLen, const char * data, // Helper method: read a TLV structure with a single tag and string and verify it matches expected string. CHIP_ERROR ReadAndVerifyTLVString(nlTestSuite * inSuite, void * inContext, const uint8_t * dataStart, uint32_t len, - const char * expected, uint16_t expectedLen) + const char * expected, size_t expectedLen) { TLV::TLVReader reader; - char tmp[64] = { 0 }; - uint32_t readLength = 0; + char tmp[64] = { 0 }; + size_t readLength = 0; VerifyOrReturnError(sizeof(tmp) > len, CHIP_ERROR_INTERNAL); reader.Init(dataStart, len); @@ -407,9 +407,9 @@ void TestInitiatingReceiverReceiverDrive(nlTestSuite * inSuite, void * inContext NL_TEST_ASSERT(inSuite, respondingSender.GetTransferBlockSize() == initiatingReceiver.GetTransferBlockSize()); // Verify parsed TLV metadata matches the original - err = - ReadAndVerifyTLVString(inSuite, inContext, outEvent.transferAcceptData.Metadata, outEvent.transferAcceptData.MetadataLength, - metadataStr, static_cast(strlen(metadataStr))); + err = ReadAndVerifyTLVString(inSuite, inContext, outEvent.transferAcceptData.Metadata, + static_cast(outEvent.transferAcceptData.MetadataLength), metadataStr, + static_cast(strlen(metadataStr))); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); // Test BlockQuery -> Block -> BlockAck @@ -491,8 +491,9 @@ void TestInitiatingSenderSenderDrive(nlTestSuite * inSuite, void * inContext) respondingReceiver, receiverOpts, transferBlockSize); // Verify parsed TLV metadata matches the original - err = ReadAndVerifyTLVString(inSuite, inContext, outEvent.transferInitData.Metadata, outEvent.transferInitData.MetadataLength, - metadataStr, static_cast(strlen(metadataStr))); + err = ReadAndVerifyTLVString(inSuite, inContext, outEvent.transferInitData.Metadata, + static_cast(outEvent.transferInitData.MetadataLength), metadataStr, + static_cast(strlen(metadataStr))); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); // Compose SendAccept parameters struct and give to respondingSender diff --git a/src/transport/raw/MessageHeader.cpp b/src/transport/raw/MessageHeader.cpp index 46d0c7f1276158..05418d9583f1dc 100644 --- a/src/transport/raw/MessageHeader.cpp +++ b/src/transport/raw/MessageHeader.cpp @@ -144,6 +144,7 @@ CHIP_ERROR PacketHeader::Decode(const uint8_t * const data, uint16_t size, uint1 CHIP_ERROR err = CHIP_NO_ERROR; LittleEndian::Reader reader(data, size); int version; + // TODO: De-uint16-ify everything related to this library uint16_t octets_read; uint16_t header; @@ -185,7 +186,7 @@ CHIP_ERROR PacketHeader::Decode(const uint8_t * const data, uint16_t size, uint1 err = reader.Read16(&mEncryptionKeyID).StatusCode(); SuccessOrExit(err); - octets_read = reader.OctetsRead(); + octets_read = static_cast(reader.OctetsRead()); VerifyOrExit(octets_read == EncodeSizeBytes(), err = CHIP_ERROR_INTERNAL); *decode_len = octets_read; @@ -245,7 +246,7 @@ CHIP_ERROR PayloadHeader::Decode(const uint8_t * const data, uint16_t size, uint mAckId.ClearValue(); } - octets_read = reader.OctetsRead(); + octets_read = static_cast(reader.OctetsRead()); VerifyOrExit(octets_read == EncodeSizeBytes(), err = CHIP_ERROR_INTERNAL); *decode_len = octets_read;