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

Improve X.509 cert writing serial number management #6843

Closed
5 tasks
mpg opened this issue Dec 23, 2022 · 1 comment · Fixed by #6883
Closed
5 tasks

Improve X.509 cert writing serial number management #6843

mpg opened this issue Dec 23, 2022 · 1 comment · Fixed by #6883
Assignees
Labels
enhancement size-s Estimated task size: small (~2d)

Comments

@mpg
Copy link
Contributor

mpg commented Dec 23, 2022

We have a module for generating (writing) X.509 certificates, which currently internally stores the serial number as an MPI (see mbedtls_x509write_cert::serial) and accepts it from the user in the same form, see mbedtls_x509write_crt_set_serial.

This creates a direct dependency of X.509 certificate writing (MBEDTLS_X509_CRT_WRITE_C) on Bignum, which is unfortunate in the context of driver-only ECC work; this task is to eliminate it.

Proposed course of action:

  • Replace mbedtls_x509write_cert::serial with two fields: one array of 20 bytes (that's enough according to the RFC, one for the actual length. Adapt existing functions to that new form of internal storage. This will require replacing the call to mbedtls_asn1_write_mpi() with a call to an equivalent internal function that takes a buffer and length as input, to be written (note: the serial number is always non-negative).
  • Document mbedtls_x509write_crt_set_serial() as rejecting numbers >= 2^20, create a ChangeLog entry about it (noting that such numbers were already forbidden by the standard).
  • Guard the old function with #if defined(MBEDTLS_BIGNUM_C). This is currently redundant as X.509 depends on PK which depends on bignum, but will be handy when that changes.
  • Add a new function mbedtls_x509write_crt_set_serial_new() that accepts the serial number as a big-endian array of bytes.
  • Mark mbedtls_x509write_crt_set_serial() as deprecated and adapt existing library code and test to use the new function (but make sure the deprecated function is still tested in builds where it's present) - add a ChangeLog entry about that too.
@mpg
Copy link
Contributor Author

mpg commented Dec 29, 2022

Note: following #6830 (comment) I think it's a good idea to add a test in component_test_full_no_deprecated ensuring that X.509 no longer directly calls any MPI function when deprecated functions are removed. (Could also assert that it doesn't directly call any ECP function while at it.)

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.

2 participants