Check remaning dependencies on ECP in PK module

[valeriosetti]

This is the 3rd step of #7460
Resolves #7460 (partially)
Requires #7463

Gatekeeper checklist

• changelog not required - internal script
• backport not required - internal script
• tests not required - internal script

[valeriosetti]

Reply to this comment
Good idea! I forgot about the syms.sh script, but here it should work quite good. I just started this PR with a proposal for an improvement of this script which can be helpful for what we are going to analyze here.

Using that script, here's what I found:

./docs/architecture/psa-migration/syms.sh full pk pkparse pkwrite
cat full-*-external | grep mbedtls_ecp | sort -u

mbedtls_ecp_check_privkey
mbedtls_ecp_check_pubkey
mbedtls_ecp_group_free
mbedtls_ecp_group_init
mbedtls_ecp_group_load
mbedtls_ecp_grp_id_list
mbedtls_ecp_keypair_free
mbedtls_ecp_point_free
mbedtls_ecp_point_read_binary
mbedtls_ecp_point_write_binary
mbedtls_ecp_restart_is_enabled
mbedtls_ecp_write_key

I opened the PR as draft just to give @mpg some ideas of what is still missing and should be addressed by this PR.

[valeriosetti]

Here's my thoughts about these remaining dependencies:

• mbedtls_ecp_check_privkey

  • once Avoid parse/unparse private ECC keys in PK with USE_PSA #7074 will be completed we will import the private key also in PSA and this checks key's validity while importing (see psa_crypto_ecp), so we will skip this check

• mbedtls_ecp_check_pubkey

  • basically the same as the previous one, but it depends on Avoid parse/unparse public ECC keys in PK with USE_PSA when !ECP_C #7073. The check on the PSA side is still done in psa_crypto_ecp

• mbedtls_ecp_group_free, mbedtls_ecp_group_init, mbedtls_ecp_grp_id_list, mbedtls_ecp_point_free

  • these are used only when MBEDTLS_PK_PARSE_EC_EXTENDED is enabled. It is already planned to have ECP_LIGHT enabled when MBEDTLS_PK_PARSE_EC_EXTENDED also is, otherwise it would not be possible to parse custom curves

• pk_use_ecparams

  • a part from the MBEDTLS_PK_PARSE_EC_EXTENDED dependency which goes back to the previous point, this is also used in pk_use_ecparams. We can get rid of this part once Avoid parse/unparse public ECC keys in PK with USE_PSA when !ECP_C #7073 and Avoid parse/unparse private ECC keys in PK with USE_PSA #7074 are done

• mbedtls_ecp_keypair_free

  • easily removable after Avoid parse/unparse public ECC keys in PK with USE_PSA when !ECP_C #7073 and Avoid parse/unparse private ECC keys in PK with USE_PSA #7074

• mbedtls_ecp_point_read_binary

  • 1 reference depends on MBEDTLS_PK_PARSE_EC_EXTENDED, while the other 2 should disappear once Avoid parse/unparse public ECC keys in PK with USE_PSA when !ECP_C #7073 and Avoid parse/unparse private ECC keys in PK with USE_PSA #7074 are addressed

• mbedtls_ecp_point_write_binary

  • references in eckey_check_pair_psa and ecdsa_verify_wrap will be dropped in Avoid parse/unparse public ECC keys in PK with USE_PSA when !ECP_C #7073 + Avoid parse/unparse private ECC keys in PK with USE_PSA #7074

• mbedtls_ecp_restart_is_enabled

  • restartable functions are not available in PSA yet, so having ECP_LIGHT enabled is mandatory to use this functionality. As a consequence the dependency will disappear by disabling MBEDTLS_ECP_RESTARTABLE

• mbedtls_ecp_write_key

  • will probably be replaced by psa_export in Avoid parse/unparse private ECC keys in PK with USE_PSA #7074

[valeriosetti]

Following my previous comment, here is the list of dependencies when MBEDTLS_ECP_RESTARTABLE and MBEDTLS_PK_PARSE_EC_EXTENDED are also disabled on top of the full configuration

mbedtls_ecp_check_privkey
mbedtls_ecp_check_pubkey
mbedtls_ecp_group_load
mbedtls_ecp_keypair_free
mbedtls_ecp_point_read_binary
mbedtls_ecp_point_write_binary
mbedtls_ecp_write_key

[mpg]

./docs/architecture/psa-migration/syms.sh full pk pkparse pkwrite

I think we should add pk_wrap to that list.

[valeriosetti]

./docs/architecture/psa-migration/syms.sh full pk pkparse pkwrite

I think we should add pk_wrap to that list.

Right, but fortunately this doesn't add much to the list:

• mbedtls_ecp_keypair_free and mbedtls_ecp_keypair_init can be removed after Avoid parse/unparse public ECC keys in PK with USE_PSA when !ECP_C #7073 and Avoid parse/unparse private ECC keys in PK with USE_PSA #7074
• mbedtls_ecp_point_write_binary is used in ecdsa_verify_wrap but can be removed after Avoid parse/unparse public ECC keys in PK with USE_PSA when !ECP_C #7073

[mpg]

Requires #7463

I'm not sure I understand why. Can't we have just the changes to all.sh and syms.sh without the others?

[valeriosetti]

I'm not sure I understand why. Can't we have just the changes to all.sh and syms.sh without the others?

TBH this was mostly for me to ease the rebase process for the reshape of #7202. There is no strict requirement for that.
Do you want me to rebase this PR on development?

[mpg]

Yes, I'd prefer to have this based on development, that way we can merge it quickly.

(For the rework of 7202, I think you can start with merging the 3 pre-requisite PRs into your feature branch. Then you can remove the merge commits once all 3 are merged to development.)

[mpg]

LGTM, both the changes to the script, and the analysis.