Investigate ECP light #7357

mpg · 2023-03-30T11:19:52Z

Context: looking for a better stepping stone towards the full ECP goald in #6839

Just dumping the current state or my investigations about defining a subset of ECP without the arithmetic.

Currently nothing is expected to work except tests/scripts/all.sh -k test_psa_crypto_config_accel_all_ec_algs_use_psa builds, and all test suites pass except pk and pkparse.

The FULL macro is obviously not the final shape of the option but only a quick hack.

TODO:

Investigate failures in pkparse: 4 of them are expected as the description says "no public key", the 5th is unclear: Parse EC Key #8a (SEC1 PEM, secp224r1, compressed)
Look at failures in pk: are all of them expected? We expect the following assertions to fail: !"!FULL not supported" and pk_genkey(&pk, parameter) == 0 (key generation) and everything to do with check_pair().
Establish a list of public functions that are removed from ecp.c when !FULL.
Start prototyping a replacement to eckey_check_pair (pk_wrap.c) along the same lines as rsa_alt_check_pair(): do a signature with the private key, verify it with the public key.
Starting from a fresh copy of development, just apply the change in pkparse.c about not completing the public key if it wasn't encoded, an see what fails: we expect the "no public key" cases in test_suite_parse to fail, but does anything else fail?
Check (in docs/proposed/psa-driver-interface.md and/or by testing or reading the code) if reconstructing the public key from the private key is something drivers are supposed to support, and if that's the case with libtestdriver1: call psa_import_key() with private key material, then psa_export_public() -> does it work?
Start prototyping a replacement to the key generation helper(s) used in test_suite_pk.function that would be based on PSA functions only (psa_generate_key() mostly).

valeriosetti · 2023-03-30T17:51:11Z

Investigate failures in pkparse: 4 of them are expected as the description says "no public key", the 5th is unclear: Parse EC Key #8a (SEC1 PEM, secp224r1, compressed)

I debugged this issue a little and it seems that the p224 does not satisfy this condition for compressed points, whereas p256/384/521 do. As a consequence the pk_parse_key_sec1_der tries to fall into the key creation which is commented out by the new ECP_FULL symbol (renamed from FULL).

Edit: it was also mentioned here

Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

…ests This symbol will be removed as soon as the proper function (not using MPI) will be added to the ECP module. This symbol can be seen as a sort of "Failure reason is known and its solution is already planned" Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

This mimic the previous temporary symbol added to pkparse, but in this case the idea is to signal the possibility to check private/public key pair. It will be removed as soon as the functionality will be implemented in ECP module Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

valeriosetti · 2023-03-31T09:23:16Z

As for the first 2 points I added 2 new temporary symbols (ECP_HAS_KEY_GENERATION and ECP_HAS_CHECK_PAIR) to:

have all the tests passing (in the component_test_psa_crypto_config_accel_all_ec_algs_use_psa component) skipping some cases for which there is no support now
mark what is the reason of the failure for the skipped cases

The idea is that both of these symbols will be removed at the end once both functions will be re-implemented

This is a first attempt to create an alternative function for EC key pair verification. It is based on PSA functions instead of ECP ones. It is included when the ECP_FULL symbol is not defined. Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

This is based on PSA functions and helps removing the dependency on ECP_FULL math functions inside the ECP module Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

This is an alternative function for key-pair generation when ECP_FULL is not enabled; in this case PSA based functions are used instead. Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

valeriosetti · 2023-03-31T16:05:44Z

@mpg
I think I have added all the required functions to make component_test_psa_crypto_config_accel_all_ec_algs_use_psa() pass all the test_suites_ without having ECP_FULL defined on the library side and without skipping any test.

Of course the work is not completed yet because now there are all other components in all.sh and ssl-opt.sh to be tested, but I think it's worth an initial quick review

mpg · 2023-04-03T07:35:07Z

library/pk_wrap.c

+
+    psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_KEY_PAIR(curve));
+    psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_SIGN_HASH);
+    // TODO: forcing SHA256 because this is included by default when building


Ah, this is a problem with the "sign-verify" approach I suggested, indeed. In the meantime, I had another idea, inspired by what you did for key completion: we could just serialize the public key for both prv and pub (using mbedtls_pk_write_pubkey() I think) and see if the results are identical.

(Then later when #7202 is done, that would be mbedtls_pk_write_pubkey() on prv and directly compare to the byte array in pub, then when and its private counterpart is done, that would be psa_import() + psa_export_public() on prv and compare to byte array in pub.)

Wdyt?

I like this idea!
However I have a question: since also for the key creation we are already importing the private key into PSA to get its public part, why can't we go that way directly also here? In theory also 7202 is a sort of middle step toward having everything on the PSA side (or at least there is a discussion on this)

Let's continue this discussion in #7387.

mpg · 2023-04-03T07:40:19Z

library/ecp.c

@@ -3198,6 +3206,57 @@ int mbedtls_ecp_gen_keypair(mbedtls_ecp_group *grp,
 {
    return mbedtls_ecp_gen_keypair_base(grp, &grp->G, d, Q, f_rng, p_rng);
 }
+#else /* ECP_FULL */
+int mbedtls_ecp_alt_gen_keypair(mbedtls_ecp_group *grp,


This is looking good, but I'm not sure the implementation belongs here, as opposed to test_suite_pk.function which seems to be the only place using this.

Yes, the mbedtls_ecp_alt_gen_keypair function itself is only explicitly called by test_suite_pk.function, but it is also indirectly called from mbedtls_ecp_gen_key which is instead used also elsewhere such as psa_crypto_ecp.c and test_suite_ecp.function

Ah, thanks for the info. Regarding test_suite_ecp.function I think that's pretty simple: we'll just skip those cases. For psa_crypto_ecp.c I'll have a look right now. (I had grepped for mbedtls_ecp_gen_key() before commenting, but had completely missed the line from psa_crypto_ecp.c in the results. Should have looked more carefully.)

Just had a look, and the call in psa_crypto_ecp.c is guarded by MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIR so it shouldn't be a problem in builds where KEY_TYPE_ECC_KEY_PAIR is accelerated, which are our target here.

Regarding test_suite_ecp.function I think that's pretty simple: we'll just skip those cases

Yes, sorry for not thinking about this, but it doesn't make much sense to test the ECP module with functions that are actually outside it. I'll fix this!

Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

Albeit using the sign/verify technique for verifying the private/public key pair works fine, it imposes requirements on the supported SHA algorithm which is not great. Here we move to a new approach which simply imports: - imports the private key to PSA and extracts its public part - write the public key to be checked to a local buffer in raw format - compares the two results Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

mpg · 2023-04-07T08:51:18Z

Closing this prototype as the real PR is now up: #7410

WIP

fc2b797

mpg added the DO-NOT-MERGE label Mar 30, 2023

valeriosetti assigned mpg and valeriosetti Mar 30, 2023

valeriosetti added 3 commits March 31, 2023 09:27

rename FULL symbol to ECP_FULL to be more future proof

fc0c64e

Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

valeriosetti added 3 commits March 31, 2023 14:37

pkparse: adding a function for generating public key from private

95b3003

This is based on PSA functions and helps removing the dependency on ECP_FULL math functions inside the ECP module Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

ecp: adding alternate function for key-pair derivation based on PSA

0c22798

This is an alternative function for key-pair generation when ECP_FULL is not enabled; in this case PSA based functions are used instead. Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

mpg commented Apr 3, 2023

View reviewed changes

valeriosetti added 2 commits April 3, 2023 11:48

pk: keep function for generating EC keypair in test_suite_pk

d064b4a

Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

mpg closed this Apr 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Investigate ECP light #7357

Investigate ECP light #7357

mpg commented Mar 30, 2023 •

edited

Loading

valeriosetti commented Mar 30, 2023 •

edited

Loading

valeriosetti commented Mar 31, 2023

valeriosetti commented Mar 31, 2023

mpg Apr 3, 2023

valeriosetti Apr 3, 2023

mpg Apr 3, 2023

mpg Apr 3, 2023

valeriosetti Apr 3, 2023

mpg Apr 3, 2023

mpg Apr 3, 2023

valeriosetti Apr 3, 2023

mpg commented Apr 7, 2023

Investigate ECP light #7357

Investigate ECP light #7357

Conversation

mpg commented Mar 30, 2023 • edited Loading

valeriosetti commented Mar 30, 2023 • edited Loading

valeriosetti commented Mar 31, 2023

valeriosetti commented Mar 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpg commented Apr 7, 2023

mpg commented Mar 30, 2023 •

edited

Loading

valeriosetti commented Mar 30, 2023 •

edited

Loading