Change remote host finding logic

* When the remote host header values contains a comma separated list,
  only consider the first entry. Previously we considered every item in
  the list.
This commit is contained in:
Chris Meyers
2026-02-13 10:09:29 -05:00
committed by Chris Meyers
parent 994a2b3c04
commit 08f1507f70
4 changed files with 289 additions and 2 deletions

View File

@@ -55,7 +55,6 @@ from wsgiref.util import FileWrapper
from drf_spectacular.utils import extend_schema_view, extend_schema
# django-ansible-base
from ansible_base.lib.utils.requests import get_remote_hosts
from ansible_base.rbac.models import RoleEvaluation
from ansible_base.lib.utils.schema import extend_schema_if_available
@@ -98,6 +97,7 @@ from awx.main.utils import (
from awx.main.utils.encryption import encrypt_value
from awx.main.utils.filters import SmartFilter
from awx.main.utils.plugins import compute_cloud_inventory_sources
from awx.main.utils.proxy import get_first_remote_host_from_headers
from awx.main.utils.common import memoize
from awx.main.redact import UriCleaner
from awx.api.permissions import (
@@ -2877,7 +2877,8 @@ class JobTemplateCallback(GenericAPIView):
host for the current request.
"""
# Find the list of remote host names/IPs to check.
remote_hosts = set(get_remote_hosts(self.request))
# Only consider the first entry from each header (for comma-separated values like X-Forwarded-For)
remote_hosts = get_first_remote_host_from_headers(self.request, settings.REMOTE_HOST_HEADERS)
# Add the reverse lookup of IP addresses.
for rh in list(remote_hosts):
try:

View File

@@ -485,3 +485,47 @@ class TestJobTemplateCallbackProxyIntegration:
expect=400,
**headers
)
@override_settings(REMOTE_HOST_HEADERS=['HTTP_X_FROM_THE_LOAD_BALANCER', 'REMOTE_ADDR', 'REMOTE_HOST'], PROXY_IP_ALLOWED_LIST=[])
def test_only_first_entry_in_comma_separated_header_is_considered(self, job_template, admin_user, post):
"""
Test that only the first entry in a comma-separated header value is used for host matching.
This is important for X-Forwarded-For style headers where the format is "client, proxy1, proxy2".
Only the original client (first entry) should be matched against inventory hosts.
"""
# Create host that matches the SECOND entry in the comma-separated list
job_template.inventory.hosts.create(name='second-host.example.com')
headers = {
# First entry is 'first-host.example.com', second is 'second-host.example.com'
# Only the first should be considered, so this should NOT match
'HTTP_X_FROM_THE_LOAD_BALANCER': 'first-host.example.com, second-host.example.com',
'REMOTE_ADDR': 'unrelated-addr',
'REMOTE_HOST': 'unrelated-host',
}
# Should return 400 because only 'first-host.example.com' is considered,
# and that host is NOT in the inventory
r = post(
url=reverse('api:job_template_callback', kwargs={'pk': job_template.pk}), data={'host_config_key': 'abcd'}, user=admin_user, expect=400, **headers
)
assert r.data['msg'] == 'No matching host could be found!'
@override_settings(REMOTE_HOST_HEADERS=['HTTP_X_FROM_THE_LOAD_BALANCER', 'REMOTE_ADDR', 'REMOTE_HOST'], PROXY_IP_ALLOWED_LIST=[])
def test_first_entry_in_comma_separated_header_matches(self, job_template, admin_user, post):
"""
Test that the first entry in a comma-separated header value correctly matches an inventory host.
"""
# Create host that matches the FIRST entry in the comma-separated list
job_template.inventory.hosts.create(name='first-host.example.com')
headers = {
# First entry is 'first-host.example.com', second is 'second-host.example.com'
# The first entry matches the inventory host
'HTTP_X_FROM_THE_LOAD_BALANCER': 'first-host.example.com, second-host.example.com',
'REMOTE_ADDR': 'unrelated-addr',
'REMOTE_HOST': 'unrelated-host',
}
# Should return 201 because 'first-host.example.com' is the first entry and matches
post(url=reverse('api:job_template_callback', kwargs={'pk': job_template.pk}), data={'host_config_key': 'abcd'}, user=admin_user, expect=201, **headers)

View File

@@ -0,0 +1,207 @@
# Copyright (c) 2024 Ansible, Inc.
# All Rights Reserved.
from unittest import mock
from awx.main.utils.proxy import get_first_remote_host_from_headers, is_proxy_in_headers
class TestGetFirstRemoteHostFromHeaders:
"""Tests for get_first_remote_host_from_headers function."""
def _make_mock_request(self, environ):
"""Create a mock request with the given environ dict."""
request = mock.MagicMock()
request.environ = environ
return request
def test_single_value_headers(self):
"""Test extraction from headers with single values (no commas)."""
request = self._make_mock_request(
{
"REMOTE_ADDR": "192.168.1.1",
"REMOTE_HOST": "client.example.com",
}
)
headers = ["REMOTE_ADDR", "REMOTE_HOST"]
result = get_first_remote_host_from_headers(request, headers)
assert result == {"192.168.1.1", "client.example.com"}
def test_comma_separated_only_first_entry(self):
"""Test that only the first entry is extracted from comma-separated values."""
request = self._make_mock_request(
{
"HTTP_X_FORWARDED_FOR": "10.0.0.1, 192.168.1.1, 172.16.0.1",
}
)
headers = ["HTTP_X_FORWARDED_FOR"]
result = get_first_remote_host_from_headers(request, headers)
# Only the first IP should be included
assert result == {"10.0.0.1"}
# Subsequent IPs should NOT be included
assert "192.168.1.1" not in result
assert "172.16.0.1" not in result
def test_comma_separated_with_whitespace(self):
"""Test that whitespace is properly stripped from first entry."""
request = self._make_mock_request(
{
"HTTP_X_FORWARDED_FOR": " 10.0.0.1 , 192.168.1.1",
}
)
headers = ["HTTP_X_FORWARDED_FOR"]
result = get_first_remote_host_from_headers(request, headers)
assert result == {"10.0.0.1"}
def test_multiple_headers_with_comma_separated(self):
"""Test multiple headers where some have comma-separated values."""
request = self._make_mock_request(
{
"HTTP_X_FORWARDED_FOR": "client.example.com, proxy1.example.com, proxy2.example.com",
"REMOTE_ADDR": "172.16.0.1",
"REMOTE_HOST": "proxy2.example.com",
}
)
headers = ["HTTP_X_FORWARDED_FOR", "REMOTE_ADDR", "REMOTE_HOST"]
result = get_first_remote_host_from_headers(request, headers)
# Should have first entry from X-Forwarded-For plus the single values from other headers
assert result == {"client.example.com", "172.16.0.1", "proxy2.example.com"}
# Should NOT have subsequent entries from X-Forwarded-For
assert "proxy1.example.com" not in result
def test_empty_header_value(self):
"""Test handling of empty header values."""
request = self._make_mock_request(
{
"HTTP_X_FORWARDED_FOR": "",
"REMOTE_ADDR": "192.168.1.1",
}
)
headers = ["HTTP_X_FORWARDED_FOR", "REMOTE_ADDR"]
result = get_first_remote_host_from_headers(request, headers)
assert result == {"192.168.1.1"}
def test_missing_header(self):
"""Test handling of headers that don't exist in environ."""
request = self._make_mock_request(
{
"REMOTE_ADDR": "192.168.1.1",
}
)
headers = ["HTTP_X_FORWARDED_FOR", "REMOTE_ADDR", "REMOTE_HOST"]
result = get_first_remote_host_from_headers(request, headers)
assert result == {"192.168.1.1"}
def test_empty_headers_list(self):
"""Test with no headers specified."""
request = self._make_mock_request(
{
"REMOTE_ADDR": "192.168.1.1",
}
)
headers = []
result = get_first_remote_host_from_headers(request, headers)
assert result == set()
def test_whitespace_only_first_entry(self):
"""Test handling when first entry is whitespace only."""
request = self._make_mock_request(
{
"HTTP_X_FORWARDED_FOR": " , 192.168.1.1",
}
)
headers = ["HTTP_X_FORWARDED_FOR"]
result = get_first_remote_host_from_headers(request, headers)
# Empty/whitespace first entry should be skipped
assert result == set()
def test_single_entry_with_trailing_comma(self):
"""Test single entry that happens to have a trailing comma."""
request = self._make_mock_request(
{
"HTTP_X_FORWARDED_FOR": "10.0.0.1,",
}
)
headers = ["HTTP_X_FORWARDED_FOR"]
result = get_first_remote_host_from_headers(request, headers)
assert result == {"10.0.0.1"}
class TestIsProxyInHeaders:
"""Tests for is_proxy_in_headers function."""
def _make_mock_request(self, environ):
"""Create a mock request with the given environ dict."""
request = mock.MagicMock()
request.environ = environ
return request
def test_proxy_found_in_single_value(self):
"""Test proxy detection in single-value header."""
request = self._make_mock_request(
{
"REMOTE_ADDR": "192.168.1.1",
}
)
result = is_proxy_in_headers(request, ["192.168.1.1"], ["REMOTE_ADDR"])
assert result is True
def test_proxy_found_in_comma_separated(self):
"""Test proxy detection in comma-separated header value."""
request = self._make_mock_request(
{
"HTTP_X_FORWARDED_FOR": "10.0.0.1, 192.168.1.1, 172.16.0.1",
}
)
result = is_proxy_in_headers(request, ["192.168.1.1"], ["HTTP_X_FORWARDED_FOR"])
assert result is True
def test_proxy_not_found(self):
"""Test when proxy is not in any header."""
request = self._make_mock_request(
{
"REMOTE_ADDR": "10.0.0.1",
}
)
result = is_proxy_in_headers(request, ["192.168.1.1"], ["REMOTE_ADDR"])
assert result is False
def test_multiple_proxies_one_match(self):
"""Test with multiple allowed proxies, one matches."""
request = self._make_mock_request(
{
"REMOTE_HOST": "proxy.example.com",
}
)
result = is_proxy_in_headers(
request,
["proxy1.example.com", "proxy.example.com", "proxy2.example.com"],
["REMOTE_HOST"],
)
assert result is True

View File

@@ -45,3 +45,38 @@ def delete_headers_starting_with_http(request: Request, headers: list[str]):
for header in headers:
if header.startswith('HTTP_'):
request.environ.pop(header, None)
def get_first_remote_host_from_headers(request: Request, headers: list[str]) -> set[str]:
"""
Extract remote host addresses from headers, considering only the first entry
in comma-separated values.
For headers like X-Forwarded-For that may contain multiple IPs (e.g., "client, proxy1, proxy2"),
only the first entry (the original client) is considered.
Example:
request.environ = {
"HTTP_X_FORWARDED_FOR": "10.0.0.1, 192.168.1.1, 172.16.0.1",
"REMOTE_ADDR": "192.168.1.1",
"REMOTE_HOST": "proxy.example.com"
}
headers = ["HTTP_X_FORWARDED_FOR", "REMOTE_ADDR", "REMOTE_HOST"]
Returns: {"10.0.0.1", "192.168.1.1", "proxy.example.com"}
(Only the first IP "10.0.0.1" from X-Forwarded-For, not the full chain)
request: The DRF/Django request. request.environ dict will be used for extracting hosts
headers: A list of header keys to check for remote host values
"""
remote_hosts = set()
for header in headers:
header_value = request.environ.get(header, '')
if header_value:
# Only take the first entry if comma-separated
first_value = header_value.split(',')[0].strip()
if first_value:
remote_hosts.add(first_value)
return remote_hosts