Closed mpg closed 1 year ago
We can get a list of external symbols used by libmbedtls in a full config (which has MBEDTLS_USE_PSA_CRYPTO
enabled) with:
scripts/config.py full
make lib
docs/architecture/psa-migration/syms.sh full
and then look at full-tls-modules
and/or grep full-tls-external
for functions of interest.
The good: this confirms no dependency on ecdh or ecdsa. This confirms dependencies on ecjpake which are known and will be addressed in #5847. (Also the same method for X.509 confirms no dependency on ecdh, ecdsa, ecjpake or ecp.)
The less good: grep mbedtls_ecp full-tls-external
has the following non-empty output:
mbedtls_ecp_curve_info_from_grp_id
mbedtls_ecp_curve_info_from_tls_id
mbedtls_ecp_point_write_binary
mbedtls_ecp_write_key
Preliminary analysis:
mbedtls_ecp_curve_info
(bitsize, human-readable name for debug) from there.mbedtls_ecp_point_write_binary()
is called only once, from ssl_get_ecdh_params_from_cert()
(the one in ssl_tls12_client.c
) for static ECDH key exchange (TLS 1.2) client-side, to serialize the peer's public key from the parsed certificate. The certificate probably already has that serialized somewhere and perhaps we could get it from the pk_raw
field.mbedtls_ecp_write_key()
is called only once, from ssl_get_ecdh_params_from_cert()
(the one in ssl_tls12_server.c
) for static ECDH key exchange (TLS 1.2) server-side, to export our own private key before we import it back in PSA. At this point the replacement is not clear to me. We could try using a pk_write
function instead but the would only move the problem, unless we can implement PK parse/write in a way that doesn't depend on ECP.Hi @mpg, I took a quick look at this activity and here's what I found/what I would do:
mbedtls_ecp_curve_info_from_grp_id()
seems to be used also in psa_crypto_ecp
, so I don't think moving it to TLS would be the right solution. Therefore I would move both mbedtls_ecp_curve_info_from_grp_id()
and mbedtls_ecp_curve_info_from_tls_id
to an external common file, such as common_ecp.c
. In this way is would be available to both TLS modules as well as psa_crypto_ecp
mbedtls_ecp_write_key()
and mbedtls_ecp_read_key()
in the same common file. Unfortunately this does not get rid of the BIGNUM_C dependency, but at least it should take ECP_C dependency awaymbedtls_ecp_point_write_binary()
I'm still working on this. At first I thought I could take the key from ssl->session_negotiate->peer_cert->raw.p
(if SSL_KEEP_PEER_CERTIFICATE) or from ssl->handshake->peer_pubkey
(if !SSL_KEEP_PEER_CERTIFICATE), but it seems not to work properly.Please note that this is the first time for me to work on this repo, so I'm aware that my proposal can be totally garbage. But in case it makes any sense to you, just let me know and I'll open a PR to let you check it.
Just some quick thoughts on the first part. The type mbedtls_ecp_curve_info()
has a layering problem, in that it generic crypto information with TLS-specific information (namely tls_id
as the name implies). That probably made sense when it was introduced, as libmbedcrypto
had no ambition of being a generic crypto library, just ad-hoc support for implementing X.509 and TLS (actually when this was introduced, make lib
produced only one library with everything in it, having it build libmbedcrypto
, libmbedx509
and libmbedtls
as separate outputs only came later).
This is highly visible in a lot of places: what's the input/output formats of mbedtls_dhm_xxx()
functions? The one defined in the TLS RFC. That of mbedtls_ecjpake_xxx()
functions? You guess it, that from the EC J-PAKE in TLS draft. And it goes on and on.
But now things are changing, we intend to retire all those mbedtls_xxx()
crypto APIs and leave PSA as the only Crypto API, which aims to be a generic crypto API that can be used to build all kinds of higher-level protocols, not just TLS; also the reference implementation of PSA Crypto is going to be split into its own independent project. So we should really have better layering / separation of concerns between generic crypto stuff on one hand, and TLS-specific things on the other hand.
So, all the TLS-specific constants and helper functions should now live in TLS: that's the tls_id
part of curve_info
, constants like MBEDTLS_ECP_TLS_NAMED_CURVE
(I mean, the name's quite a hint), probably functions like mbedtls_ecp_tls_xxx()
(same remark). Until Mbed TLS 4.0 we can't change or remove existing APIs, so that probably means some temporary duplication between what's already there in ECP and what we add to TLS where it belongs.
Regarding the generic data, like bit size etc, I'm not sure yet where that should live, might be a new module common_ecp.c
indeed, might be something else... I just wanted to point out that we want to strive for a better separation of layers in the future.
Regarding the generic data, like bit size etc,
Code using the PSA API (e.g. all parts of the code under MBEDTLS_USE_PSA_CRYPTO
) should be able to get this information via a PSA API, otherwise it indicates a possible gap in the API. But we may want to have the information in parts of the pk/x509/tls code that is common to both with and without USE_PSA. So for that, a generic ECP metadata module makes sense.
Note (just as I was going through old issues and noticed one that might be related): similarly to what we're gonna do for hashes identifiers in MD, we might want to make ECP group ID to make them more similar to PSA identifiers, which are a pair (family, bitsize). The primary purpose would be to save code size in the conversion functions ECP <-> PSA, but as it happens, I think that would also be a step towards https://github.com/Mbed-TLS/mbedtls/issues/2375
but as it happens, I think that would also be a step towards https://github.com/Mbed-TLS/mbedtls/issues/2375
Sorry, but it's not totally clear to me how I can help here.
we might want to make ECP group ID to make them more similar to PSA identifiers
) to be created? If this is the case, can I create the issue autonomously?@valeriosetti I'll create issues tomorrow! Sorry, been a bit busy recently and hadn't noticed you were already done with 6702.
Closing, superseded by #6839 that takes a larger view of the issues at hand.
As of current development, when
MBEDTLS_USE_PSA_CRYPTO
is enabled, TLS uses only PSA Crypto for all ECDH(E) and ECDSA crypto operations; after #5847 that will be the case for EC J-PAKE operations as well. However, TLS still uses functions from the ECP module for non-crypto operations, such as information management (converting between TLS/IANA id and internalgrp_id
, querying curve size, etc.) and key import/export.This task is to study how to get rid of those dependencies and create an estimated series of task achieving this result.