Skip to content

Commit de60d7e

Browse files
juanjuxjuanjux
authored
chore(asm): update the list of IP headers to the latest RFC (#5185)
## Motivation Update the list of headers from which we search for an IP address to the latest version of the "Client IP address resolution" RFC. ## Checklist - [X] Change(s) are motivated and described in the PR description. - [X] Testing strategy is described if automated tests are not included in the PR. - [X] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [X] Change is maintainable (easy to change, telemetry, documentation). - [X] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [X] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [X] Author is aware of the performance implications of this PR as reported in the benchmarks PR comment. ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer is aware of, and discussed the performance implications of this PR as reported in the benchmarks PR comment. --------- Signed-off-by: Juanjo Alvarez <[email protected]>
1 parent 0e854eb commit de60d7e

16 files changed

+46
-33
lines changed

‎ddtrace/appsec/processor.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,13 @@ def get_appsec_obfuscation_parameter_value_regexp():
9494
"accept",
9595
"accept-encoding",
9696
"accept-language",
97+
"cf-connecting-ip",
98+
"cf-connecting-ipv6",
9799
"content-encoding",
98100
"content-language",
99101
"content-length",
100102
"content-type",
103+
"fastly-client-ip",
101104
"forwarded",
102105
"forwarded-for",
103106
"host",

‎ddtrace/contrib/trace_utils.py‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,14 @@
6161
IP_PATTERNS = (
6262
"x-forwarded-for",
6363
"x-real-ip",
64-
"client-ip",
64+
"true-client-ip",
65+
"x-client-ip",
6566
"x-forwarded",
66-
"x-cluster-client-ip",
6767
"forwarded-for",
68-
"forwarded",
69-
"via",
70-
"true-client-ip",
68+
"x-cluster-client-ip",
69+
"fastly-client-ip",
70+
"cf-connecting-ip",
71+
"cf-connecting-ipv6",
7172
)
7273

7374

‎releasenotes/notes/asm-update-ip-resolution-04c9d92ea3138051.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
other:
3+
- |
4+
ASM: The list of headers for retrieving the IP when Application Security Management is enabled or the
5+
`DD_TRACE_CLIENT_IP_ENABLED` environment variable is set has been updated. "Via" has been
6+
removed as it rarely contains IP data and some common vendor headers have been added. You can
7+
also set the environment variable `DD_TRACE_CLIENT_IP_HEADER` to always retrieve the IP
8+
from the header specified as the value.

‎tests/contrib/django/test_django_appsec.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ def test_django_client_ip_header_set_by_env_var_valid(client, test_spans, tracer
349349
test_spans,
350350
tracer,
351351
url="/?a=1&b&c=d",
352-
headers={"HTTP_CLIENT_IP": "8.8.8.8", "HTTP_X_USE_THIS": "4.4.4.4"},
352+
headers={"HTTP_X_CLIENT_IP": "8.8.8.8", "HTTP_X_USE_THIS": "4.4.4.4"},
353353
)
354354
assert root_span.get_tag(http.CLIENT_IP) == "4.4.4.4"
355355

@@ -364,10 +364,10 @@ def test_django_client_ip_nothing(client, test_spans, tracer):
364364
@pytest.mark.parametrize(
365365
"kwargs,expected",
366366
[
367-
({"HTTP_CLIENT_IP": "", "HTTP_X_FORWARDED_FOR": "4.4.4.4"}, "4.4.4.4"),
368-
({"HTTP_CLIENT_IP": "192.168.1.3,4.4.4.4"}, "4.4.4.4"),
369-
({"HTTP_CLIENT_IP": "4.4.4.4,8.8.8.8"}, "4.4.4.4"),
370-
({"HTTP_CLIENT_IP": "192.168.1.10,192.168.1.20"}, "192.168.1.10"),
367+
({"HTTP_X_CLIENT_IP": "", "HTTP_X_FORWARDED_FOR": "4.4.4.4"}, "4.4.4.4"),
368+
({"HTTP_X_CLIENT_IP": "192.168.1.3,4.4.4.4"}, "4.4.4.4"),
369+
({"HTTP_X_CLIENT_IP": "4.4.4.4,8.8.8.8"}, "4.4.4.4"),
370+
({"HTTP_X_CLIENT_IP": "192.168.1.10,192.168.1.20"}, "192.168.1.10"),
371371
],
372372
)
373373
def test_django_client_ip_headers(client, test_spans, tracer, kwargs, expected):

‎tests/contrib/django/test_django_appsec_snapshots.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ def test_request_ipblock_nomatch_200():
114114
"DD_APPSEC_RULES": RULES_GOOD_PATH,
115115
},
116116
) as client:
117-
result = client.get("/", headers={"Via": _ALLOWED_IP})
117+
result = client.get("/", headers={"X-Real-Ip": _ALLOWED_IP})
118118
assert result.status_code == 200
119119
assert result.content == b"Hello, test app."
120120

@@ -143,7 +143,7 @@ def test_request_ipblock_match_403():
143143
result = client.get(
144144
"/",
145145
headers={
146-
"Via": _BLOCKED_IP,
146+
"X-Real-Ip": _BLOCKED_IP,
147147
"Accept": "text/html",
148148
},
149149
)
@@ -178,7 +178,7 @@ def test_request_ipblock_match_403_json():
178178
result = client.get(
179179
"/",
180180
headers={
181-
"Via": _BLOCKED_IP,
181+
"X-Real-Ip": _BLOCKED_IP,
182182
},
183183
)
184184
assert result.status_code == 403

‎tests/contrib/django/test_django_snapshots.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ def test_safe_string_encoding(client, snapshot_context):
111111
"111x": (1, 9) <= django.VERSION < (1, 12),
112112
"21x": (1, 12) < django.VERSION < (2, 2),
113113
"": django.VERSION >= (2, 2),
114-
}
114+
},
115+
ignores=["metrics._dd.tracer_kr"],
115116
):
116117
assert client.get("/safe-template/").status_code == 200
117118

@@ -156,7 +157,7 @@ def test_psycopg_query_default(client, snapshot_context, psycopg2_patched):
156157
from django.db import connections
157158
from psycopg2.sql import SQL
158159

159-
with snapshot_context(ignores=["meta.out.host"]):
160+
with snapshot_context(ignores=["meta.out.host", "metrics._dd.tracer_kr"]):
160161
query = SQL("""select 'one' as x""")
161162
conn = connections["postgres"]
162163
with conn.cursor() as cur:

‎tests/contrib/flask/test_appsec_flask_snapshot.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ def flask_client(flask_command, flask_port, flask_wsgi_application, flask_env_ar
127127
)
128128
@pytest.mark.parametrize("flask_env_arg", (flask_appsec_good_rules_env,))
129129
def test_flask_ipblock_match_403(flask_client):
130-
resp = flask_client.get("/", headers={"Via": _BLOCKED_IP, "ACCEPT": "text/html"})
130+
resp = flask_client.get("/", headers={"X-Real-Ip": _BLOCKED_IP, "ACCEPT": "text/html"})
131131
assert resp.status_code == 403
132132
if hasattr(resp, "text"):
133133
assert resp.text == APPSEC_BLOCKED_RESPONSE_HTML
@@ -154,7 +154,7 @@ def test_flask_ipblock_match_403(flask_client):
154154
)
155155
@pytest.mark.parametrize("flask_env_arg", (flask_appsec_good_rules_env,))
156156
def test_flask_ipblock_match_403_json(flask_client):
157-
resp = flask_client.get("/", headers={"Via": _BLOCKED_IP})
157+
resp = flask_client.get("/", headers={"X-Real-Ip": _BLOCKED_IP})
158158
assert resp.status_code == 403
159159
if hasattr(resp, "text"):
160160
assert resp.text == APPSEC_BLOCKED_RESPONSE_JSON

‎tests/contrib/flask/test_flask_appsec.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ def test_flask_useragent(self):
140140

141141
def test_flask_client_ip_header_set_by_env_var_valid(self):
142142
with override_global_config(dict(_appsec_enabled=True, client_ip_header="X-Use-This")):
143-
self.client.get("/?a=1&b&c=d", headers={"HTTP_CLIENT_IP": "8.8.8.8", "X-Use-This": "4.4.4.4"})
143+
self.client.get("/?a=1&b&c=d", headers={"HTTP_X_CLIENT_IP": "8.8.8.8", "X-Use-This": "4.4.4.4"})
144144
spans = self.pop_spans()
145145
root_span = spans[0]
146146
assert root_span.get_tag(http.CLIENT_IP) == "4.4.4.4"
@@ -268,7 +268,7 @@ def test_flask_body_xml_empty_logs_warning(self):
268268
def test_flask_ipblock_match_403_json(self):
269269
with override_global_config(dict(_appsec_enabled=True)), override_env(dict(DD_APPSEC_RULES=RULES_GOOD_PATH)):
270270
self._aux_appsec_prepare_tracer()
271-
resp = self.client.get("/", headers={"Via": _BLOCKED_IP})
271+
resp = self.client.get("/", headers={"X-Real-Ip": _BLOCKED_IP})
272272
assert resp.status_code == 403
273273
if hasattr(resp, "text"):
274274
assert resp.text == constants.APPSEC_BLOCKED_RESPONSE_JSON

‎tests/contrib/flask/test_flask_appsec_telemetry.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def test_telemetry_metrics_block(self):
3131
dict(DD_APPSEC_RULES=RULES_GOOD_PATH)
3232
):
3333
self._aux_appsec_prepare_tracer()
34-
resp = self.client.get("/", headers={"Via": _BLOCKED_IP})
34+
resp = self.client.get("/", headers={"X-Real-Ip": _BLOCKED_IP})
3535
assert resp.status_code == 403
3636
if hasattr(resp, "text"):
3737
assert resp.text == APPSEC_BLOCKED_RESPONSE_JSON

‎tests/snapshots/tests.contrib.django.test_django_appsec_snapshots.test_request_ipblock_match_403.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
"http.request.headers.accept-encoding": "gzip, deflate, br",
2929
"http.request.headers.host": "localhost:8000",
3030
"http.request.headers.user-agent": "python-requests/2.28.1",
31-
"http.request.headers.via": "8.8.4.4",
31+
"http.request.headers.x-real-ip": "8.8.4.4",
3232
"http.url": "http://localhost:8000/",
3333
"http.useragent": "python-requests/2.28.1",
3434
"http.version": "1.1",

0 commit comments

Comments
 (0)