Repository navigation
use ECDSA with SHA256 signers and verifiers for elliptic curve algorithms #8
Description
Please correct me if I'm wrong here, but I've noticed two things that together seem strange:
- The
RSA-SHAxxxhash functions are being used to create the signers and verifiers increateKeySignerandcreateKeyVerifier. createECDSASignerandcreateECDSAVeriferare simply wrappers aroundcreateKeySignerandcreateKeyVerifier, with a single modification to reformat the signature
This leads me to believe that the signers and verifiers are performing signing and verifying with the RSA-SHAxxx hash functions provided by Node's built-in crypto library, when they should be using ECDSA with SHA256.
Is there something I'm missing here? Is there another place where the signers and verifiers are being defined?
I came across an issue when I browserify-ed node-jsonwebtoken and noticed that the signing wasn't working.
Thanks!
Reference code:
function createKeySigner(bits) {
return function sign(thing, privateKey) {
if (!bufferOrString(privateKey) && !(typeof privateKey === 'object'))
throw typeError(MSG_INVALID_SIGNER_KEY);
thing = normalizeInput(thing);
// Even though we are specifying "RSA" here, this works with ECDSA
// keys as well.
const signer = crypto.createSign('RSA-SHA' + bits);
const sig = (signer.update(thing), signer.sign(privateKey, 'base64'));
return base64url.fromBase64(sig);
}
}
function createKeyVerifier(bits) {
return function verify(thing, signature, publicKey) {
if (!bufferOrString(publicKey))
throw typeError(MSG_INVALID_VERIFIER_KEY);
thing = normalizeInput(thing);
signature = base64url.toBase64(signature);
const verifier = crypto.createVerify('RSA-SHA' + bits);
verifier.update(thing);
return verifier.verify(publicKey, signature, 'base64');
}
}
function createECDSASigner(bits) {
const inner = createKeySigner(bits);
return function sign() {
var signature = inner.apply(null, arguments);
signature = formatEcdsa.derToJose(signature, 'ES' + bits);
return signature;
};
}
function createECDSAVerifer(bits) {
const inner = createKeyVerifier(bits);
return function verify(thing, signature, publicKey) {
signature = formatEcdsa.joseToDer(signature, 'ES' + bits).toString('base64');
const result = inner(thing, signature, publicKey);
return result;
};
}
Activity
pgaubatz commentedon Oct 10, 2015
Hi,
I ran in exactly the same issue.
My (probably too quick-and-dirty) solution looks like this: pgaubatz/browserify-sign@0dd151d
I'm not quite sure if it makes sense to submit a PR...
@shea256 What do you think?
Cheers,
Patrick
shea256 commentedon Oct 10, 2015
@pgaubatz I don't think this actually fixes the problem.
I just ended up writing my own JWT library: https://github.com/blockstack/jwt-js
Maybe it'll be helpful for you. It only currently supports the curve SECP256k1, but it is designed in a way that anyone can easily write a client for ES256, RS256, etc.
omsmith commentedon Oct 10, 2015
Yeah, so at the time the module was written, while openssl supported EC, there wasn't an algorithm available to call it with. So specifying RSA here gets it into openssl, and aftet it parses the PEM, it does the right thing.
I would be happy to accept a pull request which used the correct name (I looked when this issue first opened - it appears it might exist in openssl now) - perhaps deciding based off if the openssl or node.js version.
omsmith commentedon Oct 10, 2015
@shea256 ES256 is secp256r1, not k1
shea256 commentedon Oct 10, 2015
@omsmith yes, I'm aware.
I mentioned that my library only currently supports SECP256k1 (which I abbreviate as ES256k) and while my library currently doesn't support the widely accepted ES256 and RS256 standards, those could easily be added in. I might actually add in an ES256 client soon, which will use SECP256r1 (according to the standards).
samuelhorwitz commentedon Aug 7, 2016
There is an open PR on browserify-sign (what browserify uses to get node crypto to work) which addresses this. When that PR goes through, a change to this library to not masquerade as rsa and just use 'sha' + bits should work. Tried it out by monkeypatching locally.
samuelhorwitz commentedon Aug 8, 2016
Alright they merged that PR over in Browserify. @omsmith what were the limitations of a PR over here? I have a patch I wrote that's very dumb that basically just switches from
RSA-SHAtoshaprefixing for all ECDSA and works with Browserify as soon as that PR lands in a release. Unfortunately, I don't know too much about how this works with regards to Node or other versioning caveats. All I know is that Browserifying with these fixes works.omsmith commentedon Aug 8, 2016
@samuelhorwitz feel free to submit your changes as a PR and we'll see what happens with the test suite (which will run it against older versions of node as well).
use genericized names for openssl signed digests
use genericized names for openssl signed digests