Skip to content

Commit 3ec641e

Browse files
chore(tracer): add forwarded ip header format support (#14418)
Following #14400 This PR improves the support for `forwarded` header by adding ip format extraction from the forwarded format described in https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Forwarded https://www.rfc-editor.org/rfc/rfc7239 - We only extract the ip from the `for` parameter. - Also add tests for different possible ip formats (ipv4, ipv6, quoted or unquoted, with or without port) This will also be supported by system tests DataDog/system-tests#5134 APPSEC-58261 ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
1 parent 7c5fac0 commit 3ec641e

File tree

2 files changed

+66
-3
lines changed

2 files changed

+66
-3
lines changed

ddtrace/contrib/internal/trace_utils.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,23 @@ def _get_request_header_referrer_host(headers, headers_are_case_sensitive=False)
133133
return ""
134134

135135

136+
def _parse_ip_header(ip_header_value: str) -> str:
137+
"""Parse the ip header, either in Forwarded-For format or Forwarded format.
138+
139+
references: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Forwarded
140+
"""
141+
IP_EXTRACTIONS = [
142+
r"^\s*(?P<ip>[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)$", # ipv4 simple format
143+
r'(?:^|;)\s*for="?(?P<ip>[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)', # ipv4 forwarded format
144+
r"^\s*(?P<ip>[0-9a-fA-F:]+)$", # ipv6 simple format
145+
r'(?:^|;)\s*for="\[(?P<ip>[0-9a-fA-F:]+)\]', # ipv6 forwarded format
146+
]
147+
for pattern in IP_EXTRACTIONS:
148+
if m := re.search(pattern, ip_header_value, re.IGNORECASE):
149+
return m.group("ip")
150+
return ""
151+
152+
136153
def _get_request_header_client_ip(headers, peer_ip=None, headers_are_case_sensitive=False):
137154
# type: (Optional[Mapping[str, str]], Optional[str], bool) -> str
138155

@@ -185,12 +202,12 @@ def get_header_value(key): # type: (str) -> Optional[str]
185202

186203
if ip_header_value:
187204
# At this point, we have one IP header, check its value and retrieve the first public IP
205+
188206
ip_list = ip_header_value.split(",")
189207
for ip in ip_list:
190-
ip = ip.strip()
208+
ip = _parse_ip_header(ip)
191209
if not ip:
192210
continue
193-
194211
try:
195212
if ip_is_global(ip):
196213
return ip

tests/tracer/test_trace_utils.py

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,7 @@ def test_set_http_meta_case_sensitive_headers_notfound(mock_store_headers, span,
632632
("x-real-ip", "2.2.2.2"),
633633
("true-client-ip", "3.3.3.3"),
634634
("x-client-ip", "4.4.4.4"),
635-
("forwarded", "5.5.5.5"),
635+
("forwarded", 'by=1.2.3.4;for="5.5.5.5:2000";host=test.zouzou.ncom'),
636636
("forwarded-for", "6.6.6.6"),
637637
("x-cluster-client-ip", "7.7.7.7"),
638638
("fastly-client-ip", "8.8.8.8"),
@@ -645,6 +645,9 @@ def test_set_http_meta_case_sensitive_headers_notfound(mock_store_headers, span,
645645
["", dict(ALL_IP_HEADERS[-1 : -i - 2 : -1]), ALL_IP_HEADERS[-1 - i][1]] for i in range(len(ALL_IP_HEADERS))
646646
]
647647

648+
# expected value for forwarded is extracted from for
649+
ALL_TESTS[5][2] = "5.5.5.5"
650+
648651

649652
@pytest.mark.parametrize(
650653
"header_env_var,headers_dict,expected",
@@ -687,6 +690,49 @@ def test_set_http_meta_case_sensitive_headers_notfound(mock_store_headers, span,
687690
{"x-forwarded-for": "4.4.4.4", "x-real-ip": "8.8.4.4"},
688691
"8.8.4.4",
689692
),
693+
(
694+
"",
695+
{"forwarded": 'by=1.2.3.4;for="[9f7b:5e67:5472:4464:90b0:6b0a:9aa6:f9dc]:2000";host=test.zouzou.ncom'},
696+
"9f7b:5e67:5472:4464:90b0:6b0a:9aa6:f9dc",
697+
),
698+
(
699+
"",
700+
{"forwarded": 'by=1.2.3.4;for="[9f7b:5e67:5472:4464:90b0:6b0a:9aa6:f9dc]";host=test.zouzou.ncom'},
701+
"9f7b:5e67:5472:4464:90b0:6b0a:9aa6:f9dc",
702+
),
703+
(
704+
"",
705+
{"forwarded": 'by=1.2.3.4;for="3.3.3.3:1321";host=test.zouzou.ncom'},
706+
"3.3.3.3",
707+
),
708+
(
709+
"",
710+
{"forwarded": 'by=1.2.3.4;For="3.3.3.3";host=test.zouzou.ncom'},
711+
"3.3.3.3",
712+
),
713+
(
714+
"",
715+
{"forwarded": "by=1.2.3.4;FOR=3.3.3.3;host=test.zouzou.ncom"},
716+
"3.3.3.3",
717+
),
718+
(
719+
"",
720+
{"forwarded": "by=1.2.3.4;FOR=127.0.0.1;host=test.zouzou.ncom, For=4.5.6.7, For=5.6.7.8"},
721+
"4.5.6.7",
722+
),
723+
(
724+
"",
725+
{"x-forwarded-for": "127.0.0.1, 3.4.5.6"},
726+
"3.4.5.6",
727+
),
728+
(
729+
"",
730+
{
731+
"forwarded": "by=1.2.3.4;FOR=127.0.0.3;host=test.zouzou.ncom,"
732+
" by=1.2.3.4;FOR=127.0.0.4, by=1.2.3.4;FOR=127.0.0.5"
733+
},
734+
"127.0.0.3",
735+
),
690736
]
691737
+ ALL_TESTS,
692738
)

0 commit comments

Comments
 (0)