Skip to content

Use the same encryption growth macro consistently (3.1/3.0)#19624

Closed
mattcaswell wants to merge 1 commit intoopenssl:openssl-3.1from
mattcaswell:fix-encryption-growth2-31
Closed

Use the same encryption growth macro consistently (3.1/3.0)#19624
mattcaswell wants to merge 1 commit intoopenssl:openssl-3.1from
mattcaswell:fix-encryption-growth2-31

Conversation

@mattcaswell
Copy link
Member

We had two different macros for calculating the potential growth due to encryption. The macro we use for allocating the underlying buffer should be the same one that we use for reserving bytes for encryption growth.

Also if we are adding the MAC independently of the cipher algorithm then the encryption growth will not include that MAC so we should remove it from the amount of bytes that we reserve for that growth. Otherwise we might exceed our buffer size and the WPACKET_reserve operation will fail.

This is a backport of #19622 for 3.1/3.0.

Checklist
  • documentation is added or updated
  • tests are added or updated

We had two different macros for calculating the potential growth due to
encryption. The macro we use for allocating the underlying buffer should be
the same one that we use for reserving bytes for encryption growth.

Also if we are adding the MAC independently of the cipher algorithm then
the encryption growth will not include that MAC so we should remove it
from the amount of bytes that we reserve for that growth. Otherwise we
might exceed our buffer size and the WPACKET_reserve operation will
fail.
@mattcaswell mattcaswell added approval: review pending This pull request needs review by a committer approval: otc review pending branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) labels Nov 7, 2022
@hlandau
Copy link
Member

hlandau commented Nov 7, 2022

Agree urgent

@t8m t8m added the triaged: bug The issue/pr is/fixes a bug label Nov 7, 2022
@t8m t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Nov 7, 2022
@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

@mattcaswell
Copy link
Member Author

CI issue is not relevant. Setting to "ready to merge"

@mattcaswell mattcaswell added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Nov 14, 2022
@mattcaswell
Copy link
Member Author

Pushed to 3.1 and 3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants