From 23287528dd63e194ce69646e7d34a1f622363d4c Mon Sep 17 00:00:00 2001 From: andrei-menzopol <96489227+andrei-menzopol@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:42:53 +0300 Subject: [PATCH] [NXP][k32w1] Remove the need for extra DAC private key conversion binary (#32699) * [NXP][k32w1] Add SSS DAC private key conversion at init / Use plain-text DAC key * Do SSS DAC private key conversion at initialization if needed. * Do not use extra SSS conversion binary anymore. * Add option to use DAC private key in plain text. Signed-off-by: Andrei Menzopol * [NXP][k32w1] Update manufacturing flow to not use chip_convert_dac_private_key * Using chip_convert_dac_private_key option is not needed anymore. * Use chip_use_plain_dac_key=true for plain-text DAC private key. Signed-off-by: Andrei Menzopol * [NXP][k32w1] Remove example_convert_dac_private_key.jlink * Using chip_convert_dac_private_key option is not needed anymore, making the jlink script obsolete. Signed-off-by: Andrei Menzopol * Restyled by prettier-markdown * Restyled by prettier-markdown * Check SSS_ExportBlob return status Signed-off-by: Andrei Menzopol --------- Signed-off-by: Andrei Menzopol Co-authored-by: Restyled.io --- docs/guides/nxp_manufacturing_flow.md | 28 +++---- .../example_convert_dac_private_key.jlink | 16 ---- src/platform/nxp/k32w/k32w1/BUILD.gn | 10 ++- .../k32w/k32w1/FactoryDataProviderImpl.cpp | 83 ++++++++++++++++--- .../nxp/k32w/k32w1/FactoryDataProviderImpl.h | 28 ++----- src/platform/nxp/k32w/k32w1/args.gni | 2 +- 6 files changed, 96 insertions(+), 71 deletions(-) delete mode 100644 scripts/tools/nxp/factory_data_generator/k32w1/example_convert_dac_private_key.jlink diff --git a/docs/guides/nxp_manufacturing_flow.md b/docs/guides/nxp_manufacturing_flow.md index 6cc3005b412289..bad70db294a9ae 100644 --- a/docs/guides/nxp_manufacturing_flow.md +++ b/docs/guides/nxp_manufacturing_flow.md @@ -215,26 +215,15 @@ converted to an encrypted blob. This blob will overwrite the DAC private key in factory data and will be imported in the `SSS` at initialization, by the factory data provider instance. -The conversion process shall happen at manufacturing time and should be run one -time only: - -- Write factory data binary. -- Build the application with - `chip_with_factory_data=1 chip_convert_dac_private_key=1` set. -- Write the application to the board and let it run. - -After the conversion process: +The application will check at initialization whether the DAC private key has +been converted or not and convert it if needed. However, the conversion process +should be done at manufacturing time for security reasons. -- Make sure the application is built with `chip_with_factory_data=1`, but - without `chip_convert_dac_private_key` arg, since conversion already - happened. -- Write the application to the board. - -If you are using Jlink, you can see a conversion script example in: +There is no need for an extra binary. -```shell -./scripts/tools/nxp/factory_data_generator/k32w1/example_convert_dac_private_key.jlink -``` +- Write factory data binary. +- Build the application with `chip_with_factory_data=1` set. +- Write the application to the board and use it as usual. Factory data should now contain a corresponding encrypted blob instead of the DAC private key. @@ -251,6 +240,9 @@ python3 ./scripts/tools/nxp/factory_data_generator/generate.py -i 10000 -s UXKLz Please note that `--dac_key` now points to a binary file that contains the encrypted blob. +The user can use the DAC private in plain text instead of using the `SSS` by +adding the following gn argument `chip_use_plain_dac_key=true`. + ### 6.2 RW61X Supported platforms: diff --git a/scripts/tools/nxp/factory_data_generator/k32w1/example_convert_dac_private_key.jlink b/scripts/tools/nxp/factory_data_generator/k32w1/example_convert_dac_private_key.jlink deleted file mode 100644 index d31c1370933ace..00000000000000 --- a/scripts/tools/nxp/factory_data_generator/k32w1/example_convert_dac_private_key.jlink +++ /dev/null @@ -1,16 +0,0 @@ -reset -halt -// Factory data size is one internal flash sector (8K). -// Factory data address is retrieved from the map file. -erase 0xec000 0xee000 noreset -// Load factory data and conversion application, then -// wait for 10 seconds and load the "real" application. -loadfile factory_data.bin 0xec000 -loadfile chip-k32w1-light-example-before-conversion.srec -reset -go -Sleep 10000 -loadfile chip-k32w1-light-example-after-conversion.srec -reset -go -quit \ No newline at end of file diff --git a/src/platform/nxp/k32w/k32w1/BUILD.gn b/src/platform/nxp/k32w/k32w1/BUILD.gn index 53a4b4dce10509..30352203eebaaa 100644 --- a/src/platform/nxp/k32w/k32w1/BUILD.gn +++ b/src/platform/nxp/k32w/k32w1/BUILD.gn @@ -61,6 +61,12 @@ static_library("nxp_platform") { "ram_storage.h", ] + if (chip_use_plain_dac_key) { + defines += [ "CHIP_USE_PLAIN_DAC_KEY=1" ] + } else { + defines += [ "CHIP_USE_PLAIN_DAC_KEY=0" ] + } + public = [ "${chip_root}/src/credentials/DeviceAttestationCredsProvider.h", "${chip_root}/src/credentials/examples/DeviceAttestationCredsExample.h", @@ -107,10 +113,6 @@ static_library("nxp_platform") { "${chip_root}/src/credentials/CHIPCert.h", "${chip_root}/src/credentials/CertificationDeclaration.h", ] - - if (chip_convert_dac_private_key == 1) { - defines += [ "CHIP_DEVICE_CONFIG_SECURE_DAC_PRIVATE_KEY=1" ] - } } public_deps += [ "${mbedtls_root}:mbedtls" ] diff --git a/src/platform/nxp/k32w/k32w1/FactoryDataProviderImpl.cpp b/src/platform/nxp/k32w/k32w1/FactoryDataProviderImpl.cpp index c3c913858ee5f7..764e3f6ac48300 100644 --- a/src/platform/nxp/k32w/k32w1/FactoryDataProviderImpl.cpp +++ b/src/platform/nxp/k32w/k32w1/FactoryDataProviderImpl.cpp @@ -15,29 +15,30 @@ * limitations under the License. */ -#include - -#if CHIP_DEVICE_CONFIG_SECURE_DAC_PRIVATE_KEY #include "fsl_adapter_flash.h" -#endif +#include namespace chip { namespace DeviceLayer { +#if !CHIP_USE_PLAIN_DAC_KEY // SSS adds 24 bytes of metadata when creating the blob static constexpr size_t kSssBlobMetadataLength = 24; static constexpr size_t kPrivateKeyBlobLength = Crypto::kP256_PrivateKey_Length + kSssBlobMetadataLength; +#endif FactoryDataProviderImpl::~FactoryDataProviderImpl() { +#if !CHIP_USE_PLAIN_DAC_KEY SSS_KEY_OBJ_FREE(&mContext); +#endif } CHIP_ERROR FactoryDataProviderImpl::Init() { CHIP_ERROR error = CHIP_NO_ERROR; -#if CHIP_DEVICE_CONFIG_ENABLE_SSS_API_TEST +#if CHIP_DEVICE_CONFIG_ENABLE_SSS_API_TEST && !CHIP_USE_PLAIN_DAC_KEY SSS_RunApiTest(); #endif @@ -47,16 +48,56 @@ CHIP_ERROR FactoryDataProviderImpl::Init() ChipLogError(DeviceLayer, "Factory data init failed with: %s", ErrorStr(error)); } +#if !CHIP_USE_PLAIN_DAC_KEY ReturnErrorOnFailure(SSS_InitContext()); -#if CHIP_DEVICE_CONFIG_SECURE_DAC_PRIVATE_KEY ReturnErrorOnFailure(SSS_ConvertDacKey()); - ReturnErrorOnFailure(Validate()); -#endif ReturnErrorOnFailure(SSS_ImportPrivateKeyBlob()); +#endif return error; } +#if CHIP_USE_PLAIN_DAC_KEY +CHIP_ERROR FactoryDataProviderImpl::SignWithDacKey(const ByteSpan & messageToSign, MutableByteSpan & outSignBuffer) +{ + CHIP_ERROR error = CHIP_NO_ERROR; + Crypto::P256ECDSASignature signature; + Crypto::P256Keypair keypair; + Crypto::P256SerializedKeypair serializedKeypair; + uint8_t keyBuf[Crypto::kP256_PrivateKey_Length]; + MutableByteSpan dacPrivateKeySpan(keyBuf); + uint16_t keySize = 0; + + VerifyOrExit(!outSignBuffer.empty(), error = CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrExit(!messageToSign.empty(), error = CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrExit(outSignBuffer.size() >= signature.Capacity(), error = CHIP_ERROR_BUFFER_TOO_SMALL); + + /* Get private key of DAC certificate from reserved section */ + error = SearchForId(FactoryDataId::kDacPrivateKeyId, dacPrivateKeySpan.data(), dacPrivateKeySpan.size(), keySize); + SuccessOrExit(error); + dacPrivateKeySpan.reduce_size(keySize); + VerifyOrExit(keySize == Crypto::kP256_PrivateKey_Length, error = CHIP_ERROR_WRONG_KEY_TYPE); + + /* Only the private key is used when signing */ + error = serializedKeypair.SetLength(Crypto::kP256_PublicKey_Length + dacPrivateKeySpan.size()); + SuccessOrExit(error); + memcpy(serializedKeypair.Bytes() + Crypto::kP256_PublicKey_Length, dacPrivateKeySpan.data(), dacPrivateKeySpan.size()); + + error = keypair.Deserialize(serializedKeypair); + SuccessOrExit(error); + + error = keypair.ECDSA_sign_msg(messageToSign.data(), messageToSign.size(), signature); + SuccessOrExit(error); + + error = CopySpanToMutableSpan(ByteSpan{ signature.ConstBytes(), signature.Length() }, outSignBuffer); + +exit: + /* Sanitize temporary buffer */ + memset(keyBuf, 0, Crypto::kP256_PrivateKey_Length); + return error; +} + +#else CHIP_ERROR FactoryDataProviderImpl::SignWithDacKey(const ByteSpan & messageToSign, MutableByteSpan & outSignBuffer) { Crypto::P256ECDSASignature signature; @@ -118,7 +159,6 @@ CHIP_ERROR FactoryDataProviderImpl::SSS_Sign(uint8_t * digest, Crypto::P256ECDSA return error; } -#if CHIP_DEVICE_CONFIG_SECURE_DAC_PRIVATE_KEY CHIP_ERROR FactoryDataProviderImpl::SSS_ConvertDacKey() { size_t blobSize = kPrivateKeyBlobLength; @@ -126,10 +166,18 @@ CHIP_ERROR FactoryDataProviderImpl::SSS_ConvertDacKey() uint8_t blob[kPrivateKeyBlobLength] = { 0 }; uint8_t * data = static_cast(chip::Platform::MemoryAlloc(newSize)); uint32_t offset = 0; + bool convNeeded = true; VerifyOrReturnError(data != nullptr, CHIP_ERROR_INTERNAL); - ReturnErrorOnFailure(SSS_ExportBlob(blob, &blobSize, offset)); + ReturnErrorOnFailure(SSS_ExportBlob(blob, &blobSize, offset, convNeeded)); + if (!convNeeded) + { + ChipLogError(DeviceLayer, "SSS: DAC private key already converted to blob"); + chip::Platform::MemoryFree(data); + return CHIP_NO_ERROR; + } + ChipLogError(DeviceLayer, "SSS: extracted blob from DAC private key"); hal_flash_status_t status = HAL_FlashRead(kFactoryDataStart, newSize - kSssBlobMetadataLength, data); @@ -149,22 +197,31 @@ CHIP_ERROR FactoryDataProviderImpl::SSS_ConvertDacKey() chip::Platform::MemoryFree(data); ChipLogError(DeviceLayer, "SSS: sanitized RAM cache"); + ReturnErrorOnFailure(Validate()); + return CHIP_NO_ERROR; } -CHIP_ERROR FactoryDataProviderImpl::SSS_ExportBlob(uint8_t * data, size_t * dataLen, uint32_t & offset) +CHIP_ERROR FactoryDataProviderImpl::SSS_ExportBlob(uint8_t * data, size_t * dataLen, uint32_t & offset, bool & isNeeded) { CHIP_ERROR error = CHIP_NO_ERROR; auto res = kStatus_SSS_Success; - uint8_t keyBuf[Crypto::kP256_PrivateKey_Length]; + uint8_t keyBuf[kPrivateKeyBlobLength]; MutableByteSpan dacPrivateKeySpan(keyBuf); uint16_t keySize = 0; + isNeeded = true; error = SearchForId(FactoryDataId::kDacPrivateKeyId, dacPrivateKeySpan.data(), dacPrivateKeySpan.size(), keySize, &offset); SuccessOrExit(error); dacPrivateKeySpan.reduce_size(keySize); + if (keySize == kPrivateKeyBlobLength) + { + isNeeded = false; + return CHIP_NO_ERROR; + } + res = SSS_KEY_STORE_SET_KEY(&mContext, dacPrivateKeySpan.data(), Crypto::kP256_PrivateKey_Length, keySize * 8, kSSS_KeyPart_Private); VerifyOrExit(res == kStatus_SSS_Success, error = CHIP_ERROR_INTERNAL); @@ -197,7 +254,6 @@ CHIP_ERROR FactoryDataProviderImpl::ReplaceWithBlob(uint8_t * data, uint8_t * bl return CHIP_NO_ERROR; } -#endif // CHIP_DEVICE_CONFIG_SECURE_DAC_PRIVATE_KEY #if CHIP_DEVICE_CONFIG_ENABLE_SSS_API_TEST @@ -298,6 +354,7 @@ void FactoryDataProviderImpl::SSS_RunApiTest() SSS_KEY_OBJ_FREE(&mContext); } #endif // CHIP_DEVICE_CONFIG_ENABLE_SSS_API_TEST +#endif // CHIP_USE_PLAIN_DAC_KEY } // namespace DeviceLayer } // namespace chip diff --git a/src/platform/nxp/k32w/k32w1/FactoryDataProviderImpl.h b/src/platform/nxp/k32w/k32w1/FactoryDataProviderImpl.h index 67bef1959ea503..f81b5d141837df 100644 --- a/src/platform/nxp/k32w/k32w1/FactoryDataProviderImpl.h +++ b/src/platform/nxp/k32w/k32w1/FactoryDataProviderImpl.h @@ -19,21 +19,8 @@ #include #include +#if !CHIP_USE_PLAIN_DAC_KEY #include "sss_crypto.h" - -/* This flag should be defined when the factory data contains - * the DAC private key in plain text. It usually occurs in - * manufacturing. - * - * The init phase will use S200 to export an encrypted blob, - * then overwrite the private key section from internal flash. - * - * Should be used one time only for securing the private key. - * The manufacturer will then flash the real image, which shall - * not define this flag. - */ -#ifndef CHIP_DEVICE_CONFIG_SECURE_DAC_PRIVATE_KEY -#define CHIP_DEVICE_CONFIG_SECURE_DAC_PRIVATE_KEY 0 #endif /* This flag should be defined to run SSS_RunApiTest tests. @@ -59,12 +46,13 @@ class FactoryDataProviderImpl : public FactoryDataProvider CHIP_ERROR SignWithDacKey(const ByteSpan & messageToSign, MutableByteSpan & outSignBuffer) override; private: +#if !CHIP_USE_PLAIN_DAC_KEY + CHIP_ERROR SSS_InitContext(); CHIP_ERROR SSS_ImportPrivateKeyBlob(); CHIP_ERROR SSS_Sign(uint8_t * digest, Crypto::P256ECDSASignature & signature); -#if CHIP_DEVICE_CONFIG_SECURE_DAC_PRIVATE_KEY /*! - * \brief Convert DAC private key to an SSS encrypted blob and update factory data + * \brief Convert DAC private key to an SSS encrypted blob and update factory data if not already done * * @note This API should be called in manufacturing process context to replace * DAC private key with an SSS encrypted blob. The conversion will be a @@ -74,15 +62,16 @@ class FactoryDataProviderImpl : public FactoryDataProvider CHIP_ERROR SSS_ConvertDacKey(); /*! - * \brief Export an SSS encrypted blob from the DAC private key found in factory data + * \brief Check and export an SSS encrypted blob from the DAC private key found in factory data if needed * * @param data Pointer to an allocated buffer * @param dataLen Pointer to a variable that will store the blob length * @param offset Offset of private key from the start of factory data payload address (after header) + * @param isNeeded Will be set to true if conversion is needed * * @retval #CHIP_NO_ERROR if conversion to blob was successful. */ - CHIP_ERROR SSS_ExportBlob(uint8_t * data, size_t * dataLen, uint32_t & offset); + CHIP_ERROR SSS_ExportBlob(uint8_t * data, size_t * dataLen, uint32_t & offset, bool & isNeeded); /*! * \brief Replace DAC private key with the specified SSS encrypted blob @@ -97,12 +86,13 @@ class FactoryDataProviderImpl : public FactoryDataProvider * @retval #CHIP_NO_ERROR if conversion to blob was successful. */ CHIP_ERROR ReplaceWithBlob(uint8_t * data, uint8_t * blob, size_t blobLen, uint32_t offset); -#endif + #if CHIP_DEVICE_CONFIG_ENABLE_SSS_API_TEST void SSS_RunApiTest(); #endif sss_sscp_object_t mContext; +#endif // CHIP_USE_PLAIN_DAC_KEY }; } // namespace DeviceLayer diff --git a/src/platform/nxp/k32w/k32w1/args.gni b/src/platform/nxp/k32w/k32w1/args.gni index 2de35770927a63..8b90982be93ca3 100644 --- a/src/platform/nxp/k32w/k32w1/args.gni +++ b/src/platform/nxp/k32w/k32w1/args.gni @@ -19,8 +19,8 @@ import("//build_overrides/openthread.gni") declare_args() { chip_with_ot_cli = 0 chip_with_low_power = 0 - chip_convert_dac_private_key = 0 sdk_release = 1 + chip_use_plain_dac_key = false } nxp_platform = "k32w/k32w1"