Trunk ssl handshake nonblocking

CHANGES

@@ -1,6 +1,12 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.1
 
+  *) event: Add support for non blocking behaviour in the
+     CONN_STATE_READ_REQUEST_LINE phase, in addition to the existing
+     CONN_STATE_WRITE_COMPLETION phase. Add AP_MPM_CAN_AGAIN and AGAIN to
+     signal to the MPM that non blocking behaviour is requested. Update
+     mod_ssl to perform non blocking TLS handshakes. [Graham Leggett]
+
   *) http: Enforce that fully qualified uri-paths not to be forward-proxied
      have an http(s) scheme, and that the ones to be forward proxied have a
      hostname, per HTTP specifications.  [Ruediger Pluem, Yann Ylavic]

changes-entries/ab-ssl-sense-fix.txt

@@ -0,0 +1,5 @@
+  *) ab: Respond appropriately to SSL_ERROR_WANT_READ and SSL_ERROR_WANT_WRITE.
+     Previously the correct event was polled for, but the response to the poll
+     would call write instead of read, and read instead of write. PR 55952
+     [Graham Leggett]
+

include/ap_mmn.h

@@ -698,6 +698,8 @@
  * 20210926.2 (2.5.1-dev)  Add ap_post_read_request()
  * 20211221.0 (2.5.1-dev)  Bump PROXY_WORKER_MAX_NAME_SIZE from 256 to 384,
  *                         add PROXY_WORKER_UDS_PATH_SIZE.
+ * 20211221.1 (2.5.1-dev)  Add read_line to scoreboard.
+ * 20211221.2 (2.5.1-dev)  Add AGAIN, AP_MPMQ_CAN_AGAIN.
  * 20211221.3 (2.5.1-dev)  Add ap_thread_create(), ap_thread_main_create()
  *                         and ap_thread_current()
  *

include/ap_mpm.h

@@ -182,6 +182,8 @@ AP_DECLARE(apr_status_t) ap_os_create_privileged_process(
 #define AP_MPMQ_CAN_SUSPEND          17
 /** MPM supports additional pollfds */
 #define AP_MPMQ_CAN_POLL             18
+/** MPM reacts to AGAIN response */
+#define AP_MPMQ_CAN_AGAIN            19
 /** @} */
 
 /**

include/httpd.h

@@ -465,6 +465,9 @@ AP_DECLARE(const char *) ap_get_server_built(void);
                                  */
 #define SUSPENDED -3 /**< Module will handle the remainder of the request.
                       * The core will never invoke the request again, */
+#define AGAIN -4                /**< Module wants to be called again when
+                                  *  more data is availble.
+                                  */
 
 /** Returned by the bottom-most filter if no data was written.
  *  @see ap_pass_brigade(). */

include/scoreboard.h

@@ -148,6 +148,7 @@ struct process_score {
     apr_uint32_t lingering_close;   /* async connections in lingering close */
     apr_uint32_t keep_alive;        /* async connections in keep alive */
     apr_uint32_t suspended;         /* connections suspended by some module */
+    apr_uint32_t read_line;         /* async connections doing read line */
 };
 
 /* Scoreboard is now in 'local' memory, since it isn't updated once created,

modules/generators/mod_status.c

@@ -557,7 +557,7 @@ static int status_handler(request_rec *r)
         ap_rputs("</dl>", r);
 
     if (is_async) {
-        int write_completion = 0, lingering_close = 0, keep_alive = 0,
+        int read_line = 0, write_completion = 0, lingering_close = 0, keep_alive = 0,
             connections = 0, stopping = 0, procs = 0;
         /*
          * These differ from 'busy' and 'ready' in how gracefully finishing
@@ -574,11 +574,12 @@ static int status_handler(request_rec *r)
                          "<th colspan=\"3\">Async connections</th></tr>\n"
                      "<tr><th>total</th><th>accepting</th>"
                          "<th>busy</th><th>idle</th>"
-                         "<th>writing</th><th>keep-alive</th><th>closing</th></tr>\n", r);
+                         "<th>reading</th><th>writing</th><th>keep-alive</th><th>closing</th></tr>\n", r);
         for (i = 0; i < server_limit; ++i) {
             ps_record = ap_get_scoreboard_process(i);
             if (ps_record->pid) {
                 connections      += ps_record->connections;
+                read_line        += ps_record->read_line;
                 write_completion += ps_record->write_completion;
                 keep_alive       += ps_record->keep_alive;
                 lingering_close  += ps_record->lingering_close;
@@ -600,14 +601,15 @@ static int status_handler(request_rec *r)
                                       "<td>%s%s</td>"
                                       "<td>%u</td><td>%s</td>"
                                       "<td>%u</td><td>%u</td>"
-                                      "<td>%u</td><td>%u</td><td>%u</td>"
+                                      "<td>%u</td><td>%u</td><td>%u</td><td>%u</td>"
                                       "</tr>\n",
                                i, ps_record->pid,
                                dying, old,
                                ps_record->connections,
                                ps_record->not_accepting ? "no" : "yes",
                                thread_busy_buffer[i],
                                thread_idle_buffer[i],
+                               ps_record->read_line,
                                ps_record->write_completion,
                                ps_record->keep_alive,
                                ps_record->lingering_close);
@@ -619,26 +621,27 @@ static int status_handler(request_rec *r)
                           "<td>%d</td><td>%d</td>"
                           "<td>%d</td><td>&nbsp;</td>"
                           "<td>%d</td><td>%d</td>"
-                          "<td>%d</td><td>%d</td><td>%d</td>"
+                          "<td>%d</td><td>%d</td><td>%d</td><td>%d</td>"
                           "</tr>\n</table>\n",
                           procs, stopping,
                           connections,
                           busy_workers, idle_workers,
-                          write_completion, keep_alive, lingering_close);
+                          read_line, write_completion, keep_alive, lingering_close);
         }
         else {
             ap_rprintf(r, "Processes: %d\n"
                           "Stopping: %d\n"
                           "BusyWorkers: %d\n"
                           "IdleWorkers: %d\n"
                           "ConnsTotal: %d\n"
+                          "ConnsAsyncReading: %d\n"
                           "ConnsAsyncWriting: %d\n"
                           "ConnsAsyncKeepAlive: %d\n"
                           "ConnsAsyncClosing: %d\n",
                           procs, stopping,
                           busy_workers, idle_workers,
                           connections,
-                          write_completion, keep_alive, lingering_close);
+                          read_line, write_completion, keep_alive, lingering_close);
         }
     }
 

modules/ssl/mod_ssl.c

@@ -30,6 +30,7 @@
 #include "util_md5.h"
 #include "util_mutex.h"
 #include "ap_provider.h"
+#include "ap_mpm.h"
 #include "http_config.h"
 
 #include "mod_proxy.h" /* for proxy_hook_section_post_config() */
@@ -689,31 +690,80 @@ static int ssl_hook_process_connection(conn_rec* c)
 {
     SSLConnRec *sslconn = myConnConfig(c);
 
+    int status = DECLINED;
+
     if (sslconn && !sslconn->disabled) {
         /* On an active SSL connection, let the input filters initialize
          * themselves which triggers the handshake, which again triggers
          * all kinds of useful things such as SNI and ALPN.
          */
         apr_bucket_brigade* temp;
-        apr_status_t rv;
+
+        int again_mpm = 0;
 
         temp = apr_brigade_create(c->pool, c->bucket_alloc);
-        rv = ap_get_brigade(c->input_filters, temp,
-                            AP_MODE_INIT, APR_BLOCK_READ, 0);
-        apr_brigade_destroy(temp);
 
-        if (APR_SUCCESS != APR_SUCCESS) {
-            if (c->cs) {
-                c->cs->state = CONN_STATE_LINGER;
+        if (ap_mpm_query(AP_MPMQ_CAN_AGAIN, &again_mpm) != APR_SUCCESS) {
+            again_mpm = 0;
+        }
+
+        if (again_mpm) {
+
+            /* Take advantage of an async MPM. If we see an EAGAIN,
+             * loop round and don't block.
+             */
+            conn_state_t *cs = c->cs;
+
+            apr_status_t rv;
+
+            rv = ap_get_brigade(c->input_filters, temp,
+                           AP_MODE_INIT, APR_NONBLOCK_READ, 0);
+
+            if (rv == APR_SUCCESS) {
+                /* great news, lets continue */
+
+                ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10370)
+                              "SSL handshake completed, continuing");
+
+                status = DECLINED;
             }
-            ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO(10373)
-                          "SSL handshake was not completed, "
-                          "closing connection");
-            return OK;
+            else if (rv == APR_EAGAIN) {
+                /* we've been asked to come around again, don't block */
+
+                ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10371)
+                              "SSL handshake in progress, continuing");
+
+                status = AGAIN;
+            }
+            else if (rv == AP_FILTER_ERROR) {
+                /* handshake error, but mod_ssl handled it */
+
+                ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10372)
+                              "SSL handshake failed, returning error response");
+
+                status = DECLINED;
+            }
+            else {
+                /* we failed, give up */
+
+                cs->state = CONN_STATE_LINGER;
+
+                ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO(10373)
+                              "SSL handshake was not completed, "
+                              "closing connection");
+
+                status = OK;
+            }
+        }
+        else {
+            ap_get_brigade(c->input_filters, temp,
+                           AP_MODE_INIT, APR_BLOCK_READ, 0);
         }
+
+        apr_brigade_destroy(temp);
     }
-    
-    return DECLINED;
+
+    return status;
 }
 
 /*

modules/ssl/ssl_engine_io.c

@@ -323,6 +323,7 @@ typedef struct {
 } char_buffer_t;
 
 typedef struct {
+    conn_rec *c;
     SSL *ssl;
     BIO *bio_out;
     ap_filter_t *f;
@@ -795,6 +796,32 @@ static apr_status_t ssl_io_input_read(bio_filter_in_ctx_t *inctx,
                  * (This is usually the case when the client forces an SSL
                  * renegotiation which is handled implicitly by OpenSSL.)
                  */
+                if (inctx->c->cs) {
+                    inctx->c->cs->sense = CONN_SENSE_WANT_READ;
+                }
+                inctx->rc = APR_EAGAIN;
+
+                if (*len > 0) {
+                    inctx->rc = APR_SUCCESS;
+                    break;
+                }
+                if (inctx->block == APR_NONBLOCK_READ) {
+                    break;
+                }
+                continue;  /* Blocking and nothing yet?  Try again. */
+            }
+            if (ssl_err == SSL_ERROR_WANT_WRITE) {
+                /*
+                 * If OpenSSL wants to write during read, and we were
+                 * nonblocking, report as an EAGAIN.  Otherwise loop,
+                 * pulling more data from network filter.
+                 *
+                 * (This is usually the case when the client forces an SSL
+                 * renegotiation which is handled implicitly by OpenSSL.)
+                 */
+                if (inctx->c->cs) {
+                    inctx->c->cs->sense = CONN_SENSE_WANT_WRITE;
+                }
                 inctx->rc = APR_EAGAIN;
 
                 if (*len > 0) {
@@ -960,7 +987,9 @@ static apr_status_t ssl_filter_write(ap_filter_t *f,
              * (This is usually the case when the client forces an SSL
              * renegotiation which is handled implicitly by OpenSSL.)
              */
-            outctx->c->cs->sense = CONN_SENSE_WANT_READ;
+            if (outctx->c->cs) {
+                outctx->c->cs->sense = CONN_SENSE_WANT_READ;
+            }
             outctx->rc = APR_EAGAIN;
             ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, outctx->c,
                           "Want read during nonblocking write");
@@ -1489,10 +1518,25 @@ static apr_status_t ssl_io_filter_handshake(ssl_filter_ctx_t *filter_ctx)
         }
         else if (ssl_err == SSL_ERROR_WANT_READ) {
             /*
-             * This is in addition to what was present earlier. It is
-             * borrowed from openssl_state_machine.c [mod_tls].
-             * TBD.
+             * Call us back when ready to read *\/
              */
+            ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, outctx->c,
+                          "Want read during nonblocking accept");
+        	if (outctx->c->cs) {
+                outctx->c->cs->sense = CONN_SENSE_WANT_READ;
+        	}
+            outctx->rc = APR_EAGAIN;
+            return APR_EAGAIN;
+        }
+        else if (ssl_err == SSL_ERROR_WANT_WRITE) {
+            /*
+             * Call us back when ready to write *\/
+             */
+            ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, outctx->c,
+                          "Want write during nonblocking accept");
+        	if (outctx->c->cs) {
+                outctx->c->cs->sense = CONN_SENSE_WANT_WRITE;
+        	}
             outctx->rc = APR_EAGAIN;
             return APR_EAGAIN;
         }
@@ -2295,6 +2339,7 @@ static apr_status_t ssl_io_input_add_filter(ssl_filter_ctx_t *filter_ctx, conn_r
     }
     BIO_set_data(filter_ctx->pbioRead, (void *)inctx);
 
+    inctx->c = c;
     inctx->ssl = ssl;
     inctx->bio_out = filter_ctx->pbioWrite;
     inctx->f = filter_ctx->pInputFilter;