* Fixed a bug that prevented immediate memory release of reset streams

   on rare occasions. Memory was reclaimed instead at connection close.
   This could be exploited, unless nghttp2 v1.57.0 is used, in a variation
   of the Rapid Reset attack. Apache httpd issued CVE-2023-45802 for this.
 * Synchronized with Apache httpd svn trunk

Author: icing

ChangeLog

@@ -1,3 +1,9 @@
+ * Fixed a bug that prevented immediate memory release of reset streams
+   on rare occasions. Memory was reclaimed instead at connection close.
+   This could be exploited, unless nghttp2 v1.57.0 is used, in a variation
+   of the Rapid Reset attack. Apache httpd issued CVE-2023-45802 for this.
+ * Synchronized with Apache httpd svn trunk
+
 v2.0.24
 --------------------------------------------------------------------------------
  * fixed a compile time issue for Windows builds.

mod_http2/h2.h

@@ -20,6 +20,8 @@
 #include <apr_version.h>
 #include <ap_mmn.h>
 
+#include <nghttp2/nghttp2ver.h>
+
 struct h2_session;
 struct h2_stream;
 
@@ -39,7 +41,9 @@ struct h2_stream;
 #define H2_USE_POLLFD_FROM_CONN 0
 #endif
 
-#if H2_USE_PIPES
+/* WebSockets support requires apr 1.7.0 for apr_encode.h, plus the
+ * WebSockets features of nghttp2 1.34.0 and later. */
+#if H2_USE_PIPES && defined(NGHTTP2_VERSION_NUM) && NGHTTP2_VERSION_NUM >= 0x012200 && APR_VERSION_AT_LEAST(1,7,0)
 #define H2_USE_WEBSOCKETS       1
 #else
 #define H2_USE_WEBSOCKETS       0

mod_http2/h2_mplx.c

@@ -384,10 +384,11 @@ apr_status_t h2_mplx_c1_streams_do(h2_mplx *m, h2_mplx_stream_cb *cb, void *ctx)
 {
     stream_iter_ctx_t x;
 
-    H2_MPLX_ENTER(m);
-
     x.cb = cb;
     x.ctx = ctx;
+
+    H2_MPLX_ENTER(m);
+
     h2_ihash_iter(m->streams, m_stream_iter_wrap, &x);
 
     H2_MPLX_LEAVE(m);
@@ -1119,14 +1120,32 @@ static int reset_is_acceptable(h2_stream *stream)
     return 1; /* otherwise, be forgiving */
 }
 
-apr_status_t h2_mplx_c1_client_rst(h2_mplx *m, int stream_id)
+apr_status_t h2_mplx_c1_client_rst(h2_mplx *m, int stream_id, h2_stream *stream)
 {
-    h2_stream *stream;
     apr_status_t status = APR_SUCCESS;
+    int registered;
 
     H2_MPLX_ENTER_ALWAYS(m);
-    stream = h2_ihash_get(m->streams, stream_id);
-    if (stream && !reset_is_acceptable(stream)) {
+    registered = (h2_ihash_get(m->streams, stream_id) != NULL);
+    if (!stream) {
+      /* a RST might arrive so late, we have already forgotten
+       * about it. Seems ok. */
+      ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c1,
+                    H2_MPLX_MSG(m, "RST on unknown stream %d"), stream_id);
+      AP_DEBUG_ASSERT(!registered);
+    }
+    else if (!registered) {
+      /* a RST on a stream that mplx has not been told about, but
+       * which the session knows. Very early and annoying. */
+      ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c1,
+                    H2_STRM_MSG(stream, "very early RST, drop"));
+      h2_stream_set_monitor(stream, NULL);
+      h2_stream_rst(stream, H2_ERR_STREAM_CLOSED);
+      h2_stream_dispatch(stream, H2_SEV_EOS_SENT);
+      m_stream_cleanup(m, stream);
+      m_be_annoyed(m);
+    }
+    else if (!reset_is_acceptable(stream)) {
         m_be_annoyed(m);
     }
     H2_MPLX_LEAVE(m);

mod_http2/h2_mplx.h

@@ -201,7 +201,8 @@ int h2_mplx_c1_all_streams_want_send_data(h2_mplx *m);
  * any processing going on and remove from processing
  * queue.
  */
-apr_status_t h2_mplx_c1_client_rst(h2_mplx *m, int stream_id);
+apr_status_t h2_mplx_c1_client_rst(h2_mplx *m, int stream_id,
+                                   struct h2_stream *stream);
 
 /**
  * Get readonly access to a stream for a secondary connection.

mod_http2/h2_proxy_session.c

@@ -789,7 +789,7 @@ static apr_status_t session_start(h2_proxy_session *session)
     apr_socket_t *s;
 
     s = ap_get_conn_socket(session->c);
-#if (!defined(WIN32) && !defined(NETWARE)) || defined(DOXYGEN)
+#if !defined(WIN32) && !defined(NETWARE)
     if (s) {
         ap_sock_disable_nagle(s);
     }
@@ -1681,7 +1681,17 @@ static int done_iter(void *udata, void *val)
     h2_proxy_stream *stream = val;
     int touched = (stream->data_sent || stream->data_received ||
                    stream->id <= ctx->session->last_stream_id);
-    ctx->done(ctx->session, stream->r, APR_ECONNABORTED, touched, stream->error_code);
+    if (touched && stream->output) {
+      apr_bucket *b = ap_bucket_error_create(HTTP_BAD_GATEWAY, NULL,
+                                             stream->r->pool,
+                                             stream->cfront->bucket_alloc);
+      APR_BRIGADE_INSERT_TAIL(stream->output, b);
+      b = apr_bucket_eos_create(stream->cfront->bucket_alloc);
+      APR_BRIGADE_INSERT_TAIL(stream->output, b);
+      ap_pass_brigade(stream->r->output_filters, stream->output);
+    }
+    ctx->done(ctx->session, stream->r, APR_ECONNABORTED, touched,
+              stream->error_code);
     return 1;
 }
 

mod_http2/h2_session.c

@@ -402,6 +402,10 @@ static int on_frame_recv_cb(nghttp2_session *ng2s,
                           H2_SSSN_STRM_MSG(session, frame->hd.stream_id,
                           "RST_STREAM by client, error=%d"),
                           (int)frame->rst_stream.error_code);
+            if (stream) {
+                rv = h2_stream_recv_frame(stream, NGHTTP2_RST_STREAM, frame->hd.flags,
+                    frame->hd.length + H2_FRAME_HDR_LEN);
+            }
             if (stream && stream->initiated_on) {
                 /* A stream reset on a request we sent it. Normal, when the
                  * client does not want it. */
@@ -410,7 +414,8 @@ static int on_frame_recv_cb(nghttp2_session *ng2s,
             else {
                 /* A stream reset on a request it sent us. Could happen in a browser
                  * when the user navigates away or cancels loading - maybe. */
-                h2_mplx_c1_client_rst(session->mplx, frame->hd.stream_id);
+                h2_mplx_c1_client_rst(session->mplx, frame->hd.stream_id,
+                                      stream);
             }
             ++session->streams_reset;
             break;
@@ -812,6 +817,17 @@ static apr_status_t session_cleanup(h2_session *session, const char *trigger)
                       "goodbye, clients will be confused, should not happen"));
     }
 
+    if (!h2_iq_empty(session->ready_to_process)) {
+        int sid;
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c,
+                      H2_SSSN_LOG(APLOGNO(10485), session,
+                      "cleanup, resetting %d streams in ready-to-process"),
+                      h2_iq_count(session->ready_to_process));
+        while ((sid = h2_iq_shift(session->ready_to_process)) > 0) {
+          h2_mplx_c1_client_rst(session->mplx, sid, get_stream(session, sid));
+        }
+    }
+
     transit(session, trigger, H2_SESSION_ST_CLEANUP);
     h2_mplx_c1_destroy(session->mplx);
     session->mplx = NULL;
@@ -1069,11 +1085,13 @@ static apr_status_t h2_session_start(h2_session *session, int *rv)
         settings[slen].value = win_size;
         ++slen;
     }
+#if H2_USE_WEBSOCKETS
     if (h2_config_sgeti(session->s, H2_CONF_WEBSOCKETS)) {
       settings[slen].settings_id = NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL;
       settings[slen].value = 1;
       ++slen;
     }
+#endif
 
     ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, session->c1,
                   H2_SSSN_LOG(APLOGNO(03201), session, 

mod_http2/h2_stream.c

@@ -125,7 +125,7 @@ static int trans_on_event[][H2_SS_MAX] = {
 { S_XXX, S_ERR,  S_ERR,  S_CL_L, S_CLS,  S_XXX,  S_XXX,  S_XXX, },/* EV_CLOSED_L*/
 { S_ERR, S_ERR,  S_ERR,  S_CL_R, S_ERR,  S_CLS,  S_NOP,  S_NOP, },/* EV_CLOSED_R*/
 { S_CLS, S_CLS,  S_CLS,  S_CLS,  S_CLS,  S_CLS,  S_NOP,  S_NOP, },/* EV_CANCELLED*/
-{ S_NOP, S_XXX,  S_XXX,  S_XXX,  S_XXX,  S_CLS,  S_CLN,  S_XXX, },/* EV_EOS_SENT*/
+{ S_NOP, S_XXX,  S_XXX,  S_XXX,  S_XXX,  S_CLS,  S_CLN,  S_NOP, },/* EV_EOS_SENT*/
 { S_NOP, S_XXX,  S_CLS,  S_XXX,  S_XXX,  S_CLS,  S_XXX,  S_XXX, },/* EV_IN_ERROR*/
 };
 

mod_http2/h2_ws.c

@@ -19,7 +19,6 @@
 #include "apr.h"
 #include "apr_strings.h"
 #include "apr_lib.h"
-#include "apr_encode.h"
 #include "apr_sha1.h"
 #include "apr_strmatch.h"
 
@@ -45,6 +44,8 @@
 
 #if H2_USE_WEBSOCKETS
 
+#include "apr_encode.h" /* H2_USE_WEBSOCKETS is conditional on APR 1.7+ */
+
 static ap_filter_rec_t *c2_ws_out_filter_handle;
 
 struct ws_filter_ctx {
@@ -238,7 +239,7 @@ static void ws_handle_resp(conn_rec *c2, h2_conn_ctx_t *conn_ctx,
                  * or in the request processings implementation of WebSockets */
                 ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c2, APLOGNO(10463)
                               "h2_c2(%s-%d): websocket CONNECT, 101 response "
-                              "with 'Sec-WebSocket-Accept: %s' but expected %s",
+                              "without 'Sec-WebSocket-Accept: %s' but expected %s",
                               conn_ctx->id, conn_ctx->stream_id, hd,
                               ws_ctx->ws_accept_base64);
             }

mod_http2/h2_ws.h

@@ -23,7 +23,7 @@
  * Rewrite a websocket request.
  *
  * @param req the h2 request to rewrite
- * @param c2 the connection to process the request on
+ * @param conn the connection to process the request on
  * @param no_body != 0 iff the request is known to have no body
  * @return the websocket request for internal submit
  */

mod_http2/mod_proxy_http2.c

@@ -65,7 +65,7 @@ typedef struct h2_proxy_ctx {
     unsigned is_ssl : 1;
 
     request_rec *r;            /* the request processed in this ctx */
-    apr_status_t r_status;     /* status of request work */
+    int r_status;              /* status of request work */
     int r_done;                /* request was processed, not necessarily successfully */
     int r_may_retry;           /* request may be retried */
     int has_reusable_session;  /* http2 session is live and clean */
@@ -413,7 +413,7 @@ static int proxy_http2_handler(request_rec *r,
                       "setup new connection: is_ssl=%d %s %s %s", 
                       ctx->p_conn->is_ssl, ctx->p_conn->ssl_hostname, 
                       locurl, ctx->p_conn->hostname);
-        ctx->r_status = status;
+        ctx->r_status = ap_map_http_request_error(status, HTTP_SERVICE_UNAVAILABLE);
         goto cleanup;
     }
 
@@ -427,7 +427,7 @@ static int proxy_http2_handler(request_rec *r,
     if (ctx->cfront->aborted) goto cleanup;
     status = ctx_run(ctx);
 
-    if (ctx->r_status != APR_SUCCESS && ctx->r_may_retry && !ctx->cfront->aborted) {
+    if (ctx->r_status != OK && ctx->r_may_retry && !ctx->cfront->aborted) {
         /* Not successfully processed, but may retry, tear down old conn and start over */
         if (ctx->p_conn) {
             ctx->p_conn->close = 1;
@@ -463,6 +463,12 @@ static int proxy_http2_handler(request_rec *r,
     ap_set_module_config(ctx->cfront->conn_config, &proxy_http2_module, NULL);
     ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->cfront,
                   APLOGNO(03377) "leaving handler");
+    if (ctx->r_status != OK) {
+        ap_die(ctx->r_status, r);
+    }
+    else if (status != APR_SUCCESS) {
+        ap_die(ap_map_http_request_error(status, HTTP_SERVICE_UNAVAILABLE), r);
+    }
     return ctx->r_status;
 }