Initial work to support RSA signing

[ChadKillingsworth]

I didn't really like any of the existing options to offload JWT signing from the main thread. Utilizing https://gist.github.com/irbull/08339ddcd5686f509e9826964b17bb59 as a guide, this is a very basic start to add signing.

I was able to test that the signatures are equal with:

const cryptoAsync = require('./');
const source = Buffer.from('foo');
const privateKey = Buffer.from(`-----BEGIN RSA PRIVATE KEY-----
MIICWwIBAAKBgQDdlatRjRjogo3WojgGHFHYLugdUWAY9iR3fy4arWNA1KoS8kVw
33cJibXr8bvwUAUparCwlvdbH6dvEOfou0/gCFQsHUfQrSDv+MuSUMAe8jzKE4qW
+jK+xQU9a03GUnKHkkle+Q0pX/g6jXZ7r1/xAK5Do2kQ+X5xK9cipRgEKwIDAQAB
AoGAD+onAtVye4ic7VR7V50DF9bOnwRwNXrARcDhq9LWNRrRGElESYYTQ6EbatXS
3MCyjjX2eMhu/aF5YhXBwkppwxg+EOmXeh+MzL7Zh284OuPbkglAaGhV9bb6/5Cp
uGb1esyPbYW+Ty2PC0GSZfIXkXs76jXAu9TOBvD0ybc2YlkCQQDywg2R/7t3Q2OE
2+yo382CLJdrlSLVROWKwb4tb2PjhY4XAwV8d1vy0RenxTB+K5Mu57uVSTHtrMK0
GAtFr833AkEA6avx20OHo61Yela/4k5kQDtjEf1N0LfI+BcWZtxsS3jDM3i1Hp0K
Su5rsCPb8acJo5RO26gGVrfAsDcIXKC+bQJAZZ2XIpsitLyPpuiMOvBbzPavd4gY
6Z8KWrfYzJoI/Q9FuBo6rKwl4BFoToD7WIUS+hpkagwWiz+6zLoX1dbOZwJACmH5
fSSjAkLRi54PKJ8TFUeOP15h9sQzydI8zJU+upvDEKZsZc/UhT/SySDOxQ4G/523
Y0sz/OZtSWcol/UMgQJALesy++GdvoIDLfJX5GBQpuFgFenRiRDabxrE9MNUZ2aP
FaFp+DyAe+b4nDwuJaW2LURbr8AEZga7oQj0uYxcYw==
-----END RSA PRIVATE KEY-----`);
cryptoAsync.sign('RSA-SHA256', privateKey, source, (err, signature) => {
  console.log('signature: ', signature.toString('base64'));
});

const crypto = require('crypto');
const signature2 = crypto.createSign('RSA-SHA256').update(source).sign(privateKey);
console.log('signature2:', signature2.toString('base64'));

Fair warning: it's been a LONG time since I've written C code. I'm willing to do the work needed to get this in a fully mergeable state, but I'm also perfectly happy if someone else wants to help contribute as well.

[brandonros]

Closes #6

[ChadKillingsworth]

We benchmarked this technique vs using Node 12 worker threads. We got better CPU and memory performance with crypto-async.

[jorangreef]

Thank you @ChadKillingsworth this is great work. I like how you have stuck to the conventions and nailed almost everything across the code base. Are you comfortable adding to the fuzz test and benchmark?

[jorangreef]

Also, it would be great to change the design slightly, so as to make the sign() method more versatile, so we can do verifications using the same code, without adding a verify() method.

See how the cipher() does this for encryption/decryption just using encrypt=0/1. We could do the same here. When signing, the signature argument would be written to as the target, when verifying, the signature argument would be read from as the signature to be verified. This is also similar to how the tag argument works for the AEAD ciphers.

[ChadKillingsworth]

I'd be happy to make these changes. The fuzzing / tests are a bit overwhelming. At a high level I understand what they are doing, but it's not very clear what I need to add for signing.

As for making a common sign/verify method - that sounds like a grand idea. Any suggestions as to what it should be called?

[brandonros]

@jorangreef this might be slightly offtopic but, does anything in this PR jump out at you as "memory leak worthy"?

[ChadKillingsworth]

I settled on the name signature for the method. The napi_external hint was enough for me to figure out how to make the key value safely.

With a 2048 bit key, creating the signatures, 50,000 iterations:

14279.814736008644 'milliseconds' - original method parsing key every call
8080.25746101141 'milliseconds' - new method reusing a napi_external key

Making a separate key function was definitely worth it.

The only part that's a little odd is that verifying a signature returns a boolean where as creating one returns a buffer.

[ChadKillingsworth]

I've now tested this successfully with both RSA and ECDSA keys (public and private).

[ChadKillingsworth]

@jorangreef What needs to be done to finish up this PR?

[jorangreef]

Thanks for all the work on this @ChadKillingsworth.

I am happy to take it from here to make a few small changes and update the test and benchmark.

I hope to get to this in a few days.

[jorangreef]

The test needs to verify that all possible exceptions are thrown for invalid arguments, to test the interface in the negative. And the test also needs to verify that outputs are correct for a few thousand randomly generated inputs, to test the interface in the positive. You can see how the tests for cipher, hash etc. work.

But don't worry, I will do this. Just thought you might appreciate the explanation.

[ChadKillingsworth]

Thanks - I hate leaving others with work but I'm having a hard time wrapping my head around what needs to be done with those tests.

We've been running this code in production for a couple of weeks now and there are no memory leaks.

[brandonros]

@jorangreef Are you able to help get this merged? :)

[jorangreef]

Thanks @brandonros, yes and I would like to. I will be able to get to this in 2 or 3 months. I am sorry for the delay.