node icon indicating copy to clipboard operation
node copied to clipboard

Published 20 hours ago • verified publisher icon nodejs

test: use 2048 bit RSA keys

Open mmomtchev opened this issue 2 years ago • 7 comments

OpenSSL now requires at least 2048 Refs: https://github.com/nodejs/node/issues/44497

mmomtchev avatar Sep 02 '22 19:09 mmomtchev

Are you planning to update certs and keys in a follow-up PR?

lpinca avatar Sep 03 '22 08:09 lpinca

@lpinca Just realized that these were comitted too, done

mmomtchev avatar Sep 03 '22 11:09 mmomtchev

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

nodejs-github-bot avatar Sep 03 '22 11:09 nodejs-github-bot

@lpinca This turned to be much more laborious that I previously thought so I scaled it back to simply replacing all 1024 bit keys with 2048 bit keys, I am leaving the 2048->4096 transition to the next unlucky soul because there is a list of hard-coded test vectors produced with another crypto system meant for testing compatibility.

There is also the DH512 keys which have been blocked (?) because of the logjam attack, I will do them separately

mmomtchev avatar Sep 04 '22 00:09 mmomtchev

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

nodejs-github-bot avatar Sep 05 '22 19:09 nodejs-github-bot

node-test-commit-linuxone-rhel8-s390x is an abort on an out of memory condition - is this a flaky test?

mmomtchev avatar Sep 05 '22 20:09 mmomtchev

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

nodejs-github-bot avatar Sep 17 '22 19:09 nodejs-github-bot

Commit Queue failed
- Loading data for nodejs/node/pull/44498
✔  Done loading data for nodejs/node/pull/44498
----------------------------------- PR info ------------------------------------
Title      test: use 2048 bit RSA keys (#44498)
Author     Momtchil Momtchev  (@mmomtchev)
Branch     mmomtchev:rsa-key-length -> nodejs:main
Labels     crypto, test, needs-ci, review wanted
Commits    6
 - test: upgrade all 1024 bit RSA keys to 2048 bits
 - add missed fingerprints
 - renew ca1 with a 2048 bit key
 - run the linter
 - deprecate 1024 bit rsa keys for benchmarking
 - change all default keylengths to 2048
Committers 1
 - Momtchil Momtchev 
PR-URL: https://github.com/nodejs/node/pull/44498
Refs: https://github.com/nodejs/node/issues/44497
Reviewed-By: Luigi Pinca 
Reviewed-By: Michael Dawson 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/44498
Refs: https://github.com/nodejs/node/issues/44497
Reviewed-By: Luigi Pinca 
Reviewed-By: Michael Dawson 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 02 Sep 2022 19:35:00 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/44498#pullrequestreview-1095481223
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/44498#pullrequestreview-1119033840
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-09-17T19:12:17Z: https://ci.nodejs.org/job/node-test-pull-request/46641/
- Querying data for job/node-test-pull-request/46641/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 44498
From https://github.com/nodejs/node
 * branch                  refs/pull/44498/merge -> FETCH_HEAD
✔  Fetched commits as 7e0097d8a33f..a27d6ae652db
--------------------------------------------------------------------------------
[main 9b5334fe96] test: upgrade all 1024 bit RSA keys to 2048 bits
 Author: Momtchil Momtchev 
 Date: Sat Sep 3 14:46:47 2022 +0000
 72 files changed, 868 insertions(+), 658 deletions(-)
 create mode 100644 test/fixtures/keys/fake-startcom-root-issued-certs/03.pem
 create mode 100644 test/fixtures/keys/fake-startcom-root-issued-certs/04.pem
 delete mode 100644 test/fixtures/keys/rsa_private_1024.pem
 delete mode 100644 test/fixtures/keys/rsa_public_1024.pem
[main 2e82ef021c] add missed fingerprints
 Author: Momtchil Momtchev 
 Date: Sat Sep 3 21:12:26 2022 +0000
 1 file changed, 8 insertions(+), 8 deletions(-)
[main 5e00f420d5] renew ca1 with a 2048 bit key
 Author: Momtchil Momtchev 
 Date: Sat Sep 3 23:41:08 2022 +0000
 19 files changed, 192 insertions(+), 161 deletions(-)
[main 6ac0aad9f9] run the linter
 Author: Momtchil Momtchev 
 Date: Sun Sep 4 00:17:30 2022 +0000
 2 files changed, 2 insertions(+), 2 deletions(-)
[main bb5bd14d62] deprecate 1024 bit rsa keys for benchmarking
 Author: Momtchil Momtchev 
 Date: Sun Sep 4 16:07:16 2022 +0000
 2 files changed, 2 insertions(+), 2 deletions(-)
[main a8264f7716] change all default keylengths to 2048
 Author: Momtchil Momtchev 
 Date: Sun Sep 4 16:07:37 2022 +0000
 11 files changed, 11 insertions(+), 11 deletions(-)
   ✔  Patches applied
There are 6 commits in the PR. Attempting autorebase.
Rebasing (2/12)

Executing: git node land --amend --yes
✖  Git found no trailers in the original commit message, but 'Refs: https://github.com/nodejs/node/issues/44497' is present and should be a trailer.
warning: execution failed: git node land --amend --yes
You can fix the problem, and then run


git rebase --continue


Couldn't rebase 6 commits in the PR automatically
Please run the following commands to complete landing


$ git rebase origin/main -i -x "git node land --amend"
$ git node land --continue
https://github.com/nodejs/node/actions/runs/3117174528

nodejs-github-bot avatar Sep 24 '22 05:09 nodejs-github-bot

@lpinca @mhdawson is this a problem with the commit message?

mmomtchev avatar Sep 24 '22 14:09 mmomtchev

There should be a blank line between the commit message body and the Refs: line.

lpinca avatar Sep 24 '22 18:09 lpinca

Commit Queue failed
- Loading data for nodejs/node/pull/44498
✔  Done loading data for nodejs/node/pull/44498
----------------------------------- PR info ------------------------------------
Title      test: use 2048 bit RSA keys (#44498)
Author     Momtchil Momtchev  (@mmomtchev)
Branch     mmomtchev:rsa-key-length -> nodejs:main
Labels     crypto, test, needs-ci, review wanted, commit-queue-failed, commit-queue-squash
Commits    6
 - test: upgrade all 1024 bit RSA keys to 2048 bits
 - add missed fingerprints
 - renew ca1 with a 2048 bit key
 - run the linter
 - deprecate 1024 bit rsa keys for benchmarking
 - change all default keylengths to 2048
Committers 1
 - Momtchil Momtchev 
PR-URL: https://github.com/nodejs/node/pull/44498
Refs: https://github.com/nodejs/node/issues/44497
Reviewed-By: Luigi Pinca 
Reviewed-By: Michael Dawson 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/44498
Refs: https://github.com/nodejs/node/issues/44497
Reviewed-By: Luigi Pinca 
Reviewed-By: Michael Dawson 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 02 Sep 2022 19:35:00 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/44498#pullrequestreview-1095481223
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/44498#pullrequestreview-1119033840
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-09-24T05:36:18Z: https://ci.nodejs.org/job/node-test-pull-request/46641/
- Querying data for job/node-test-pull-request/46641/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 44498
From https://github.com/nodejs/node
 * branch                  refs/pull/44498/merge -> FETCH_HEAD
✔  Fetched commits as abfeed868442..a27d6ae652db
--------------------------------------------------------------------------------
[main 200070ef9e] test: upgrade all 1024 bit RSA keys to 2048 bits
 Author: Momtchil Momtchev 
 Date: Sat Sep 3 14:46:47 2022 +0000
 72 files changed, 868 insertions(+), 658 deletions(-)
 create mode 100644 test/fixtures/keys/fake-startcom-root-issued-certs/03.pem
 create mode 100644 test/fixtures/keys/fake-startcom-root-issued-certs/04.pem
 delete mode 100644 test/fixtures/keys/rsa_private_1024.pem
 delete mode 100644 test/fixtures/keys/rsa_public_1024.pem
[main 7f913b2c36] add missed fingerprints
 Author: Momtchil Momtchev 
 Date: Sat Sep 3 21:12:26 2022 +0000
 1 file changed, 8 insertions(+), 8 deletions(-)
[main b656b46dc4] renew ca1 with a 2048 bit key
 Author: Momtchil Momtchev 
 Date: Sat Sep 3 23:41:08 2022 +0000
 19 files changed, 192 insertions(+), 161 deletions(-)
[main e7d70b5260] run the linter
 Author: Momtchil Momtchev 
 Date: Sun Sep 4 00:17:30 2022 +0000
 2 files changed, 2 insertions(+), 2 deletions(-)
[main a8112cc86a] deprecate 1024 bit rsa keys for benchmarking
 Author: Momtchil Momtchev 
 Date: Sun Sep 4 16:07:16 2022 +0000
 2 files changed, 2 insertions(+), 2 deletions(-)
[main c574ef798e] change all default keylengths to 2048
 Author: Momtchil Momtchev 
 Date: Sun Sep 4 16:07:37 2022 +0000
 11 files changed, 11 insertions(+), 11 deletions(-)
   ✔  Patches applied
There are 6 commits in the PR. Attempting to fixup everything into first commit.
[main 0c7e21470e] test: upgrade all 1024 bit RSA keys to 2048 bits
 Author: Momtchil Momtchev 
 Date: Sat Sep 3 14:46:47 2022 +0000
 88 files changed, 962 insertions(+), 721 deletions(-)
 create mode 100644 test/fixtures/keys/fake-startcom-root-issued-certs/03.pem
 create mode 100644 test/fixtures/keys/fake-startcom-root-issued-certs/04.pem
 delete mode 100644 test/fixtures/keys/rsa_private_1024.pem
 delete mode 100644 test/fixtures/keys/rsa_public_1024.pem
   ✖  Git found no trailers in the original commit message, but 'Refs: https://github.com/nodejs/node/issues/44497' is present and should be a trailer.
https://github.com/nodejs/node/actions/runs/3119390838

nodejs-github-bot avatar Sep 24 '22 18:09 nodejs-github-bot

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

nodejs-github-bot avatar Sep 27 '22 12:09 nodejs-github-bot

No space left on the disk in Jenkins

mmomtchev avatar Sep 27 '22 15:09 mmomtchev

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

nodejs-github-bot avatar Sep 27 '22 15:09 nodejs-github-bot

Commit Queue failed
- Loading data for nodejs/node/pull/44498
✔  Done loading data for nodejs/node/pull/44498
----------------------------------- PR info ------------------------------------
Title      test: use 2048 bit RSA keys (#44498)
Author     Momtchil Momtchev  (@mmomtchev)
Branch     mmomtchev:rsa-key-length -> nodejs:main
Labels     crypto, test, needs-ci, review wanted, commit-queue-squash
Commits    6
 - test: upgrade all 1024 bit RSA keys to 2048 bits
 - add missed fingerprints
 - renew ca1 with a 2048 bit key
 - run the linter
 - deprecate 1024 bit rsa keys for benchmarking
 - change all default keylengths to 2048
Committers 1
 - Momtchil Momtchev 
PR-URL: https://github.com/nodejs/node/pull/44498
Refs: https://github.com/nodejs/node/issues/44497
Reviewed-By: Luigi Pinca 
Reviewed-By: Michael Dawson 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/44498
Refs: https://github.com/nodejs/node/issues/44497
Reviewed-By: Luigi Pinca 
Reviewed-By: Michael Dawson 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - test: upgrade all 1024 bit RSA keys to 2048 bits
   ⚠  - add missed fingerprints
   ⚠  - renew ca1 with a 2048 bit key
   ⚠  - run the linter
   ⚠  - deprecate 1024 bit rsa keys for benchmarking
   ⚠  - change all default keylengths to 2048
   ℹ  This PR was created on Fri, 02 Sep 2022 19:35:00 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/44498#pullrequestreview-1095481223
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/44498#pullrequestreview-1119033840
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-09-27T15:51:00Z: https://ci.nodejs.org/job/node-test-pull-request/46864/
- Querying data for job/node-test-pull-request/46864/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3137549599

nodejs-github-bot avatar Sep 27 '22 17:09 nodejs-github-bot

Landed in 8671e4a116e5.

lpinca avatar Sep 27 '22 18:09 lpinca

This is not landing clean in the v16.x branch; marking as "dont-land-on-v16.x"

=== release test-crypto-x509 ===
Path: parallel/test-crypto-x509
node:assert:124
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ 'C=US\nST=CA\nL=SF\nO=Joyent\nOU=Node.js\nCN=agent1\[email protected]'
- [Object: null prototype] {
-   C: 'US',
-   CN: 'agent1',
-   L: 'SF',
-   O: 'Joyent',
-   OU: 'Node.js',
-   ST: 'CA',
-   emailAddress: '[email protected]'
- }
    at Object.<anonymous> (/home/juanarbol/GitHub/node/test/parallel/test-crypto-x509.js:249:10)
    at Module._compile (node:internal/modules/cjs/loader:1155:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1209:10)
    at Module.load (node:internal/modules/cjs/loader:1033:32)
    at Function.Module._load (node:internal/modules/cjs/loader:868:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:22:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 'C=US\n' +
    'ST=CA\n' +
    'L=SF\n' +
    'O=Joyent\n' +
    'OU=Node.js\n' +
    'CN=agent1\n' +
    '[email protected]',
  expected: [Object: null prototype] {
    C: 'US',
    ST: 'CA',
    L: 'SF',
    O: 'Joyent',
    OU: 'Node.js',
    CN: 'agent1',
    emailAddress: '[email protected]'
  },
  operator: 'strictEqual'
}
Command: out/Release/node --expose-internals /home/juanarbol/GitHub/node/test/parallel/test-crypto-x509.js

juanarbol avatar Oct 04 '22 01:10 juanarbol