Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.07k stars 2.52k forks source link

Cannot anymore load public key into a ecp_keypair in v3.0 #6833

Closed HomeACcessoryKid closed 5 months ago

HomeACcessoryKid commented 1 year ago

Summary

I start using ESP IDF v5.0 and this has mbedtls updated from 2.x to 3.0 Now private struct members are shielded and that is OK, BUT there is no API to load a ECP public key.

System information

Mbed TLS version (number or commit id): as in IDF v5.0 Operating system and version: Configuration (if not default, please attach mbedtls_config.h): Compiler and options (if you used a pre-built binary, please indicate how you obtained it): Additional environment information:

Expected behavior

There is an API to do mbedtls_ecp_read_pubkey(grp_id, &mbedtls_ecp_key, but, len); There is an API to do mbedtls_ecp_check_pubkey(&mbedtls_ecdsa_ctx);

Actual behavior

this used to work in ESP IDF v4.4.3 but without support for private struct entries, it is not possible

mbedtls_ecdsa_context mbedtls_ecdsa_ctx;
    mbedtls_ecdsa_init(&mbedtls_ecdsa_ctx);
    mbedtls_ecp_group_load(&mbedtls_ecdsa_ctx.grp, MBEDTLS_ECP_DP_SECP384R1);
    ret=mbedtls_ecp_point_read_binary(&mbedtls_ecdsa_ctx.grp,&mbedtls_ecdsa_ctx.Q,buffer+23,length);
    printf("keycheck: 0x%02x\n",mbedtls_ecp_check_pubkey(&mbedtls_ecdsa_ctx.grp,&mbedtls_ecdsa_ctx.Q));

    answer=mbedtls_ecdsa_read_signature(&mbedtls_ecdsa_ctx,signature->hash,HASHSIZE,signature->sign,signature->sign[1]+2);

Steps to reproduce

https://github.com/HomeACcessoryKid/LCM4ESP32/blob/main/main/ota.c#L412 use docker to compile this in esp idf v5.0 image remove #include "mbedtls/certs.h" first

Additional information

mpg commented 1 year ago

Thanks for your report. I'm labeling "bug" and scheduling for the next release as this looks like a usability regression in 3.0.

mpg commented 1 year ago

A few thoughts and questions:

There is an API to do mbedtls_ecp_read_pubkey(grp_id, &mbedtls_ecp_key, but, len); There is an API to do mbedtls_ecp_check_pubkey(&mbedtls_ecdsa_ctx);

I'm not sure the second one would be necessary, as read_pubkey() should probably validate the public key anyway.

Taking a step back, currently the intention in the API is that key parsing (and writing) is handled by the PK layer: mbedtls_pk_parse_key() and mbedtls_pk_parse_public_key() which support a variety of standard formats. This populates an mbedtls_pk_context that you can then use to verify a signature, in the same format as mbedtls_ecdsa_read_signature().

Would switching to PK work for you? It probably depends in which format you're getting your key, but the buffer+23 in you code snippet suggests you're already getting more than just the "point" part.

Also, I would like to point out that in the next major version of Mbed TLS (that is, 4.0, no due date so far), we intend to remove all of ecdsa.h and ecp.h from the public API, leaving only the PSA Crypto API (and probably some of pk.h, to be determined). Would switching to the PSA Crypto API work for you? It looks like psa_import_key() does accept the same format for public keys as you are currently using, namely just an uncompressed EC point. However, switching to PSA Crypto is likely to be more work.

HomeACcessoryKid commented 1 year ago

Just to let you know I saw your feedback. Thanks for that and sorry for the delayed response.

My considerations is that I want to just use what is available in ESP IDF. As you refer to a PSA API, which is an ARM initiative, I expect that to port to ESP only once you have switched over in mbedtls and then ESP has adopted it. If you have a link to show that it is in IDF v5 already, then that would be worth pointing out.

So, I want to think inside mbedtls v3 and you ask if I could use PK layer. The thing is that I use the raw EC point and without having verified this, I guess that makes PK not compatible. Is that correct?

One thing is, that I never found a good document helping one to get the bigger picture of all the mbedtls stuff. If you have that, I might have a look there.

Otherwise, I hope that we keep to use mbed also as a generic crypto library and not just a https server library. Similar to wolfcrypt as part of wolfssl.

So, bottom line, please keep the regression bug and I will continue with my current code.

Thanks again for your help and product.

gilles-peskine-arm commented 1 year ago

PSA APIs have been available in Mbed TLS for a long time. I don't know ESP IDF, but unless they've deliberately removed PSA APIs, they're available. And psa_import_key can read the same format as mbedtls_ecp_point_read_binary.

That being said, as Manuel said, converting your code to use the PSA might to be a significant effort. But it wouldn't be wasted effort, since it would prepare you for Mbed TLS 4.0. Whether you should do it now depends partly on whether you would run into problems (some “exotic” things in TLS might not be easy to make work with PSA yet, and it might increase the code size), which depends heavily on what your application does.

gilles-peskine-arm commented 5 months ago

This has been resolved by https://github.com/Mbed-TLS/mbedtls/pull/7815 which is merged in the development branch and will be in the next minor release of Mbed TLS (3.6.0). This introduces a function mbedtls_ecp_set_public_key(). Combined with mbedtls_ecp_point_read_binary, this function allows filling an mbedtls_ecp_keypair object with a public key, taking the input in the standard binary format (compressed or uncompressed point).