Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upConversation
benjamn
added some commits
Aug 2, 2016
benjamn
added this to the Release 1.4.1 milestone
Aug 2, 2016
benjamn
self-assigned this
Aug 2, 2016
abernix
referenced this pull request
Aug 2, 2016
Closed
Document that npm-node-aes-gcm and dependents need OpenSSL to run on Windows #4461
benjamn
merged commit 7755d27
into
devel
Aug 2, 2016
glasser
reviewed
Aug 2, 2016
| decipher.setAuthTag(new Buffer(ciphertext.authTag, "base64")); | ||
| var chunks = [decipher.update( | ||
| new Buffer(ciphertext.ciphertext, "base64"))]; | ||
| chunks.push(decipher.final()); |
This comment has been minimized.
This comment has been minimized.
glasser
Aug 2, 2016
Member
Not clear to me at which step crypto throws or otherwise indicates decryption or authentication errors but it looks like this change does affect how errors are detected? Or do you need to add an error handler?
This comment has been minimized.
This comment has been minimized.
glasser
Aug 2, 2016
Member
Oh never mind. I remembered that we tried to remove details from errors but not how it was implemented. Should be fine.
This comment has been minimized.
This comment has been minimized.
benjamn
Aug 2, 2016
Member
For what it's worth, my understanding is that (de)cipher.final() throws if there are encryption/decryption errors.
This comment has been minimized.
This comment has been minimized.
|
There's also a no longer accurate comment in encrypt.js saying that the crypto API doesn't support AAD. Though I doubt we actually want to incur the compatibility issues of changing to follow the suggestion there. |
added a commit
that referenced
this pull request
Aug 2, 2016
benjamn commentedAug 2, 2016
Inspired by this comment from @glasser.