Unverified Commit 4faecd5c authored by Isis Lovecruft's avatar Isis Lovecruft
Browse files

Merge branch 'fix/24432-ignore-loopback' into develop

parents 8b4f7367 bf7ec9ee
......@@ -345,6 +345,11 @@ MOAT_HTTP_PORT = None
# the *last* entry from its X-Forwarded-For header as the client's IP.
MOAT_USE_IP_FROM_FORWARDED_HEADER = True
# If True, there is a misconfigured proxy relaying incoming messages
# to us: take the *last* entry in the X-Forwarded-For header which is
# not a loopback address (127.0.0.1/8) as the client's IP.
MOAT_SKIP_LOOPBACK_ADDRESSES = True
# How many clusters do we group IPs in when distributing bridges based on IP?
# Note that if PROXY_LIST_FILES is set (below), what we actually do here
# is use one higher than the number here, and the extra cluster is used
......
......@@ -21,6 +21,7 @@ import logging
import os
from bridgedb.parse.addr import isIPAddress
from bridgedb.parse.addr import isLoopback
#: The fully-qualified domain name for any and all web servers we run.
......@@ -54,7 +55,7 @@ def getFQDN():
"""
return SERVER_PUBLIC_FQDN
def getClientIP(request, useForwardedHeader=False):
def getClientIP(request, useForwardedHeader=False, skipLoopback=False):
"""Get the client's IP address from the ``'X-Forwarded-For:'``
header, or from the :api:`request <twisted.web.server.Request>`.
......@@ -62,6 +63,9 @@ def getClientIP(request, useForwardedHeader=False):
:param request: A ``Request`` for a :api:`twisted.web.resource.Resource`.
:param bool useForwardedHeader: If ``True``, attempt to get the client's
IP address from the ``'X-Forwarded-For:'`` header.
:param bool skipLoopback: If ``True``, and ``useForwardedHeader`` is
also ``True``, then skip any loopback addresses (127.0.0.1/8) when
parsing the X-Forwarded-For header.
:rtype: ``None`` or :any:`str`
:returns: The client's IP address, if it was obtainable.
"""
......@@ -70,10 +74,20 @@ def getClientIP(request, useForwardedHeader=False):
if useForwardedHeader:
header = request.getHeader("X-Forwarded-For")
if header:
ip = header.split(",")[-1].strip()
if not isIPAddress(ip):
logging.warn("Got weird X-Forwarded-For value %r" % header)
ip = None
index = -1
ip = header.split(",")[index].strip()
if skipLoopback:
logging.info(("Parsing X-Forwarded-For again, ignoring "
"loopback addresses..."))
while isLoopback(ip):
index -= 1
ip = header.split(",")[index].strip()
if not skipLoopback and isLoopback(ip):
logging.warn("Accepting loopback address: %s" % ip)
else:
if not isIPAddress(ip):
logging.warn("Got weird X-Forwarded-For value %r" % header)
ip = None
else:
ip = request.getClientIP()
......
......@@ -134,9 +134,17 @@ class JsonAPIResource(resource.Resource):
"""A resource which conforms to the `JSON API spec <http://jsonapi.org/>`__.
"""
def __init__(self, useForwardedHeader=True):
def __init__(self, useForwardedHeader=True, skipLoopback=False):
"""Create a JSON API resource, containing either error(s) or data.
:param bool useForwardedHeader: If ``True``, obtain the client's IP
address from the ``X-Forwarded-For`` HTTP header.
:param bool skipLoopback: Skip loopback addresses when parsing the
X-Forwarded-For header.
"""
resource.Resource.__init__(self)
self.useForwardedHeader = useForwardedHeader
self.skipLoopback = skipLoopback
def getClientIP(self, request):
"""Get the client's IP address from the ``'X-Forwarded-For:'``
......@@ -148,7 +156,7 @@ class JsonAPIResource(resource.Resource):
:rtype: ``None`` or :any:`str`
:returns: The client's IP address, if it was obtainable.
"""
return getClientIP(request, self.useForwardedHeader)
return getClientIP(request, self.useForwardedHeader, self.skipLoopback)
def formatDataForResponse(self, data, request):
"""Format a dictionary of ``data`` into JSON and add necessary response
......@@ -233,8 +241,8 @@ class CustomErrorHandlingResource(resource.Resource):
class JsonAPIDataResource(JsonAPIResource):
"""A resource which returns some JSON API data."""
def __init__(self, useForwardedHeader=True):
JsonAPIResource.__init__(self, useForwardedHeader)
def __init__(self, useForwardedHeader=True, skipLoopback=False):
JsonAPIResource.__init__(self, useForwardedHeader, skipLoopback)
def checkRequestHeaders(self, request):
"""The JSON API specification requires servers to respond with certain HTTP
......@@ -288,8 +296,8 @@ class CaptchaResource(JsonAPIDataResource):
"""A CAPTCHA."""
def __init__(self, hmacKey=None, publicKey=None, secretKey=None,
useForwardedHeader=True):
JsonAPIDataResource.__init__(self, useForwardedHeader)
useForwardedHeader=True, skipLoopback=False):
JsonAPIDataResource.__init__(self, useForwardedHeader, skipLoopback)
self.hmacKey = hmacKey
self.publicKey = publicKey
self.secretKey = secretKey
......@@ -301,7 +309,7 @@ class CaptchaFetchResource(CaptchaResource):
isLeaf = True
def __init__(self, hmacKey=None, publicKey=None, secretKey=None,
captchaDir="captchas", useForwardedHeader=True):
captchaDir="captchas", useForwardedHeader=True, skipLoopback=False):
"""DOCDOC
:param bytes hmacKey: The master HMAC key, used for validating CAPTCHA
......@@ -319,6 +327,8 @@ class CaptchaFetchResource(CaptchaResource):
:param str captchaDir: The directory where the cached CAPTCHA images
:param bool useForwardedHeader: If ``True``, obtain the client's IP
address from the ``X-Forwarded-For`` HTTP header.
:param bool skipLoopback: Skip loopback addresses when parsing the
X-Forwarded-For header.
"""
CaptchaResource.__init__(self, hmacKey, publicKey, secretKey,
useForwardedHeader)
......@@ -477,7 +487,7 @@ class CaptchaCheckResource(CaptchaResource):
def __init__(self, distributor, schedule, N=1,
hmacKey=None, publicKey=None, secretKey=None,
useForwardedHeader=True):
useForwardedHeader=True, skipLoopback=False):
"""Create a new resource for checking CAPTCHA solutions and returning
bridges to a client.
......@@ -490,6 +500,8 @@ class CaptchaCheckResource(CaptchaResource):
:param int N: The number of bridges to hand out per query.
:param bool useForwardedHeader: Whether or not we should use the the
X-Forwarded-For header instead of the source IP address.
:param bool skipLoopback: Skip loopback addresses when parsing the
X-Forwarded-For header.
"""
CaptchaResource.__init__(self, hmacKey, publicKey, secretKey,
useForwardedHeader)
......@@ -748,6 +760,7 @@ def addMoatServer(config, distributor):
MOAT_BRIDGES_PER_ANSWER
MOAT_TRANSPORT_PREFERENCE_LIST
MOAT_USE_IP_FROM_FORWARDED_HEADER
MOAT_SKIP_LOOPBACK_ADDRESSES
MOAT_ROTATION_PERIOD
MOAT_GIMP_CAPTCHA_HMAC_KEYFILE
MOAT_GIMP_CAPTCHA_RSA_KEYFILE
......@@ -760,6 +773,7 @@ def addMoatServer(config, distributor):
captcha = None
fwdHeaders = config.MOAT_USE_IP_FROM_FORWARDED_HEADER
numBridges = config.MOAT_BRIDGES_PER_ANSWER
skipLoopback = config.MOAT_SKIP_LOOPBACK_ADDRESSES
logging.info("Starting moat servers...")
......@@ -785,9 +799,11 @@ def addMoatServer(config, distributor):
meek = CustomErrorHandlingResource()
moat = CustomErrorHandlingResource()
fetch = CaptchaFetchResource(hmacKey, publicKey, secretKey,
config.GIMP_CAPTCHA_DIR, fwdHeaders)
config.GIMP_CAPTCHA_DIR,
fwdHeaders, skipLoopback)
check = CaptchaCheckResource(distributor, sched, numBridges,
hmacKey, publicKey, secretKey, fwdHeaders)
hmacKey, publicKey, secretKey,
fwdHeaders, skipLoopback)
moat.putChild("fetch", fetch)
moat.putChild("check", check)
......
......@@ -435,6 +435,28 @@ def isValidIP(ip):
return False
return True
def isLoopback(ip):
"""Determine if ``ip`` is a loopback or localhost address (127.0.0.1/8).
:type: ``str`` or ``ipaddr.IPAddress``
:param ip: An IP address.
:rtype: bool
:returns: ``True`` if the IP was a loopback or localhost address; ``False``
otherwise.
"""
try:
if isinstance(ip, basestring):
ip = ipaddr.IPAddress(ip)
if ip.is_loopback:
return True
else:
return False
except Exception as err:
logging.debug("Error attempting to parse IP address: %s", ip)
return False
def normalizeEmail(emailaddr, domainmap, domainrules, ignorePlus=True):
"""Normalise an email address according to the processing rules for its
canonical originating domain.
......
......@@ -42,6 +42,7 @@ MOAT_HTTPS_PORT = None
MOAT_HTTP_IP = None
MOAT_HTTP_PORT = None
MOAT_USE_IP_FROM_FORWARDED_HEADER = True
MOAT_SKIP_LOOPBACK_ADDRESSES = True
MOAT_N_IP_CLUSTERS = 4
MOAT_ROTATION_PERIOD = "3 hours"
MOAT_GIMP_CAPTCHA_HMAC_KEYFILE = 'moat_captcha_hmac_key'
......@@ -63,6 +64,7 @@ MOAT_HTTP_PORT = %r
MOAT_BRIDGES_PER_ANSWER = %r
MOAT_TRANSPORT_PREFERENCE_LIST = %r
MOAT_USE_IP_FROM_FORWARDED_HEADER = %r
MOAT_SKIP_LOOPBACK_ADDRESSES = %r
MOAT_N_IP_CLUSTERS = %r
MOAT_ROTATION_PERIOD = %r
MOAT_GIMP_CAPTCHA_HMAC_KEYFILE = %r
......@@ -82,6 +84,7 @@ MOAT_GIMP_CAPTCHA_RSA_KEYFILE = %r
MOAT_BRIDGES_PER_ANSWER,
MOAT_TRANSPORT_PREFERENCE_LIST,
MOAT_USE_IP_FROM_FORWARDED_HEADER,
MOAT_SKIP_LOOPBACK_ADDRESSES,
MOAT_N_IP_CLUSTERS,
MOAT_ROTATION_PERIOD,
MOAT_GIMP_CAPTCHA_HMAC_KEYFILE,
......
......@@ -91,6 +91,24 @@ class GetClientIPTests(unittest.TestCase):
clientIP = server.getClientIP(request, useForwardedHeader=True)
self.assertEqual(clientIP, None)
def test_getClientIP_XForwardedFor_skip_loopback(self):
request = self.createRequestWithIPs()
request.headers.update({'x-forwarded-for': '3.3.3.3, 127.0.0.1'})
clientIP = server.getClientIP(request, useForwardedHeader=True, skipLoopback=True)
self.assertEqual(clientIP, '3.3.3.3')
def test_getClientIP_XForwardedFor_skip_loopback_multiple(self):
request = self.createRequestWithIPs()
request.headers.update({'x-forwarded-for': '3.3.3.3, 127.0.0.6, 127.0.0.1'})
clientIP = server.getClientIP(request, useForwardedHeader=True, skipLoopback=True)
self.assertEqual(clientIP, '3.3.3.3')
def test_getClientIP_XForwardedFor_no_skip_loopback(self):
request = self.createRequestWithIPs()
request.headers.update({'x-forwarded-for': '3.3.3.3, 127.0.0.1'})
clientIP = server.getClientIP(request, useForwardedHeader=True, skipLoopback=False)
self.assertEqual(clientIP, '127.0.0.1')
def test_getClientIP_fromRequest(self):
"""getClientIP() should return the IP address from the request instance
when ``useForwardedHeader=False``.
......
......@@ -337,6 +337,7 @@ class CaptchaFetchResourceTests(unittest.TestCase):
self.root.putChild(self.pagename, self.resource)
self.make_captcha_directory()
server.setPreferredTransports(['obfs4', 'vanilla'])
def make_captcha_directory(self):
if not os.path.isdir(self.captchaDir):
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment