Revert "[nxp fromtree] [NXP][k32w1] Remove the need for extra DAC pri…

…vate key conversion binary (project-chip#32699)" This reverts commit 932c21a. Signed-off-by: Andrei Menzopol <andrei.menzopol@nxp.com>
NXP · Apr 10, 2024 · ef2cf85 · ef2cf85
1 parent 4cca897
commit ef2cf85
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 96 deletions.
diff --git a/docs/guides/nxp_manufacturing_flow.md b/docs/guides/nxp_manufacturing_flow.md
@@ -215,15 +215,26 @@ 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 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.
-
-There is no need for an extra binary.
+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` set.
--   Write the application to the board and use it as usual.
+-   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:
+
+-   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:
+
+```shell
+./scripts/tools/nxp/factory_data_generator/k32w1/example_convert_dac_private_key.jlink
+```
 
 Factory data should now contain a corresponding encrypted blob instead of the
 DAC private key.
@@ -240,9 +251,6 @@ 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
@@ -0,0 +1,16 @@
+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
diff --git a/src/platform/nxp/k32w/k32w1/BUILD.gn b/src/platform/nxp/k32w/k32w1/BUILD.gn
@@ -93,12 +93,6 @@ static_library("nxp_platform") {
     ]
   }
 
-  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",
@@ -145,6 +139,10 @@ 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
@@ -15,30 +15,29 @@
  *    limitations under the License.
  */
 
-#include "fsl_adapter_flash.h"
 #include <platform/nxp/k32w/k32w1/FactoryDataProviderImpl.h>
 
+#if CHIP_DEVICE_CONFIG_SECURE_DAC_PRIVATE_KEY
+#include "fsl_adapter_flash.h"
+#endif
+
 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 && !CHIP_USE_PLAIN_DAC_KEY
+#if CHIP_DEVICE_CONFIG_ENABLE_SSS_API_TEST
     SSS_RunApiTest();
 #endif
 
@@ -48,56 +47,16 @@ 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(SSS_ImportPrivateKeyBlob());
+    ReturnErrorOnFailure(Validate());
 #endif
+    ReturnErrorOnFailure(SSS_ImportPrivateKeyBlob());
 
     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;
@@ -159,25 +118,18 @@ 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;
     size_t newSize                      = sizeof(FactoryDataProvider::Header) + mHeader.size + kSssBlobMetadataLength;
     uint8_t blob[kPrivateKeyBlobLength] = { 0 };
     uint8_t * data                      = static_cast<uint8_t *>(chip::Platform::MemoryAlloc(newSize));
     uint32_t offset                     = 0;
-    bool convNeeded                     = true;
 
     VerifyOrReturnError(data != nullptr, CHIP_ERROR_INTERNAL);
 
-    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;
-    }
-
+    ReturnErrorOnFailure(SSS_ExportBlob(blob, &blobSize, offset));
     ChipLogError(DeviceLayer, "SSS: extracted blob from DAC private key");
 
     hal_flash_status_t status = HAL_FlashRead(kFactoryDataStart, newSize - kSssBlobMetadataLength, data);
@@ -197,31 +149,22 @@ 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, bool & isNeeded)
+CHIP_ERROR FactoryDataProviderImpl::SSS_ExportBlob(uint8_t * data, size_t * dataLen, uint32_t & offset)
 {
     CHIP_ERROR error = CHIP_NO_ERROR;
     auto res         = kStatus_SSS_Success;
 
-    uint8_t keyBuf[kPrivateKeyBlobLength];
+    uint8_t keyBuf[Crypto::kP256_PrivateKey_Length];
     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);
@@ -254,6 +197,7 @@ 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
 
@@ -354,7 +298,6 @@ 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
@@ -19,8 +19,21 @@
 #include <crypto/CHIPCryptoPAL.h>
 #include <platform/nxp/k32w/common/FactoryDataProvider.h>
 
-#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.
@@ -46,13 +59,12 @@ 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 if not already done
+     * \brief Convert DAC private key to an SSS encrypted blob and update factory data
      *
      * @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
@@ -62,16 +74,15 @@ class FactoryDataProviderImpl : public FactoryDataProvider
     CHIP_ERROR SSS_ConvertDacKey();
 
     /*!
-     * \brief Check and export an SSS encrypted blob from the DAC private key found in factory data if needed
+     * \brief Export an SSS encrypted blob from the DAC private key found in factory data
      *
      * @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, bool & isNeeded);
+    CHIP_ERROR SSS_ExportBlob(uint8_t * data, size_t * dataLen, uint32_t & offset);
 
     /*!
      * \brief Replace DAC private key with the specified SSS encrypted blob
@@ -86,13 +97,12 @@ 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
@@ -26,7 +26,6 @@ declare_args() {
   # The key storage solution. Developers can select between "littlefs", "nvs"
   # and the older "fwk_nvm".
   chip_key_storage = "fwk_nvm"
-  chip_use_plain_dac_key = false
 }
 
 if(!sdk_release && is_sdk_2_15) {