node icon indicating copy to clipboard operation
node copied to clipboard

Failing unit test: recent OpenSSL (logjam protection) / TLS 1.2 / DH

Open mmomtchev opened this issue 1 year ago • 3 comments

Version

18.8.0

Platform

Ubuntu 22.04 Jammy

Subsystem

crypto

What steps will reproduce the bug?

./node  test/parallel/test-tls-dhe.js

Error is visible with

/node/out/Release/openssl-cli s_client -cipher DHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256 -CAfile /node/test/fixtures/keys/agent2-cert.pem -connect 127.0.0.1:<insert port here>

How often does it reproduce? Is there a required condition?

maxVersion: 'TLSv1.2' in options of tls.createServer

Removing this line fixes the test.

What is the expected behavior?

Server uses specified DH key length and OpenSSL does not complain

What do you see instead?

4037CF125B7F0000:error:0A00018A:SSL routines:tls_process_ske_dhe:dh key too small:../deps/openssl/openssl/ssl/statem/statem_clnt.c:2098:

Additional information

No response

mmomtchev avatar Sep 04 '22 16:09 mmomtchev

In fact, after investigating - Ubuntu Jammy now requires a minimum of 2048 bit DH exchange keys - because of the Logjam attack. The problem is that this unit test verifies that the 512 bit keys are rejected and that the 1024/2048 keys are accepted. Now it will have to check what OpenSSL version/system configuration it is working with and either require the 1024 bit test to pass or to fail. Frankly, I don't think this will be a good test.

mmomtchev avatar Sep 07 '22 13:09 mmomtchev

In openSUSE, we have this defined to work-around system level policies in unit tests,

# Relax the crypto policies for the test-suite
export OPENSSL_SYSTEM_CIPHERS_OVERRIDE=xyz_nonexistent_file
export OPENSSL_CONF=''

Would this help in your case?

AdamMajer avatar Sep 16 '22 12:09 AdamMajer

@AdamMajer Yes, definitely, this is one of the solutions, but the real question here is what should the Node.js behavior be:

  • Adhere to the OS policy or
  • Enforce its own policy

mmomtchev avatar Sep 18 '22 15:09 mmomtchev

My take is that Node.js should enforce its own policy and ignore the OS configuration file. Why?

  • Node.js includes its own copy of the OpenSSL library and there cannot be any guarantees that it will be compatible with the OS configuration file
  • Node.js has a dedicated security team that can handle updates/decisions - which is the downside of ignoring the OS configuration

So, Node.js should probably proceed to deprecate 1024-bit DH exchanges - something that will apply to all OS

I see that a duplicate has been opened: https://github.com/nodejs/node/issues/44539

I am closing this one

mmomtchev avatar Oct 11 '22 07:10 mmomtchev

My take is that Node.js should enforce its own policy and ignore the OS configuration file.

@mmomtchev this should be the case since July -- Node.js will now look for a node_conf section in the openssl config file: https://nodejs.org/en/blog/vulnerability/july-2022-security-releases/ In practice this means that Node.js should use default OpenSSL 3 settings unless such as section has been added to the configuration file.

richardlau avatar Oct 11 '22 15:10 richardlau