Turned On Wconversion for ASN1 Library and Coding Style Updates. (#9711)

* Turned On Wconversion for ASN1 Library and Coding Style Updates. -- Turned on -Wconversion in src/lib/asn1/BUILD.gn -- Added required SafeCast checks. -- Update code to support only short (uint8_t) ASN1 Tags. -- Updated ASN1 library conding style to use ReturnErrorCodeIf() VerifyOrReturnError() macros. -- Other minor updates. * Apply suggestions from code review Co-authored-by: Boris Zbarsky <[email protected]> * Addressed review comments. * Update src/lib/asn1/ASN1Writer.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Returned (ValueLen > sizeof(int64_t)) check in the ASN1Reader::GetInteger() method Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Sep 16, 2021 · 8e86c9e · 8e86c9e
1 parent 17d85c2
commit 8e86c9e
Show file tree

Hide file tree

Showing 9 changed files with 166 additions and 196 deletions.
diff --git a/src/credentials/CHIPCert.cpp b/src/credentials/CHIPCert.cpp
@@ -787,18 +787,15 @@ CHIP_ERROR ConvertIntegerDERToRaw(ByteSpan derInt, uint8_t * rawInt, const uint1
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, uint8_t * derSig, const uint16_t derSigBufSize,
-                                         uint16_t & derSigLen)
+CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, MutableByteSpan & derSig)
 {
     ASN1Writer writer;
 
-    VerifyOrReturnError(derSig != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
-
-    writer.Init(derSig, derSigBufSize);
+    writer.Init(derSig);
 
     ReturnErrorOnFailure(ConvertECDSASignatureRawToDER(rawSig, writer));
 
-    derSigLen = writer.GetLengthWritten();
+    derSig.reduce_size(writer.GetLengthWritten());
 
     return CHIP_NO_ERROR;
 }

diff --git a/src/credentials/CHIPCert.h b/src/credentials/CHIPCert.h
@@ -740,15 +740,14 @@ CHIP_ERROR ConvertIntegerDERToRaw(ByteSpan derInt, uint8_t * rawInt, const uint1
 /**
  * @brief Convert a raw CHIP signature to an ASN.1 DER encoded signature structure.
  *
- * @param rawSig        P256 ECDSA signature in raw form.
- * @param derSig        Buffer to store converted ASN.1 DER encoded signature.
- * @param derSigBufSize The size of the buffer to store ASN.1 DER encoded signature.
- * @param derSigLen     The length of the ASN.1 DER encoded signature.
+ * @param[in]     rawSig  P256 ECDSA signature in raw form.
+ * @param[in,out] derSig  Output buffer to receive the converted ASN.1 DER encoded signature.
+ *                        `derSig` must be at least `kMax_ECDSA_Signature_Length_Der` bytes long.
+ *                        The `derSig` size will be set to the actual DER encoded signature length on success.
  *
  * @retval  #CHIP_NO_ERROR  If the signature value was successfully converted.
  */
-CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, uint8_t * derSig, const uint16_t derSigBufSize,
-                                         uint16_t & derSigLen);
+CHIP_ERROR ConvertECDSASignatureRawToDER(P256ECDSASignatureSpan rawSig, MutableByteSpan & derSig);
 
 /**
  * @brief Convert a raw CHIP ECDSA signature to an ASN.1 DER encoded signature structure.

diff --git a/src/credentials/CHIPCertToX509.cpp b/src/credentials/CHIPCertToX509.cpp
@@ -58,7 +58,7 @@ static CHIP_ERROR DecodeConvertDN(TLVReader & reader, ASN1Writer & writer, ChipD
     uint64_t tlvTag;
     uint32_t tlvTagNum;
     OID attrOID;
-    uint32_t asn1Tag;
+    uint8_t asn1Tag;
     const uint8_t * asn1AttrVal;
     uint32_t asn1AttrValLen;
     uint8_t chipAttrStr[17];

diff --git a/src/credentials/GenerateChipX509Cert.cpp b/src/credentials/GenerateChipX509Cert.cpp
@@ -320,12 +320,8 @@ CHIP_ERROR EncodeChipECDSASignature(Crypto::P256ECDSASignature & signature, ASN1
     ASN1_START_BIT_STRING_ENCAPSULATED
     {
         // Convert RAW signature to DER when generating X509 certs.
-        uint8_t sig_der[Crypto::kMax_ECDSA_Signature_Length_Der];
-        uint16_t sig_der_size = 0;
         P256ECDSASignatureSpan raw_sig(signature.Bytes());
-
-        ReturnErrorOnFailure(ConvertECDSASignatureRawToDER(raw_sig, &sig_der[0], sizeof(sig_der), sig_der_size));
-        ReturnErrorOnFailure(writer.PutConstructedType(&sig_der[0], static_cast<uint16_t>(sig_der_size)));
+        ReturnErrorOnFailure(ConvertECDSASignatureRawToDER(raw_sig, writer));
     }
     ASN1_END_ENCAPSULATED;
 

diff --git a/src/lib/asn1/ASN1.h b/src/lib/asn1/ASN1.h
@@ -61,7 +61,7 @@ enum ASN1TagClasses
     kASN1TagClass_Private         = 0xC0
 };
 
-enum ASN1UniversalTags
+enum ASN1UniversalTags : uint8_t
 {
     kASN1UniversalTag_Boolean         = 1,
     kASN1UniversalTag_Integer         = 2,
@@ -111,7 +111,7 @@ class DLL_EXPORT ASN1Reader
     }
 
     uint8_t GetClass(void) const { return Class; };
-    uint32_t GetTag(void) const { return Tag; };
+    uint8_t GetTag(void) const { return Tag; };
     const uint8_t * GetValue(void) const { return Value; };
     uint32_t GetValueLen(void) const { return ValueLen; };
     bool IsConstructed(void) const { return Constructed; };
@@ -145,7 +145,7 @@ class DLL_EXPORT ASN1Reader
     };
 
     uint8_t Class;
-    uint32_t Tag;
+    uint8_t Tag;
     const uint8_t * Value;
     uint32_t ValueLen;
     bool Constructed;
@@ -177,28 +177,28 @@ class DLL_EXPORT ASN1Writer
         Init(data, N);
     }
     void InitNullWriter(void);
-    uint16_t GetLengthWritten(void) const;
+    size_t GetLengthWritten(void) const;
 
     CHIP_ERROR PutInteger(int64_t val);
     CHIP_ERROR PutBoolean(bool val);
     CHIP_ERROR PutObjectId(const uint8_t * val, uint16_t valLen);
     CHIP_ERROR PutObjectId(OID oid);
-    CHIP_ERROR PutString(uint32_t tag, const char * val, uint16_t valLen);
+    CHIP_ERROR PutString(uint8_t tag, const char * val, uint16_t valLen);
     CHIP_ERROR PutOctetString(const uint8_t * val, uint16_t valLen);
-    CHIP_ERROR PutOctetString(uint8_t cls, uint32_t tag, const uint8_t * val, uint16_t valLen);
-    CHIP_ERROR PutOctetString(uint8_t cls, uint32_t tag, chip::TLV::TLVReader & tlvReader);
+    CHIP_ERROR PutOctetString(uint8_t cls, uint8_t tag, const uint8_t * val, uint16_t valLen);
+    CHIP_ERROR PutOctetString(uint8_t cls, uint8_t tag, chip::TLV::TLVReader & tlvReader);
     CHIP_ERROR PutBitString(uint32_t val);
     CHIP_ERROR PutBitString(uint8_t unusedBits, const uint8_t * val, uint16_t valLen);
     CHIP_ERROR PutBitString(uint8_t unusedBits, chip::TLV::TLVReader & tlvReader);
     CHIP_ERROR PutTime(const ASN1UniversalTime & val);
     CHIP_ERROR PutNull(void);
     CHIP_ERROR PutConstructedType(const uint8_t * val, uint16_t valLen);
-    CHIP_ERROR StartConstructedType(uint8_t cls, uint32_t tag);
+    CHIP_ERROR StartConstructedType(uint8_t cls, uint8_t tag);
     CHIP_ERROR EndConstructedType(void);
-    CHIP_ERROR StartEncapsulatedType(uint8_t cls, uint32_t tag, bool bitStringEncoding);
+    CHIP_ERROR StartEncapsulatedType(uint8_t cls, uint8_t tag, bool bitStringEncoding);
     CHIP_ERROR EndEncapsulatedType(void);
-    CHIP_ERROR PutValue(uint8_t cls, uint32_t tag, bool isConstructed, const uint8_t * val, uint16_t valLen);
-    CHIP_ERROR PutValue(uint8_t cls, uint32_t tag, bool isConstructed, chip::TLV::TLVReader & tlvReader);
+    CHIP_ERROR PutValue(uint8_t cls, uint8_t tag, bool isConstructed, const uint8_t * val, uint16_t valLen);
+    CHIP_ERROR PutValue(uint8_t cls, uint8_t tag, bool isConstructed, chip::TLV::TLVReader & tlvReader);
 
 private:
     static constexpr size_t kMaxDeferredLengthDepth = kMaxConstructedAndEncapsulatedTypesDepth;
@@ -209,7 +209,7 @@ class DLL_EXPORT ASN1Writer
     uint8_t * mDeferredLengthLocations[kMaxDeferredLengthDepth];
     uint8_t mDeferredLengthCount;
 
-    CHIP_ERROR EncodeHead(uint8_t cls, uint32_t tag, bool isConstructed, int32_t len);
+    CHIP_ERROR EncodeHead(uint8_t cls, uint8_t tag, bool isConstructed, int32_t len);
     CHIP_ERROR WriteDeferredLength(void);
     static uint8_t BytesForLength(int32_t len);
     static void EncodeLength(uint8_t * buf, uint8_t bytesForLen, int32_t lenToEncode);

diff --git a/src/lib/asn1/ASN1OID.cpp b/src/lib/asn1/ASN1OID.cpp
@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2020 Project CHIP Authors
+ *    Copyright (c) 2020-2021 Project CHIP Authors
  *    Copyright (c) 2013-2017 Nest Labs, Inc.
  *    All rights reserved.
  *
@@ -30,6 +30,7 @@
 #include <string.h>
 
 #include <lib/support/DLLUtil.h>
+#include <lib/support/SafeInt.h>
 
 #define ASN1_DEFINE_OID_TABLE
 #define ASN1_DEFINE_OID_NAME_TABLE
@@ -41,58 +42,76 @@ namespace ASN1 {
 DLL_EXPORT OID ParseObjectID(const uint8_t * encodedOID, uint16_t encodedOIDLen)
 {
     if (encodedOID == nullptr or encodedOIDLen == 0)
+    {
         return kOID_NotSpecified;
+    }
 
     for (uint32_t i = 0; i < sOIDTableSize; i++)
+    {
         if (encodedOIDLen == sOIDTable[i].EncodedOIDLen && memcmp(encodedOID, sOIDTable[i].EncodedOID, encodedOIDLen) == 0)
+        {
             return sOIDTable[i].EnumVal;
+        }
+    }
 
     return kOID_Unknown;
 }
 
 bool GetEncodedObjectID(OID oid, const uint8_t *& encodedOID, uint16_t & encodedOIDLen)
 {
     for (uint32_t i = 0; i < sOIDTableSize; i++)
+    {
         if (oid == sOIDTable[i].EnumVal)
         {
             encodedOID    = sOIDTable[i].EncodedOID;
             encodedOIDLen = sOIDTable[i].EncodedOIDLen;
             return true;
         }
+    }
 
     return false;
 }
 
 OIDCategory GetOIDCategory(OID oid)
 {
     if (oid == kOID_Unknown)
+    {
         return kOIDCategory_Unknown;
+    }
     if (oid == kOID_NotSpecified)
+    {
         return kOIDCategory_NotSpecified;
-    return (OIDCategory)(oid & kOIDCategory_Mask);
+    }
+    return static_cast<OIDCategory>(oid & kOIDCategory_Mask);
 }
 
 const char * GetOIDName(OID oid)
 {
     if (oid == kOID_Unknown)
+    {
         return "Unknown";
+    }
     if (oid == kOID_NotSpecified)
+    {
         return "NotSpecified";
+    }
     for (uint32_t i = 0; i < sOIDTableSize; i++)
+    {
         if (oid == sOIDNameTable[i].EnumVal)
+        {
             return sOIDNameTable[i].Name;
+        }
+    }
     return "Unknown";
 }
 
 CHIP_ERROR ASN1Reader::GetObjectId(OID & oid)
 {
-    if (Value == nullptr)
-        return ASN1_ERROR_INVALID_STATE;
-    if (ValueLen < 1)
-        return ASN1_ERROR_INVALID_ENCODING;
-    if (mElemStart + mHeadLen + ValueLen > mContainerEnd)
-        return ASN1_ERROR_UNDERRUN;
-    oid = ParseObjectID(Value, ValueLen);
+    ReturnErrorCodeIf(Value == nullptr, ASN1_ERROR_INVALID_STATE);
+    ReturnErrorCodeIf(ValueLen < 1, ASN1_ERROR_INVALID_ENCODING);
+    ReturnErrorCodeIf(mElemStart + mHeadLen + ValueLen > mContainerEnd, ASN1_ERROR_UNDERRUN);
+    VerifyOrReturnError(CanCastTo<uint16_t>(ValueLen), ASN1_ERROR_INVALID_ENCODING);
+    oid = ParseObjectID(Value, static_cast<uint16_t>(ValueLen));
     return CHIP_NO_ERROR;
 }
 
@@ -101,8 +120,7 @@ CHIP_ERROR ASN1Writer::PutObjectId(OID oid)
     const uint8_t * encodedOID;
     uint16_t encodedOIDLen;
 
-    if (!GetEncodedObjectID(oid, encodedOID, encodedOIDLen))
-        return ASN1_ERROR_UNKNOWN_OBJECT_ID;
+    VerifyOrReturnError(GetEncodedObjectID(oid, encodedOID, encodedOIDLen), ASN1_ERROR_UNKNOWN_OBJECT_ID);
 
     return PutObjectId(encodedOID, encodedOIDLen);
 }