Skip to content

ECDSA Support#925

Open
hamano wants to merge 5 commits into
digitalbazaar:mainfrom
OSSTech-JP:ecdsa
Open

ECDSA Support#925
hamano wants to merge 5 commits into
digitalbazaar:mainfrom
OSSTech-JP:ecdsa

Conversation

@hamano
Copy link
Copy Markdown

@hamano hamano commented Nov 22, 2021

I just added ECDSA support with elliptic.

Supported algo:

  • secp192r1, prime192v1
  • secp256r1, prime256v1
  • secp224r1
  • secp384r1
  • secp521r1
  • secp256k1

Features:

  • key generation
  • DER/PEM encoding/decoding
  • sign and verify
  • ECDSA certifiate validation

TODO:

  • derivation public key from private key

@msalado
Copy link
Copy Markdown

msalado commented Jan 12, 2022

Hi @hamano

I've tried this code:

`var keys = forge.pki.ecdsa.generateKeyPair({name:"p256"});

let privateKeyToPem = keys.privateKey.toPem();
let publicKeyToPem = keys.publicKey.toPem();
console.log('Key-pair created. privateKey' + privateKeyToPem);
console.log('Key-pair created. publicKey' + publicKeyToPem);
`

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. privateKey-----BEGIN EC PRIVATE KEY-----
MDECAQEEINQYnDIgVwCD881LL5NYArpFs9oZQZfWedn/X6ZfAL5loAoGCCqGSM49
AwEH
-----END EC PRIVATE KEY-----

Key-pair created. publicKey-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEZtBpf6rIVFsTWb9ss1uph5ECNowo
paijMJ6Ki/NrIeSQ38EwIfu7125b00H40EkyKrTEkIUFOscJppX0BG6/2g==
-----END PUBLIC KEY-----
`

openssl output:
-----BEGIN EC PARAMETERS----- BggqhkjOPQMBBw== -----END EC PARAMETERS----- -----BEGIN EC PRIVATE KEY----- MHcCAQEEIAHtTd1ScmU2mX3l/JayWj03hoOw97wE8XB0RAJ3HQAgoAoGCCqGSM49 AwEHoUQDQgAEMlXaf/9rgoOhqyUzYwKYE3ca5u4S7O8JsHT9Wrrvzex+L2Rn3NFs ZkLT688ySc9MiKaIVcpwcadbp2vAPxmrFg== -----END EC PRIVATE KEY-----


Also the CSR generation is failing, cause the CSR generation seems to be not be implemented

@cipherboy
Copy link
Copy Markdown

cipherboy commented Jan 12, 2022

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 EC PARAMETERS block is optional (see e.g., RFC 5840) when the curve is a known, named curve. Indeed, the blob you include above merely decodes to the OID of P-256.

@msalado
Copy link
Copy Markdown

msalado commented Jan 13, 2022

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
Certificate Request:
Data:
Version: 1 (0x0)
Subject: CN = test, O = testing, C = MX
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
pub:
04:66:d0:69:7f:aa:c8:54:5b:13:59:bf:6c:b3:5b:
a9:87:91:02:36:8c:28:a5:a8:a3:30:9e:8a:8b:f3:
6b:21:e4:90:df:c1:30:21:fb:bb:d7:6e:5b:d3:41:
f8:d0:49:32:2a:b4:c4:90:85:05:3a:c7:09:a6:95:
f4:04:6e:bf:da
ASN1 OID: prime256v1
NIST CURVE: P-256
Attributes:
a0:00
Signature Algorithm: ecdsa-with-SHA256
30:44:02:20:6a:87:fc:37:0c:db:8d:85:f0:f6:9f:ce:c2:a5:
71:d4:17:9b:c4:d2:fa:55:cf:5f:58:ce:7f:53:26:df:e5:bd:
02:20:40:a6:7a:0c:64:db:6d:7e:3a:d5:96:b0:1c:e1:3f:7a:
5c:10:3b:cf:28:00:e6:47:b8:58:ce:59:3f:40:30:94
-----BEGIN CERTIFICATE REQUEST-----
MIHoMIGQAgEAMC4xDTALBgNVBAMMBHRlc3QxEDAOBgNVBAoMB3Rlc3RpbmcxCzAJ
BgNVBAYTAk1YMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEZtBpf6rIVFsTWb9s
s1uph5ECNowopaijMJ6Ki/NrIeSQ38EwIfu7125b00H40EkyKrTEkIUFOscJppX0
BG6/2qAAMAoGCCqGSM49BAMCA0cAMEQCIGqH/DcM242F8PafzsKlcdQXm8TS+lXP
X1jOf1Mm3+W9AiBApnoMZNttfjrVlrAc4T96XBA7zygA5ke4WM5ZP0AwlA==
-----END CERTIFICATE REQUEST-----
`

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:

`
var forge = require('..');

console.log('Generating 1024-bit key-pair...');
var keys = forge.pki.ecdsa.generateKeyPair();

let privateKeyToPem = keys.privateKey.toPem();
let publicKeyToPem = keys.publicKey.toPem();
console.log('Key-pair created. privateKey' + privateKeyToPem);
console.log('Key-pair created. publicKey' + publicKeyToPem);

console.log('Creating certification request (CSR) ...');
var csr = forge.pki.createCertificationRequest();
csr.publicKey = keys.publicKey;
csr.setSubject([{
name: 'commonName',
value: 'example.org'
}, {
name: 'countryName',
value: 'US'
}, {
shortName: 'ST',
value: 'Virginia'
}, {
name: 'localityName',
value: 'Blacksburg'
}, {
name: 'organizationName',
value: 'Test'
}, {
shortName: 'OU',
value: 'Test'
}]);
// add optional attributes
csr.setAttributes([{
name: 'challengePassword',
value: 'password'
}, {
name: 'unstructuredName',
value: 'My company'
}]);

// sign certification request
csr.sign(keys.privateKey/, forge.md.sha256.create()/);
console.log('Certification request (CSR) created.');

// PEM-format keys and csr
var pem = {
privateKey: privateKeyToPem,
publicKey: publicKeyToPem,
csr: forge.pki.certificationRequestToPem(csr)
};

console.log('\nKey-Pair:');
console.log(pem.privateKey);
console.log(pem.publicKey);

console.log('\nCertification Request (CSR):');
console.log(pem.csr);

// verify certification request
try {
if(csr.verify()) {
console.log('Certification request (CSR) verified.');
} else {
throw new Error('Signature not verified.');
}
} catch(err) {
console.log('Certification request (CSR) verification failure: ' +
JSON.stringify(err, null, 2));
}

`

But I get the following error
`
Key-pair created. privateKey-----BEGIN EC PRIVATE KEY-----
MDECAQEEIJvjyKawPBDQSFCRPxr21ADIdQVjHUL2oW8mc9koiXLWoAoGCCqGSM49
AwEH
-----END EC PRIVATE KEY-----

Key-pair created. publicKey-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE7gwPsSiXdVy6remOaS59vqWUEpBe
j+foESrU7mtZ6BqjCgRruG8qcnREilkFs3ObYipTmf3HPPfjyZdzoVxMpA==
-----END PUBLIC KEY-----

Creating certification request (CSR) ...
/Users/msalado/git/forge-hamano/lib/rsa.js:1721
var hex = b.toString(16);
^

TypeError: Cannot read properties of undefined (reading 'toString')
at _bnToBytes (/Users/msalado/git/forge-hamano/lib/rsa.js:1721:15)
at Object.pki.publicKeyToRSAPublicKey (/Users/msalado/git/forge-hamano/lib/rsa.js:1430:7)
at Object.pki.publicKeyToAsn1.pki.publicKeyToSubjectPublicKeyInfo (/Users/msalado/git/forge-hamano/lib/rsa.js:1413:11)
at Object.pki.getCertificationRequestInfo (/Users/msalado/git/forge-hamano/lib/x509.js:2603:9)
at Object.csr.sign (/Users/msalado/git/forge-hamano/lib/x509.js:1828:40)
at Object. (/Users/msalado/git/forge-hamano/examples/create-csr.js:43:5)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)

`

@hamano
Copy link
Copy Markdown
Author

hamano commented Jan 14, 2022

@msalado
The OpenSSL EC private key format includes the public key, but it is optional in the specification.
I have listed the EC private key structure in the comments.

/*
 * RCF5915: Elliptic Curve Private Key Format
 * https://datatracker.ietf.org/doc/html/rfc5915
 *
 * ECPrivateKey ::= SEQUENCE {
 *   version        INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1),
 *   privateKey     OCTET STRING,
 *   parameters [0] ECParameters {{ NamedCurve }} OPTIONAL,
 *   publicKey  [1] BIT STRING OPTIONAL
 * }
 */

You can modify to include the public key, however some people prefer a compact format.
We could add a switch flag to include the public key.
Thanks.

@hamano
Copy link
Copy Markdown
Author

hamano commented Jan 14, 2022

Many code still assume RSA keys and need to be modified for EC keys.
I think we need an abstraction of the key object.

ex:

  • ECublicKey.prototype.toAsn1(options)
  • RSAPublicKey.prototype.toAsn1(options)

@sanaullah82
Copy link
Copy Markdown

Thanks @hamano for the response.
Could you please let us know, how to generate the CSR ? as the code shared by @msalado is failing.

Creating certification request (CSR) ...
/Users/msalado/git/forge-hamano/lib/rsa.js:1721
var hex = b.toString(16);
^

TypeError: Cannot read properties of undefined (reading 'toString')
at _bnToBytes (/Users/msalado/git/forge-hamano/lib/rsa.js:1721:15)
at Object.pki.publicKeyToRSAPublicKey (/Users/msalado/git/forge-hamano/lib/rsa.js:1430:7)
at Object.pki.publicKeyToAsn1.pki.publicKeyToSubjectPublicKeyInfo (/Users/msalado/git/forge-hamano/lib/rsa.js:1413:11)
at Object.pki.getCertificationRequestInfo (/Users/msalado/git/forge-hamano/lib/x509.js:2603:9)
at Object.csr.sign (/Users/msalado/git/forge-hamano/lib/x509.js:1828:40)
at Object. (/Users/msalado/git/forge-hamano/examples/create-csr.js:43:5)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)

@tanmac2905
Copy link
Copy Markdown

hi @hamano Could you please let us know, how to using ECDSA in type script angular 12, i need read .csr file

@ghost
Copy link
Copy Markdown

ghost commented Apr 16, 2022

@hamano Could you rebase this PR?

@hamano
Copy link
Copy Markdown
Author

hamano commented Jul 8, 2022

@ayanamidev rebased

@stoprocent
Copy link
Copy Markdown

Hi what is a status on this one ? Can I help somehow to speed it up?

@dany74q
Copy link
Copy Markdown

dany74q commented Nov 7, 2022

+1, any updates on this ?

/cc @davidlehn, @dlongley

@dany74q
Copy link
Copy Markdown

dany74q commented Nov 22, 2022

Ping @davidlehn , @dlongley ❤️

@dany74q
Copy link
Copy Markdown

dany74q commented Feb 23, 2023

Ping @davidlehn, @dlongley (:

@dlongley
Copy link
Copy Markdown
Member

So I think the problem here is the use of elliptic which has been criticized over the years for not doing its best to use constant timing where possible. The ASN.1 / related work is great and useful though. The core crypto implementation here should be swapped out, however, with either WebCrypto or https://github.com/paulmillr/noble-curves. The former being the best option and the latter being a secure and audited JS implementation that can provide backup when WebCrypto isn't available.

@LunaSquee
Copy link
Copy Markdown

How far are we from having this merged?

@otimoshc
Copy link
Copy Markdown

otimoshc commented Mar 1, 2024

Thanks @hamano for the response. Could you please let us know, how to generate the CSR ? as the code shared by @msalado is failing.

Creating certification request (CSR) ...
/Users/msalado/git/forge-hamano/lib/rsa.js:1721
var hex = b.toString(16);
^

TypeError: Cannot read properties of undefined (reading 'toString')
at _bnToBytes (/Users/msalado/git/forge-hamano/lib/rsa.js:1721:15)
at Object.pki.publicKeyToRSAPublicKey (/Users/msalado/git/forge-hamano/lib/rsa.js:1430:7)
at Object.pki.publicKeyToAsn1.pki.publicKeyToSubjectPublicKeyInfo (/Users/msalado/git/forge-hamano/lib/rsa.js:1413:11)
at Object.pki.getCertificationRequestInfo (/Users/msalado/git/forge-hamano/lib/x509.js:2603:9)
at Object.csr.sign (/Users/msalado/git/forge-hamano/lib/x509.js:1828:40)
at Object. (/Users/msalado/git/forge-hamano/examples/create-csr.js:43:5)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)

@sanaullah82 I'm getting the same error, have you found a workaround?

@cottiar
Copy link
Copy Markdown

cottiar commented May 15, 2024

@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.

@01101sam
Copy link
Copy Markdown

01101sam commented Jun 6, 2024

Thanks @hamano for the response. Could you please let us know, how to generate the CSR ? as the code shared by @msalado is failing.

Creating certification request (CSR) ...
/Users/msalado/git/forge-hamano/lib/rsa.js:1721
var hex = b.toString(16);
^

TypeError: Cannot read properties of undefined (reading 'toString')
at _bnToBytes (/Users/msalado/git/forge-hamano/lib/rsa.js:1721:15)
at Object.pki.publicKeyToRSAPublicKey (/Users/msalado/git/forge-hamano/lib/rsa.js:1430:7)
at Object.pki.publicKeyToAsn1.pki.publicKeyToSubjectPublicKeyInfo (/Users/msalado/git/forge-hamano/lib/rsa.js:1413:11)
at Object.pki.getCertificationRequestInfo (/Users/msalado/git/forge-hamano/lib/x509.js:2603:9)
at Object.csr.sign (/Users/msalado/git/forge-hamano/lib/x509.js:1828:40)
at Object. (/Users/msalado/git/forge-hamano/examples/create-csr.js:43:5)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)

@sanaullah82 I'm getting the same error, have you found a workaround?

The whole code base is so old and need someone to pay the time to reconstruct it.
Therefore, only partial code is implemented for own use and willing to share (Like this one)
Thanks to @hamano , I can add more s*it code to make those function working.
Please see my code of this PR, and add your own solution.

For example, based on your traceback I suggest you to add this patch:

Location: https://github.com/01101sam/node-forge/blob/2bb97afb5058285ef09bcf1d04d6bd6b87cffd58/lib/x509.js#L2501-L2516

Diff Change
pki.getCertificationRequestInfo = function(csr) {
  // CertificationRequestInfo
  var cri = asn1.create(asn1.Class.UNIVERSAL, asn1.Type.SEQUENCE, true, [
    // version
    asn1.create(asn1.Class.UNIVERSAL, asn1.Type.INTEGER, false,
      asn1.integerToDer(csr.version).getBytes()),
    // subject
    _dnToAsn1(csr.subject),
    // SubjectPublicKeyInfo
-   csr.publicKey.toAsn1(),
    // attributes
    _CRIAttributesToAsn1(csr)
  ]);

  return cri;
};
pki.getCertificationRequestInfo = function(csr) {
  // CertificationRequestInfo
  var cri = asn1.create(asn1.Class.UNIVERSAL, asn1.Type.SEQUENCE, true, [
    // version
    asn1.create(asn1.Class.UNIVERSAL, asn1.Type.INTEGER, false,
      asn1.integerToDer(csr.version).getBytes()),
    // subject
    _dnToAsn1(csr.subject),
    // SubjectPublicKeyInfo
+   csr.publicKey instanceof ECPublicKey ? csr.publicKey.toAsn1() : pki.publicKeyToAsn1(csr.publicKey),
    // attributes
    _CRIAttributesToAsn1(csr)
  ]);

  return cri;
};

Remember, this is only a temporary solution, and I havn't test this patch.
If you get a new error please fix it yourself by looking at stack traceback.

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.

@sibelius
Copy link
Copy Markdown

what plans to get this merged in?

@jkrajinovic
Copy link
Copy Markdown

+1

@defacto64
Copy link
Copy Markdown

Please merge this PR, there is a lot of people needing ECC support since a few years now....

@serdarde
Copy link
Copy Markdown

serdarde commented Nov 7, 2024

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.

@alexjs87
Copy link
Copy Markdown

alexjs87 commented Dec 1, 2024

+1

@k01255322
Copy link
Copy Markdown

+1

1 similar comment
@Blackbam
Copy link
Copy Markdown

+1

@petetnt
Copy link
Copy Markdown

petetnt commented Jun 11, 2025

Thanks for this @hamano, the change set solved my use case.

I made a single file .patch of it if someone else needs one, use it with patch-package

node-forge+1.3.1.patch

Save it to patches/node-forge+1.3.1+ecdsa-support.patch and run npx patch-package (or see the docs on how to run it in post-install).

@jacobshirley
Copy link
Copy Markdown

+1

3 similar comments
@statico
Copy link
Copy Markdown

statico commented Sep 12, 2025

+1

@Yiming075
Copy link
Copy Markdown

+1

@OptoCloud
Copy link
Copy Markdown

+1

@jacobshirley
Copy link
Copy Markdown

In case anyone still needs this, I made a library which supports ECDSA keys: https://github.com/jacobshirley/pki-lite

@paulmillr
Copy link
Copy Markdown

This PR should absolutely not be merged, since it uses elliptic.

See blog post: We found cryptography bugs in the elliptic library using Wycheproof.

@Blackbam
Copy link
Copy Markdown

Blackbam commented Dec 11, 2025

@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?

@paulmillr
Copy link
Copy Markdown

@Blackbam

pure JavaScript cryptography is dangerous

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.

@Blackbam
Copy link
Copy Markdown

@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?

@paulmillr
Copy link
Copy Markdown

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.

@Blackbam
Copy link
Copy Markdown

Blackbam commented Dec 12, 2025

@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.

@paulmillr
Copy link
Copy Markdown

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.