net: strip the ephemeral source port from the forwarded client IP
IIS ARR sets X-Forwarded-For to clientip:port, and the port changes every connection. Left in, the audit log showed IP:PORT, the dashboard IP fallback never matched a stored (portless) DashboardDefault.ipaddress, and login rate limiting keyed per-connection instead of per-host. Add an IPv6-safe clientip.client_ip / strip_port helper and use it in the audit log, the dashboard resolver, and the login rate-limit key.
This commit is contained in:
@@ -26,11 +26,10 @@ LOCKOUT_MINUTES = 15
|
||||
|
||||
|
||||
def _login_ip():
|
||||
"""Caller IP for rate limiting, honoring the first X-Forwarded-For hop."""
|
||||
forwarded = request.headers.get('X-Forwarded-For')
|
||||
if forwarded:
|
||||
return forwarded.split(',')[0].strip()
|
||||
return request.remote_addr or 'unknown'
|
||||
"""Caller IP for rate limiting, port stripped so the key is per-host, not
|
||||
per-connection (ARR forwards clientip:port with an ephemeral port)."""
|
||||
from shopdb.utils.clientip import client_ip
|
||||
return client_ip(request) or 'unknown'
|
||||
|
||||
|
||||
def _login_ratelimited():
|
||||
|
||||
@@ -17,11 +17,10 @@ dashboarddefaults_bp = Blueprint('dashboarddefaults', __name__)
|
||||
|
||||
|
||||
def _request_ip():
|
||||
"""Caller IP, honoring a single proxy hop via X-Forwarded-For."""
|
||||
forwarded = request.headers.get('X-Forwarded-For')
|
||||
if forwarded:
|
||||
return forwarded.split(',')[0].strip()
|
||||
return request.remote_addr
|
||||
"""Caller IP for the IP fallback, port stripped so it matches a stored
|
||||
(portless) DashboardDefault.ipaddress. ARR forwards clientip:port."""
|
||||
from shopdb.utils.clientip import client_ip
|
||||
return client_ip(request)
|
||||
|
||||
|
||||
def _serialize(default):
|
||||
|
||||
@@ -117,10 +117,9 @@ class AuditLog(db.Model):
|
||||
useragent = None
|
||||
|
||||
if request_obj:
|
||||
# Handle proxy forwarding
|
||||
ipaddress = request_obj.headers.get('X-Forwarded-For', request_obj.remote_addr)
|
||||
if ipaddress and ',' in ipaddress:
|
||||
ipaddress = ipaddress.split(',')[0].strip()
|
||||
# First-hop client IP, port stripped (ARR forwards clientip:port).
|
||||
from shopdb.utils.clientip import client_ip
|
||||
ipaddress = client_ip(request_obj)
|
||||
useragent = request_obj.headers.get('User-Agent', '')[:255]
|
||||
|
||||
entry = cls(
|
||||
|
||||
39
shopdb/utils/clientip.py
Normal file
39
shopdb/utils/clientip.py
Normal file
@@ -0,0 +1,39 @@
|
||||
"""Client IP extraction behind a reverse proxy.
|
||||
|
||||
Prod runs behind IIS ARR, which sets X-Forwarded-For to "clientip:port" (and
|
||||
appends a hop per proxy). The trailing source port is ephemeral - it changes
|
||||
every connection - so it must be stripped before the value is stored (audit),
|
||||
matched (dashboard IP fallback), or used as a rate-limit key (login). Left in,
|
||||
it makes every request look like a different client.
|
||||
|
||||
Port stripping is IPv6-aware: a bracketed [2001:db8::1]:443 loses its port; a
|
||||
bare 2001:db8::1 (many colons, no brackets) is returned untouched.
|
||||
"""
|
||||
|
||||
|
||||
def strip_port(ipaddress):
|
||||
"""Return ipaddress without a trailing :port. IPv6-safe. None/empty-safe."""
|
||||
if not ipaddress:
|
||||
return ipaddress
|
||||
ipaddress = ipaddress.strip()
|
||||
if ipaddress.startswith('['):
|
||||
# [IPv6] or [IPv6]:port -> the address inside the brackets
|
||||
return ipaddress[1:].split(']', 1)[0]
|
||||
if ipaddress.count(':') == 1:
|
||||
# exactly one colon = IPv4:port (a bare IPv6 has several colons)
|
||||
return ipaddress.split(':', 1)[0]
|
||||
return ipaddress
|
||||
|
||||
|
||||
def client_ip(request_obj):
|
||||
"""First-hop client IP behind one proxy, port stripped.
|
||||
|
||||
Honors the first X-Forwarded-For hop, else remote_addr. Returns None only
|
||||
when neither is present (callers that need a non-null key coerce it).
|
||||
"""
|
||||
forwarded = request_obj.headers.get('X-Forwarded-For')
|
||||
if forwarded:
|
||||
raw = forwarded.split(',')[0].strip()
|
||||
else:
|
||||
raw = request_obj.remote_addr
|
||||
return strip_port(raw)
|
||||
45
tests/test_core/test_clientip.py
Normal file
45
tests/test_core/test_clientip.py
Normal file
@@ -0,0 +1,45 @@
|
||||
"""strip_port / client_ip normalization (ARR forwards clientip:port)."""
|
||||
|
||||
from shopdb.utils.clientip import strip_port, client_ip
|
||||
|
||||
|
||||
def test_strip_ipv4_port():
|
||||
assert strip_port('10.134.48.20:52344') == '10.134.48.20'
|
||||
|
||||
|
||||
def test_ipv4_without_port_untouched():
|
||||
assert strip_port('10.134.48.20') == '10.134.48.20'
|
||||
|
||||
|
||||
def test_bare_ipv6_untouched():
|
||||
assert strip_port('2001:db8::1') == '2001:db8::1'
|
||||
|
||||
|
||||
def test_bracketed_ipv6_with_port():
|
||||
assert strip_port('[2001:db8::1]:443') == '2001:db8::1'
|
||||
|
||||
|
||||
def test_bracketed_ipv6_without_port():
|
||||
assert strip_port('[2001:db8::1]') == '2001:db8::1'
|
||||
|
||||
|
||||
def test_none_and_empty_safe():
|
||||
assert strip_port(None) is None
|
||||
assert strip_port('') == ''
|
||||
|
||||
|
||||
class _FakeRequest:
|
||||
def __init__(self, headers=None, remote_addr=None):
|
||||
self.headers = headers or {}
|
||||
self.remote_addr = remote_addr
|
||||
|
||||
|
||||
def test_client_ip_prefers_forwarded_first_hop_stripped():
|
||||
request_obj = _FakeRequest(headers={'X-Forwarded-For': '10.0.0.5:51000, 10.0.0.1'},
|
||||
remote_addr='127.0.0.1')
|
||||
assert client_ip(request_obj) == '10.0.0.5'
|
||||
|
||||
|
||||
def test_client_ip_falls_back_to_remote_addr_stripped():
|
||||
request_obj = _FakeRequest(headers={}, remote_addr='192.168.1.9:60000')
|
||||
assert client_ip(request_obj) == '192.168.1.9'
|
||||
Reference in New Issue
Block a user