mbedtls
mbedtls copied to clipboard
Mbed-TLS
`x509_info_subject_alt_name`: Render HardwareModuleName as hex
Description
Change x509_info_subject_alt_name to render the serial numbers in HardwareModuleName SAN extensions (RFC 4108) in hex encoding.
Fixes #6262. More details about the reason of this change in the underlying issue.
Status
READY
Hi @Kabbah, thanks for your contribution!
Would you be able to rebase your first 2 commits on top of the development branch to avoid having merge commits in this PR?
Hi @Kabbah, thanks for your contribution!
Would you be able to rebase your first 2 commits on top of the
developmentbranch to avoid having merge commits in this PR?
Done!
We generally apply bug fixes to all supported versions of Mbed TLS: development, and the long-term support branches, currently mbedtls-2.28. It looks like the relevant code was already present in 2.28 so the bug fix should be backported.
In this case, it's possible that we might not want the bug fix, because there might be applications that have worked around the bug and that would be broken by the fix. I'm not sure how likely it is: I've only looked superficially at the bug description. @paul-elliott-arm @davidhorstmann-arm If you think it's better to leave 2.28 alone, please argue why.
@gilles-peskine-arm an omission on my part. I agree this needs a backport - it could conceivably break scripts that parse the output of programs/cert_app but it would be unlikely to break C programs using Mbed TLS (unless someone is doing something strange with the output of mbedtls_x509_crt_info()).
Thank you all for the reviews. At first I was unsure whether this would need a backport or not, because the bugfix requires a breaking change in the behavior of mbedtls_x509_crt_info() (even though it's simply an informational function). In the event we decide to backport this bugfix, should I reproduce the changes on top of the mbedtls-2.28 branch and open a new pull request?
@Kabbah that's correct, please open a new PR on top of mbedtls-2.28 (usually we use the same name with "[Backport 2.28]" in front) then link to it from here.
Looks good to me, thanks again! Please raise a backport on 2.28.
For future reference, when making new changes we'd prefer you to add new commits on the branch rather than force-pushing as it makes it easier to review new additions.
Thank you. I just submitted the backport PR (#6380). The changes I force-pushed were very minor: a simple code style fix (one space) and a test description update, and I preferred to fixup my previous commits rather than making new ones to avoid polluting upstream.
@paul-elliott-arm looks like this has had a very light update since your approval - if you can give it a quick look & the backport also, it should be a very quick job to get this merged & stop it hanging around / lighten the CI load.
