[qpid-proton] branch main updated: PROTON-1870: openssl layer: flush TLS error alert to peer for better error reporting

[cl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

cliffjansen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git


The following commit(s) were added to refs/heads/main by this push:
     new dfebbe8c PROTON-1870: openssl layer: flush TLS error alert to peer for better error reporting
dfebbe8c is described below

commit dfebbe8c10666fecb65f74c99199fc7e013997ef
Author: Clifford Jansen <cl...@apache.org>
AuthorDate: Thu May 12 07:11:45 2022 -0700

    PROTON-1870: openssl layer: flush TLS error alert to peer for better error reporting
---
 c/src/ssl/openssl.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/c/src/ssl/openssl.c b/c/src/ssl/openssl.c
index b4b75645..2681d2d6 100644
--- a/c/src/ssl/openssl.c
+++ b/c/src/ssl/openssl.c
@@ -114,6 +114,7 @@ struct pni_ssl_t {
   bool ssl_closed;      // shutdown complete, or SSL error
   bool read_blocked;    // SSL blocked until more network data is read
   bool write_blocked;   // SSL blocked until data is written to network
+  int err_reason;
 
   char *subject;
   X509 *peer_certificate;
@@ -193,14 +194,30 @@ static void ssl_log_clear_data(pn_transport_t *transport, const char *data, size
 }
 
 // unrecoverable SSL failure occurred, notify transport and generate error code.
-static int ssl_failed(pn_transport_t *transport)
+static int ssl_failed(pn_transport_t *transport, int reason)
 {
   pni_ssl_t *ssl = transport->ssl;
-  SSL_set_shutdown(ssl->ssl, SSL_SENT_SHUTDOWN|SSL_RECEIVED_SHUTDOWN);
-  ssl->ssl_closed = true;
+  bool first_fail = true;
+  if (ssl->err_reason != 0)
+    first_fail = false;  // No need for second transport error event.
+  else
+    ssl->err_reason = reason;
+
   ssl->app_input_closed = ssl->app_output_closed = PN_EOS;
   // fake a shutdown so the i/o processing code will close properly
   SSL_set_shutdown(ssl->ssl, SSL_SENT_SHUTDOWN|SSL_RECEIVED_SHUTDOWN);
+
+  if (first_fail && reason == SSL_ERROR_SSL) {
+    // Protocol error. Toss input and pending app output but best efforts for SSL generated output (i.e. error alerts)
+    ssl->out_count = 0;  // Discard.  No more writing into SSL engine allowed.
+  } else {
+    // SSL_ERROR_SYS or unknown error.  SSL engine state unknown.  Stop IO in all directions immediately.
+    ssl->ssl_closed = true;
+  }
+  // transport->io_layers[layer] is updated in handle_error_ssl
+  if (!first_fail)
+    return PN_EOS;
+
   // try to grab the first SSL error to add to the failure log
   char buf[256] = "Unknown error";
   unsigned long ssl_err = ERR_get_error();
@@ -208,6 +225,7 @@ static int ssl_failed(pn_transport_t *transport)
     ERR_error_string_n( ssl_err, buf, sizeof(buf) );
   }
   ssl_log_flush(transport, PN_LEVEL_ERROR);    // spit out any remaining errors to the log file
+  // Following call invokes hanlde_error_ssl which sets transport->io_layers[layer]
   pn_do_error(transport, "amqp:connection:framing-error", "SSL Failure: %s", buf);
   return PN_EOS;
 }
@@ -1059,7 +1077,7 @@ static ssize_t process_input_ssl( pn_transport_t *transport, unsigned int layer,
             break;
            default:
             // unexpected error
-            return (ssize_t)ssl_failed(transport);
+            return (ssize_t)ssl_failed(transport, reason);
           }
         } else {
           if (BIO_should_write( ssl->bio_ssl )) {
@@ -1152,7 +1170,12 @@ static ssize_t process_input_ssl( pn_transport_t *transport, unsigned int layer,
 
 static void handle_error_ssl(pn_transport_t *transport, unsigned int layer)
 {
-  transport->io_layers[layer] = &ssl_closed_layer;
+  // External errors do not affect SSL operation and graceful shutdown.
+  if (transport->ssl->err_reason == SSL_ERROR_SSL) {
+    transport->io_layers[layer] = &ssl_input_closed_layer;
+  } else {
+    transport->io_layers[layer] = &ssl_closed_layer;
+  }
 }
 
 static ssize_t process_output_ssl( pn_transport_t *transport, unsigned int layer, char *buffer, size_t max_len)
@@ -1209,7 +1232,7 @@ static ssize_t process_output_ssl( pn_transport_t *transport, unsigned int layer
               break;
              default:
               // unexpected error
-              return (ssize_t)ssl_failed(transport);
+              return (ssize_t)ssl_failed(transport, reason);
             }
           } else {
             if (BIO_should_read( ssl->bio_ssl )) {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org