diff --git a/changes-entries/remoteip-proxylist-scope.txt b/changes-entries/remoteip-proxylist-scope.txt new file mode 100644 index 00000000000..7bcff9c51b3 --- /dev/null +++ b/changes-entries/remoteip-proxylist-scope.txt @@ -0,0 +1,4 @@ + *) mod_remoteip: Apply RemoteIP{Trusted,Internal}ProxyList to the virtual host + they are configured in, and merge the proxy entries of the main server + and of the virtual host, logging a warning when both have some. + PR 70207. [Arturo Bernal] diff --git a/docs/log-message-tags/next-number b/docs/log-message-tags/next-number index 577ecbb3ce2..c20c25f1622 100644 --- a/docs/log-message-tags/next-number +++ b/docs/log-message-tags/next-number @@ -1 +1 @@ -10633 +10634 diff --git a/modules/metadata/mod_remoteip.c b/modules/metadata/mod_remoteip.c index e017d9aec4f..40eb7702782 100644 --- a/modules/metadata/mod_remoteip.c +++ b/modules/metadata/mod_remoteip.c @@ -176,9 +176,18 @@ static void *merge_remoteip_server_config(apr_pool_t *p, void *globalv, config->proxies_header_name = server->proxies_header_name ? server->proxies_header_name : global->proxies_header_name; - config->proxymatch_ip = server->proxymatch_ip - ? server->proxymatch_ip - : global->proxymatch_ip; + if (server->proxymatch_ip && global->proxymatch_ip) { + /* Both have entries: merge them. remoteip_modify_request() uses the + * first match, so the more specific entries of the vhost go first. + */ + config->proxymatch_ip = apr_array_append(p, server->proxymatch_ip, + global->proxymatch_ip); + } + else { + config->proxymatch_ip = server->proxymatch_ip + ? server->proxymatch_ip + : global->proxymatch_ip; + } return config; } @@ -495,8 +504,9 @@ static const char *remoteip_disable_networks(cmd_parms *cmd, void *d, static int remoteip_hook_post_config(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_t *ptemp, server_rec *s) { - remoteip_config_t *conf; + remoteip_config_t *conf, *vconf; remoteip_addr_info *info; + server_rec *vs; char buf[INET6_ADDRSTRLEN]; conf = ap_get_module_config(ap_server_conf->module_config, @@ -513,6 +523,23 @@ static int remoteip_hook_post_config(apr_pool_t *pconf, apr_pool_t *plog, "RemoteIPProxyProtocol: disabled on %s:%hu", buf, info->addr->port); } + /* A vhost with entries of its own got a new array in the config merge, + * otherwise it shares the one of the main server. + */ + if (conf->proxymatch_ip) { + for (vs = s->next; vs; vs = vs->next) { + vconf = ap_get_module_config(vs->module_config, &remoteip_module); + if (vconf->proxymatch_ip + && vconf->proxymatch_ip != conf->proxymatch_ip) { + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, vs, APLOGNO(10633) + "RemoteIP proxy entries are configured in both " + "the main server and the virtual host defined " + "at %s:%u, they are merged", + vs->defn_name, vs->defn_line_number); + } + } + } + return OK; } @@ -1247,11 +1274,11 @@ static const command_rec remoteip_cmds[] = "Specifies one or more internal (transparent) proxies " "which are trusted to present IP headers"), AP_INIT_TAKE1("RemoteIPTrustedProxyList", proxylist_read, 0, - RSRC_CONF | EXEC_ON_READ, + RSRC_CONF, "The filename to read the list of trusted proxies, " "see the RemoteIPTrustedProxy directive"), AP_INIT_TAKE1("RemoteIPInternalProxyList", proxylist_read, (void*)1, - RSRC_CONF | EXEC_ON_READ, + RSRC_CONF, "The filename to read the list of internal proxies, " "see the RemoteIPInternalProxy directive"), AP_INIT_FLAG("RemoteIPProxyProtocol", remoteip_enable_proxy_protocol, NULL, diff --git a/test/modules/metadata/env.py b/test/modules/metadata/env.py index 1d9672d3951..63f0d9388ca 100644 --- a/test/modules/metadata/env.py +++ b/test/modules/metadata/env.py @@ -15,7 +15,7 @@ def __init__(self, env: 'HttpdTestEnv'): self.add_modules(["mime", "mime_magic", "env", "include"]) # mod_mime_libmagic needs libmagic at build time, so it must not # be a hard requirement; its tests skip when it is absent. - self.add_optional_modules(["mime_libmagic"]) + self.add_optional_modules(["mime_libmagic", "remoteip"]) class MetadataTestEnv(HttpdTestEnv): diff --git a/test/modules/metadata/test_004_remoteip.py b/test/modules/metadata/test_004_remoteip.py new file mode 100644 index 00000000000..de2ce41cb8f --- /dev/null +++ b/test/modules/metadata/test_004_remoteip.py @@ -0,0 +1,117 @@ +import os +import re + +import pytest + +from pyhttpd.conf import HttpdConf +from .env import MetadataTestEnv + + +@pytest.mark.skipif(condition=not MetadataTestEnv.has_shared_module("remoteip"), + reason="mod_remoteip not available") +class TestRemoteIp: + LOG_FILE = "test_remoteip.log" + PEER = "127.0.0.1" + CLIENT = "203.0.113.7" + CDN = "198.51.100.9" + + # The test requests come from the internal proxy PEER and carry + # "X-Forwarded-For: CLIENT, CDN". The proxies are given directly or in + # a list file, in the server config or in the vhost. Each case expects + # the client and the (external) proxies seen by mod_remoteip. + @pytest.mark.parametrize(["scope", "directives", "listed", "proxies"], [ + ["vhost", [f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"], + [], CDN], + ["vhost", [f"RemoteIPInternalProxy {PEER}", "RemoteIPTrustedProxyList {list}"], + [CDN], CDN], + ["vhost", ["RemoteIPTrustedProxyList {list}", f"RemoteIPInternalProxy {PEER}"], + [CDN], CDN], + ["vhost", ["RemoteIPTrustedProxy 10.142.1.99", "RemoteIPInternalProxyList {list}"], + [PEER, CDN], "-"], + ["server", [f"RemoteIPInternalProxy {PEER}", "RemoteIPTrustedProxyList {list}"], + [CDN], CDN], + ]) + def test_metadata_004_01(self, env, scope, directives, listed, proxies): + list_file = os.path.join(env.gen_dir, "remoteip_proxies.lst") + with open(list_file, "w") as f: + f.write("".join(f"{ip}\n" for ip in listed)) + lines = [d.format(list=list_file) for d in directives] + conf = HttpdConf(env, extras={ + "base": [ + "RemoteIPHeader X-Forwarded-For", + f'CustomLog logs/{self.LOG_FILE} "%a %{{remoteip-proxy-ip-list}}n"', + ] + (lines if scope == "server" else []) + }) + conf.start_vhost(domains=[f"test1.{env.http_tld}"], doc_root="htdocs/test1") + if scope == "vhost": + conf.add(lines) + conf.end_vhost() + conf.install() + assert env.apache_restart() == 0 + + log_path = os.path.join(env.server_logs_dir, self.LOG_FILE) + open(log_path, 'w').close() + r = env.curl_get(env.mkurl("https", "test1", "/"), options=[ + '-H', f"X-Forwarded-For: {self.CLIENT}, {self.CDN}"]) + assert r.response["status"] == 200 + with open(log_path) as f: + assert f.read().strip() == f"{self.CLIENT} {proxies}" + + WARNING = r".*both the main server and the virtual host" + + # The proxy entries of the main server and of a vhost are merged, the + # ones of the vhost taking precedence where subnets overlap, and a + # warning tells when both have some. + @pytest.mark.parametrize(["main", "vhost", "proxies", "warning"], [ + # only one of the scopes + [[f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"], + [], CDN, False], + [[], [f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"], + CDN, False], + # RemoteIPInternalProxy in the main server, a list in the vhost + [[f"RemoteIPInternalProxy {PEER}"], ["RemoteIPInternalProxyList {cdn}"], + "-", True], + [[f"RemoteIPInternalProxy {PEER}"], ["RemoteIPTrustedProxyList {cdn}"], + CDN, True], + # a list in the main server, a direct entry in the vhost + [["RemoteIPInternalProxyList {peer}"], [f"RemoteIPTrustedProxy {CDN}"], + CDN, True], + # direct entries in both + [[f"RemoteIPInternalProxy {PEER}"], [f"RemoteIPTrustedProxy {CDN}"], + CDN, True], + # the CDN is trusted by the main server and internal for the vhost + [[f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"], + [f"RemoteIPInternalProxy {CDN}"], "-", True], + [[f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"], + ["RemoteIPInternalProxyList {cdn}"], "-", True], + ]) + def test_metadata_004_02(self, env, main, vhost, proxies, warning): + files = {"cdn": self.CDN, "peer": self.PEER} + for name, ip in files.items(): + with open(os.path.join(env.gen_dir, f"remoteip_{name}.lst"), "w") as f: + f.write(f"{ip}\n") + paths = {n: os.path.join(env.gen_dir, f"remoteip_{n}.lst") for n in files} + conf = HttpdConf(env, extras={ + "base": [ + "RemoteIPHeader X-Forwarded-For", + f'CustomLog logs/{self.LOG_FILE} "%a %{{remoteip-proxy-ip-list}}n"', + ] + [d.format(**paths) for d in main] + }) + conf.start_vhost(domains=[f"test1.{env.http_tld}"], doc_root="htdocs/test1") + conf.add([d.format(**paths) for d in vhost]) + conf.end_vhost() + conf.install() + pos = env.httpd_error_log.current_pos() + assert env.apache_restart() == 0 + warned = env.httpd_error_log.wait_for(re.compile(self.WARNING), pos, timeout=1) + assert warned == warning + if warned: + env.httpd_error_log.ignore_recent(matches=[self.WARNING]) + + log_path = os.path.join(env.server_logs_dir, self.LOG_FILE) + open(log_path, 'w').close() + r = env.curl_get(env.mkurl("https", "test1", "/"), options=[ + '-H', f"X-Forwarded-For: {self.CLIENT}, {self.CDN}"]) + assert r.response["status"] == 200 + with open(log_path) as f: + assert f.read().strip() == f"{self.CLIENT} {proxies}"