Introduce const qualifiers in the API wherever appropriate

[k-stachowiak]

• Type: Enhancement
• Priority: Minor

There are many places in the code that could benefit from using const qualifiers in the function arguments' lists. However introducing such change at this moment will cause a wide range API change. It is worth considering as it is an idiomatic C approach and the lack of the const qualifiers has been reported to cause inconvenience in third party code, which lead to submitting the following (potentially more) PRs:

• add various const annotations #1004 (pk.h)
• add various const specifiers to pk utils #1194 (pk.h)
• Constify init/free of mpi #1669 (bignum.h)
• ecp.h, ecdsa.h: the mbedtls_ecp_group *grp argument to many functions, and as a consequence pointers to structures containing an ECP group such as the ctx argument of mbedtls_ecdsa_read_signature and mbedtls_ecdsa_write_signature.

The main problem with the current API is that if somebody wishes to use const qualifiers in their code, they will need to cast away the constness of their pointers.

[RonEld]

related issue: #396

[jack-fortanix]

I would like to suggest a change that would not break any existing code but might make life easier for applications that want to use const where possible. Right now, many functions take non-const arguments that are logically const, for instance pk_write_key_der or x509_crt_verify. There is no reason for x509_crt_verify to modify its argument, but also nothing in the API contract that seems to prohibit it. For example, for all I know, you might decide sometime later that, since x509_crt_verify takes a non-const pointer argument, there is no problem with verification modifying some secret state of the mbedtls_x509_crt* passed in (say to cache the verification result).

If you cannot actually make any of the arguments const right now, so be it. But please for each such argument that you would like to make const and is (beyond being passed as a non-const pointer for historical reasons) actually const, just document in the Doxygen headers that "Despite taking them as non-const pointers, this function does not modify arguments x,z,y. In a future major release they will be made const.". Then, callers can know it is actually safe to cast const pointers to a non-const pointer and provide it to this function, knowing the function takes a non-const pointer just due to this compatibility issue and not because it does or might modify its argument.

An example where the situation is currently ambiguous - ecdh_calc_secret takes the ecdh_context as a non-const pointer. Is this just because you forgot to const it long ago and are now stuck with it, or because ecdh_calc_secret actually modifies something in the context? I mention this one as I know there are some hidden mutations happening in parts of the ECP code, and I have not dug enough yet to know which way it is for ecdh_calc_secret.

[RonEld]

Note: This issue enhances #803

[mpg]

* `ecp.h`, `ecdsa.h`: the `mbedtls_ecp_group *grp` argument to many functions, and as a consequence pointers to structures containing an ECP group such as the `ctx` argument of `mbedtls_ecdsa_read_signature` and `mbedtls_ecdsa_write_signature`.

I don't think that part is valid: mbedtls_ecp_mul() (and as a consequence, all functions that call it, which includes all crypto operations (as opposed to (de)serialiasing for example) do modify the grp structure (specifically its T member) when MBEDTLS_ECP_FIXED_POINT_OPTIM == 1. Arguably, this is a design mistake in the implementation of this optimisation, but until it's re-implemented with a better design, we just can't make grp const in most places.

[mpg]

I'm closing this as it has been superseded by #4033