-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
Update crypto.md to correct function description for decipher.setAAD
#33095
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
According to the [NodeJS CCM example](https://nodejs.org/docs/latest-v14.x/api/crypto.html#crypto_ccm_mode], when decrypting the `plaintextLength` parameter actually refers to the ciphertext length, not the plaintext length: ``` decipher.setAAD(aad, { plaintextLength: ciphertext.length }); ``` The same can be seen in the [OpenSSL docs](https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption) where a call to `EVP_DecryptUpdate` passes the ciphertext length: ``` /* Provide the total ciphertext length */ if(1 != EVP_DecryptUpdate(ctx, NULL, &len, NULL, ciphertext_len)) handleErrors(); ``` This parameter probably should have been called `inputLength` or `bufferLength` instead of `plaintextLength`, so that it makes sense both when encrypting and decrypting, but at least we can correct the sentence in the documentation for now to refer to the correct value.
@nodejs/crypto |
I am not sure if the PR in its current state is an improvement.
The
When I implemented this, I called the option I think a better approach would be to explain that the ciphertext length is equal to the plaintext length. |
That's a great explanation, it would be helpful to add it to the documentation. Since there is already a link to the CCM Mode section, how about adding it there? |
Add explanation in [CCM mode](https://nodejs.org/docs/latest-v14.x/api/crypto.html#crypto_ccm_mode], that ciphertext length is equal to plaintext length in Node.js crypto output because the authentication tag is returned separately: ``` decipher.setAAD(aad, { plaintextLength: ciphertext.length }); ``` The same can be seen in the [OpenSSL docs](https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption) where a call to `EVP_DecryptUpdate` passes the ciphertext length: ``` /* Provide the total ciphertext length */ if(1 != EVP_DecryptUpdate(ctx, NULL, &len, NULL, ciphertext_len)) handleErrors(); ```
@tniessen Do you want to take another look 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.
The entire paragraph is a verbatim quote from me, so I guess I'll have to approve it :)
Landed in d093e78 🎉 Thanks for the PR again! |
According to the [NodeJS CCM example](https://nodejs.org/docs/latest-v14.x/api/crypto.html#crypto_ccm_mode], when decrypting the `plaintextLength` parameter actually refers to the ciphertext length, not the plaintext length: ``` decipher.setAAD(aad, { plaintextLength: ciphertext.length }); ``` The same can be seen in the [OpenSSL docs](https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption) where a call to `EVP_DecryptUpdate` passes the ciphertext length: ``` /* Provide the total ciphertext length */ if(1 != EVP_DecryptUpdate(ctx, NULL, &len, NULL, ciphertext_len)) handleErrors(); ``` This parameter probably should have been called `inputLength` or `bufferLength` instead of `plaintextLength`, so that it makes sense both when encrypting and decrypting, but at least we can correct the sentence in the documentation for now to refer to the correct value. PR-URL: #33095 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
According to the [NodeJS CCM example](https://nodejs.org/docs/latest-v14.x/api/crypto.html#crypto_ccm_mode], when decrypting the `plaintextLength` parameter actually refers to the ciphertext length, not the plaintext length: ``` decipher.setAAD(aad, { plaintextLength: ciphertext.length }); ``` The same can be seen in the [OpenSSL docs](https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption) where a call to `EVP_DecryptUpdate` passes the ciphertext length: ``` /* Provide the total ciphertext length */ if(1 != EVP_DecryptUpdate(ctx, NULL, &len, NULL, ciphertext_len)) handleErrors(); ``` This parameter probably should have been called `inputLength` or `bufferLength` instead of `plaintextLength`, so that it makes sense both when encrypting and decrypting, but at least we can correct the sentence in the documentation for now to refer to the correct value. PR-URL: #33095 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
According to the [NodeJS CCM example](https://nodejs.org/docs/latest-v14.x/api/crypto.html#crypto_ccm_mode], when decrypting the `plaintextLength` parameter actually refers to the ciphertext length, not the plaintext length: ``` decipher.setAAD(aad, { plaintextLength: ciphertext.length }); ``` The same can be seen in the [OpenSSL docs](https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption) where a call to `EVP_DecryptUpdate` passes the ciphertext length: ``` /* Provide the total ciphertext length */ if(1 != EVP_DecryptUpdate(ctx, NULL, &len, NULL, ciphertext_len)) handleErrors(); ``` This parameter probably should have been called `inputLength` or `bufferLength` instead of `plaintextLength`, so that it makes sense both when encrypting and decrypting, but at least we can correct the sentence in the documentation for now to refer to the correct value. PR-URL: #33095 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
According to the [NodeJS CCM example](https://nodejs.org/docs/latest-v14.x/api/crypto.html#crypto_ccm_mode], when decrypting the `plaintextLength` parameter actually refers to the ciphertext length, not the plaintext length: ``` decipher.setAAD(aad, { plaintextLength: ciphertext.length }); ``` The same can be seen in the [OpenSSL docs](https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption) where a call to `EVP_DecryptUpdate` passes the ciphertext length: ``` /* Provide the total ciphertext length */ if(1 != EVP_DecryptUpdate(ctx, NULL, &len, NULL, ciphertext_len)) handleErrors(); ``` This parameter probably should have been called `inputLength` or `bufferLength` instead of `plaintextLength`, so that it makes sense both when encrypting and decrypting, but at least we can correct the sentence in the documentation for now to refer to the correct value. PR-URL: nodejs#33095 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
According to the [NodeJS CCM example](https://nodejs.org/docs/latest-v14.x/api/crypto.html#crypto_ccm_mode], when decrypting the `plaintextLength` parameter actually refers to the ciphertext length, not the plaintext length: ``` decipher.setAAD(aad, { plaintextLength: ciphertext.length }); ``` The same can be seen in the [OpenSSL docs](https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption) where a call to `EVP_DecryptUpdate` passes the ciphertext length: ``` /* Provide the total ciphertext length */ if(1 != EVP_DecryptUpdate(ctx, NULL, &len, NULL, ciphertext_len)) handleErrors(); ``` This parameter probably should have been called `inputLength` or `bufferLength` instead of `plaintextLength`, so that it makes sense both when encrypting and decrypting, but at least we can correct the sentence in the documentation for now to refer to the correct value. PR-URL: nodejs/node#33095 Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
According to the NodeJS CCM example, when decrypting the
plaintextLength
parameter actually refers to the ciphertext length, not the plaintext length:The same can be seen in the OpenSSL docs where a call to
EVP_DecryptUpdate
passes the ciphertext length:This parameter probably should have been called
inputLength
orbufferLength
instead ofplaintextLength
, so that it makes sense both when encrypting and decrypting, but at least we can correct the sentence in the documentation for now to refer to the correct value.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes