added fixes for issue #32, hopefully this closes the security hole that @mmetince found

This commit is contained in:
Ken Cochrane 2015-02-24 18:16:08 -05:00
parent f95874d22d
commit 9c50d8e833
5 changed files with 46 additions and 85 deletions

View file

@ -19,8 +19,16 @@ Sites using Defender:
=====================
- https://hub.docker.com
0.1 Features
============
Versions
========
- 0.2 - security fix for XFF headers
- 0.1.1 - setup.py fix
- 0.1 - initial release
Features
========
- Log all login attempts to the database
- support for reverse proxies with different headers for IP addresses
@ -255,9 +263,8 @@ These should be defined in your ``settings.py`` file.
* ``DEFENDER_LOGIN_FAILURE_LIMIT``: Int: The number of login attempts allowed before a
record is created for the failed logins. [Default: ``3``]
* ``DEFENDER_USE_USER_AGENT``: Boolean: If ``True``, lock out / log based on an IP address
AND a user agent. This means requests from different user agents but from
the same IP are treated differently. [Default: ``False``]
* ``DEFENDER_BEHIND_REVERSE_PROXY``: Boolean: Is defender behind a reverse proxy?
[Default: False]
* ``DEFENDER_COOLOFF_TIME``: Int: If set, defines a period of inactivity after which
old failed login attempts will be forgotten. An integer, will be interpreted as a
number of seconds. If ``0``, the locks will not expire. [Default: ``300``]

View file

@ -15,8 +15,6 @@ MOCK_REDIS = get_setting('DEFENDER_MOCK_REDIS', False)
# see if the user has overridden the failure limit
FAILURE_LIMIT = get_setting('DEFENDER_LOGIN_FAILURE_LIMIT', 3)
USE_USER_AGENT = get_setting('DEFENDER_USE_USER_AGENT', False)
# use a specific username field to retrieve from login POST data
USERNAME_FORM_FIELD = get_setting('DEFENDER_USERNAME_FORM_FIELD', 'username')

View file

@ -140,7 +140,8 @@ class AccessAttemptTest(DefenderTestCase):
def test_valid_login(self):
""" Tests a valid login for a real username
"""
response = self._login(username=VALID_USERNAME, password=VALID_PASSWORD)
response = self._login(username=VALID_USERNAME,
password=VALID_PASSWORD)
self.assertNotContains(response, LOGIN_FORM_KEY, status_code=302)
def test_reset_after_valid_login(self):
@ -206,6 +207,7 @@ class AccessAttemptTest(DefenderTestCase):
self.assertNotContains(response, LOGIN_FORM_KEY, status_code=302)
@patch('defender.config.BEHIND_REVERSE_PROXY', True)
@patch('defender.config.REVERSE_PROXY_HEADER', 'HTTP_X_FORWARDED_FOR')
def test_get_ip_reverse_proxy(self):
""" Tests if can handle a long user agent
"""
@ -344,8 +346,7 @@ class AccessAttemptTest(DefenderTestCase):
self.assertIsNotNone(str(AccessAttempt.objects.all()[0]))
def test_is_valid_ip(self):
""" Test the is_valid_ip() method
"""
""" Test the is_valid_ip() method """
self.assertEquals(utils.is_valid_ip('192.168.0.1'), True)
self.assertEquals(utils.is_valid_ip('130.80.100.24'), True)
self.assertEquals(utils.is_valid_ip('8.8.8.8'), True)
@ -353,6 +354,8 @@ class AccessAttemptTest(DefenderTestCase):
self.assertEquals(utils.is_valid_ip('fish'), False)
self.assertEquals(utils.is_valid_ip(None), False)
self.assertEquals(utils.is_valid_ip(''), False)
self.assertEquals(utils.is_valid_ip('0x41.0x41.0x41.0x41'), False)
self.assertEquals(utils.is_valid_ip('192.168.100.34.y'), False)
def test_parse_redis_url(self):
""" test the parse_redis_url method """
@ -407,52 +410,28 @@ class AccessAttemptTest(DefenderTestCase):
def test_get_ip_address_from_request(self):
req = HttpRequest()
req.META['HTTP_X_FORWARDED_FOR'] = '1.2.3.4'
req.META['REMOTE_ADDR'] = '1.2.3.4'
ip = utils.get_ip_address_from_request(req)
self.assertEqual(ip, '1.2.3.4')
req = HttpRequest()
req.META['HTTP_X_FORWARDED_FOR'] = ','.join(
['192.168.100.23', '1.2.3.4']
)
req.META['REMOTE_ADDR'] = '1.2.3.4 '
ip = utils.get_ip_address_from_request(req)
self.assertEqual(ip, '1.2.3.4')
req = HttpRequest()
req.META['HTTP_X_FORWARDED_FOR'] = '192.168.100.34'
req.META['REMOTE_ADDR'] = '192.168.100.34.y'
ip = utils.get_ip_address_from_request(req)
self.assertEqual(ip, '127.0.0.1')
req = HttpRequest()
req.META['HTTP_X_FORWARDED_FOR'] = '127.0.0.1'
req.META['HTTP_X_REAL_IP'] = '1.2.3.4'
req.META['REMOTE_ADDR'] = 'cat'
ip = utils.get_ip_address_from_request(req)
self.assertEqual(ip, '1.2.3.4')
self.assertEqual(ip, '127.0.0.1')
req = HttpRequest()
req.META['HTTP_X_FORWARDED_FOR'] = '1.2.3.4'
req.META['HTTP_X_REAL_IP'] = '5.6.7.8'
ip = utils.get_ip_address_from_request(req)
self.assertEqual(ip, '1.2.3.4')
req = HttpRequest()
req.META['HTTP_X_REAL_IP'] = '5.6.7.8'
ip = utils.get_ip_address_from_request(req)
self.assertEqual(ip, '5.6.7.8')
req = HttpRequest()
req.META['REMOTE_ADDR'] = '1.2.3.4'
ip = utils.get_ip_address_from_request(req)
self.assertEqual(ip, '1.2.3.4')
req = HttpRequest()
req.META['HTTP_X_FORWARDED_FOR'] = ','.join(
['127.0.0.1', '192.168.132.98']
)
req.META['HTTP_X_REAL_IP'] = '10.0.0.34'
req.META['REMOTE_ADDR'] = '1.2.3.4'
ip = utils.get_ip_address_from_request(req)
self.assertEqual(ip, '1.2.3.4')
self.assertEqual(ip, '127.0.0.1')
@patch('defender.config.BEHIND_REVERSE_PROXY', True)
@patch('defender.config.REVERSE_PROXY_HEADER', 'HTTP_X_PROXIED')
@ -469,6 +448,8 @@ class AccessAttemptTest(DefenderTestCase):
req.META['REMOTE_ADDR'] = '1.2.3.4'
self.assertEqual(utils.get_ip(req), '1.2.3.4')
@patch('defender.config.BEHIND_REVERSE_PROXY', True)
@patch('defender.config.REVERSE_PROXY_HEADER', 'HTTP_X_REAL_IP')
def test_get_user_attempts(self):
ip_attempts = random.randint(3, 12)
username_attempts = random.randint(3, 12)

View file

@ -1,5 +1,6 @@
import logging
import socket
import re
from django.http import HttpResponse
from django.http import HttpResponseRedirect
@ -17,63 +18,37 @@ log = logging.getLogger(__name__)
def is_valid_ip(ip_address):
""" Check Validity of an IP address """
valid = True
try:
socket.inet_aton(ip_address.strip())
except (socket.error, AttributeError):
valid = False
return valid
if not ip_address:
return False
ip_address = ip_address.strip()
ipv4_re = r'^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$'
ipv6_re = r'\[[0-9a-f:\.]+\]'
ipv4_regex = re.compile(ipv4_re)
ipv6_regex = re.compile(ipv6_re)
if ipv4_regex.match(ip_address) or ipv6_regex.match(ip_address):
return True
return False
def get_ip_address_from_request(request):
""" Makes the best attempt to get the client's real IP or return
the loopback """
PRIVATE_IPS_PREFIX = ('10.', '172.', '192.', '127.')
ip_address = ''
x_forwarded_for = request.META.get('HTTP_X_FORWARDED_FOR', '')
if x_forwarded_for and ',' not in x_forwarded_for:
if not x_forwarded_for.startswith(PRIVATE_IPS_PREFIX) and is_valid_ip(
x_forwarded_for):
ip_address = x_forwarded_for.strip()
else:
ips = [ip.strip() for ip in x_forwarded_for.split(',')]
for ip in ips:
if ip.startswith(PRIVATE_IPS_PREFIX):
continue
elif not is_valid_ip(ip):
continue
else:
ip_address = ip
break
if not ip_address:
x_real_ip = request.META.get('HTTP_X_REAL_IP', '')
if x_real_ip:
if not x_real_ip.startswith(PRIVATE_IPS_PREFIX) and is_valid_ip(
x_real_ip):
ip_address = x_real_ip.strip()
if not ip_address:
remote_addr = request.META.get('REMOTE_ADDR', '')
if remote_addr:
if not remote_addr.startswith(PRIVATE_IPS_PREFIX) and is_valid_ip(
remote_addr):
ip_address = remote_addr.strip()
if remote_addr.startswith(PRIVATE_IPS_PREFIX) and is_valid_ip(
remote_addr):
ip_address = remote_addr.strip()
if not ip_address:
ip_address = '127.0.0.1'
return ip_address
remote_addr = request.META.get('REMOTE_ADDR', '')
if remote_addr and is_valid_ip(remote_addr):
return remote_addr.strip()
return '127.0.0.1'
def get_ip(request):
""" get the ip address from the request """
if not config.BEHIND_REVERSE_PROXY:
ip = get_ip_address_from_request(request)
else:
if config.BEHIND_REVERSE_PROXY:
ip = request.META.get(config.REVERSE_PROXY_HEADER, '')
ip = ip.split(",", 1)[0].strip()
if ip == '':
ip = request.META.get('REMOTE_ADDR', '')
else:
ip = get_ip_address_from_request(request)
return ip

View file

@ -6,7 +6,7 @@ except ImportError:
from distutils.core import setup
version = '0.1.1'
version = '0.2'
setup(name='django-defender',
version=version,