Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macro PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE does not deal with curve25519 (PSA_ECC_FAMILY_MONTGOMERY) correctly #7056

Closed
chrihop opened this issue Feb 7, 2023 · 3 comments

Comments

@chrihop
Copy link

chrihop commented Feb 7, 2023

Summary

PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE(PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_MONTGOMERY), 255)

gives incorrect results.

The public key size of curve25519 is 32bytes, but PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE(..) gives 65 bytes.

see: https://www.iacr.org/cryptodb/archive/2006/PKC/3351/3351.pdf

System information

Mbed TLS version (number or commit id): a0c806a
Operating system and version: Linux (Ubuntu 20.04)
Configuration (if not default, please attach mbedtls_config.h): default
Compiler and options (if you used a pre-built binary, please indicate how you obtained it): default
Additional environment information: N/A

Expected behavior

PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE(PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_MONTGOMERY), 255) returns 32.

Actual behavior

PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE(PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_MONTGOMERY), 255) returns 65.

Steps to reproduce

printf("%lu\n", PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE(PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_MONTGOMERY), 255));

Additional information

N/A

@tom-cosgrove-arm
Copy link
Contributor

PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE is documented as returning "Sufficient output buffer size for psa_export_public_key()" - i.e. it's the buffer size that the caller should allocate in order to receive a public key of the specified type. It might well be a bit larger than the key, but it must never be less than the memory required to receive such a public key.

Since 65 > 32, this API seems to be functioning as documented, so I'm closing this issue.

@gilles-peskine-arm
Copy link
Contributor

The current definition is correct, but not optimal in terms of RAM usage.

There's a compromise here between source code complexity, code size (depending on the configuration) and memory usage. Currently the code is simple but wastes memory for Montgomery curves. We may revise this compromise in the future.

@gilles-peskine-arm
Copy link
Contributor

On a related note, in a build with only Montgomery curves, PSA_EXPORT_PUBLIC_KEY_MAX_SIZE is similarly larger than it needs to be. This might change when we resolve #6622, in which case we may need to change PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE as well to maintain the invariant that PSA_EXPORT_PUBLIC_KEY_OUTPUT_SIZE(…) <= PSA_EXPORT_PUBLIC_KEY_OUTPUT_MAX_SIZE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants