-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
TLS error closes entire process #39363
Comments
Hi! Can you please a reproducible test case (preferably with no other dependencies)? That would help us debug this issue. Thanks! |
Basically the error happens randomly on a live server with an unpredictable frequency and we do not really have a reproduction case. We do see it on the server, happening frequently enough that it escalated to a problem. simple code as the following will not be able to catch the error.: to make things even worse it won't get caught by process.on('unhandledException import http2 from 'http2';
const ssl = {
key: fs.readFileSync('./privatekey.pem'),
cert: fs.readFileSync('./fullchain.pem')
}
const server = http2.createSecureServer({
key: ssl.key,
cert: ssl.cert
});
server.on('tlsClientError', (err) => {
console.error("Error tls", err);
})
server.on('error', (err) => {
console.error("Server error", err);
})
server.on('session', session => {
session.on('error', (sessionError) => {
console.log("error on session ", sessionError)
});
session.on('stream', (stream, headers, flags) => {
stream.on('error', (streamError) => {
console.error('Stream Errorred ', streamError);
})
// pass the stream and the session to the application handlers.
if(gatheredResponseHeaders.responded === false) {
stream.respond(gatheredResponseHeaders);
stream.write(gatheredResponseBody);
stream.end();
}
})
})
I don't really understand how exactly the error happens but from giving it a read through TLSWrap there's a bunch of other errors that can happen in the same way on a TLS Socket. They haven't been caught because, most likely, too many people use node without SSL choosing to have something else that does the TLS in front of it. This can not be done properly for HTTP/2. An easy to reproduce error is to cause an ECONNRESET, which most SSL / TLS penetration tools check for. This causes 100% of the time an uncapturable error that shuts down the process instantly. This is unacceptable under any kind of circumstances since the server might be doing handling of multiple streams at the same time.
A proper rewrite of the whole SSL and TLS wrap would be in order as this is not a problem with HTTP/2 per se but with EVERYTHING that has to do with TLS and SSL. Nothing on the TLS has been tested in real world yet. The SSL3 related routines can be alleviated seemingly by just disabling SSL3 altogether (poodle attack alone would demand that), and nginx has already done so; but the other errors for some twisted reason do result in an error event but I have no idea what exactly is the place where the error event should be captured and is not. |
Oh, and I would give it a TLS label instead of HTTP2 |
After giving it thorough read, the conclusion is that the errors happen between the socket creation and the process.nextTick that allows the socket.on('error) listener to be set. Now, this is likely the case because as one can read on the error itself, it's an error event emitted and it was captured properly by the tlsWrap itself and emitted too via _emitTLSError. Normally I would think that the errors that occur EXACTLY between the moment the server sets its releaseControl to true and the moment of being able to call the error handler as it was attached to the socket. This is possibly because the openssl runs on a different thread or something in the likes of it. Given the findings I have, I must mention that we're using the server via the cluster api . I haven't gone into further details about the way the TLS handles the tls tickets but I would suggest having a go at it from there having in mind what I found with the error timing. By the looks of it, this HAS to be a timing problem with the moment the event listeners are added. |
Any feedback on this issue? |
Here is the simplest example I could make: import fs from 'fs';
import http2 from 'http2';
process.on('error', (err) => {
console.error('Process error: ', err);
process.exit();
});
const server = http2.createSecureServer({
key: fs.readFileSync('./privkey.pem'),
cert: fs.readFileSync('./fullchain.pem')
// allowHTTP1: true
});
server.on('error', (error) => {
console.error('Server error: ', error);
});
server.on('close', () => {
console.info('Server closed');
});
server.on('session', (session) => {
session.on('error', (err) => {
console.info('Session error', err);
});
session.on('close', () => {
console.info('Session closed');
});
});
server.on('stream', (stream) => {
stream.on('error', (err) => {
console.info('Stream error', err);
});
stream.on('close', () => {
console.info('Stream closed');
});
});
server.on('request', (req, res) => {
res.statusCode = 200;
res.end('Hello');
});
server.listen(3000, '0.0.0.0'); With Edit 1: Here is an SSL error from a test with Digicert SSL Tool node:events:371
throw er; // Unhandled 'error' event
^
Error: 140346147399616:error:1408F119:SSL routines:ssl3_get_record:decryption failed or bad record mac:../deps/openssl/openssl/ssl/record/ssl3_record.c:677:
Emitted 'error' event on TLSSocket instance at:
at TLSSocket._emitTLSError (node:_tls_wrap:900:10)
at TLSWrap.onerror (node:_tls_wrap:432:11) {
library: 'SSL routines',
function: 'ssl3_get_record',
reason: 'decryption failed or bad record mac',
code: 'ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC'
} Edit 2: Even with the server.on('connection', (socket) => {
socket.on('error', (err) => {
console.info('Socket error', err);
});
}); |
The ECONNRESET flaw might be addressed by #36111, but that won't address the larger issue here |
I am able to reproduce a very similar error message using the openssl command Listening to the server 'unknownProtocol' event seems to fix the error. Error message
Node.js code to reproduceRequires openssl. Run as an es module.
|
Perhaps, this can be related to this issue too - Unhandled 'error' event with self-signed TLS certificate on node:http2 and wget I have an unhandled exception when I connect to the HTTPS server with self-signed cert through the UPDATE: There is no error when I use
|
I'm still seeing this on 22.8.0 (Using self-signed certificate, indeed :) ) |
Version
v16.4.0
Platform
Linux 4.19.0-16-cloud-amd64 #1 SMP Debian 4.19.181-1 (2021-03-19) x86_64 GNU/Linux
Subsystem
No response
What steps will reproduce the bug?
A simple http2 server, with no external libraries fails to catch a TLS error.
I added error listeners to server, session and stream, but none of them manages to catch the following error:
How often does it reproduce? Is there a required condition?
Daily on a low traffic server (~200 daily users).
What is the expected behavior?
To be able to catch the error or http2 module to be able to handle it
What do you see instead?
Unhandled error event
Additional information
No response
The text was updated successfully, but these errors were encountered: