node icon indicating copy to clipboard operation
node copied to clipboard
verified publisher icon nodejs

Test failures with OpenSSL 3.2.2

Open richardlau opened this issue 1 year ago • 6 comments

A number of Node.js tests fail when Node.js is compiled dynamically linked against OpenSSL 3.2 (tested with OpenSSL 3.2.2). i.e.

./configure --shared-openssl
make -j 4 test-ci

Full tap results: https://gist.github.com/richardlau/ce642daf2ffd581755232a924f9f8f63

Failures:

  • [x] out/Release/node /home/nodejs/node/test/parallel/test-crypto-dh.js https://github.com/nodejs/node/pull/53503
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-http2-https-fallback.js Also fails with OpenSSL 3.0.14 https://github.com/nodejs/node/pull/53373
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-http2-server-unknown-protocol.js Also fails with OpenSSL 3.0.14 https://github.com/nodejs/node/pull/53373
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-https-client-checkServerIdentity.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-https-strict.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-alert-handling.js https://github.com/nodejs/node/pull/54909
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-cert-regression.js https://github.com/nodejs/node/pull/54968
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-client-getephemeralkeyinfo.js https://github.com/nodejs/node/pull/55030
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-alpn-server-client.js Also fails with OpenSSL 3.0.14 https://github.com/nodejs/node/pull/53373
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-client-mindhsize.js https://github.com/nodejs/node/pull/54739
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-client-renegotiation-13.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-client-auth.js https://github.com/nodejs/node/pull/54610
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-client-verify.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-empty-sni-context.js https://github.com/nodejs/node/pull/53384
  • [x] out/Release/node --no-warnings /home/nodejs/node/test/parallel/test-tls-dhe.js https://github.com/nodejs/node/pull/54903
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-getcipher.js https://github.com/nodejs/node/pull/54972
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-junk-server.js https://github.com/nodejs/node/pull/54926
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-multiple-cas-as-string.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-peer-certificate-encoding.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-multi-key.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-psk-circuit.js https://github.com/nodejs/node/pull/53384
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-sni-server-client.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-set-ciphers.js https://github.com/nodejs/node/pull/55016
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-sni-option.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-server-verify.js https://github.com/nodejs/node/pull/54599
  • [x] out/Release/node /home/nodejs/node/test/parallel/test-tls-junk-closes-server.js also reported in https://github.com/nodejs/node/issues/52482 https://github.com/nodejs/node/pull/55089#top

In our Jenkins CI we currently test in node-test-commit-linux-containered Node.js dynamically linked against OpenSSL 3.0 and 3.1. Addressing the above test failures would be required before we can add testing against OpenSSL 3.2 to the CI.

richardlau avatar Jun 07 '24 15:06 richardlau

FWIW I've added testing against OpenSSl 3.2 to my copy of node-test-commit-linux-containered: richardlau-node-test-commit-linux-containered

e.g. https://ci.nodejs.org/job/richardlau-node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_openssl32_x64/26/consoleFull which is showing the current test failures with main when linked against OpenSSL 3.2.2.

richardlau avatar Jun 13 '24 17:06 richardlau

In terms of updating what is included in Node.js, for now we want to stay on the LTS version of OpenSSL (3.0) for as long as possible. OpenSSL have not yet decided/announced what the next LTS version of OpenSSL will be. Making sure Node.js can build and execute tests successfully on later OpenSSL versions should hopefully ease the migration when it is eventually time.

richardlau avatar Jun 13 '24 17:06 richardlau

For those that want to help move it forward, you can run this job to confirm if you have fixed one of the tests - https://ci.nodejs.org/job/richardlau-node-test-commit-linux-containered/

mhdawson avatar Jun 24 '24 14:06 mhdawson

OpenSSL 3.2 does document:

The default security level can be configured when OpenSSL is compiled by setting -DOPENSSL_TLS_SECURITY_LEVEL=level. If not set then 2 is used.

https://docs.openssl.org/3.2/man3/SSL_CTX_set_security_level/#notes

Compare this to earlier versions (3.1, 3.0) where the default seclevel is 1:

  • OpenSSL 3.1: https://docs.openssl.org/3.1/man3/SSL_CTX_set_security_level/#notes
  • OpenSSL 3.0: https://docs.openssl.org/3.0/man3/SSL_CTX_set_security_level/#notes

We don't set -DOPENSSL_TLS_SECURITY_LEVEL when compiling OpenSSL in Node.js in upstream so will inherit the default.

Since we've had other issues relating to key sizes – perhaps the right long term solution would be to provide an API (either public or just for tests) to get the seclevel from OpenSSL at runtime and then Node.js' test could be selective (i.e. match the size of keys being used to the seclevel being run at).

The changing default seclevel also highlights that while OpenSSL 3.x is ABI compatible, updating (e.g. 3.0 -> 3.2) may still cause observable changes (i.e. with the increased seclevel smaller keys are no longer allowed out of the box without dropping the seclevel down to what it was previously).

richardlau avatar Aug 28 '24 14:08 richardlau

I ran the CI variant again on main: https://ci.nodejs.org/job/richardlau-node-test-commit-linux-containered/32/nodes=ubuntu2204_sharedlibs_openssl32_x64/

This is showing a new failure in parallel.test-tls-set-sigalgs:

  (node:2717679) [DEP0060] DeprecationWarning: The `util._extend` API is deprecated. Please use Object.assign() instead.
  (Use `node --trace-deprecation ...` to show where the warning was created)
  node:assert:126
    throw new AssertionError(obj);
    ^

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

  + 'ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE'
  - 'ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE'
      at /home/iojs/build/workspace/richardlau-node-test-commit-linux-containered/test/parallel/test-tls-set-sigalgs.js:51:16
      at /home/iojs/build/workspace/richardlau-node-test-commit-linux-containered/test/common/index.js:488:15
      at /home/iojs/build/workspace/richardlau-node-test-commit-linux-containered/test/common/index.js:488:15
      at maybeCallback (/home/iojs/build/workspace/richardlau-node-test-commit-linux-containered/test/fixtures/tls-connect.js:97:7)
      at TLSSocket.<anonymous> (/home/iojs/build/workspace/richardlau-node-test-commit-linux-containered/test/fixtures/tls-connect.js:73:13)
      at TLSSocket.emit (node:events:520:28)
      at emitErrorNT (node:internal/streams/destroy:170:8)
      at emitErrorCloseNT (node:internal/streams/destroy:129:3)
      at process.processTicksAndRejections (node:internal/process/task_queues:90:21) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: 'ERR_SSL_SSL/TLS_ALERT_HANDSHAKE_FAILURE',
    expected: 'ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE',
    operator: 'strictEqual'
  }

  Node.js v23.0.0-pre

This didn't fail before (see description) likely due to the changes from https://github.com/nodejs/node/pull/54208 landing since the run for the description.

richardlau avatar Aug 28 '24 15:08 richardlau

Great news -- main now passes all tests with OpenSSL 3.2.3! Big thanks to @mhdawson for working through many of the issues.

Once we have the fixes backported to all supported release lines we can add OpenSSL 3.2.3 testing to the main node-test-commit-linux-containered job.

richardlau avatar Sep 26 '24 12:09 richardlau

With v20.18.1 merged, all supported release lines now have the test fixes for OpenSSL 3.2 support.

I've added testing with OpenSSL 3.2 to node-test-commit-linux-containered.

Test builds:

  • main: https://ci.nodejs.org/job/node-test-commit-linux-containered/47504/nodes=ubuntu2204_sharedlibs_openssl32_x64/
  • v23.x-staging: https://ci.nodejs.org/job/node-test-commit-linux-containered/47508/nodes=ubuntu2204_sharedlibs_openssl32_x64/
  • v22.x-staging: https://ci.nodejs.org/job/node-test-commit-linux-containered/47507/nodes=ubuntu2204_sharedlibs_openssl32_x64/
  • v20.x-staging: https://ci.nodejs.org/job/node-test-commit-linux-containered/47506/nodes=ubuntu2204_sharedlibs_openssl32_x64/
  • v18.x-staging: https://ci.nodejs.org/job/node-test-commit-linux-containered/47505/nodes=ubuntu2204_sharedlibs_openssl32_x64/

richardlau avatar Nov 20 '24 18:11 richardlau