Skip to content
This repository has been archived by the owner on Oct 23, 2020. It is now read-only.

Add ECDSA prototype #18

Merged
merged 1 commit into from
Sep 2, 2019
Merged

Add ECDSA prototype #18

merged 1 commit into from
Sep 2, 2019

Conversation

tniessen
Copy link
Member

This should have been easy, but OpenSSL generates ASN.1 signatures while WebCrypto uses a simpler format, so we need to implement some (potentially slow) conversion logic.

As for RSA, only SPKI and PKCS#8 are currently supported for import/export.

Fixes: #6

@tniessen tniessen added the new algorithm Issues and PRs about new algorithms label Aug 17, 2019
@tniessen tniessen requested a review from sam-github August 17, 2019 10:38
@panva
Copy link
Member

panva commented Aug 17, 2019

@tniessen
Copy link
Member Author

Seems to be another thing that was only standardized as part of JOSE / JWK / WebCrypto. I'm not sure if it is a good idea to add small parts of those things to node core, but it would be easy to implement efficiently.

@panva
Copy link
Member

panva commented Aug 17, 2019

But you do kind of feel like doing this conversion on a library level sucks right? :)

@tniessen
Copy link
Member Author

Sure, it's the daily "What belongs in core" discussion: It would definitely be useful to have this in core for a few use cases, but I am worried about turning node core into a patchwork of partially supported JOSE / JWK features. I'm not saying that it should not be in core, just sharing my concerns.

@panva
Copy link
Member

panva commented Aug 17, 2019

COSE follows the same path btw.

@tniessen
Copy link
Member Author

I know. They probably do because the whole COSE standard was designed based on JOSE / JWK, and CBOR itself is just a neat way to express JSON IIRC.

Again, I am not saying it should not be a part of node core, but I want to be careful not to create an even worse patchwork than node crypto currently is. Adding more and more small features that only make sense within JOSE / COSE (without intending to ever fully support JOSE / COSE) does not seem like a good idea to me. But I do see the advantages :-)

@tniessen
Copy link
Member Author

I researched this a bit more. It seems that the ASN.1 format (despite being fully standardized in 2002 in RFC3279, and extended in SEC 1 in 2009) predates the year 2000. Most crypto libraries and protocols seem to employ this well-standardized and widely used format.

In 2000, the IEEE P1363 standard was published. Section E.3.1 suggests concatenation of r and s to represent DSA signatures (and ECDSA is not at all different from DSA in this case), but also explicitely mentions ASN.1 as a more structured alternative. And while that section can't really be counted as a specification, some crypto libraries seem to have picked it up. As far as I can tell, at least .NET and pycryptodome use the P1363 format by default, and Crypto++ at least supports it.

So it seems that this "P1363 encoding" does have some (more or less standardized) existence outside of JOSE / COSE, which, in my opinion, makes it a candidate for inclusion in node core. What do you think, @panva and @sam-github?

@panva
Copy link
Member

panva commented Aug 18, 2019

It is of no surprise to me that the webcrypto/JOSE/COSE signature format is not proprietary and is rooted in something generally available and I'd love to see the option to produce and accept r and s simply concatenated as an option for core sign/verify functions.

I'm going to repeat myself but here goes - one should not need to write asn1 schemas and use community parsers for something this fundamental (by today's standards).

This goes for ASN.1 ECDSA signatures, x509 certs and likely more as you'll find out yourself while working on webcrypto.

As a sidenote - EdDSA signatures produced by OpenSSL are already in this "r || s" format.

@tniessen tniessen merged commit 7d87f8c into nodejs:master Sep 2, 2019
@tniessen tniessen deleted the add-ecdsa branch September 2, 2019 18:50
@tniessen tniessen added this to the Standard algorithms milestone Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new algorithm Issues and PRs about new algorithms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for ECDSA
2 participants