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.

The early checks avoid failing the request later on and thus save cycles
for those invalid cases.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1895921 13f79535-47bb-0310-9956-ffa450edef68

7 files changed, 42 insertions, 10 deletions

diff --git a/changes-entries/http_enforcements.txt b/changes-entries/http_enforcements.txt
new file mode 100644
index 0000000000..3e16f109f2
--- /dev/null
+++ b/changes-entries/http_enforcements.txt
@@ -0,0 +1,3 @@
+  *) 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.  [Yann Ylavic]
diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 822a41eaf5..67f3637448 100644
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -695,6 +695,7 @@
  * 20210926.0 (2.5.1-dev)  Add dav_get_liveprop_element(), remove DAV_PROP_ELEMENT.
  * 20210926.1 (2.5.1-dev)  Add ap_unescape_url_ex() and deprecate
  *                         AP_NORMALIZE_DROP_PARAMETERS
+ * 20210926.2 (2.5.1-dev)  Add ap_post_read_request()
  * 
  */
 
@@ -703,7 +704,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20210926
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 1             /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 2             /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
diff --git a/include/http_protocol.h b/include/http_protocol.h
index 9c9cb952b2..38eef396a3 100644
--- a/include/http_protocol.h
+++ b/include/http_protocol.h
@@ -96,6 +96,13 @@ AP_DECLARE(void) ap_get_mime_headers(request_rec *r);
 AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r,
                                           apr_bucket_brigade *bb);
 
+/**
+ * Run post_read_request hook and validate.
+ * @param r The current request
+ * @return OK or HTTP_...
+ */
+AP_DECLARE(int) ap_post_read_request(request_rec *r);
+
 /* Finish up stuff after a request */
 
 /**
diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index a5fdaf44f9..5327dd0e04 100644
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -690,7 +690,7 @@ static request_rec *internal_internal_redirect(const char *new_uri,
      * to do their thing on internal redirects as well.  Perhaps this is a
      * misnamed function.
      */
-    if ((access_status = ap_run_post_read_request(new))) {
+    if ((access_status = ap_post_read_request(new))) {
         ap_die(access_status, new);
         return NULL;
     }
diff --git a/modules/http2/h2_request.c b/modules/http2/h2_request.c
index 7c9f38a26a..54721c45af 100644
--- a/modules/http2/h2_request.c
+++ b/modules/http2/h2_request.c
@@ -383,7 +383,7 @@ request_rec *h2_create_request_rec(const h2_request *req, conn_rec *c)
     ap_add_input_filter_handle(ap_http_input_filter_handle,
                                NULL, r, r->connection);
     
-    if ((access_status = ap_run_post_read_request(r))) {
+    if ((access_status = ap_post_read_request(r))) {
         /* Request check post hooks failed. An example of this would be a
          * request for a vhost where h2 is disabled --> 421.
          */
diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c
index 622b936dde..b0dc09ecdf 100644
--- a/modules/proxy/mod_proxy.c
+++ b/modules/proxy/mod_proxy.c
@@ -781,13 +781,13 @@ static int proxy_detect(request_rec *r)
 
     /* Ick... msvc (perhaps others) promotes ternary short results to int */
 
-    if (conf->req && r->parsed_uri.scheme) {
+    if (conf->req && r->parsed_uri.scheme && r->parsed_uri.hostname) {
         /* but it might be something vhosted */
-        if (!(r->parsed_uri.hostname
-              && !ap_cstr_casecmp(r->parsed_uri.scheme, ap_http_scheme(r))
-              && ap_matches_request_vhost(r, r->parsed_uri.hostname,
-                                          (apr_port_t)(r->parsed_uri.port_str ? r->parsed_uri.port
-                                                       : ap_default_port(r))))) {
+        if (ap_cstr_casecmp(r->parsed_uri.scheme, ap_http_scheme(r)) != 0
+            || !ap_matches_request_vhost(r, r->parsed_uri.hostname,
+                                         (apr_port_t)(r->parsed_uri.port_str
+                                                      ? r->parsed_uri.port
+                                                      : ap_default_port(r)))) {
             r->proxyreq = PROXYREQ_PROXY;
             r->uri = r->unparsed_uri;
             r->filename = apr_pstrcat(r->pool, "proxy:", r->uri, NULL);
diff --git a/server/protocol.c b/server/protocol.c
index c4dc7b5763..0c3b770ad5 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -1595,7 +1595,7 @@ request_rec *ap_read_request(conn_rec *conn)
     /* we may have switched to another server */
     apply_server_config(r);
 
-    if ((access_status = ap_run_post_read_request(r))) {
+    if ((access_status = ap_post_read_request(r))) {
         goto die;
     }
 
@@ -1650,6 +1650,27 @@ ignore:
     return NULL;
 }
 
+AP_DECLARE(int) ap_post_read_request(request_rec *r)
+{
+    int status;
+
+    if ((status = ap_run_post_read_request(r))) {
+        return status;
+    }
+
+    /* Enforce http(s) only scheme for non-forward-proxy requests */
+    if (!r->proxyreq
+            && r->parsed_uri.scheme
+            && (ap_cstr_casecmpn(r->parsed_uri.scheme, "http", 4) != 0
+                || (r->parsed_uri.scheme[4] != '\0'
+                    && (apr_tolower(r->parsed_uri.scheme[4]) != 's'
+                        || r->parsed_uri.scheme[5] != '\0')))) {
+        return HTTP_BAD_REQUEST;
+    }
+
+    return OK;
+}
+
 /* if a request with a body creates a subrequest, remove original request's
  * input headers which pertain to the body which has already been read.
  * out-of-line helper function for ap_set_sub_req_protocol.