wrong RSA_PRV_DER_MAX_BYTES for odd MBEDTLS_MPI_MAX_SIZE #4094
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…MAX_SIZE is odd. if MBEDTLS_MPI_MAX_SIZE is odd then RSA_PRV_DER_MAX_BYTES will be two less than expected, since the macros are lacking parentheses. Signed-off-by: Daniel Otte <d.otte@wut.de>
…ble mistakes in usage. Signed-off-by: Daniel Otte <d.otte@wut.de>
chris-jones-arm
added
bug
Community
component-crypto
chris-jones-arm
added this to To Do
in OBSOLETE - PLEASE SEE https://github.com/orgs/Mbed-TLS/projects/2
via automation
chris-jones-arm
moved this from To Do
to In Progress
in OBSOLETE - PLEASE SEE https://github.com/orgs/Mbed-TLS/projects/2
chris-jones-arm
suggested changes
Feb 1, 2021
There was a problem hiding this comment.
Hi @d-otte, thanks for the well written bug report and subsequent PR. I'm generally happy with this PR apart from a few minor house-keeping points shown below.
Also as you mentioned in your issue, this bug exists in the 2.16 branch as well as the 2.7 branch. Could you please provide backports for both of these branches along with the changelog entry (mentioned below).
chris-jones-arm
removed
the
needs-review
Signed-off-by: Daniel Otte <d.otte@wut.de>
|
Thanks for adding the changelog @d-otte! Unfortunately it now seems to be failing CI (trailing whitespace and missing newline in the changelog). Could you please fix these errors and also rename the changelog entry to "Security" as opposed to "Bugfix" as I think this constitutes a security risk due to the potential buffer overflow. Sorry if my earlier wording was not clear on this. |
Signed-off-by: Daniel Otte <d.otte@wut.de>
chris-jones-arm
added
needs-ci
chris-jones-arm
approved these changes
Feb 2, 2021
chris-jones-arm
moved this from In Progress
to Has Approval
in OBSOLETE - PLEASE SEE https://github.com/orgs/Mbed-TLS/projects/2
yanesca
added
approved
OBSOLETE - PLEASE SEE https://github.com/orgs/Mbed-TLS/projects/2
automation
moved this from Has Approval
to Done
Feb 2, 2021
bensze01
removed this from Done
in OBSOLETE - PLEASE SEE https://github.com/orgs/Mbed-TLS/projects/2
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the PR for issue #4093
Description
It adds parenthesis to fix the computation of
RSA_PRV_DER_MAX_BYTESand also adds parenthesis to other macros to make them safer to use.