Investigate ECP light #7357
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1095,13 +1095,117 @@ static int eckey_sign_rs_wrap(void *ctx, mbedtls_md_type_t md_alg, | |
| } | ||
| #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ | ||
|
|
||
| #if !defined(ECP_FULL) | ||
| /* | ||
| * Alternative function used to verify that the EC private/public key pair | ||
| * is valid using PSA functions instead of ECP ones. | ||
| * The flow is: | ||
| * - sign a hash message using the provided private key | ||
| * - verify the signature using the public key | ||
| */ | ||
| static int eckey_alt_check_pair(const void *pub, const void *prv, | ||
| int (*f_rng)(void *, unsigned char *, size_t), | ||
| void *p_rng) | ||
| { | ||
| (void)f_rng; | ||
| (void)p_rng; | ||
| psa_status_t status; | ||
| psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; | ||
| mbedtls_ecp_keypair *prv_ctx = (mbedtls_ecp_keypair *) prv; | ||
| mbedtls_ecp_keypair *pub_ctx = (mbedtls_ecp_keypair *) pub; | ||
| unsigned char sig[MBEDTLS_MPI_MAX_SIZE]; | ||
| size_t sig_len = 0; | ||
| unsigned char hash[32]; | ||
| int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; | ||
| size_t curve_bits; | ||
| psa_ecc_family_t curve = | ||
| mbedtls_ecc_group_to_psa(prv_ctx->grp.id, &curve_bits); | ||
| unsigned char key_buf[MBEDTLS_PSA_MAX_EC_KEY_PAIR_LENGTH]; | ||
| size_t key_len = PSA_BITS_TO_BYTES(curve_bits); | ||
| mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; | ||
|
|
||
| memset(hash, 0x2a, sizeof(hash)); | ||
|
|
||
| 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 | ||
|
There was a problem hiding this comment. 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 (Then later when #7202 is done, that would be Wdyt? There was a problem hiding this comment. I like this idea! There was a problem hiding this comment. Let's continue this discussion in #7387. |
||
| // the library (even though it's not granted that the built-in version | ||
| // is supported). Is there a more general purpose solution? | ||
| psa_set_key_algorithm(&key_attr, PSA_ALG_ECDSA(PSA_ALG_SHA_256)); | ||
|
|
||
| ret = mbedtls_mpi_write_binary(&prv_ctx->d, key_buf, key_len); | ||
| if (ret != 0) { | ||
| return ret; | ||
| } | ||
|
|
||
| status = psa_import_key(&key_attr, key_buf, key_len, &key_id); | ||
| if (status != PSA_SUCCESS) { | ||
| ret = PSA_PK_TO_MBEDTLS_ERR(status); | ||
| return ret; | ||
| } | ||
|
|
||
| status = psa_sign_hash(key_id, PSA_ALG_ECDSA(PSA_ALG_SHA_256), | ||
| hash, sizeof(hash), sig, sizeof(sig), &sig_len); | ||
| if (status != PSA_SUCCESS) { | ||
| ret = PSA_PK_TO_MBEDTLS_ERR(status); | ||
| status = psa_destroy_key(key_id); | ||
| return (status != PSA_SUCCESS) ? PSA_PK_TO_MBEDTLS_ERR(status) : ret; | ||
| } | ||
|
|
||
| status = psa_destroy_key(key_id); | ||
| if (status != PSA_SUCCESS) { | ||
| return PSA_PK_TO_MBEDTLS_ERR(status); | ||
| } | ||
| psa_reset_key_attributes(&key_attr); | ||
| mbedtls_platform_zeroize(key_buf, sizeof(key_buf)); | ||
|
|
||
| psa_set_key_type(&key_attr, PSA_KEY_TYPE_ECC_PUBLIC_KEY(curve)); | ||
| psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_VERIFY_HASH); | ||
| psa_set_key_algorithm(&key_attr, PSA_ALG_ECDSA(PSA_ALG_SHA_256)); | ||
|
|
||
| ret = mbedtls_ecp_point_write_binary(&pub_ctx->grp, &pub_ctx->Q, | ||
| MBEDTLS_ECP_PF_UNCOMPRESSED, | ||
| &key_len, key_buf, sizeof(key_buf)); | ||
| if (ret != 0) { | ||
| return ret; | ||
| } | ||
|
|
||
| status = psa_import_key(&key_attr, key_buf, key_len, &key_id); | ||
| if (status != PSA_SUCCESS) { | ||
| ret = PSA_PK_TO_MBEDTLS_ERR(status); | ||
| return ret; | ||
| } | ||
|
|
||
| status = psa_verify_hash(key_id, PSA_ALG_ECDSA(PSA_ALG_SHA_256), | ||
| hash, sizeof(hash), sig, sig_len); | ||
| if (status != PSA_SUCCESS) { | ||
| ret = PSA_PK_TO_MBEDTLS_ERR(status); | ||
| status = psa_destroy_key(key_id); | ||
| return (status != PSA_SUCCESS) ? PSA_PK_TO_MBEDTLS_ERR(status) : ret; | ||
| } | ||
| status = psa_destroy_key(key_id); | ||
| if (status != PSA_SUCCESS) { | ||
| return PSA_PK_TO_MBEDTLS_ERR(status); | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
| #endif /* ECP_FULL */ | ||
|
|
||
| static int eckey_check_pair(const void *pub, const void *prv, | ||
| int (*f_rng)(void *, unsigned char *, size_t), | ||
| void *p_rng) | ||
| { | ||
| #if defined(ECP_FULL) | ||
| return mbedtls_ecp_check_pub_priv((const mbedtls_ecp_keypair *) pub, | ||
| (const mbedtls_ecp_keypair *) prv, | ||
| f_rng, p_rng); | ||
| #else /* ECP_FULL */ | ||
| return eckey_alt_check_pair((const mbedtls_ecp_keypair *) pub, | ||
| (const mbedtls_ecp_keypair *) prv, | ||
| f_rng, p_rng); | ||
| #endif /* ECP_FULL */ | ||
| return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; | ||
| } | ||
|
|
||
| static void *eckey_alloc_wrap(void) | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good, but I'm not sure the implementation belongs here, as opposed to
test_suite_pk.functionwhich seems to be the only place using this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the
mbedtls_ecp_alt_gen_keypairfunction itself is only explicitly called bytest_suite_pk.function, but it is also indirectly called frommbedtls_ecp_gen_keywhich is instead used also elsewhere such aspsa_crypto_ecp.candtest_suite_ecp.functionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks for the info. Regarding
test_suite_ecp.functionI think that's pretty simple: we'll just skip those cases. Forpsa_crypto_ecp.cI'll have a look right now. (I had grepped formbedtls_ecp_gen_key()before commenting, but had completely missed the line frompsa_crypto_ecp.cin the results. Should have looked more carefully.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just had a look, and the call in
psa_crypto_ecp.cis guarded byMBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIRso it shouldn't be a problem in builds whereKEY_TYPE_ECC_KEY_PAIRis accelerated, which are our target here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!