Switch from Node's crypto to WebCrypto.subtle
#27
Conversation
keyz-tk
requested review from
jack-kearney,
andrewkmin,
MaxBlaushild,
r-n-o and
timurnkey
keyz-tk
force-pushed
the
isomorphic-crypto
branch
from
f44a2f5 to
361d31c
Compare
| key: privateKeyPkcs8Der, | ||
| }); | ||
| return await webcrypto.subtle.importKey( | ||
| "pkcs8", |
There was a problem hiding this comment.
webcrypto.subtle.importKey supports raw as encoding (link to docs). Can we use this instead of pkcs8? That'd be easier to deal with if it works!
There was a problem hiding this comment.
unfortunately no -- you'd think that it should work right? Then you see:
There was a problem hiding this comment.
😿
Dang it. Well, thanks for checking!
| * `SubtleCrypto.sign(...)` outputs signature in IEEE P1363 format: | ||
| * - https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/sign#ecdsa | ||
| * | ||
| * Turnkey expects the signature encoding to be DER-encoded ASN.1: | ||
| * - https://github.com/tkhq/tkcli/blob/7f0159af5a73387ff050647180d1db4d3a3aa033/src/internal/apikey/apikey.go#L149 | ||
| * | ||
| * Code modified from https://github.com/google/tink/blob/6f74b99a2bfe6677e3670799116a57268fd067fa/javascript/subtle/elliptic_curves.ts#L114 |
There was a problem hiding this comment.
Oh man. Painful. Thanks for going through this 🎖️
| * @param ieee the ECDSA signature in IEEE encoding | ||
| * @return ECDSA signature in DER encoding | ||
| */ | ||
| function convertEcdsaIeee1363ToDer(ieee: Uint8Array): Uint8Array { |
There was a problem hiding this comment.
I think some basic tests are in order for this function? I know it's indirectly tested by the high-level "signature is valid" test, but I'd feel better with a few unit tests given this function has pretty complex, hard-to-review-at-a-glance logic.
There was a problem hiding this comment.
yeah good point -- i'll copy some tests over from google's repo (assuming they have good coverage)
Summary & Motivation
Spiritual successor to #26. See inline comments re: the s u b t l e difference in signature encoding
How I Tested These Changes