-
Notifications
You must be signed in to change notification settings - Fork 38
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
Make AES GCM more consistent cross platform #189
Conversation
Co-authored-by: Michael Graeb <graebm@amazon.com>
Co-authored-by: Michael Graeb <graebm@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix & ship
source/windows/bcrypt_aes.c
Outdated
int ret_val = s_aes_default_encrypt(cipher, &final_cur, out); | ||
int ret_val = AWS_OP_SUCCESS; | ||
if (final_cur.len > 0) { | ||
s_aes_default_encrypt(cipher, &final_cur, out); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still not checking result here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugh, im dumb. stashed the fix to merge in auto merges and never poped it
Issue #, if available:
Description of changes:
Warning Backwards Incompatible change: moved gcm tag out of the constructor into a separate setter, since it was leading to a whole bunch of annoying corner cases, that was not handled consistently cross platform. And arguably having tag as a separate setter is a bit cleaner design, since we already complicate logic, by abstracting away encryptor vs decryptor
Outside of that, changes are mostly extra tests and corresponding changes to make everything consistent cross platform
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.