From 3bd6a54b0e498b7bc854c435758aaff0613adfef Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Thu, 27 Aug 2026 19:48:32 +0100 Subject: [PATCH] mod_proxy_connect: Implement "AllowCONNECT None". The value is documented possibly by accident in r1932337, but not implemented. * modules/proxy/mod_proxy_connect.c (connect_conf): Add "none". (set_allowed_ports): Accept "None"; reject combining it with ports in the same context. (merge_config): Special-case "None": it disallows all ports and overrides an inherited list, and an inherited "None" is overridden by a port list in a vhost; two port lists still merge as a union. (allowed_port): Disallow all ports when "none" is set. * docs/manual/mod/mod_proxy_connect.xml: Clarify the merge behaviour and the "None" restriction. * test/modules/proxy/test_07_connect.py: Added. * test/modules/proxy/env.py: Load mod_proxy_connect for the suite. Co-Authored-By: Claude Fable 5 --- changes-entries/proxy-connect-none.txt | 2 + docs/manual/mod/mod_proxy_connect.xml | 12 ++- modules/proxy/mod_proxy_connect.c | 47 ++++++++++- test/modules/proxy/env.py | 3 +- test/modules/proxy/test_07_connect.py | 108 +++++++++++++++++++++++++ test/pytest_suite/t/basic1 | 3 - 6 files changed, 166 insertions(+), 9 deletions(-) create mode 100644 changes-entries/proxy-connect-none.txt create mode 100644 test/modules/proxy/test_07_connect.py delete mode 100644 test/pytest_suite/t/basic1 diff --git a/changes-entries/proxy-connect-none.txt b/changes-entries/proxy-connect-none.txt new file mode 100644 index 00000000000..7b2896f5ae0 --- /dev/null +++ b/changes-entries/proxy-connect-none.txt @@ -0,0 +1,2 @@ + *) mod_proxy_connect: Implement "AllowCONNECT None", which + disallows CONNECT to all ports. [Joe Orton] diff --git a/docs/manual/mod/mod_proxy_connect.xml b/docs/manual/mod/mod_proxy_connect.xml index e974f22dc64..e662c39c58f 100644 --- a/docs/manual/mod/mod_proxy_connect.xml +++ b/docs/manual/mod/mod_proxy_connect.xml @@ -101,8 +101,16 @@ Port ranges available since Apache 2.3.7. AllowCONNECT directive to override this default and allow connections to the listed ports only.

-

Set the value to None to disallow - CONNECT requests to all ports, including the defaults.

+

Set the value to None to disallow CONNECT + requests to all ports, including the defaults. None cannot + be combined with port numbers in the same context.

+ +

Where AllowCONNECT is set in both the main + server and a virtual host, the lists of ports are merged, so a virtual + host may permit additional ports. A value of None takes + precedence and disallows all ports where it is set; but an inherited + None is in turn overridden by an explicit list of ports in a + virtual host.

diff --git a/modules/proxy/mod_proxy_connect.c b/modules/proxy/mod_proxy_connect.c index c46a67f13cf..42a7222256c 100644 --- a/modules/proxy/mod_proxy_connect.c +++ b/modules/proxy/mod_proxy_connect.c @@ -48,6 +48,7 @@ module AP_MODULE_DECLARE_DATA proxy_connect_module; typedef struct { apr_array_header_t *allowed_connect_ports; + int none; /* AllowCONNECT None: disallow CONNECT to all ports */ } connect_conf; typedef struct { @@ -68,9 +69,31 @@ static void *merge_config(apr_pool_t *p, void *basev, void *overridesv) connect_conf *base = (connect_conf *) basev; connect_conf *overrides = (connect_conf *) overridesv; - c->allowed_connect_ports = apr_array_append(p, + if (overrides->none) { + /* "AllowCONNECT None" here disallows all ports regardless of base. */ + c->allowed_connect_ports = + apr_array_copy(p, overrides->allowed_connect_ports); + c->none = 1; + } + else if (apr_is_empty_array(overrides->allowed_connect_ports)) { + /* Nothing set here: inherit the base. */ + c->allowed_connect_ports = + apr_array_copy(p, base->allowed_connect_ports); + c->none = base->none; + } + else if (base->none || apr_is_empty_array(base->allowed_connect_ports)) { + /* Base was "None" or unset, so the ports set here stand alone; e.g. + * "AllowCONNECT 443" in a vhost re-allows that port under a global + * "AllowCONNECT None". */ + c->allowed_connect_ports = + apr_array_copy(p, overrides->allowed_connect_ports); + } + else { + /* Two port lists merge as a union, as they always have. */ + c->allowed_connect_ports = apr_array_append(p, base->allowed_connect_ports, overrides->allowed_connect_ports); + } return c; } @@ -90,8 +113,21 @@ static const char * char *endptr; const char *p = arg; + /* "None" disallows CONNECT to all ports, including the defaults, and + * cannot be combined with a port list in the same context. */ + if (!ap_cstr_casecmp(arg, "None")) { + if (!apr_is_empty_array(conf->allowed_connect_ports)) { + return "AllowCONNECT: \"None\" cannot be combined with port numbers"; + } + conf->none = 1; + return NULL; + } + if (conf->none) { + return "AllowCONNECT: \"None\" cannot be combined with port numbers"; + } + if (!apr_isdigit(arg[0])) - return "AllowCONNECT: port numbers must be numeric"; + return "AllowCONNECT: port numbers must be numeric or \"None\""; first = strtol(p, &endptr, 10); if (*endptr == '-') { @@ -119,6 +155,10 @@ static int allowed_port(connect_conf *conf, int port) int i; port_range *list = (port_range *) conf->allowed_connect_ports->elts; + if (conf->none) { + return 0; + } + if (apr_is_empty_array(conf->allowed_connect_ports)) { return port == APR_URI_HTTPS_DEFAULT_PORT || port == APR_URI_SNEWS_DEFAULT_PORT; @@ -389,7 +429,8 @@ static void ap_proxy_connect_register_hook(apr_pool_t *p) static const command_rec cmds[] = { AP_INIT_ITERATE("AllowCONNECT", set_allowed_ports, NULL, RSRC_CONF, - "A list of ports or port ranges which CONNECT may connect to"), + "A list of ports or port ranges which CONNECT may connect to, or " + "\"None\" to disallow all"), {NULL} }; diff --git a/test/modules/proxy/env.py b/test/modules/proxy/env.py index fc443370754..d73f9db9be9 100644 --- a/test/modules/proxy/env.py +++ b/test/modules/proxy/env.py @@ -69,7 +69,8 @@ def __init__(self, env: 'HttpdTestEnv'): super().__init__(env=env) self.add_source_dir(os.path.dirname(inspect.getfile(ProxyTestSetup))) self.add_modules(["proxy", "proxy_http", "proxy_ajp", "proxy_balancer", - "proxy_uwsgi", "lbmethod_byrequests", "remoteip"]) + "proxy_connect", "proxy_uwsgi", "lbmethod_byrequests", + "remoteip"]) class ProxyTestEnv(HttpdTestEnv): diff --git a/test/modules/proxy/test_07_connect.py b/test/modules/proxy/test_07_connect.py new file mode 100644 index 00000000000..571a2226961 --- /dev/null +++ b/test/modules/proxy/test_07_connect.py @@ -0,0 +1,108 @@ +import socket + +import pytest + +from pyhttpd.conf import HttpdConf + + +class TestProxyConnect: + """AllowCONNECT, including the documented "None" value which disallows + CONNECT to all ports (including the 443/563 defaults).""" + + def _mk_proxy(self, env, allow_connect): + # A forward proxy on its own (plain HTTP) port, so a raw CONNECT can + # be sent to it and the vhost handling it is unambiguous. + conf = HttpdConf(env) + conf.add(f"Listen {env.proxy_port}") + conf.start_vhost(domains=[env.d_forward], port=env.proxy_port) + conf.add([ + "ProxyRequests on", + allow_connect, + ]) + conf.end_vhost() + conf.install() + assert env.apache_restart() == 0 + + def _connect_status(self, env, target): + # Send a raw CONNECT request to the forward proxy and return the + # numeric status of its response. + req = f"CONNECT {target} HTTP/1.0\r\nHost: {target}\r\n\r\n" + with socket.create_connection(("127.0.0.1", env.proxy_port), + timeout=5) as s: + s.sendall(req.encode()) + buf = b"" + while b"\r\n" not in buf: + data = s.recv(1024) + if not data: + break + buf += data + line = buf.split(b"\r\n", 1)[0].decode("latin-1") + return int(line.split(" ", 2)[1]) + + def test_proxy_connect_07_none(self, env): + # AllowCONNECT None rejects every CONNECT with 403, including the + # default https port and an otherwise-reachable target. + self._mk_proxy(env, "AllowCONNECT None") + assert self._connect_status(env, "127.0.0.1:443") == 403 + assert self._connect_status(env, f"127.0.0.1:{env.http_port}") == 403 + # the two rejections each log "Connect to remote machine blocked" + env.httpd_error_log.ignore_recent(lognos=["AH00898"]) + + def test_proxy_connect_07_allowed(self, env): + # Control: with the port explicitly allowed, the same CONNECT tunnels + # (200 Connection Established) - so it is None, above, which blocks it. + self._mk_proxy(env, f"AllowCONNECT {env.http_port}") + assert self._connect_status(env, f"127.0.0.1:{env.http_port}") == 200 + + def test_proxy_connect_07_inherit_override(self, env): + # "AllowCONNECT None" at the main-server level is overridden by an + # explicit port list in the vhost: that port is allowed there (200), + # while a port not listed remains blocked (403). + conf = HttpdConf(env) + conf.add("AllowCONNECT None") + conf.add(f"Listen {env.proxy_port}") + conf.start_vhost(domains=[env.d_forward], port=env.proxy_port) + conf.add([ + "ProxyRequests on", + f"AllowCONNECT {env.http_port}", + ]) + conf.end_vhost() + conf.install() + assert env.apache_restart() == 0 + assert self._connect_status(env, f"127.0.0.1:{env.http_port}") == 200 + assert self._connect_status(env, "127.0.0.1:443") == 403 + env.httpd_error_log.ignore_recent(lognos=["AH00898"]) + + def test_proxy_connect_07_merge_union(self, env): + # Two port lists (main-server and vhost) still merge as a union, so a + # port allowed at the main-server level stays allowed in the vhost. + conf = HttpdConf(env) + conf.add(f"AllowCONNECT {env.http_port}") + conf.add(f"Listen {env.proxy_port}") + conf.start_vhost(domains=[env.d_forward], port=env.proxy_port) + conf.add([ + "ProxyRequests on", + "AllowCONNECT 563", + ]) + conf.end_vhost() + conf.install() + assert env.apache_restart() == 0 + assert self._connect_status(env, f"127.0.0.1:{env.http_port}") == 200 + assert self._connect_status(env, "127.0.0.1:9") == 403 + env.httpd_error_log.ignore_recent(lognos=["AH00898"]) + + def test_proxy_connect_07_conflict(self, env): + # "None" and a port number in the same context is a config error. + conf = HttpdConf(env) + conf.add(f"Listen {env.proxy_port}") + conf.start_vhost(domains=[env.d_forward], port=env.proxy_port) + conf.add([ + "ProxyRequests on", + "AllowCONNECT None", + "AllowCONNECT 443", + ]) + conf.end_vhost() + conf.install() + assert env.apache_restart() != 0 + # leave a working server behind for teardown / any later test + self._mk_proxy(env, "AllowCONNECT None") diff --git a/test/pytest_suite/t/basic1 b/test/pytest_suite/t/basic1 deleted file mode 100644 index 1bbed268e6a..00000000000 --- a/test/pytest_suite/t/basic1 +++ /dev/null @@ -1,3 +0,0 @@ -user1:NYSYdf7MU5KpU -user2:KJ7Yxzr1VVzAI -user3:xnpSvZ2iqti/c