Add accessor (getter/setter) function for MBEDTLS_PRIVATE

[ko-maren]

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_ssl_context
  • session_negotiate->peer_cert similar to mbedtls_ssl_get_peer_cert()
  • in_msg
  • p_bio
• mbedtls_x509_crt
  • sig
  • sig_md
• mbedtls_pk_context
  • pk_info
• mbedtls_ssl_config
  • f_rng
  • p_rng

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]

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.

[ko-maren]

@gilles-peskine-arm Thanks for your long and detailed response. I try to give more detailed usage for the open questions.

TLS

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

In case of failure, it would be possible to have a extended logging if we get the data from the peer certificate of the session_negotiate, as it might not be available in session. mbedtls_ssl_get_peer_cert returns the peer certificate of session.

X.509

I don't see any harm in exposing sig_md. On the other hand, wouldn't you need sig_pk as well?

From what I see, we wouldn't need sig_pk, only sig_md.

PK

What problem are you trying to solve?

I double checked it, seems like we are only using pk_info to exit functions (context encryption and signing) early if it's NULL. So perhaps it would help, to have a function, which simply returns true or false if pk_info is set or not. To check the type we are already using mbedtls_pk_get_type. Indirectly this function also checks if pk_info == NULL, but it's not the main functionality, so it maybe changed and I would like not to relay on it for the check.

[gilles-peskine-arm]

I double checked it, seems like we are only using pk_info to exit functions (context encryption and signing) early if it's NULL. So perhaps it would help, to have a function, which simply returns true or false if pk_info is set or not. To check the type we are already using mbedtls_pk_get_type. Indirectly this function also checks if pk_info == NULL, but it's not the main functionality, so it maybe changed and I would like not to relay on it for the check.

It's the opposite: pk_info == NULL is an implementation detail, whereas mbedtls_pk_get_type is a public interface. If the 3.x branch had gone on longer, we might have removed the info field (and that will happen in 4.x). On the other hand there will always be a type that's 0 for empty contexts and nonzero for filled contexts (and that will still be the case in 4.x, although with a different set of types).