digitalbazaar / forge

A native implementation of TLS in Javascript and tools to write crypto-based and network-heavy webapps
https://digitalbazaar.com/
Other
5.04k stars 778 forks source link

PBKDF2: Use UTF-8 encoding by default #487

Open kspearrin opened 7 years ago

kspearrin commented 7 years ago

SJCL creates the expected result (compared with other crypto libs as well). Forge makes something else.

Forge version 0.7.0

Example: https://jsfiddle.net/tgb1z1rn/2/

Correct

password: 123 salt: abc forge result: H8gykxsD40tS7/hbR95gqIKCy5SIUbgdaKq4S1hXNhI= sjcl result: H8gykxsD40tS7/hbR95gqIKCy5SIUbgdaKq4S1hXNhI=

Incorrect

password: 123ù salt: abc forge result: oUSVM/dLB8+NAd9Kq273ZyILVPp3BqWYEx4vUhWHkkc= sjcl result: UNSf8ufpIMgbVW/rn11BBbYkrm3kEHhb/W6gCcI/ozc=

Incorrect

password: 123 salt: abcù forge result: 8En13SHfP3HELhM3IGqbpML7al0dCury4etfU1vzZx0= sjcl result: XmLVG3R7rrsVOzYUjAguAooUqPHFE7M54DYSLb9+fts=

kspearrin commented 7 years ago

Looks like encoding the values with utf8 bytes prior to passing them to forge.pbkdf2 solves the issue:

forge.pbkdf2(forge.util.encodeUtf8(password.value), forge.util.encodeUtf8(salt.value), 5000, 256 / 8, "sha256")

https://jsfiddle.net/tgb1z1rn/3/

Perhaps this should be the default?

FGasper commented 7 years ago

I just ran into this issue as well. I first was doing unescape( encodeURIComponent( password ) ), but encodeUtf8() is at least marginally cleaner.

… and, yes, I agree that this is desirable as default behavior. At least have it try again if the decode fails and there are code points above 255 in the string.