Skip to content

Commit

Permalink
Code review 2
Browse files Browse the repository at this point in the history
  • Loading branch information
@ArekBalysNordic
ArekBalysNordic committed Jan 2, 2024
1 parent 73682ad commit eeb097f
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 66 deletions.
6 changes: 0 additions & 6 deletions config/nrfconnect/chip-module/Kconfig.defaults
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,6 @@ config CHIP_MALLOC_SYS_HEAP_SIZE
config NVS_LOOKUP_CACHE_SIZE
default 512

# See config/zephyr/Kconfig for full definition
config CHIP_FACTORY_RESET_ERASE_NVS
# For now erasing whole NVS sector is not supported for PSA Crypto
default y if !CHIP_CRYPTO_PSA


# ==============================================================================
# Zephyr networking configuration
# ==============================================================================
Expand Down
2 changes: 0 additions & 2 deletions src/platform/nrfconnect/CHIPPlatformConfig.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,9 @@
#endif // CHIP_CONFIG_SHA256_CONTEXT_ALIGN
#endif // CONFIG_CHIP_CRYPTO_PSA

#ifdef CONFIG_CHIP_CRYPTO_PSA
#ifndef CHIP_CONFIG_CRYPTO_PSA_KEY_ID_BASE
#define CHIP_CONFIG_CRYPTO_PSA_KEY_ID_BASE 0x30000
#endif // CHIP_CONFIG_CRYPTO_PSA_KEY_ID_BASE
#endif // CONFIG_CHIP_CRYPTO_PSA

// ==================== General Configuration Overrides ====================

Expand Down
55 changes: 6 additions & 49 deletions src/platform/nrfconnect/FactoryDataParser.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,8 @@
#include <ctype.h>
#include <string.h>

#define zcbor_tstr_put_lit_cast(state, string) zcbor_tstr_encode_ptr(state, (char *) string, sizeof(string) - 1)

#define MAX_FACTORY_DATA_NESTING_LEVEL 2

static inline void updateSize(struct FactoryData * factoryData, const char * name, size_t len)
{
if (factoryData)
{
if (name)
{
// Add name string length + 1, actual length of the entry and 1 Byte of CBOR item header.
factoryData->actualSize += strlen(name) + 1 + len + 1;
}
else
{
factoryData->actualSize += len + 1;
}
}
}

static inline bool uint16_decode(zcbor_state_t * states, uint16_t * value)
{
uint32_t u32;
Expand Down Expand Up @@ -152,7 +134,6 @@ bool ParseFactoryData(uint8_t * buffer, uint16_t bufferSize, struct FactoryData
ZCBOR_STATE_D(states, MAX_FACTORY_DATA_NESTING_LEVEL, buffer, bufferSize, 1);

bool res = zcbor_map_start_decode(states);
updateSize(factoryData, NULL, 0);
struct zcbor_string currentString;

while (res)
Expand All @@ -168,54 +149,44 @@ bool ParseFactoryData(uint8_t * buffer, uint16_t bufferSize, struct FactoryData
if (strncmp("version", (const char *) currentString.value, currentString.len) == 0)
{
res = res && uint16_decode(states, &factoryData->version);
updateSize(factoryData, "version", sizeof(uint32_t));
}
else if (strncmp("hw_ver", (const char *) currentString.value, currentString.len) == 0)
{
res = res && uint16_decode(states, &factoryData->hw_ver);
factoryData->hwVerPresent = res;
updateSize(factoryData, "hw_ver", sizeof(uint32_t));
}
else if (strncmp("spake2_it", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_uint32_decode(states, &factoryData->spake2_it);
updateSize(factoryData, "spake2_it", sizeof(uint32_t));
}
else if (strncmp("vendor_id", (const char *) currentString.value, currentString.len) == 0)
{
res = res && uint16_decode(states, &factoryData->vendor_id);
factoryData->vendorIdPresent = res;
updateSize(factoryData, "vendor_id", sizeof(uint32_t));
}
else if (strncmp("product_id", (const char *) currentString.value, currentString.len) == 0)
{
res = res && uint16_decode(states, &factoryData->product_id);
factoryData->productIdPresent = res;
updateSize(factoryData, "product_id", sizeof(uint32_t));
}
else if (strncmp("discriminator", (const char *) currentString.value, currentString.len) == 0)
{
res = res && uint16_decode(states, &factoryData->discriminator);
factoryData->discriminatorPresent = res;
updateSize(factoryData, "discriminator", sizeof(uint32_t));
}
else if (strncmp("passcode", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_uint32_decode(states, &factoryData->passcode);

updateSize(factoryData, "passcode", sizeof(uint32_t));
}
else if (strncmp("sn", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->sn);
updateSize(factoryData, "sn", factoryData->sn.len);
}
else if (strncmp("date", (const char *) currentString.value, currentString.len) == 0)
{
// Date format is YYYY-MM-DD, so format needs to be validated and string parse to integer parts.
struct zcbor_string date;
res = res && zcbor_bstr_decode(states, &date);
updateSize(factoryData, "date", date.len);
if (date.len == 10 && isdigit(date.value[0]) && isdigit(date.value[1]) && isdigit(date.value[2]) &&
isdigit(date.value[3]) && date.value[4] == '-' && isdigit(date.value[5]) && isdigit(date.value[6]) &&
date.value[7] == '-' && isdigit(date.value[8]) && isdigit(date.value[9]))
Expand All @@ -234,94 +205,80 @@ bool ParseFactoryData(uint8_t * buffer, uint16_t bufferSize, struct FactoryData
else if (strncmp("hw_ver_str", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->hw_ver_str);
updateSize(factoryData, "hw_ver_str", factoryData->hw_ver_str.len);
}
else if (strncmp("rd_uid", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->rd_uid);
updateSize(factoryData, "rd_uid", factoryData->rd_uid.len);
}
else if (strncmp("dac_cert", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->dac_cert);
updateSize(factoryData, "dac_cert", factoryData->dac_cert.len);
}
else if (strncmp("dac_key", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->dac_priv_key);
updateSize(factoryData, "dac_key", factoryData->dac_priv_key.len);
factoryData->dac_priv_key_offset = (size_t) ((void *) states->payload - (void *) buffer) - factoryData->dac_priv_key.len;
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->dac_priv_key);
factoryData->dac_priv_key_offset = (size_t) ((uint8_t *) factoryData->dac_priv_key.data - buffer);
}
else if (strncmp("pai_cert", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->pai_cert);
updateSize(factoryData, "pai_cert", factoryData->pai_cert.len);
}
else if (strncmp("spake2_salt", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->spake2_salt);
updateSize(factoryData, "spake2_salt", factoryData->spake2_salt.len);
}
else if (strncmp("spake2_verifier", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->spake2_verifier);
updateSize(factoryData, "spake2_verifier", factoryData->spake2_verifier.len);
}
else if (strncmp("vendor_name", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->vendor_name);
updateSize(factoryData, "vendor_name", factoryData->vendor_name.len);
}
else if (strncmp("product_name", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->product_name);
updateSize(factoryData, "product_name", factoryData->product_name.len);
}
else if (strncmp("part_number", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->part_number);
updateSize(factoryData, "part_number", factoryData->part_number.len);
}
else if (strncmp("product_url", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->product_url);
updateSize(factoryData, "product_url", factoryData->product_url.len);
}
else if (strncmp("product_label", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->product_label);
updateSize(factoryData, "product_label", factoryData->product_label.len);
}
else if (strncmp("enable_key", (const char *) currentString.value, currentString.len) == 0)
{
res = res && zcbor_bstr_decode(states, (struct zcbor_string *) &factoryData->enable_key);
updateSize(factoryData, "enable_key", factoryData->enable_key.len);
}
else if (strncmp("product_finish", (const char *) currentString.value, currentString.len) == 0)
{
res = res && uint8_decode(states, &factoryData->product_finish);
factoryData->productFinishPresent = res;
updateSize(factoryData, "product_finish", sizeof(uint32_t));
}
else if (strncmp("primary_color", (const char *) currentString.value, currentString.len) == 0)
{
res = res && uint8_decode(states, &factoryData->primary_color);
factoryData->primaryColorPresent = res;
updateSize(factoryData, "primary_color", sizeof(uint32_t));
}
else if (strncmp("user", (const char *) currentString.value, currentString.len) == 0)
{
factoryData->user.data = (void *) states->payload;
res = res && zcbor_any_skip(states, NULL);
factoryData->user.len = (size_t) ((void *) states->payload - factoryData->user.data);
updateSize(factoryData, "user", factoryData->user.len);
factoryData->userDataPresent = true;
}
else
{
res = res && zcbor_any_skip(states, NULL);
}
}

return res && zcbor_list_map_end_force_decode(states);
res = res && zcbor_list_map_end_force_decode(states);
factoryData->actualSize = (size_t) (states->payload - buffer);

return res;
}
1 change: 0 additions & 1 deletion src/platform/nrfconnect/FactoryDataParser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ struct FactoryData
bool discriminatorPresent;
bool productFinishPresent;
bool primaryColorPresent;
bool userDataPresent;
size_t actualSize;
size_t dac_priv_key_offset;
};
Expand Down
20 changes: 13 additions & 7 deletions src/platform/nrfconnect/FactoryDataProvider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,19 @@ CHIP_ERROR FactoryDataProvider<FlashFactoryData>::MoveDACPrivateKeyToSecureStora
size_t factoryDataSize)
{

CHIP_ERROR err = CHIP_NO_ERROR;
uint8_t ClearedDACPrivKey[kDACPrivateKeyLength];
memset(ClearedDACPrivKey, 0xFF, sizeof(ClearedDACPrivKey));

if (!factoryDataPartition || factoryDataSize == 0)
{
return CHIP_ERROR_INVALID_ARGUMENT;
}

if (mFactoryData.dac_priv_key.len != kDACPrivateKeyLength)
{
return CHIP_ERROR_INCORRECT_STATE;
}

uint8_t ClearedDACPrivKey[kDACPrivateKeyLength];
memset(ClearedDACPrivKey, 0xFF, sizeof(ClearedDACPrivKey));

// Check if factory data contains DAC private key
if (memcmp(mFactoryData.dac_priv_key.data, ClearedDACPrivKey, kDACPrivateKeyLength) != 0)
{
Expand All @@ -142,15 +146,17 @@ CHIP_ERROR FactoryDataProvider<FlashFactoryData>::MoveDACPrivateKeyToSecureStora
psa_set_key_algorithm(&attributes, PSA_ALG_ECDSA(PSA_ALG_SHA_256));
psa_set_key_lifetime(&attributes, PSA_KEY_LIFETIME_PERSISTENT);
psa_set_key_id(&attributes, mDACPrivKeyId);
psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_EXPORT | PSA_KEY_USAGE_SIGN_MESSAGE);
psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_SIGN_MESSAGE);

VerifyOrReturnError(psa_import_key(&attributes, reinterpret_cast<uint8_t *>(mFactoryData.dac_priv_key.data),
kDACPrivateKeyLength, &mDACPrivKeyId) == PSA_SUCCESS,
CHIP_ERROR_INTERNAL);
}

// Allocate needed memory space to perform removal and moving DAC private key
size_t alignedSize = mFactoryData.actualSize + (sizeof(uint32_t) - (mFactoryData.actualSize % sizeof(uint32_t)));
const flash_parameters * flashParameters = flash_get_parameters(kFlashDev);
VerifyOrReturnError(flashParameters, CHIP_ERROR_INTERNAL);
size_t alignedSize = ROUND_UP(mFactoryData.actualSize, flashParameters->write_block_size);
chip::Platform::ScopedMemoryBuffer<uint8_t> factoryDataBuff;
VerifyOrReturnError(factoryDataBuff.Calloc(alignedSize), CHIP_ERROR_NO_MEMORY);

Expand All @@ -162,7 +168,7 @@ CHIP_ERROR FactoryDataProvider<FlashFactoryData>::MoveDACPrivateKeyToSecureStora

// Replace the old factory data set with the new one.
VerifyOrReturnError(0 == flash_erase(kFlashDev, kFactoryDataPartitionAddress, kFactoryDataPartitionSize),
CHIP_ERROR_NO_MEMORY);
CHIP_ERROR_INTERNAL);
VerifyOrReturnError(0 == flash_write(kFlashDev, kFactoryDataPartitionAddress, factoryDataBuff.Get(), alignedSize),
CHIP_ERROR_INTERNAL);

Expand Down
1 change: 0 additions & 1 deletion src/platform/nrfconnect/FactoryDataProvider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ class FactoryDataProvider : public chip::Credentials::DeviceAttestationCredentia
CHIP_ERROR Init();
#ifdef CONFIG_CHIP_CRYPTO_PSA
CHIP_ERROR MoveDACPrivateKeyToSecureStorage(uint8_t * factoryDataPartition, size_t factoryDataSize);
CHIP_ERROR ImportDACPrivateKey();
#endif

// ===== Members functions that implement the DeviceAttestationCredentialsProvider
Expand Down

0 comments on commit eeb097f

Please sign in to comment.