From fb82d56307977b90d86eac6ccb2889ca147031db Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 3 Mar 2019 20:26:25 +0100 Subject: [PATCH] create suspicious request detector and policy, add request to policy engine --- passbook/audit/models.py | 6 +-- passbook/core/auth/view.py | 2 +- passbook/core/models.py | 2 +- passbook/core/policies.py | 29 ++++++++++- passbook/core/settings.py | 1 + passbook/core/signals.py | 2 +- .../templatetags/passbook_user_settings.py | 2 +- passbook/password_expiry_policy/models.py | 2 +- passbook/saml_idp/views.py | 2 +- passbook/suspicious_policy/__init__.py | 2 + passbook/suspicious_policy/admin.py | 5 ++ passbook/suspicious_policy/apps.py | 15 ++++++ passbook/suspicious_policy/forms.py | 18 +++++++ .../migrations/0001_initial.py | 49 ++++++++++++++++++ .../migrations/0002_auto_20190303_1820.py | 17 +++++++ .../migrations/0003_auto_20190303_1833.py | 25 +++++++++ .../suspicious_policy/migrations/__init__.py | 0 passbook/suspicious_policy/models.py | 51 +++++++++++++++++++ .../requirements.txt | 0 passbook/suspicious_policy/signals.py | 37 ++++++++++++++ requirements.txt | 2 +- 21 files changed, 256 insertions(+), 13 deletions(-) create mode 100644 passbook/suspicious_policy/__init__.py create mode 100644 passbook/suspicious_policy/admin.py create mode 100644 passbook/suspicious_policy/apps.py create mode 100644 passbook/suspicious_policy/forms.py create mode 100644 passbook/suspicious_policy/migrations/0001_initial.py create mode 100644 passbook/suspicious_policy/migrations/0002_auto_20190303_1820.py create mode 100644 passbook/suspicious_policy/migrations/0003_auto_20190303_1833.py create mode 100644 passbook/suspicious_policy/migrations/__init__.py create mode 100644 passbook/suspicious_policy/models.py rename passbook/{audit => suspicious_policy}/requirements.txt (100%) create mode 100644 passbook/suspicious_policy/signals.py diff --git a/passbook/audit/models.py b/passbook/audit/models.py index 29338ee7a..8f700bd6a 100644 --- a/passbook/audit/models.py +++ b/passbook/audit/models.py @@ -1,17 +1,15 @@ """passbook audit models""" -from datetime import timedelta from logging import getLogger +from ipware import get_client_ip from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.contrib.postgres.fields import JSONField from django.core.exceptions import ValidationError from django.db import models -from django.utils import timezone from django.utils.translation import gettext as _ -from ipware import get_client_ip -from passbook.lib.models import CreatedUpdatedModel, UUIDModel +from passbook.lib.models import UUIDModel LOGGER = getLogger(__name__) diff --git a/passbook/core/auth/view.py b/passbook/core/auth/view.py index 672063cd1..80f7bd835 100644 --- a/passbook/core/auth/view.py +++ b/passbook/core/auth/view.py @@ -65,7 +65,7 @@ class AuthenticationView(UserPassesTestMixin, View): self.pending_factors = [] for factor in _all_factors: policy_engine = PolicyEngine(factor.policies.all()) - policy_engine.for_user(self.pending_user) + policy_engine.for_user(self.pending_user).with_request(request).build() if policy_engine.result[0]: self.pending_factors.append((factor.uuid.hex, factor.type)) # Read and instantiate factor from session diff --git a/passbook/core/models.py b/passbook/core/models.py index cdef3c754..fbfb2cc16 100644 --- a/passbook/core/models.py +++ b/passbook/core/models.py @@ -153,7 +153,7 @@ class Application(PolicyModel): def user_is_authorized(self, user: User) -> bool: """Check if user is authorized to use this application""" from passbook.core.policies import PolicyEngine - return PolicyEngine(self.policies.all()).for_user(user).result + return PolicyEngine(self.policies.all()).for_user(user).build().result def get_provider(self): """Get casted provider instance""" diff --git a/passbook/core/policies.py b/passbook/core/policies.py index 3abfb0b27..1b4f5930e 100644 --- a/passbook/core/policies.py +++ b/passbook/core/policies.py @@ -2,12 +2,22 @@ from logging import getLogger from celery import group +from django.http import HttpRequest from passbook.core.celery import CELERY_APP from passbook.core.models import Policy, User LOGGER = getLogger(__name__) + +def get_remote_ip(request: HttpRequest) -> str: + """Return the remote's IP""" + if not request: + return '0.0.0.0' # nosec + if request.META.get('HTTP_X_FORWARDED_FOR'): + return request.META.get('HTTP_X_FORWARDED_FOR') + return request.META.get('REMOTE_ADDR') + @CELERY_APP.task() def _policy_engine_task(user_pk, policy_pk, **kwargs): """Task wrapper to run policy checking""" @@ -33,18 +43,33 @@ class PolicyEngine: policies = None _group = None + _request = None + _user = None def __init__(self, policies): self.policies = policies + self._request = None + self._user = None def for_user(self, user): """Check policies for user""" + self._user = user + return self + + def with_request(self, request): + """Set request""" + self._request = request + return self + + def build(self): + """Build task group""" signatures = [] kwargs = { - '__password__': getattr(user, '__password__', None) + '__password__': getattr(self._user, '__password__', None), + 'remote_ip': get_remote_ip(self._request) } for policy in self.policies: - signatures.append(_policy_engine_task.s(user.pk, policy.pk.hex, **kwargs)) + signatures.append(_policy_engine_task.s(self._user.pk, policy.pk.hex, **kwargs)) self._group = group(signatures)() return self diff --git a/passbook/core/settings.py b/passbook/core/settings.py index 9f42352d6..bb26ded45 100644 --- a/passbook/core/settings.py +++ b/passbook/core/settings.py @@ -76,6 +76,7 @@ INSTALLED_APPS = [ 'passbook.hibp_policy.apps.PassbookHIBPConfig', 'passbook.pretend.apps.PassbookPretendConfig', 'passbook.password_expiry_policy.apps.PassbookPasswordExpiryPolicyConfig', + 'passbook.suspicious_policy.apps.PassbookSuspiciousPolicyConfig', ] # Message Tag fix for bootstrap CSS Classes diff --git a/passbook/core/signals.py b/passbook/core/signals.py index 7a35f1418..5ca9b243b 100644 --- a/passbook/core/signals.py +++ b/passbook/core/signals.py @@ -20,7 +20,7 @@ def password_policy_checker(sender, password, **kwargs): _all_factors = PasswordFactor.objects.filter(enabled=True).order_by('order') for factor in _all_factors: policy_engine = PolicyEngine(factor.password_policies.all().select_subclasses()) - policy_engine.for_user(sender) + policy_engine.for_user(sender).build() passing, messages = policy_engine.result if not passing: raise PasswordPolicyInvalid(*messages) diff --git a/passbook/core/templatetags/passbook_user_settings.py b/passbook/core/templatetags/passbook_user_settings.py index 137cd000f..f6e11df6c 100644 --- a/passbook/core/templatetags/passbook_user_settings.py +++ b/passbook/core/templatetags/passbook_user_settings.py @@ -16,7 +16,7 @@ def user_factors(context): for factor in _all_factors: _link = factor.has_user_settings() policy_engine = PolicyEngine(factor.policies.all()) - policy_engine.for_user(user) + policy_engine.for_user(user).with_request(context.get('request')).build() if policy_engine.result[0] and _link: matching_factors.append(_link) return matching_factors diff --git a/passbook/password_expiry_policy/models.py b/passbook/password_expiry_policy/models.py index 7ac5ac292..7c5d935e8 100644 --- a/passbook/password_expiry_policy/models.py +++ b/passbook/password_expiry_policy/models.py @@ -24,7 +24,7 @@ class PasswordExpiryPolicy(Policy): """If password change date is more than x days in the past, call set_unusable_password and show a notice""" actual_days = (now() - user.password_change_date).days - days_since_expiry = now() - (user.password_change_date + timedelta(days=self.days)).days + days_since_expiry = (now() - (user.password_change_date + timedelta(days=self.days))).days if actual_days >= self.days: if not self.deny_only: user.set_unusable_password() diff --git a/passbook/saml_idp/views.py b/passbook/saml_idp/views.py index 35ceb704a..b7ba95119 100644 --- a/passbook/saml_idp/views.py +++ b/passbook/saml_idp/views.py @@ -104,7 +104,7 @@ class LoginProcessView(ProviderMixin, LoginRequiredMixin, View): def _has_access(self): """Check if user has access to application""" policy_engine = PolicyEngine(self.provider.application.policies.all()) - policy_engine.for_user(self.request.user) + policy_engine.for_user(self.request.user).with_request(self.request).build() return policy_engine.result def get(self, request, application): diff --git a/passbook/suspicious_policy/__init__.py b/passbook/suspicious_policy/__init__.py new file mode 100644 index 000000000..649094f26 --- /dev/null +++ b/passbook/suspicious_policy/__init__.py @@ -0,0 +1,2 @@ +"""passbook suspicious_policy""" +__version__ = '0.1.1-beta' diff --git a/passbook/suspicious_policy/admin.py b/passbook/suspicious_policy/admin.py new file mode 100644 index 000000000..983385432 --- /dev/null +++ b/passbook/suspicious_policy/admin.py @@ -0,0 +1,5 @@ +"""Passbook suspicious_policy Admin""" + +from passbook.lib.admin import admin_autoregister + +admin_autoregister('passbook_suspicious_policy') diff --git a/passbook/suspicious_policy/apps.py b/passbook/suspicious_policy/apps.py new file mode 100644 index 000000000..c0fb6f6bf --- /dev/null +++ b/passbook/suspicious_policy/apps.py @@ -0,0 +1,15 @@ +"""Passbook suspicious_policy app config""" +from importlib import import_module + +from django.apps import AppConfig + + +class PassbookSuspiciousPolicyConfig(AppConfig): + """Passbook suspicious_policy app config""" + + name = 'passbook.suspicious_policy' + label = 'passbook_suspicious_policy' + verbose_name = 'passbook Suspicious Request Detector' + + def ready(self): + import_module('passbook.suspicious_policy.signals') diff --git a/passbook/suspicious_policy/forms.py b/passbook/suspicious_policy/forms.py new file mode 100644 index 000000000..45758aafc --- /dev/null +++ b/passbook/suspicious_policy/forms.py @@ -0,0 +1,18 @@ +"""passbook suspicious request forms""" +from django import forms + +from passbook.core.forms.policies import GENERAL_FIELDS +from passbook.suspicious_policy.models import SuspiciousRequestPolicy + + +class SuspiciousRequestPolicyForm(forms.ModelForm): + """Form to edit SuspiciousRequestPolicy""" + + class Meta: + + model = SuspiciousRequestPolicy + fields = GENERAL_FIELDS + ['check_ip', 'check_username', 'threshold'] + widgets = { + 'name': forms.TextInput(), + 'value': forms.TextInput(), + } diff --git a/passbook/suspicious_policy/migrations/0001_initial.py b/passbook/suspicious_policy/migrations/0001_initial.py new file mode 100644 index 000000000..f48ae499c --- /dev/null +++ b/passbook/suspicious_policy/migrations/0001_initial.py @@ -0,0 +1,49 @@ +# Generated by Django 2.1.7 on 2019-03-03 18:17 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('passbook_core', '0016_auto_20190227_1355'), + ] + + operations = [ + migrations.CreateModel( + name='IPScore', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('ip', models.GenericIPAddressField()), + ('score', models.IntegerField(default=0)), + ('updated', models.DateTimeField(auto_now=True)), + ], + ), + migrations.CreateModel( + name='SuspiciousRequestPolicy', + fields=[ + ('policy_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='passbook_core.Policy')), + ('check_ip', models.BooleanField(default=True)), + ('check_username', models.BooleanField(default=True)), + ('threshold', models.IntegerField(default=-5)), + ], + options={ + 'abstract': False, + }, + bases=('passbook_core.policy',), + ), + migrations.CreateModel( + name='UserScore', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('score', models.IntegerField(default=0)), + ('updated', models.DateTimeField(auto_now=True)), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + ] diff --git a/passbook/suspicious_policy/migrations/0002_auto_20190303_1820.py b/passbook/suspicious_policy/migrations/0002_auto_20190303_1820.py new file mode 100644 index 000000000..7db3e4d1c --- /dev/null +++ b/passbook/suspicious_policy/migrations/0002_auto_20190303_1820.py @@ -0,0 +1,17 @@ +# Generated by Django 2.1.7 on 2019-03-03 18:20 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('passbook_suspicious_policy', '0001_initial'), + ] + + operations = [ + migrations.AlterModelOptions( + name='suspiciousrequestpolicy', + options={'verbose_name': 'Suspicious Request Policy', 'verbose_name_plural': 'Suspicious Request Policies'}, + ), + ] diff --git a/passbook/suspicious_policy/migrations/0003_auto_20190303_1833.py b/passbook/suspicious_policy/migrations/0003_auto_20190303_1833.py new file mode 100644 index 000000000..23d2fac40 --- /dev/null +++ b/passbook/suspicious_policy/migrations/0003_auto_20190303_1833.py @@ -0,0 +1,25 @@ +# Generated by Django 2.1.7 on 2019-03-03 18:33 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('passbook_suspicious_policy', '0002_auto_20190303_1820'), + ] + + operations = [ + migrations.AlterField( + model_name='ipscore', + name='ip', + field=models.GenericIPAddressField(unique=True), + ), + migrations.AlterField( + model_name='userscore', + name='user', + field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/passbook/suspicious_policy/migrations/__init__.py b/passbook/suspicious_policy/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/passbook/suspicious_policy/models.py b/passbook/suspicious_policy/models.py new file mode 100644 index 000000000..4429d20cf --- /dev/null +++ b/passbook/suspicious_policy/models.py @@ -0,0 +1,51 @@ +"""passbook suspicious request policy""" +from django.db import models +from django.utils.translation import gettext as _ + +from passbook.core.models import Policy, User + + +class SuspiciousRequestPolicy(Policy): + """Return true if request IP/target username's score is below a certain threshold""" + + check_ip = models.BooleanField(default=True) + check_username = models.BooleanField(default=True) + threshold = models.IntegerField(default=-5) + + form = 'passbook.suspicious_policy.forms.SuspiciousRequestPolicyForm' + + def passes(self, user: User): + remote_ip = user.remote_ip + passing = True + if self.check_ip: + ip_scores = IPScore.objects.filter(ip=remote_ip, score__lte=self.threshold) + passing = passing and ip_scores.exists() + if self.check_username: + user_scores = UserScore.objects.filter(user=user, score__lte=self.threshold) + passing = passing and user_scores.exists() + return passing + + class Meta: + + verbose_name = _('Suspicious Request Policy') + verbose_name_plural = _('Suspicious Request Policies') + +class IPScore(models.Model): + """Store score coming from the same IP""" + + ip = models.GenericIPAddressField(unique=True) + score = models.IntegerField(default=0) + updated = models.DateTimeField(auto_now=True) + + def __str__(self): + return "IPScore for %s @ %d" % (self.ip, self.score) + +class UserScore(models.Model): + """Store score attempting to log in as the same username""" + + user = models.OneToOneField(User, on_delete=models.CASCADE) + score = models.IntegerField(default=0) + updated = models.DateTimeField(auto_now=True) + + def __str__(self): + return "UserScore for %s @ %d" % (self.user, self.score) diff --git a/passbook/audit/requirements.txt b/passbook/suspicious_policy/requirements.txt similarity index 100% rename from passbook/audit/requirements.txt rename to passbook/suspicious_policy/requirements.txt diff --git a/passbook/suspicious_policy/signals.py b/passbook/suspicious_policy/signals.py new file mode 100644 index 000000000..382e2c595 --- /dev/null +++ b/passbook/suspicious_policy/signals.py @@ -0,0 +1,37 @@ +"""passbook suspicious request signals""" +from logging import getLogger + +from django.contrib.auth.signals import user_logged_in, user_login_failed +from django.dispatch import receiver +from ipware import get_client_ip + +from passbook.core.models import User +from passbook.suspicious_policy.models import IPScore, UserScore + +LOGGER = getLogger(__name__) + + +def update_score(request, username, amount): + """Update score for IP and User""" + remote_ip = get_client_ip(request) + ip_score, _ = IPScore.objects.update_or_create(ip=remote_ip) + ip_score.score += amount + ip_score.save() + LOGGER.debug("Added %s to score of IP %s", amount, remote_ip) + user = User.objects.filter(username=username) + if not user.exists(): + return + user_score, _ = UserScore.objects.update_or_create(user=user.first()) + user_score.score += amount + user_score.save() + LOGGER.debug("Added %s to score of User %s", amount, username) + +@receiver(user_login_failed) +def handle_failed_login(sender, request, credentials, **kwargs): + """Lower Score for failed loging attempts""" + update_score(request, credentials.get('username'), -1) + +@receiver(user_logged_in) +def handle_successful_login(sender, request, user, **kwargs): + """Raise score for successful attempts""" + update_score(request, user.username, 1) diff --git a/requirements.txt b/requirements.txt index 65aceaac4..f9290c23d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ -r passbook/saml_idp/requirements.txt -r passbook/otp/requirements.txt -r passbook/oauth_provider/requirements.txt --r passbook/audit/requirements.txt +-r passbook/suspicious_policy/requirements.txt -r passbook/captcha_factor/requirements.txt -r passbook/admin/requirements.txt -r passbook/api/requirements.txt