Skip to content

Commit b6dcca0

Browse files
committed
Fix mod_remoteip proxy list handling
PR: 70207
1 parent 8dbd283 commit b6dcca0

5 files changed

Lines changed: 156 additions & 8 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
*) mod_remoteip: Apply RemoteIP{Trusted,Internal}ProxyList to the virtual host
2+
they are configured in, and merge the proxy entries of the main server
3+
and of the virtual host, logging a warning when both have some.
4+
PR 70207. [Arturo Bernal]

‎docs/log-message-tags/next-number‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
10633
1+
10634

‎modules/metadata/mod_remoteip.c‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,18 @@ static void *merge_remoteip_server_config(apr_pool_t *p, void *globalv,
176176
config->proxies_header_name = server->proxies_header_name
177177
? server->proxies_header_name
178178
: global->proxies_header_name;
179-
config->proxymatch_ip = server->proxymatch_ip
180-
? server->proxymatch_ip
181-
: global->proxymatch_ip;
179+
if (server->proxymatch_ip && global->proxymatch_ip) {
180+
/* Both have entries: merge them. remoteip_modify_request() uses the
181+
* first match, so the more specific entries of the vhost go first.
182+
*/
183+
config->proxymatch_ip = apr_array_append(p, server->proxymatch_ip,
184+
global->proxymatch_ip);
185+
}
186+
else {
187+
config->proxymatch_ip = server->proxymatch_ip
188+
? server->proxymatch_ip
189+
: global->proxymatch_ip;
190+
}
182191
return config;
183192
}
184193

@@ -495,8 +504,9 @@ static const char *remoteip_disable_networks(cmd_parms *cmd, void *d,
495504
static int remoteip_hook_post_config(apr_pool_t *pconf, apr_pool_t *plog,
496505
apr_pool_t *ptemp, server_rec *s)
497506
{
498-
remoteip_config_t *conf;
507+
remoteip_config_t *conf, *vconf;
499508
remoteip_addr_info *info;
509+
server_rec *vs;
500510
char buf[INET6_ADDRSTRLEN];
501511

502512
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,
513523
"RemoteIPProxyProtocol: disabled on %s:%hu", buf, info->addr->port);
514524
}
515525

526+
/* A vhost with entries of its own got a new array in the config merge,
527+
* otherwise it shares the one of the main server.
528+
*/
529+
if (conf->proxymatch_ip) {
530+
for (vs = s->next; vs; vs = vs->next) {
531+
vconf = ap_get_module_config(vs->module_config, &remoteip_module);
532+
if (vconf->proxymatch_ip
533+
&& vconf->proxymatch_ip != conf->proxymatch_ip) {
534+
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, vs, APLOGNO(10633)
535+
"RemoteIP proxy entries are configured in both "
536+
"the main server and the virtual host defined "
537+
"at %s:%u, they are merged",
538+
vs->defn_name, vs->defn_line_number);
539+
}
540+
}
541+
}
542+
516543
return OK;
517544
}
518545

@@ -1247,11 +1274,11 @@ static const command_rec remoteip_cmds[] =
12471274
"Specifies one or more internal (transparent) proxies "
12481275
"which are trusted to present IP headers"),
12491276
AP_INIT_TAKE1("RemoteIPTrustedProxyList", proxylist_read, 0,
1250-
RSRC_CONF | EXEC_ON_READ,
1277+
RSRC_CONF,
12511278
"The filename to read the list of trusted proxies, "
12521279
"see the RemoteIPTrustedProxy directive"),
12531280
AP_INIT_TAKE1("RemoteIPInternalProxyList", proxylist_read, (void*)1,
1254-
RSRC_CONF | EXEC_ON_READ,
1281+
RSRC_CONF,
12551282
"The filename to read the list of internal proxies, "
12561283
"see the RemoteIPInternalProxy directive"),
12571284
AP_INIT_FLAG("RemoteIPProxyProtocol", remoteip_enable_proxy_protocol, NULL,

‎test/modules/metadata/env.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ def __init__(self, env: 'HttpdTestEnv'):
1515
self.add_modules(["mime", "mime_magic", "env", "include"])
1616
# mod_mime_libmagic needs libmagic at build time, so it must not
1717
# be a hard requirement; its tests skip when it is absent.
18-
self.add_optional_modules(["mime_libmagic"])
18+
self.add_optional_modules(["mime_libmagic", "remoteip"])
1919

2020

2121
class MetadataTestEnv(HttpdTestEnv):
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import os
2+
import re
3+
4+
import pytest
5+
6+
from pyhttpd.conf import HttpdConf
7+
from .env import MetadataTestEnv
8+
9+
10+
@pytest.mark.skipif(condition=not MetadataTestEnv.has_shared_module("remoteip"),
11+
reason="mod_remoteip not available")
12+
class TestRemoteIp:
13+
LOG_FILE = "test_remoteip.log"
14+
PEER = "127.0.0.1"
15+
CLIENT = "203.0.113.7"
16+
CDN = "198.51.100.9"
17+
18+
# The test requests come from the internal proxy PEER and carry
19+
# "X-Forwarded-For: CLIENT, CDN". The proxies are given directly or in
20+
# a list file, in the server config or in the vhost. Each case expects
21+
# the client and the (external) proxies seen by mod_remoteip.
22+
@pytest.mark.parametrize(["scope", "directives", "listed", "proxies"], [
23+
["vhost", [f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"],
24+
[], CDN],
25+
["vhost", [f"RemoteIPInternalProxy {PEER}", "RemoteIPTrustedProxyList {list}"],
26+
[CDN], CDN],
27+
["vhost", ["RemoteIPTrustedProxyList {list}", f"RemoteIPInternalProxy {PEER}"],
28+
[CDN], CDN],
29+
["vhost", ["RemoteIPTrustedProxy 10.142.1.99", "RemoteIPInternalProxyList {list}"],
30+
[PEER, CDN], "-"],
31+
["server", [f"RemoteIPInternalProxy {PEER}", "RemoteIPTrustedProxyList {list}"],
32+
[CDN], CDN],
33+
])
34+
def test_metadata_004_01(self, env, scope, directives, listed, proxies):
35+
list_file = os.path.join(env.gen_dir, "remoteip_proxies.lst")
36+
with open(list_file, "w") as f:
37+
f.write("".join(f"{ip}\n" for ip in listed))
38+
lines = [d.format(list=list_file) for d in directives]
39+
conf = HttpdConf(env, extras={
40+
"base": [
41+
"RemoteIPHeader X-Forwarded-For",
42+
f'CustomLog logs/{self.LOG_FILE} "%a %{{remoteip-proxy-ip-list}}n"',
43+
] + (lines if scope == "server" else [])
44+
})
45+
conf.start_vhost(domains=[f"test1.{env.http_tld}"], doc_root="htdocs/test1")
46+
if scope == "vhost":
47+
conf.add(lines)
48+
conf.end_vhost()
49+
conf.install()
50+
assert env.apache_restart() == 0
51+
52+
log_path = os.path.join(env.server_logs_dir, self.LOG_FILE)
53+
open(log_path, 'w').close()
54+
r = env.curl_get(env.mkurl("https", "test1", "/"), options=[
55+
'-H', f"X-Forwarded-For: {self.CLIENT}, {self.CDN}"])
56+
assert r.response["status"] == 200
57+
with open(log_path) as f:
58+
assert f.read().strip() == f"{self.CLIENT} {proxies}"
59+
60+
WARNING = r".*both the main server and the virtual host"
61+
62+
# The proxy entries of the main server and of a vhost are merged, the
63+
# ones of the vhost taking precedence where subnets overlap, and a
64+
# warning tells when both have some.
65+
@pytest.mark.parametrize(["main", "vhost", "proxies", "warning"], [
66+
# only one of the scopes
67+
[[f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"],
68+
[], CDN, False],
69+
[[], [f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"],
70+
CDN, False],
71+
# RemoteIPInternalProxy in the main server, a list in the vhost
72+
[[f"RemoteIPInternalProxy {PEER}"], ["RemoteIPInternalProxyList {cdn}"],
73+
"-", True],
74+
[[f"RemoteIPInternalProxy {PEER}"], ["RemoteIPTrustedProxyList {cdn}"],
75+
CDN, True],
76+
# a list in the main server, a direct entry in the vhost
77+
[["RemoteIPInternalProxyList {peer}"], [f"RemoteIPTrustedProxy {CDN}"],
78+
CDN, True],
79+
# direct entries in both
80+
[[f"RemoteIPInternalProxy {PEER}"], [f"RemoteIPTrustedProxy {CDN}"],
81+
CDN, True],
82+
# the CDN is trusted by the main server and internal for the vhost
83+
[[f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"],
84+
[f"RemoteIPInternalProxy {CDN}"], "-", True],
85+
[[f"RemoteIPInternalProxy {PEER}", f"RemoteIPTrustedProxy {CDN}"],
86+
["RemoteIPInternalProxyList {cdn}"], "-", True],
87+
])
88+
def test_metadata_004_02(self, env, main, vhost, proxies, warning):
89+
files = {"cdn": self.CDN, "peer": self.PEER}
90+
for name, ip in files.items():
91+
with open(os.path.join(env.gen_dir, f"remoteip_{name}.lst"), "w") as f:
92+
f.write(f"{ip}\n")
93+
paths = {n: os.path.join(env.gen_dir, f"remoteip_{n}.lst") for n in files}
94+
conf = HttpdConf(env, extras={
95+
"base": [
96+
"RemoteIPHeader X-Forwarded-For",
97+
f'CustomLog logs/{self.LOG_FILE} "%a %{{remoteip-proxy-ip-list}}n"',
98+
] + [d.format(**paths) for d in main]
99+
})
100+
conf.start_vhost(domains=[f"test1.{env.http_tld}"], doc_root="htdocs/test1")
101+
conf.add([d.format(**paths) for d in vhost])
102+
conf.end_vhost()
103+
conf.install()
104+
pos = env.httpd_error_log.current_pos()
105+
assert env.apache_restart() == 0
106+
warned = env.httpd_error_log.wait_for(re.compile(self.WARNING), pos, timeout=1)
107+
assert warned == warning
108+
if warned:
109+
env.httpd_error_log.ignore_recent(matches=[self.WARNING])
110+
111+
log_path = os.path.join(env.server_logs_dir, self.LOG_FILE)
112+
open(log_path, 'w').close()
113+
r = env.curl_get(env.mkurl("https", "test1", "/"), options=[
114+
'-H', f"X-Forwarded-For: {self.CLIENT}, {self.CDN}"])
115+
assert r.response["status"] == 200
116+
with open(log_path) as f:
117+
assert f.read().strip() == f"{self.CLIENT} {proxies}"

0 commit comments

Comments
 (0)