Changeset 70f9c00 in mod_gnutls

Timestamp:

Sep 28, 2020, 8:56:23 AM (21 months ago)

Author:

Fiona Klute <fiona.klute@…>

Branches:

asyncio, master

Children:

c9e720f

Parents:

ae4325e

Message:

Clean up TLS error handling

The HTTP_BAD_REQUEST case was never actually called, because nothing
in the mod_gnutls filter functions returns such an error code. I'm
completely removing it to avoid any issues with the injected request,
instead the client should just get an alert.

On the other hand, a lot of places in gnutls_io.c can use a generic
"insert EOS bucket" function. :-)

File:

• src/gnutls_io.c (modified) (6 diffs)

src/gnutls_io.c

@@ -37 +37 @@
  */
 
-#define HTTP_ON_HTTPS_PORT \
-    "GET /" CRLF
-
-#define HTTP_ON_HTTPS_PORT_BUCKET(alloc) \
-    apr_bucket_immortal_create(HTTP_ON_HTTPS_PORT, \
-                               sizeof(HTTP_ON_HTTPS_PORT) - 1, \
-                               alloc)
-
 #define IS_PROXY_STR(c) \
     ((c->is_proxy == GNUTLS_ENABLED_TRUE) ? "proxy " : "")
@@ -56 +48 @@
 
 
-static apr_status_t gnutls_io_filter_error(ap_filter_t * f,
-        apr_bucket_brigade * bb,
-        apr_status_t status) {
-    mgs_handle_t *ctxt = (mgs_handle_t *) f->ctx;
-    apr_bucket *bucket;
-
-    switch (status) {
-    case HTTP_BAD_REQUEST:
-        /* log the situation */
-        ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, f->c,
-                      "GnuTLS handshake failed: HTTP spoken on HTTPS port; "
-                      "trying to send HTML error page");
-        ctxt->status = -1;
-
-        /* fake the request line */
-        bucket = HTTP_ON_HTTPS_PORT_BUCKET(f->c->bucket_alloc);
-        break;
-
-    default:
-        return status;
-    }
-
+/**
+ * Helper function, used mostly for error conditions: Insert an EOS (end
+ * of stream) bucket into the bucket brigade.
+ */
+static inline void gnutls_io_filter_eos(ap_filter_t *f,
+                                        apr_bucket_brigade *bb)
+{
+    apr_bucket *bucket = apr_bucket_eos_create(f->c->bucket_alloc);
     APR_BRIGADE_INSERT_TAIL(bb, bucket);
-    bucket = apr_bucket_eos_create(f->c->bucket_alloc);
-    APR_BRIGADE_INSERT_TAIL(bb, bucket);
-
-    return APR_SUCCESS;
-}
+}
+
+
 
 static int char_buffer_read(mgs_char_buffer_t * buffer, char *in, int inl) {
@@ -560 +536 @@
 
     if (f->c->aborted) {
-        apr_bucket *bucket =
-                apr_bucket_eos_create(f->c->bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(bb, bucket);
+        gnutls_io_filter_eos(f, bb);
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctxt->c,
                       "%s: %sconnection aborted",
@@ -582 +556 @@
                       "%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);
+        gnutls_io_filter_eos(f, bb);
         return APR_ECONNABORTED;
     }
@@ -649 +621 @@
 
         /* Close TLS session and free resources on EOF,
-         * gnutls_io_filter_error will add an EOS bucket */
+         * gnutls_io_filter_eos will add an EOS bucket */
         if (APR_STATUS_IS_EOF(status))
             mgs_bye(ctxt);
 
-        return gnutls_io_filter_error(f, bb, status);
+        gnutls_io_filter_eos(f, bb);
+        return status;
     }
 
@@ -730 +703 @@
              * 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);
+            gnutls_io_filter_eos(f, bb);
             return APR_SUCCESS;
         }