Skip to content

Commit

Permalink
[Chip-Cert] Prevent extra bytes to be encoded when converting key to …
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
willyrakotoarisoa authored and pull[bot] committed Nov 6, 2023
1 parent 1152b30 commit 8ff314b
Showing 1 changed file with 10 additions and 8 deletions.
18 changes: 10 additions & 8 deletions src/tools/chip-cert/KeyUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<uint8_t[]> chipKey(new uint8_t[chipKeyLen]);
std::unique_ptr<uint8_t[]> chipKeyBase64(new uint8_t[chipKeyBase64Len]);
uint32_t chipKeySize = serializedKeypair.Capacity();
uint32_t chipKeyBase64Size = BASE64_ENCODED_LEN(chipKeySize);
std::unique_ptr<uint8_t[]> chipKey(new uint8_t[chipKeySize]);
std::unique_ptr<uint8_t[]> chipKeyBase64(new uint8_t[chipKeyBase64Size]);

VerifyOrExit(key != nullptr, res = false);

Expand Down Expand Up @@ -282,8 +282,10 @@ bool WritePrivateKey(const char * fileName, EVP_PKEY * key, KeyFormat keyFmt)

if (keyFmt == kKeyFormat_Chip_Base64)
{
res = Base64Encode(serializedKeypair, static_cast<uint32_t>(sizeof(serializedKeypair)), chipKeyBase64.get(),
chipKeyBase64Len, chipKeyBase64Len);
uint32_t chipKeyBase64Len;

res = Base64Encode(serializedKeypair.Bytes(), static_cast<uint32_t>(serializedKeypair.Length()), chipKeyBase64.get(),
chipKeyBase64Size, chipKeyBase64Len);
VerifyTrueOrExit(res);

keyToWrite = chipKeyBase64.get();
Expand All @@ -292,7 +294,7 @@ bool WritePrivateKey(const char * fileName, EVP_PKEY * key, KeyFormat keyFmt)
else
{
keyToWrite = chipKey.get();
keyToWriteLen = chipKeyLen;
keyToWriteLen = static_cast<uint32_t>(serializedKeypair.Length());
}

if (fwrite(keyToWrite, 1, keyToWriteLen, file) != keyToWriteLen)
Expand Down

0 comments on commit 8ff314b

Please sign in to comment.