Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Jake-Gillberg opened this issue Sep 11, 2019 · 6 comments
Open

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

Jake-Gillberg opened this issue Sep 11, 2019 · 6 comments

Comments

@Jake-Gillberg
Copy link

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
Copy link

blahah commented Nov 23, 2019

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 goto-bus-stop transferred this issue from choojs/create-choo-app Nov 23, 2019
@goto-bus-stop
Copy link
Member

goto-bus-stop commented Nov 23, 2019

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!

bankai/lib/http-server.js

Lines 110 to 179 in 858a25b

function createKeys (cb) {
mkdirp(CONFIG_DIR, function (err) {
if (err) return cb(err)
fs.readdir(CONFIG_DIR, function (err, files) {
if (err) return cb(err)
var keys = {}
// check if both files exist
if (files.indexOf(KEY_NAME) !== -1 && files.indexOf(CERT_NAME) !== -1) {
return async.parallel([
function (done) {
fs.readFile(CERT_LOCATION, function (err, buf) {
if (err) return done(err)
keys.cert = buf
done()
})
},
function (done) {
fs.readFile(KEY_LOCATION, function (err, buf) {
if (err) return done(err)
keys.key = buf
done()
})
}
], function (err) {
if (err) return cb(err)
cb(null, keys)
})
}
var opts = {
days: 2048,
algorithm: 'sha256',
extensions: [
{
name: 'subjectAltName',
altNames: [
{
type: 2, // DNSName
value: 'localhost'
}
]
}
]
}
selfsigned.generate([{ name: 'commonName', value: 'localhost' }], opts, function (err, keys) {
if (err) return cb(err)
keys = {
key: keys.private,
cert: keys.cert
}
async.parallel([
function (done) {
fs.writeFile(KEY_LOCATION, keys.key, done)
},
function (done) {
fs.writeFile(CERT_LOCATION, keys.cert, done)
}
], function (err) {
if (err) return cb(err)
cb(null, keys)
})
})
})
})
}

@blahah
Copy link

blahah commented Nov 26, 2019

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
Copy link

blahah commented Nov 26, 2019

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
Copy link

blahah commented Nov 26, 2019

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
Copy link

blahah commented Nov 26, 2019

Waiting on this: jfromaniello/selfsigned#35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants