Changeset 72b669e in mod_gnutls

Timestamp:

Sep 27, 2018, 1:23:25 PM (16 months ago)

Author:

Fiona Klute <fiona.klute@…>

Branches:

debian/master, master

Children:

c33ef88

Parents:

514d4d6

Message:

Refuse to send or receive over a failed TLS connection

On a failed connection (e.g. after a refused handshake) the
input/output filters would pass data unprocessed to the next filter in
the chain. On a normal server this just led to odd log messages
(because the HTTP handler couldn't process whatever the client was
sending), but for proxy HTTPS connections it caused a security issue:
The proxy request would be sent unencrypted after the failed
handshake, and the connection only closed when mod_proxy didn't
receive a valid response.

The fix is to refuse any send or receive operations through the
filters if the TLS connection failed.

Files:

• include/mod_gnutls.h.in (modified) (2 diffs)
• src/gnutls_io.c (modified) (2 diffs)

include/mod_gnutls.h.in

@@ -204 +204 @@
 } mgs_char_buffer_t;
 
-/* GnuTLS Handle */
+/** GnuTLS connection handle */
 typedef struct {
         /* Server configuration record */
@@ -242 +242 @@
         /* Output length */
     apr_size_t output_length;
-        /* General Status */
+    /** Connection status: 0 before (re-)handshake, 1 when up, -1 on
+     * error (checks use status < 0 or status > 0) */
     int status;
 } mgs_handle_t;

src/gnutls_io.c

@@ -563 +563 @@
     }
 
-    if (ctxt->status < 0) {
-        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctxt->c,
-                      "%s %s: ap_get_brigade", __func__, IS_PROXY_STR(ctxt));
-        return ap_get_brigade(f->next, bb, mode, block, readbytes);
+    if (ctxt->status < 0)
+    {
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, ctxt->c,
+                      "%s: %sconnection failed, cannot provide data!",
+                      __func__, IS_PROXY_STR(ctxt));
+        apr_bucket *bucket =
+                apr_bucket_eos_create(f->c->bucket_alloc);
+        APR_BRIGADE_INSERT_TAIL(bb, bucket);
+        return APR_ECONNABORTED;
     }
 
@@ -706 +711 @@
     }
 
-    if (ctxt->status < 0) {
-        return ap_pass_brigade(f->next, bb);
+    if (ctxt->status < 0)
+    {
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, ctxt->c,
+                      "%s: %sconnection failed, refusing to send.",
+                      __func__, IS_PROXY_STR(ctxt));
+        if (ctxt->is_proxy)
+        {
+            /* If mod_proxy receives an error while trying to send its
+             * request it sends an "invalid request" error to the
+             * client. By pretending we could send the request
+             * mod_proxy continues its processing and sends a proper
+             * "proxy error" message when there's no response to read. */
+            apr_bucket *bucket = apr_bucket_eos_create(f->c->bucket_alloc);
+            APR_BRIGADE_INSERT_TAIL(bb, bucket);
+            return APR_SUCCESS;
+        }
+        else
+            return APR_ECONNABORTED;
     }