search

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.5k stars 2.6k forks source link

Add accessor (getter/setter) function for MBEDTLS_PRIVATE #9716

Open ko-maren opened 1 week ago

ko-maren commented 1 week ago

Suggested enhancement

I'm migrating from MBedTLS 2.28.x to 3.6.x. During that I found a few private struct members we are using, and where I didn't found any accessor function. I found this list (is it still up2date?) and went through some issue, but perhaps I missed some accessor functions, which are already decided not to be added.

mbedtls_ecp_export() exports grp, d and Q as a copy from the given keypair. However, we would need to modify these within mbedtls_ecp_keypair. Is it possible to have a getter, which returns a pointer to the corresponding struct element, or a setter, to copy a modified export back into the keypair?

Justification

For specific logging, we would need from mbedtls_ssl_context as getter, peer_cert of session_negotiate (e.g. similar function like mbedtls_ssl_get_peer_cert() and in_msg. A getter for p_bio in mbedtls_ssl_context to access the own defined struct to access I/O operations. I found a setter but now getter.

To take some more parameters into account we do have a function for validation of x509 signature. We are using mbedtls_md_info_from_type() and mbedtls_pk_verify(), however we would need to access for these function sig and sig_md from the struct mbedtls_x509_crt.

mbedtls_pk_info_from_type() for a given type is possible, but not the pk_info of mbedtls_pk_context is returned. We would need the actually pk_info.

For mbedtls_pk_parse_key both, f_rng and p_rng of mbedtls_ssl_config are necessary. As we are using mbedtls_pk_parse_key in our implementation in creating a own TLS container and setting the certificate there. For this function we would need the random number generator function.

gilles-peskine-arm commented 1 week ago

TLS

For specific logging, we would need from mbedtls_ssl_context as getter, peer_cert of session_negotiate (e.g. similar function like mbedtls_ssl_get_peer_cert()

Sorry, I don't understand: how would that be different from mbedtls_ssl_get_peer_cert()?

and in_msg.

in_msg is an implementation detail of how the library manages buffers. We could add a function that exposes its contents, but I'm not sure if we could promise anything about the contents, e.g. what starts at what offset at a given time.

A getter for p_bio in mbedtls_ssl_context to access the own defined struct to access I/O operations. I found a setter but now getter.

Our assumption here is that the application code sets the bio functions, so it doesn't need a getter for them. Admittedly that doesn't necessarily work if the application uses an autonomous module that needs the information. Why would you need the io functions for logging though?

For mbedtls_pk_parse_key both, f_rng and p_rng of mbedtls_ssl_config are necessary. As we are using mbedtls_pk_parse_key in our implementation in creating a own TLS container and setting the certificate there. For this function we would need the random number generator function.

Again, our assumption is that the application passed those parameters so it can pass them again. But I can see the usefulness for a callback, and no risk with exposing those. We can do that in 3.6. Starting with 4.0 those fields probably won't exist any longer, just call psa_generate_random.

X.509

To take some more parameters into account we do have a function for validation of x509 signature. We are using mbedtls_md_info_from_type() and mbedtls_pk_verify(), however we would need to access for these function sig and sig_md from the struct mbedtls_x509_crt.

sig is whatever data is in the certificate, so I don't see any problem with exposing that. I'm not sure how we should do it in 3.6 LTS, however. If we make the field public by renaming it, it's technically an ABI change — not one that matters, but it might trip up ABI signature comparisons? If we add an accessor function, it should copy the data, because memory management gets hard when you have pointers to sub-structures lying around, and we try to avoid this in the API design.

I don't see any harm in exposing sig_md. On the other hand, wouldn't you need sig_pk as well? That one would be ok for 3.6 (although we'd have to document it better because mbedtls_pk_type_t is weird), but not in 4.x, where we will get rid of mbedtls_pk_type_t (due to its weirdness), but that might not be ready yet by the time of the 4.0 release.

PK

mbedtls_pk_info_from_type() for a given type is possible, but not the pk_info of mbedtls_pk_context is returned. We would need the actually pk_info.

That one is a problem: the pk module tries to hide the underlying representation, so that as much as possible, applications can work indifferently with RSA/ECC keys and with transparent/opaque keys. We're already exposing more than we'd like from mbedtls_pk_type_t, and mbedtls_pk_info_t exposes even more internal distinctions. And in Mbed TLS 4.0 (or 4.x if we don't get it done in time), mbedtls_pk_info_t will go away.

So I don't think we should expose this. But I am sure that you're asking this because you have a problem that you can't solve with the current API. What problem are you trying to solve? Let's try to find a better interface that would still work in Mbed TLS 4.x.