- Notifications
You must be signed in to change notification settings - Fork5.2k
Fix over-zeroing of destination buffer for zero padding symmetric encryption#52465
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This addresses two issues with how zero padding is handled.The first is performance. When applying zero padding, we were clearingthe entire destination, then copying the plaintext over it. We werezeroing more data than required, the only data that needed to be zeroedis where the zero padding is applied.The second is in the case of overlapping buffers for the plaintextand ciphertext. However, this cannot happen currently since we alwaysensure the destination buffer does not overlap the input buffer.If overlapping is permitted in a future change, this would clear theplaintext, not just where padding is required.
ghost commentedMay 7, 2021
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly onearea label. |
ghost commentedMay 7, 2021
Tagging subscribers to this area:@bartonjs,@vcsjones,@krwq,@GrabYourPitchforks Issue DetailsThis addresses one issue with how zero padding is handled and another theoretical one. The first is performance. When applying zero padding, we were clearing the entire destination, then copying the plaintext over it. We were zeroing more data than required, the only data that needed to be zeroed is where the zero padding is applied. The second is in the case of overlapping buffers for the plaintext and ciphertext. However, this cannot happen currently since we always ensure the destination buffer does not overlap the input buffer. If overlapping is permitted in a future change, this would clear the plaintext, not just where padding is required. Contributes to#2406 I'm submitting this separately from the upcoming one-shot PR since that PR is getting fairly large and this is not immediately related to the one-shot APIs, but is adjacent.
|
vcsjones commentedMay 7, 2021
Just to be clear: nothing is "broken" right now. As-is, this is a performance improvement. But it will be more important for the one-shot APIs. |
This addresses one issue with how zero padding is handled and another theoretical one.
The first is performance. When applying zero padding, we were clearing the entire destination, then copying the plaintext over it. We were zeroing more data than required, the only data that needed to be zeroed is where the zero padding is applied.
The second is in the case of overlapping buffers for the plaintext and ciphertext. However, this cannot happen currently since we always ensure the destination buffer does not overlap the input buffer. If overlapping is permitted in a future change, this would clear the plaintext, not just where padding is required.
Contributes to#2406
I'm submitting this separately from the upcoming one-shot PR since that PR is getting fairly large and this is not immediately related to the one-shot APIs, but is adjacent.