Tests fail - dummy SSL key too small

[sgpinkus]

When running the tests on latest (v1.17.3) I get:

  ...
  124 passing (3s)
  1 failing

  1) session()
       req.session
         .cookie
           .secure
             should set cookie when secure:
     Error: error:140AB18F:SSL routines:SSL_CTX_use_certificate:ee key too small
      at node:internal/tls/secure-context:65:13
      at Array.forEach (<anonymous>)
      at setCerts (node:internal/tls/secure-context:63:3)
      at configSecureContext (node:internal/tls/secure-context:152:5)
      at Object.createSecureContext (node:_tls_common:117:3)
      at Server.setSecureContext (node:_tls_wrap:1346:27)
      at Server (node:_tls_wrap:1205:8)
      at new Server (node:https:69:3)
      at Object.createServer (node:https:105:10)
      at Context.<anonymous> (test/session.js:2012:30)
      at processImmediate (node:internal/timers:466:21)

I believe SSL libs now think 1024 bit key is too small. Regenerating 2048 bit keys in fixtures fixes the test:

openssl req -new -x509 -keyout server.key -out server.crt -newkey rsa:4096 -days 3650 -nodes -subj "/CN=express-session.local/"

I would PR with new key but probably better it project member does this.

[dougwilson]

What version of Node.js does this happen on? We are running this on all versions on CI seemingly without issue. Ideally we need a way for it to fail on CI otherwise there is no guarantee the change will not accidentally be reverted, breaking you again. I can help set that up, just need information on your environment to replicate.

[sgpinkus]

@dougwilson v16.15.0

[dougwilson]

Our CI succeeded on 16.15.0: https://github.com/expressjs/session/runs/6394111617?check_suite_focus=true#step:9:8

[sgpinkus]

Yeah I think it's node relying on system crypto libraries.

[dougwilson]

AFAIK Node.js statically links the OpenSSL lib into their binaries. Our CI downloads the libs direct from nodejs.org

[dougwilson]

Anyway, we can def regenerate the key to fix the test, but we need a way to replicate the issue at least in CI to ensure the change stays and doesn't break again in the future.

[sgpinkus]

AFAIK Node.js statically links the OpenSSL lib into their binaries. Our CI downloads the libs direct from nodejs.org

Hmm, I'm not sure TBH. Maybe it depends? Local system was Debian Buster. I just replicated test failure with docker image node:16.15.0-buster-slim.

[dougwilson]

I'm pretty sure, otherwise Node.js wouldn't need to make new releases when there is an OpenSSL security vulnerability? Burt even then, it would seem to reason that if our CI isn't having trouble, and I just tried locally on 16.15.0 without issue, it would seem I wouldn't realistically be able to generate a new key, as I don't know the parameters needed. Project maintainers like myself can only reliable generate keys that meet known parameters, like what our local and CI Node.js need. You're welcome to PR a key that meets your needs and passes the CI as well, though if you don't update the CI to fail in the same way, I cannot guarantee that any future key changes will work for you. I hope that makes sense.

[sgpinkus]

Makes sense.

I'm pretty sure, otherwise Node.js wouldn't need to make new releases when there is an OpenSSL security vulnerability?

OK confirmed it's statically linked SSL and most things on my system. I guess the build process for "v16.15.0" is different. Particular version was from the NodeSource apt repo. I'm not going to bother checking what SSL lib version is. I will PR a new 2048b dummy key though.