Repository navigation
Crypto - sign.update, verifier.update default parameter for strings 'utf8' not really true? #7712
Description
- Version:v4.4.5
- Platform:Linux debian-john 3.16.0-4-586 deps: update openssl to 1.0.1j #1 Debian 3.16.7-ckt25-2 (2016-04-08) i686 GNU/Linux
- Subsystem:crypto
In the docs here:
https://nodejs.org/api/crypto.html#crypto_sign_update_data_input_encoding
It states:
"If encoding is not provided, and the data is a string, an encoding of 'utf8' is enforced."
I found if I didn't specify that parameter the calculated signature would be incorrect (not matching a standard implementation using Bouncy Castle crypto apis in Windows) if there were any unicode characters in the source string to be signed.
If I specified the parameter utf8 then it worked correctly both ways between Bouncy Castle's crypto apis in windows and Node crytpo under linux.
Same issue with verifier.update was found as well.
I suggest the docs should be updated to specify that it's (likely) binary by default if it's a string or a buffer or the sign.update and verifier.update code changed to act as documented.
Here is a snippet of code I was using for testing:
var sign = crypto.createSign('RSA-SHA256');
var testData = 'Что такое Unicode?';
sign.update(testData,'utf8');//<<===== The 'utf8' parameter should not be necessary as this is a string according to the docs, but it definitely is necessary.
Activity
[-]Crypto - sign.update, verifier.update default parameter for strings not really true?[/-][+]Crypto - sign.update, verifier.update default parameter for strings 'utf8' not really true?[/+]MylesBorins commentedon Jul 13, 2016
/cc @nodejs/crypto
addaleax commentedon Jul 13, 2016
The default encoding has been changed to utf8 in Node v6 (#5522). The link to the docs you provided is for Node v6; Docs for v4 are at https://nodejs.org/dist/latest-v4.x/docs/api/.
I’m going ahead and closing this, but feel free to ask follow-up questions.
WhatFreshHellIsThis commentedon Jul 13, 2016
Doh! I'm very sorry about that, I forgot how old the packaged Node is with Debian. Thank you for your patience.
Just FYI those node 4 docs actually say the same thing, perhaps those docs need to be updated?:
"If encoding is not provided, and the data is a string, an encoding of 'utf8' is enforced"
addaleax commentedon Jul 13, 2016
Umm, no, you’re right, that is in the v4 docs.
The docs were changed in #5500, even before #5522… /cc @mscdex
mscdex commentedon Jul 13, 2016
@addaleax #5500 was landed after #5522. The problem was that #5500 should not have been backported to v4.x. Or at least the references in it to
'utf8'should have been changed to'binary'for v4.x.doc: correct crypto encoding information
tniessen commentedon Jun 3, 2017
I think this was resolved thanks to @addaleax in #7799, feel free to reopen if I am wrong.