crypto: add support for x25119 and x448 key pair generation

[panva]

This adds support for key pair generation for x25519 and x448 so that #26626 is one step closer.

Refs: #26626 - ECDH with x25519 and x448 keys

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[panva]

/cc @sam-github @tniessen

[bnoordhuis]

The PR basically LGTM but can you also add regression tests to test/parallel/test-crypto-key-objects.js? Thanks.

[panva]

The PR basically LGTM but can you also add regression tests to test/parallel/test-crypto-key-objects.js? Thanks.

indeed. coming up

[tniessen]

I guess we should be careful with supporting key types, but I don't have much experience around ECDH keys so I hope others do.

[tniessen]

With this in place i think (changed my mind) all these new keys should all have the same type (e.g. okp)

See #26709 (comment).

[bnoordhuis]

CI: https://ci.nodejs.org/job/node-test-pull-request/21664/

[bnoordhuis]

I guess we should be careful with supporting key types

I don't disagree but since it's mostly transparent to Node.js because it goes through openssl's EVP API, I don't see a problem with supporting them.

[panva]

The CI failures don't seem to be related to the PR. Ideas?

[tniessen]

CI is known to be flaky from time to time, let's see what happens when I resume it: https://ci.nodejs.org/job/node-test-pull-request/21671/

[panva]

✅, ping the team?

[sam-github]

I think for the next key type, we are going to need to refactor our docs to have a section on key types, and the invalid messages will have to change to be "not a supported key type" rather than an ever growing list of types. But, that shouldn't block this.

[tniessen]

CI: https://ci.nodejs.org/job/node-test-pull-request/21842/

[sam-github]

@targos @BethGriggs This was landed on v11.x-staging, but it doesn't work against openssl 1.1.0, which doesn't support those. I found this while testing #26951 against openssl 1.1.0, you'll find out soon, too. So, I think it needs popping off of v11.x-staging, and to be backported.

[panva]

>,< there will be more in that same boat i guess then (e.g. #26319, which i assumed was labeled to not land by accident). I thought CI handles these cases.

[tniessen]

Oh, right, I forgot about that, too... We are targeting 1.1.1 but still allow linking against 1.1.0 IIRC :(

[sam-github]

when prepping the release, people don't pull a single commit onto v11.x-staging, run full ci, pull another, etc. They do more of a bulk, test, hope its good (it should be), pull a bunch more. Then narrow down problems to single commits if they occur. So, when @BethGriggs started work on 11.x, she would notice this.

Maybe this is enough:

diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js
index fb1c3de1d7..f18ad41658 100644
--- a/test/parallel/test-crypto-key-objects.js
+++ b/test/parallel/test-crypto-key-objects.js
@@ -168,6 +168,9 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii');
   }, /null/);
 }
 
+if (!/1\.1\.1/.test(process.versions.openssl))
+  return common.skip('ec key types added in openssl 1.1.1');
+
 [
   { private: fixtures.readSync('test_ed25519_privkey.pem', 'ascii'),
     public: fixtures.readSync('test_ed25519_pubkey.pem', 'ascii'),

[sam-github]

Or, you could just not backport it. it depends if you are hoping to get it onto an LTS release. It will come in 12.x, but 12.x won't be LTS until the fall.

[targos]

I'm quite sure I ran tests before pushing to staging. Is it supposed to fail?

[sam-github]

@targos the shared_openssl1.1.0 test won't pass, I'm not sure what you ran, was that test part of it?

This is the one:

https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1604_sharedlibs_openssl110_x64/11817/

the key types this PR adds support for aren't supported by openssl 1.1.0, so the test has to be skipped when node is built against that openssl version.

[targos]

OK, I removed it from staging