ECDSA Support#925
Conversation
|
Hi @hamano I've tried this code: `var keys = forge.pki.ecdsa.generateKeyPair({name:"p256"}); let privateKeyToPem = keys.privateKey.toPem(); But the key length p256 seems to be smaller as compared to the ones generated with openssl Keys generated with the forge: Key-pair created. publicKey-----BEGIN PUBLIC KEY----- openssl output: Also the CSR generation is failing, cause the CSR generation seems to be not be implemented |
|
Hi @msalado, That appears to be because per RFC 5915 Section 3, the code in this PR appears to not encode the public key in the private key (marked optional per spec link above). Which, from my reading of the RFC, is within spec. See e.g., https://github.com/digitalbazaar/forge/pull/925/files#diff-f515e671436f66ff2118cc1d6c71dd3978438e8a27094fffd057568ed129f64fR87 and https://github.com/digitalbazaar/forge/pull/925/files#diff-f515e671436f66ff2118cc1d6c71dd3978438e8a27094fffd057568ed129f64fR374 -- the latter in particular doesn't include the optional public key field. Also, when using a named curve, my understanding is that the |
|
Hi @cipherboy, Thank you that makes sense, in fact we used the key to generate a CSR using openssl and it seems to work: `$ openssl req -new -key test.key -nodes -out test.csr -subj "/CN=test/O=testing/C=MX" root@ubuntu:/home/san# openssl req -in test.csr -text Do you know how we may do the same with the forge library (using private key generated with "ecdsa" implemented by @hamano ) I modified the create-csr.js file like this: ` console.log('Generating 1024-bit key-pair...'); let privateKeyToPem = keys.privateKey.toPem(); console.log('Creating certification request (CSR) ...'); // sign certification request // PEM-format keys and csr console.log('\nKey-Pair:'); console.log('\nCertification Request (CSR):'); // verify certification request ` But I get the following error Key-pair created. publicKey-----BEGIN PUBLIC KEY----- Creating certification request (CSR) ... TypeError: Cannot read properties of undefined (reading 'toString') ` |
|
@msalado You can modify to include the public key, however some people prefer a compact format. |
|
Many code still assume RSA keys and need to be modified for EC keys. ex:
|
|
Thanks @hamano for the response. |
|
hi @hamano Could you please let us know, how to using ECDSA in type script angular 12, i need read .csr file |
|
@hamano Could you rebase this PR? |
|
@ayanamidev rebased |
|
Hi what is a status on this one ? Can I help somehow to speed it up? |
|
+1, any updates on this ? /cc @davidlehn, @dlongley |
|
Ping @davidlehn , @dlongley ❤️ |
|
Ping @davidlehn, @dlongley (: |
|
So I think the problem here is the use of |
|
How far are we from having this merged? |
@sanaullah82 I'm getting the same error, have you found a workaround? |
|
@hamano thanks for the initiative. My attempt to clone the repo for generating dist-minified files failed owing to npm dependency. I am trying to resolve the conflict, but, I was wondering if you have a latest min files that you could share as I use refer these via script tag directly? Please advise. TIA. |
The whole code base is so old and need someone to pay the time to reconstruct it. For example, based on your traceback I suggest you to add this patch:
Remember, this is only a temporary solution, and I havn't test this patch. Thank you 😄 Edit: My usage is mainly on x509, so you will see some code not suitable on that location, that's normal. My target isn't create a new PR for this PR. |
|
what plans to get this merged in? |
|
+1 |
|
Please merge this PR, there is a lot of people needing ECC support since a few years now.... |
|
Many companies are switching to ECC because it offers both smaller key sizes and increased difficulty to break. For example, in Germany, the use of ECC will be mandatory starting in 2026. Adding ECC curves appears to be highly beneficial. |
|
+1 |
|
+1 |
1 similar comment
|
+1 |
|
Thanks for this @hamano, the change set solved my use case. I made a single file Save it to |
|
+1 |
3 similar comments
|
+1 |
|
+1 |
|
+1 |
|
In case anyone still needs this, I made a library which supports ECDSA keys: https://github.com/jacobshirley/pki-lite |
|
This PR should absolutely not be merged, since it uses elliptic. See blog post: We found cryptography bugs in the elliptic library using Wycheproof. |
|
@paulmillr Those cryptography bugs have still not been fixed? From what I have learned, pure JavaScript cryptography is dangerous in general and state of the art is to use the Web Crypto API for something like ECDSA support (https://developer.mozilla.org/en-US/docs/Web/API/Web_Crypto_API ) ? Would you agree? |
no, that’s nonsense. It’s okay. There are some minimal side-channel leakages but they would be there even with wasm so it doesn’t matter. Web crypto api is garbage. For example there is no secp256k1 (one of top-3 curves) and seems like there would never be. |
|
@paulmillr Afaik the reason is that WebCrypto focuses on NIST-approved curves and other curves focused on general cryptographic internet standards? The advantage is that you do not have to take care of library updates and that CVEs are fixed by the browser vendors automatically? |
|
If you are able to use WC, it’s ok. Most of the time it’s garbage because the feature set is tiny, api is extremely complicated. Pure js cryptography is not dangerous. |
|
@paulmillr Pure JS cryptography is not dangerous if properly done, sorry if this was expressed misleadingly. It requires regular maintenance though and the maintainers must be trusted, as we can see here an issue could be introduced quite fast. |
|
@Blackbam everything requires maintenance and must be trusted, including your browser, your browser’s openssl bindings for webcrypto, browser’s native randomness generator (which failed catastrophically in the past), compilers which compile browser’s openssl (randomly disabled side-channel protections in the past), operating systems. An issue in any of those can also be introduced momentarily. |
I just added ECDSA support with elliptic.
Supported algo:
Features:
TODO: