choojs / bankai

:station: - friendly web compiler
Apache License 2.0
1.09k stars 102 forks source link

SSL key too small, project doesn't start #533

Open Jake-Gillberg opened 5 years ago

Jake-Gillberg commented 5 years ago

npm start fails with below:

A critical error occured, forcing Bankai to abort:
Error: error:140AB18F:SSL routines:SSL_CTX_use_certificate:ee key too small
at Object.createSecureContext (_tls_common.js:135:17)
at new Server (_tls_wrap.js:873:27)
at new Http2SecureServer (internal/http2/core.js:2839:5)
at createSecureServer (internal/http2/core.js:2963:10)
at /home/dev/baby-math/node_modules/bankai/lib/http-server.js:61:29
at process._tickCallback (internal/process/next_tick.js:68:7)

Workaround: Changing a line in /etc/ssl/openssl.cnf from: CipherString = DEFAULT@SECLEVEL=2 to CipherString = DEFAULT@SECLEVEL=1

but it is probably better to just create a longer ssl key.

Versions: npm 6.11.3 node v10.16.3 debian buster openSSL 1.0.2g 1 Mar 2016

blahah commented 4 years ago

It's definitely sub-optimal to be failing because the key is too weak. Any secured linux distro will enforce the SSL DEFAULT@SECLEVEL=2, and compromising the security of the whole system for the sake of one module seems problematic.

This appears to be a NodeJS core TLS module issue?

goto-bus-stop commented 4 years ago

I think bankai's createKeys function can probably be updated to fix this somehow? I don't really know what it would look like, but if anyone's interested in trying to contribute a fix, this is probably the place to investigate!

https://github.com/choojs/bankai/blob/858a25bba8c37e79c14bb8115c2325c983d662c5/lib/http-server.js#L110-L179

blahah commented 4 years ago

Looks like a fix was pre-emptively attempted in the past, but a typo led to it not working (keySize rather than days should have been set to 2048).

It's better to have keys expire frequently BTW, especially in this sort of situation where they are easily regenerated by trusted applications. I'd recommend using a 90 day expiry the same as LetsEncrypt. This protects to some extent against key exfiltration by malware, bots etc. by limit the amount of time an exfiltrated key can be used maliciously.

PR incoming...

blahah commented 4 years ago

Turns out the default expiration for selfsigned, which is doing the cert generation, is 30 days, which is more secure. So in my PR I've just switched days for keySize, meaning days will default to 30 which I suspect was the intention of the original edit.

blahah commented 4 years ago

Actually I just noticed selfsigned hardcodes one side of the cert to be 1024. I am making a PR to that project to respect keySize for both keys, which we should wait for before considering this fixed.

blahah commented 4 years ago

Waiting on this: https://github.com/jfromaniello/selfsigned/pull/35