Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename functions whose deprecated variant has been removed #4212

Closed
mpg opened this issue Mar 9, 2021 · 6 comments · Fixed by #4629
Closed

Rename functions whose deprecated variant has been removed #4212

mpg opened this issue Mar 9, 2021 · 6 comments · Fixed by #4629
Assignees
Labels
enhancement size-s Estimated task size: small (~2d)

Comments

@mpg
Copy link
Contributor

mpg commented Mar 9, 2021

This is a follow-up to #4029 which is about removing deprecated things, in particular deprecated functions.

In the 2.x branch when we found that a function's prototype turned out not to be satisfying, one common pattern to fix this was to (a) add a new function with a better prototype and a new name (often just adding _ext or _ret to the original name) and (b) deprecated the old function.

In those cases, once the old function has been removed as part of #4029, it becomes possible to use a more natural name for the (new) remaining function, by removing the suffix. This task is to determine for which functions such a renaming is desirable, and create further tasks to track execution of the renamings that were deemed desirable.

@mpg mpg added enhancement mbedtls-3 size-s Estimated task size: small (~2d) labels Mar 9, 2021
@mpg
Copy link
Contributor Author

mpg commented Mar 9, 2021

Initial list of functions concerned, as a discussion starter (the final list will be produced as part of #4029):

include/mbedtls/aes.h: * \deprecated      Superseded by mbedtls_internal_aes_encrypt()
include/mbedtls/aes.h: * \deprecated      Superseded by mbedtls_internal_aes_decrypt()
include/mbedtls/bignum.h: * \deprecated     Superseded by mbedtls_mpi_is_prime_ext() which allows
include/mbedtls/cipher.h: * \deprecated          Superseded by mbedtls_cipher_auth_encrypt_ext().
include/mbedtls/cipher.h: * \deprecated          Superseded by mbedtls_cipher_auth_decrypt_ext().
include/mbedtls/ctr_drbg.h: * \deprecated         Superseded by mbedtls_ctr_drbg_update_ret()
include/mbedtls/dhm.h: * \deprecated The hex-encoded primes from RFC 5114 are deprecated and are
include/mbedtls/dhm.h: * \deprecated The hex-encoded primes from RFC 3625 are deprecated and
include/mbedtls/ecdsa.h: * \deprecated      Superseded by mbedtls_ecdsa_write_signature() in
include/mbedtls/hmac_drbg.h: * \deprecated          Superseded by mbedtls_hmac_drbg_update_ret()
include/mbedtls/md.h: * \deprecated      Superseded by mbedtls_md_setup() in 2.0.0
include/mbedtls/md2.h: * \deprecated     Superseded by mbedtls_md2_starts_ret() in 2.7.0
include/mbedtls/md2.h: * \deprecated     Superseded by mbedtls_md2_update_ret() in 2.7.0
include/mbedtls/md2.h: * \deprecated     Superseded by mbedtls_md2_finish_ret() in 2.7.0
include/mbedtls/md2.h: * \deprecated     Superseded by mbedtls_internal_md2_process() in 2.7.0
include/mbedtls/md2.h: * \deprecated     Superseded by mbedtls_md2_ret() in 2.7.0
include/mbedtls/md4.h: * \deprecated     Superseded by mbedtls_md4_starts_ret() in 2.7.0
include/mbedtls/md4.h: * \deprecated     Superseded by mbedtls_md4_update_ret() in 2.7.0
include/mbedtls/md4.h: * \deprecated     Superseded by mbedtls_md4_finish_ret() in 2.7.0
include/mbedtls/md4.h: * \deprecated     Superseded by mbedtls_internal_md4_process() in 2.7.0
include/mbedtls/md4.h: * \deprecated     Superseded by mbedtls_md4_ret() in 2.7.0
include/mbedtls/md5.h: * \deprecated     Superseded by mbedtls_md5_starts_ret() in 2.7.0
include/mbedtls/md5.h: * \deprecated     Superseded by mbedtls_md5_update_ret() in 2.7.0
include/mbedtls/md5.h: * \deprecated     Superseded by mbedtls_md5_finish_ret() in 2.7.0
include/mbedtls/md5.h: * \deprecated     Superseded by mbedtls_internal_md5_process() in 2.7.0
include/mbedtls/md5.h: * \deprecated     Superseded by mbedtls_md5_ret() in 2.7.0
include/mbedtls/net.h: * \deprecated Superseded by mbedtls/net_sockets.h
include/mbedtls/ripemd160.h: * \deprecated     Superseded by mbedtls_ripemd160_starts_ret() in 2.7.0
include/mbedtls/ripemd160.h: * \deprecated     Superseded by mbedtls_ripemd160_update_ret() in 2.7.0
include/mbedtls/ripemd160.h: * \deprecated     Superseded by mbedtls_ripemd160_finish_ret() in 2.7.0
include/mbedtls/ripemd160.h: * \deprecated     Superseded by mbedtls_internal_ripemd160_process() in 2.7.0
include/mbedtls/ripemd160.h: * \deprecated     Superseded by mbedtls_ripemd160_ret() in 2.7.0
include/mbedtls/sha1.h: * \deprecated     Superseded by mbedtls_sha1_starts_ret() in 2.7.0.
include/mbedtls/sha1.h: * \deprecated     Superseded by mbedtls_sha1_update_ret() in 2.7.0.
include/mbedtls/sha1.h: * \deprecated     Superseded by mbedtls_sha1_finish_ret() in 2.7.0.
include/mbedtls/sha1.h: * \deprecated     Superseded by mbedtls_internal_sha1_process() in 2.7.0.
include/mbedtls/sha1.h: * \deprecated     Superseded by mbedtls_sha1_ret() in 2.7.0
include/mbedtls/sha256.h: * \deprecated     Superseded by mbedtls_sha256_starts_ret() in 2.7.0.
include/mbedtls/sha256.h: * \deprecated     Superseded by mbedtls_sha256_update_ret() in 2.7.0.
include/mbedtls/sha256.h: * \deprecated     Superseded by mbedtls_sha256_finish_ret() in 2.7.0.
include/mbedtls/sha256.h: * \deprecated     Superseded by mbedtls_internal_sha256_process() in 2.7.0.
include/mbedtls/sha256.h: * \deprecated     Superseded by mbedtls_sha256_ret() in 2.7.0.
include/mbedtls/sha512.h: * \deprecated     Superseded by mbedtls_sha512_starts_ret() in 2.7.0
include/mbedtls/sha512.h: * \deprecated     Superseded by mbedtls_sha512_update_ret() in 2.7.0.
include/mbedtls/sha512.h: * \deprecated     Superseded by mbedtls_sha512_finish_ret() in 2.7.0.
include/mbedtls/sha512.h: * \deprecated     Superseded by mbedtls_internal_sha512_process() in 2.7.0.
include/mbedtls/sha512.h: * \deprecated     Superseded by mbedtls_sha512_ret() in 2.7.0
include/mbedtls/ssl.h: * \deprecated     Superseded by \c mbedtls_ssl_conf_dh_param_bin.

@mpg
Copy link
Contributor Author

mpg commented Mar 9, 2021

My initial opinion was that we generally want to rename, but Gilles pointed out that in some cases we might not. Looking at the above list, I'm revising my initial impression:

  • for functions where we added _internal_ in the name we probably want to keep it;
  • for functions where we added _ret or _ext we probably want to remove it most of the time, but we should indeed examine the consequences for each function.

@gilles-peskine-arm
Copy link
Contributor

One consideration is: what migration path do we offer for applications written for the 2.x API?

“We don't have time, just let each developer cope” is not a good answer: this is work that all applications will need to do, so the Mbed TLS project should offer a solution that works for most people. It doesn't have to be Arm doing it, but it has to ship in Mbed TLS.

Given the tight timeline for 3.0, I don't think we can count on having a solution by the time of the 3.0 release, regardless of who does the work. Therefore I propose the following guideline: a typical application that does not use any deprecated feature and does not rely on internal details such as structure content should keep working.

As a consequence, the old names should remain available in 3.0. We can declare them as informally deprecated synonyms of new names, to be deprecated later once a renaming tool is available.

@mpg
Copy link
Contributor Author

mpg commented Mar 10, 2021

a typical application that does not use any deprecated feature and does not rely on internal details such as structure content should keep working

I think that's an excellent point. I also agree that it can easily be achieved (when we'd like to rename) by providing a simple #define for the old name.

@mpg
Copy link
Contributor Author

mpg commented Apr 29, 2021

Team decision: do the renaming and provide a header with #defines for compatibility.

@mpg mpg changed the title Consider renaming functions whose deprecated variant has been removed Rename functions whose deprecated variant has been removed May 4, 2021
@gilles-peskine-arm
Copy link
Contributor

Proposal:

  • For _ret functions, where the only difference is returning void vs int, rename the function in 3.0 and define the old name as a #define alias. Most applications will just keep working.
  • For _ext functions, where there are differences in the parameter sequence, do nothing in 3.0. Applications that were using the now-removed function need to be rewritten anyway so they might as well be rewritten to use the _ext function. The short name is free for new APIs in 3.x.

@TRodziewicz TRodziewicz self-assigned this Jun 7, 2021
@bensze01 bensze01 modified the milestone: Mbed TLS 4.0 Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants