tls: trace errors can show up as SSL errors

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: #27636
PR-URL: #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>

Author: sam-github

src/tls_wrap.cc

@@ -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, constvoid* 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

test/parallel/test-tls-enable-trace-cli.js

@@ -36,8 +36,8 @@ child.on('close', common.mustCall((code, signal) => {
 assert.strictEqual(signal,null);
 assert.strictEqual(stdout.trim(),'');
 assert(/Warning:Enabling--trace-tlscanexposesensitive/.test(stderr));
+assert(/SentRecord/.test(stderr));
 assert(/ReceivedRecord/.test(stderr));
-assert(/ClientHello/.test(stderr));
 }));
 
 functiontest(){
@@ -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);
 
 returncleanup();
 }));