Changeset 9db4dcd in mod_gnutls

Timestamp:

Jan 21, 2020, 3:24:32 PM (4 weeks ago)

Author:

Fiona Klute <fiona.klute@…>

Branches:

master

Children:

fcad37b

Parents:

4a22ee6

Message:

Handle GNUTLS_E_GOT_APPLICATION_DATA on gnutls_reauth()

Pending application data is cached in the connection input buffer, so
repeated attempts won't cause more memory allocations. If the buffer
is too small HTTP_REQUEST_ENTITY_TOO_LARGE is sent to the client.

The reauthentication test now contains a POST request so this code
path may (!) be taken during the test suite run.

Files:

• src/gnutls_hooks.c (modified) (1 diff)
• src/gnutls_io.c (modified) (3 diffs)
• src/gnutls_io.h (modified) (1 diff)
• test/Makefile.am (modified) (1 diff)
• test/data/secret/mirror.cgi (added)
• test/tests/35_client_reauth/test.yml (modified) (1 diff)

src/gnutls_hooks.c

@@ -1491 +1491 @@
         gnutls_certificate_server_set_request(ctxt->session,
                                               client_verify_mode);
-        if (mgs_reauth(ctxt) != GNUTLS_E_SUCCESS) {
-            return HTTP_FORBIDDEN;
-        }
-    }
-
-    int rv = mgs_cert_verify(r, ctxt);
+        int rv = mgs_reauth(ctxt, r);
+        if (rv != GNUTLS_E_SUCCESS) {
+            if (rv == GNUTLS_E_GOT_APPLICATION_DATA)
+                return HTTP_REQUEST_ENTITY_TOO_LARGE;
+            else
+                return HTTP_FORBIDDEN;
+        }
+    }
+
+    int ret = mgs_cert_verify(r, ctxt);
     /* In "request" mode we always allow the request, otherwise the
      * verify result decides. */
     if (client_verify_mode == GNUTLS_CERT_REQUEST)
         return DECLINED;
-    return rv;
+    return ret;
 }
 

src/gnutls_io.c

@@ -451 +451 @@
 
 
-int mgs_reauth(mgs_handle_t * ctxt)
+int mgs_reauth(mgs_handle_t *ctxt, request_rec *r)
 {
     if (ctxt->session == NULL)
@@ -457 +457 @@
 
     int rv = gnutls_reauth(ctxt->session, 0);
-    // TODO: Handle non-fatal errors: GNUTLS_E_INTERRUPTED,
-    // GNUTLS_E_AGAIN, GNUTLS_E_GOT_APPLICATION_DATA
+    // TODO: Handle remaining non-fatal errors: GNUTLS_E_INTERRUPTED,
+    // GNUTLS_E_AGAIN
 
     /* GNUTLS_E_GOT_APPLICATION_DATA can (randomly, depending on
@@ -466 +466 @@
      * party is the one that did the handshake, so caching the data
      * is appropriate. */
-    /* Allocate cache to content-length (if available), with an upper
-     * limit to prevent resource exhaustion attacks. Do we have to
-     * prevent creating multiple caches for one connection? */
-    /* ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, */
-    /*               "Request content: %s bytes", */
-    /*               apr_table_get(r->headers_in, "Content-Length")); */
-    /* If the cache is too small,
-     * a) return HTTP_REQUEST_ENTITY_TOO_LARGE to the client
-     * b) IF reauth was successful set Retry-After to immediately: */
-    /* apr_table_setn(r->err_headers_out, "Retry-After", "0"); */
+    if (rv == GNUTLS_E_GOT_APPLICATION_DATA)
+    {
+        /* Fill connection input buffer using a speculative read. */
+        apr_size_t len = sizeof(ctxt->input_buffer);
+        ctxt->input_mode = AP_MODE_SPECULATIVE;
+        apr_status_t status =
+            gnutls_io_input_read(ctxt, ctxt->input_buffer, &len);
+        if (status == APR_SUCCESS)
+        {
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r,
+                          "%s: cached %" APR_SIZE_T_FMT " bytes.",
+                          __func__, len);
+            /* If the cache was too small to accept all pending data
+             * we'll get GNUTLS_E_GOT_APPLICATION_DATA again, and the
+             * authz hook will return HTTP_REQUEST_ENTITY_TOO_LARGE to
+             * the client. */
+            rv = gnutls_reauth(ctxt->session, 0);
+        }
+        else
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
+                          "%s: buffering request data failed!",
+                          __func__);
+    }
 
     if (rv != GNUTLS_E_SUCCESS)
     {
-        ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, ctxt->c,
-                      "Reauthentication failed: %s (%d)",
-                      gnutls_strerror(rv), rv);
+        ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
+                      "%s: Reauthentication failed: %s (%d)",
+                      __func__, gnutls_strerror(rv), rv);
         return rv;
     }

src/gnutls_io.h

@@ -84 +84 @@
                             const void *buffer, size_t len);
 
-int mgs_reauth(mgs_handle_t * ctxt);
+int mgs_reauth(mgs_handle_t * ctxt, request_rec *r);
 
 #endif /* __MOD_GNUTLS_IO_H__ */

test/Makefile.am

@@ -249 +249 @@
 # Apache configuration and data files
 apache_data = base_apache.conf cgi_module.conf data/dump.cgi data/ocsp.cgi \
-        data/test.txt mime.types proxy_mods.conf
+        data/test.txt data/secret/mirror.cgi data/secret/test.txt \
+        mime.types proxy_mods.conf
 
 # Documentation for the test system

test/tests/35_client_reauth/test.yml

@@ -17 +17 @@
           test
   - !request
-    method: GET
-    path: /secret/test.txt
+    method: POST
+    path: /secret/mirror.cgi
+    body: |
+      GNUTLS_E_GOT_APPLICATION_DATA can (randomly, depending on
+      timing) happen with a request containing a body. According to
+      https://tools.ietf.org/html/rfc8446#appendix-E.1.2
+      post-handshake authentication proves that the authenticated
+      party is the one that did the handshake, so caching the data
+      is appropriate.
     expect:
       status: 200
       body:
         exactly: |
-          top secret
+          GNUTLS_E_GOT_APPLICATION_DATA can (randomly, depending on
+          timing) happen with a request containing a body. According to
+          https://tools.ietf.org/html/rfc8446#appendix-E.1.2
+          post-handshake authentication proves that the authenticated
+          party is the one that did the handshake, so caching the data
+          is appropriate.
   - !request
     method: GET