1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

tls: trace errors can show up as SSL errors

Browse Source 
Calls to TLS_trace might leave errors on the SSL error stack, which then
get reported as SSL errors instead of being ignored. Wrap TLS_trace to
keep the error stack unchanged.

Fixes: https://github.com/nodejs/node/issues/27636

PR-URL: https://github.com/nodejs/node/pull/27841
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
This commit is contained in:
Sam Roberts 2019-05-23 10:56:41 -07:00
parent 490c7e0606
commit b1bd9e3dd2
2 changed files with 19 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/tls_wrap.cc
View File

@ -938,7 +938,17 @@ void TLSWrap::EnableTrace(
#if HAVE_SSL_TRACE
if (wrap->ssl_) {
wrap->bio_trace_.reset(BIO_new_fp(stderr, BIO_NOCLOSE | BIO_FP_TEXT));
SSL_set_msg_callback(wrap->ssl_.get(), SSL_trace);
SSL_set_msg_callback(wrap->ssl_.get(), [](int write_p, int version, int
content_type, const void* buf, size_t len, SSL* ssl, void* arg)
-> void {
// BIO_write(), etc., called by SSL_trace, may error. The error should
// be ignored, trace is a "best effort", and its usually because stderr
// is a non-blocking pipe, and its buffer has overflowed. Leaving errors
// on the stack that can get picked up by later SSL_ calls causes
// unwanted failures in SSL_ calls, so keep the error stack unchanged.
crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
SSL_trace(write_p, version, content_type, buf, len, ssl, arg);
});
SSL_set_msg_callback_arg(wrap->ssl_.get(), wrap->bio_trace_.get());
}
#endif

14
test/parallel/test-tls-enable-trace-cli.js
View File

@ -36,8 +36,8 @@ child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(signal, null);
assert.strictEqual(stdout.trim(), '');
assert(/Warning: Enabling --trace-tls can expose sensitive/.test(stderr));
assert(/Sent Record/.test(stderr));
assert(/Received Record/.test(stderr));
assert(/ClientHello/.test(stderr));
}));
function test() {
@ -55,12 +55,14 @@ function test() {
key: keys.agent6.key
},
}, common.mustCall((err, pair, cleanup) => {
if (err) {
console.error(err);
console.error(err.opensslErrorStack);
console.error(err.reason);
assert(err);
if (pair.server.err) {
console.trace('server', pair.server.err);
}
if (pair.client.err) {
console.trace('client', pair.client.err);
}
assert.ifError(pair.server.err);
assert.ifError(pair.client.err);
return cleanup();
}));