From 8ff314ba7362e39c13d69b3a26015baec63d04ce Mon Sep 17 00:00:00 2001 From: "RAKOTOARISOA Willy (Orange)" <101390517+willyrakotoarisoa@users.noreply.github.com> Date: Mon, 14 Mar 2022 14:34:02 +0100 Subject: [PATCH] [Chip-Cert] Prevent extra bytes to be encoded when converting key to chip-b64 (#16121) Observed : - Existing code encodes bytes beyond the buffer actually containing the key. - The issue was sizeof(P256SerializedKeypair) that includes 'length' field as well as 'bytes' field. Changes : - Make use of serializedKeypair.Length() to retrieve actual key length - Clarify usage of buffer sizes vs actual used lengths - Additionally, do not assume that P256SerializedKeypair 'bytes' field is the first field of the data structure (i.e. has implicitly same address as the structure itself) --- src/tools/chip-cert/KeyUtils.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/tools/chip-cert/KeyUtils.cpp b/src/tools/chip-cert/KeyUtils.cpp index 0bfa6e20d5541d..272c54e747bb3a 100644 --- a/src/tools/chip-cert/KeyUtils.cpp +++ b/src/tools/chip-cert/KeyUtils.cpp @@ -112,7 +112,7 @@ bool SerializeKeyPair(EVP_PKEY * key, P256SerializedKeypair & serializedKeypair) const EC_GROUP * group = nullptr; const EC_KEY * ecKey = nullptr; const EC_POINT * ecPoint = nullptr; - uint8_t * pubKey = serializedKeypair; + uint8_t * pubKey = serializedKeypair.Bytes(); uint8_t * privKey = pubKey + kP256_PublicKey_Length; size_t pubKeyLen = 0; int privKeyLen = 0; @@ -245,10 +245,10 @@ bool WritePrivateKey(const char * fileName, EVP_PKEY * key, KeyFormat keyFmt) uint8_t * keyToWrite = nullptr; uint32_t keyToWriteLen = 0; P256SerializedKeypair serializedKeypair; - uint32_t chipKeyLen = sizeof(serializedKeypair); - uint32_t chipKeyBase64Len = BASE64_ENCODED_LEN(chipKeyLen); - std::unique_ptr chipKey(new uint8_t[chipKeyLen]); - std::unique_ptr chipKeyBase64(new uint8_t[chipKeyBase64Len]); + uint32_t chipKeySize = serializedKeypair.Capacity(); + uint32_t chipKeyBase64Size = BASE64_ENCODED_LEN(chipKeySize); + std::unique_ptr chipKey(new uint8_t[chipKeySize]); + std::unique_ptr chipKeyBase64(new uint8_t[chipKeyBase64Size]); VerifyOrExit(key != nullptr, res = false); @@ -282,8 +282,10 @@ bool WritePrivateKey(const char * fileName, EVP_PKEY * key, KeyFormat keyFmt) if (keyFmt == kKeyFormat_Chip_Base64) { - res = Base64Encode(serializedKeypair, static_cast(sizeof(serializedKeypair)), chipKeyBase64.get(), - chipKeyBase64Len, chipKeyBase64Len); + uint32_t chipKeyBase64Len; + + res = Base64Encode(serializedKeypair.Bytes(), static_cast(serializedKeypair.Length()), chipKeyBase64.get(), + chipKeyBase64Size, chipKeyBase64Len); VerifyTrueOrExit(res); keyToWrite = chipKeyBase64.get(); @@ -292,7 +294,7 @@ bool WritePrivateKey(const char * fileName, EVP_PKEY * key, KeyFormat keyFmt) else { keyToWrite = chipKey.get(); - keyToWriteLen = chipKeyLen; + keyToWriteLen = static_cast(serializedKeypair.Length()); } if (fwrite(keyToWrite, 1, keyToWriteLen, file) != keyToWriteLen)