nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107k stars 29.28k forks source link

TLS error closes entire process #39363

Open arobert93 opened 3 years ago

arobert93 commented 3 years ago

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:

node:events:371
      throw er; // Unhandled 'error' event
      ^

Error: 139932573210560:error:14094123:SSL routines:ssl3_read_bytes:application data after close notify:../deps/openssl/openssl/ssl/record/rec_layer_s3.c:1608:

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_read_bytes',
  reason: 'application data after close notify',
  code: 'ERR_SSL_APPLICATION_DATA_AFTER_CLOSE_NOTIFY'
}

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

evanlucas commented 3 years ago

Hi! Can you please a reproducible test case (preferably with no other dependencies)? That would help us debug this issue. Thanks!

valipopescu commented 3 years ago

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.


Error: read ECONNRESET
    at TLSWrap.onStreamRead (node:internal/stream_base_commons:211:20)
Emitted 'error' event on TLSSocket instance at:
    at emitErrorNT (node:internal/streams/destroy:193:8)
    at emitErrorCloseNT (node:internal/streams/destroy:158:3)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  errno: -104,
  code: 'ECONNRESET',
  syscall: 'read'
}

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.

valipopescu commented 3 years ago

Oh, and I would give it a TLS label instead of HTTP2

valipopescu commented 3 years ago

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.

arobert93 commented 3 years ago

Any feedback on this issue?

arobert93 commented 3 years ago

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 allowHTTP1 the server works as expected, but I do not want to allow HTTP1 connections to the server. Please test it with a real SSL certificate (not localhost).

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 connection listener (that we are not supposed to use), I'm not able to catch the error:

server.on('connection', (socket) => {
  socket.on('error', (err) => {
    console.info('Socket error', err);
  });
});
vtjnash commented 3 years ago

The ECONNRESET flaw might be addressed by https://github.com/nodejs/node/pull/36111, but that won't address the larger issue here

canerder commented 8 months ago

I am able to reproduce a very similar error message using the openssl command openssl s_client -connect <hostname> to probe the server. I am running node.js version 20.11.0 on MacOS.

Listening to the server 'unknownProtocol' event seems to fix the error.

Error message

node:events:496
      throw er; // Unhandled 'error' event
      ^

Error: 8045180601000000:error:0A00041A:SSL routines:ssl3_read_bytes:tlsv1 alert decode error:../deps/openssl/openssl/ssl/record/rec_layer_s3.c:1586:SSL alert number 50

Emitted 'error' event on TLSSocket instance at:
    at TLSSocket._emitTLSError (node:_tls_wrap:1041:10)
    at TLSWrap.onerror (node:_tls_wrap:482:11) {
  library: 'SSL routines',
  reason: 'tlsv1 alert decode error',
  code: 'ERR_SSL_TLSV1_ALERT_DECODE_ERROR'
}

Node.js code to reproduce

Requires openssl. Run as an es module. Changing HANDLE_UNKOWN_PROTOCOL to false should reproduce the error and true should fix it.

// Tested on node version 20.11.0

import http2 from 'node:http2';
import child_process from 'node:child_process';

// Self-signed key and cert for localhost, PEM format, encoded in base64
const localhostCertBase64 = 'LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUJrekNDQVRpZ0F3SUJBZ0lUWVUySTRRTUFhVXQ4TStFcXBMVitIaUpGbmpBS0JnZ3Foa2pPUFFRREFqQVUKTVJJd0VBWURWUVFEREFsc2IyTmhiR2h2YzNRd0hoY05NalF3TWpBek1qSXpNakU0V2hjTk1qa3dOekkyTWpJegpNakU0V2pBVU1SSXdFQVlEVlFRRERBbHNiMk5oYkdodmMzUXdXVEFUQmdjcWhrak9QUUlCQmdncWhrak9QUU1CCkJ3TkNBQVRsbjVYSGk5Z0tMS21BNk1YN2tUSjhadG4xYlFzNmllaHFuSjN5VUxYTVFrMjNIb3YxWHdoV3lPVzkKYzZJZG5mM2ljaHNTMkNEOHlObWVGb1BadUM0V28ya3daekFkQmdOVkhRNEVGZ1FVWnU3NnFtREVsRDFSODB4UApiWDRtNm03cU9sRXdId1lEVlIwakJCZ3dGb0FVWnU3NnFtREVsRDFSODB4UGJYNG02bTdxT2xFd0R3WURWUjBUCkFRSC9CQVV3QXdFQi96QVVCZ05WSFJFRURUQUxnZ2xzYjJOaGJHaHZjM1F3Q2dZSUtvWkl6ajBFQXdJRFNRQXcKUmdJaEFLZE54Wm5EZ1JIeEJmaklwa3JBS2FpSmVOa25uSDZhTk1RbFBUZzRRRlY3QWlFQXV0Wmt2dis5dkcxUwpOL0dRcmdlbDZ0L0V5K21HaXhNNFRnRWZFVWhkZjl3PQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg==';
const localhostKeyBase64 = 'LS0tLS1CRUdJTiBFQyBQQVJBTUVURVJTLS0tLS0KQmdncWhrak9QUU1CQnc9PQotLS0tLUVORCBFQyBQQVJBTUVURVJTLS0tLS0KLS0tLS1CRUdJTiBFQyBQUklWQVRFIEtFWS0tLS0tCk1IY0NBUUVFSUQ0MHVkY2oxU0RZb28xeGFnSFBiUUpwbEEySU94ZFlsSktTMTlCbFh2bm9vQW9HQ0NxR1NNNDkKQXdFSG9VUURRZ0FFNVorVng0dllDaXlwZ09qRis1RXlmR2JaOVcwTE9vbm9hcHlkOGxDMXpFSk50eDZMOVY4SQpWc2psdlhPaUhaMzk0bkliRXRnZy9NalpuaGFEMmJndUZnPT0KLS0tLS1FTkQgRUMgUFJJVkFURSBLRVktLS0tLQo=';

const PORT = 9443;

// Change to true to suppress error
const HANDLE_UNKOWN_PROTOCOL = false;

const server = http2.createSecureServer({
    key: Buffer.from(localhostKeyBase64, 'base64'),
    cert: Buffer.from(localhostCertBase64, 'base64')
});
server.on('error', err => {
    console.log('Server error', err);
});
server.on('connection', socket => {
    socket.on('error', err => {
        console.log('Socket error', err);
    });
});
server.on('session', session => {
    session.on('error', err => {
        console.log('Session error', err);
    });
    session.on('frameError', err => {
        console.log('Session frame error', err);
    });
});
server.on('sessionError', err => {
    console.log('Server session error', err);
});

if (HANDLE_UNKOWN_PROTOCOL) {
    server.on('unknownProtocol', () => {
        console.log('Unknown protocol');
    });
}

server.on('stream', (stream, headers) => {
    stream.on('error', err => {
        console.log('Stream error', err);
    });
    stream.on('frameError', err => {
        console.log('Stream frame error', err);
    });

    stream.respond({
        'content-type': 'text/html',
        ':status': 200,
    });
    stream.end('<h1>Hello World</h1>');
});

server.listen(PORT);

console.log(`Sending openssl request...`);
child_process.exec(`openssl s_client -connect localhost:${PORT}`, (err, stdout, stderr) => {
    if (err) {
        console.log('Exec error', err);
        return;
    }

    console.log('stdout:\n', stdout);
    console.log('stderr:\n', stderr);
});
flancer64 commented 1 month ago

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 wget. When I do the same with Chrome browser the all works fine.

UPDATE:

There is no error when I use node:https:

import {createServer} from 'node:https';
...
const server = createServer({
    key: readFileSync('secure.key'),
    cert: readFileSync('secure.pem')
});
neoncube2 commented 3 weeks ago

I'm still seeing this on 22.8.0 (Using self-signed certificate, indeed :) )