From 6854c85b31459bdf041e7dee255a6c01c4fe40c3 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 13 Nov 2013 17:14:27 +0100 Subject: [PATCH 1/5] Various fixes. fixes #430 --- modoboa/core/templatetags/core_tags.py | 32 +------------------ modoboa/extensions/admin/forms/account.py | 7 +++- .../general_callbacks.py | 2 +- .../postfix_relay_domains/js/relay_domains.js | 4 +++ modoboa/static/js/twocols_nav.js | 2 +- 5 files changed, 13 insertions(+), 34 deletions(-) diff --git a/modoboa/core/templatetags/core_tags.py b/modoboa/core/templatetags/core_tags.py index 7b9a08930..2ae750a6d 100644 --- a/modoboa/core/templatetags/core_tags.py +++ b/modoboa/core/templatetags/core_tags.py @@ -31,36 +31,6 @@ def core_menu(selection, user): ) -@register.simple_tag -def settings_menu(selection, user): - entries = [ - {"name": "extensions", - "class": "ajaxlink", - "url": "extensions/", - "label": _("Extensions"), - "img": ""}, - {"name": "info", - "class": "ajaxlink", - "url": "info/", - "label": _("Information")}, - {"name": "logs", - "class": "ajaxlink", - "url": "logs/&sort_order=-date_created", - "label": _("Logs")}, - {"name": "parameters", - "class": "ajaxlink", - "url": "parameters/", - "img": "", - "label": _("Parameters")}, - ] - return render_to_string('common/menu.html', { - "entries": entries, - "css": "nav nav-list", - "selection": selection, - "user": user - }) - - @register.simple_tag def extensions_menu(selection, user): menu = events.raiseQueryEvent("UserMenuDisplay", "top_menu", user) @@ -83,7 +53,7 @@ def admin_menu(selection, user): "label": _("Information")}, {"name": "logs", "class": "ajaxlink", - "url": "logs/", + "url": "logs/?sort_order=-date_created", "label": _("Logs")}, {"name": "parameters", "class": "ajaxlink", diff --git a/modoboa/extensions/admin/forms/account.py b/modoboa/extensions/admin/forms/account.py index a44ba884c..476a3b9d4 100644 --- a/modoboa/extensions/admin/forms/account.py +++ b/modoboa/extensions/admin/forms/account.py @@ -15,7 +15,12 @@ from modoboa.extensions.admin.models import ( class AccountFormGeneral(forms.ModelForm): - username = forms.CharField(label=ugettext_lazy("Username")) + username = forms.CharField( + label=ugettext_lazy("Username"), + help_text=ugettext_lazy( + "The user's name. Must be a valid e-mail address for simple users." + ) + ) role = forms.ChoiceField( label=ugettext_lazy("Role"), choices=[('', ugettext_lazy("Choose"))], diff --git a/modoboa/extensions/postfix_relay_domains/general_callbacks.py b/modoboa/extensions/postfix_relay_domains/general_callbacks.py index c51c54be4..34e0b7630 100644 --- a/modoboa/extensions/postfix_relay_domains/general_callbacks.py +++ b/modoboa/extensions/postfix_relay_domains/general_callbacks.py @@ -67,7 +67,7 @@ def rdomain_modify_link(domain): 'modoboa.extensions.postfix_relay_domains.views.edit', args=[domain.id] ), - 'modalcb': 'rdomain.domainform_cb' + 'modalcb': 'rdomain.editdomain_form_cb' } diff --git a/modoboa/extensions/postfix_relay_domains/static/postfix_relay_domains/js/relay_domains.js b/modoboa/extensions/postfix_relay_domains/static/postfix_relay_domains/js/relay_domains.js index d30e10f4b..e1ab3e40d 100644 --- a/modoboa/extensions/postfix_relay_domains/static/postfix_relay_domains/js/relay_domains.js +++ b/modoboa/extensions/postfix_relay_domains/static/postfix_relay_domains/js/relay_domains.js @@ -67,6 +67,10 @@ RelayDomains.prototype = { success_cb: $.proxy(admin.reload_listing, admin) }); }, this)); + }, + + editdomain_form_cb: function() { + this.domainform_cb(); $(document).trigger('domform_init'); }, diff --git a/modoboa/static/js/twocols_nav.js b/modoboa/static/js/twocols_nav.js index 8d98e272d..c6fddc606 100644 --- a/modoboa/static/js/twocols_nav.js +++ b/modoboa/static/js/twocols_nav.js @@ -119,7 +119,7 @@ TwocolsNav.prototype = { load_section: function(e) { var $link = get_target(e); e.preventDefault(); - this.navobj.baseurl($link.attr("href")).update(); + this.navobj.parse_string($link.attr("href"), true).update(); }, From 949f922ce5f51d2cac05dcaa96aadd758acdfc2e Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 15 Nov 2013 17:05:39 +0100 Subject: [PATCH 2/5] Adding support for SHA256-CRYPT and SHA512-CRYPT. fixes #450 fixes #493 --- modoboa/core/app_settings.py | 31 ++++-- .../0003_update_md5crypt_passwords.py | 87 +++++++++++++++ .../0004_auto__chg_field_user_password.py | 82 ++++++++++++++ modoboa/core/models.py | 103 ++++++++---------- modoboa/core/password_hashers/__init__.py | 26 +++++ modoboa/core/password_hashers/advanced.py | 63 +++++++++++ modoboa/core/password_hashers/base.py | 100 +++++++++++++++++ modoboa/core/tests.py | 12 +- modoboa/extensions/admin/tests/__init__.py | 3 +- .../admin/tests/password_schemes.py | 55 ++++++++++ modoboa/lib/__init__.py | 79 -------------- requirements.txt | 1 + tests/install_from_scratch.py | 2 +- 13 files changed, 495 insertions(+), 149 deletions(-) create mode 100644 modoboa/core/migrations/0003_update_md5crypt_passwords.py create mode 100644 modoboa/core/migrations/0004_auto__chg_field_user_password.py create mode 100644 modoboa/core/password_hashers/__init__.py create mode 100644 modoboa/core/password_hashers/advanced.py create mode 100644 modoboa/core/password_hashers/base.py create mode 100644 modoboa/extensions/admin/tests/password_schemes.py diff --git a/modoboa/core/app_settings.py b/modoboa/core/app_settings.py index edb9bfe48..684138a36 100644 --- a/modoboa/core/app_settings.py +++ b/modoboa/core/app_settings.py @@ -46,15 +46,26 @@ class GeneralParametersForm(parameters.AdminParametersForm): password_scheme = forms.ChoiceField( label=ugettext_lazy("Default password scheme"), - choices=[("crypt", "crypt"), - ("md5", "md5"), - ("md5crypt", "md5crypt"), - ("sha256", "sha256"), - ("plain", "plain")], - initial="md5crypt", + choices=[("sha512crypt", "sha512crypt"), + ("sha256crypt", "sha256crypt"), + ("md5crypt", ugettext_lazy("md5crypt (weak)")), + ("sha256", ugettext_lazy("sha256 (weak)")), + ("md5", ugettext_lazy("md5 (weak)")), + ("crypt", ugettext_lazy("crypt (weak)")), + ("plain", ugettext_lazy("plain (weak)"))], + initial="sha512crypt", help_text=ugettext_lazy("Scheme used to crypt mailbox passwords") ) + rounds_number = forms.IntegerField( + label=ugettext_lazy("Rounds"), + initial=70000, + help_text=ugettext_lazy( + "Number of rounds to use (only used by sha256crypt and " + "sha512crypt). Must be between 1000 and 999999999, inclusive." + ) + ) + secret_key = forms.CharField( label=ugettext_lazy("Secret key"), initial=random_key(), @@ -173,7 +184,7 @@ class GeneralParametersForm(parameters.AdminParametersForm): "ldap_search_filter": "ldap_auth_method=searchbind", "ldap_user_dn_template": "ldap_auth_method=directbind", "ldap_password_attribute": "authentication_type=ldap", - "ldap_is_active_directory": "authentication_type=ldap" + "ldap_is_active_directory": "authentication_type=ldap", } def __init__(self, *args, **kwargs): @@ -195,6 +206,12 @@ class GeneralParametersForm(parameters.AdminParametersForm): raise forms.ValidationError(_("Invalid syntax")) return tpl + def clean_rounds_number(self): + value = self.cleaned_data["rounds_number"] + if value < 1000 or value > 999999999: + raise forms.ValidationError(_("Invalid rounds number")) + return value + def clean(self): """Custom validation method diff --git a/modoboa/core/migrations/0003_update_md5crypt_passwords.py b/modoboa/core/migrations/0003_update_md5crypt_passwords.py new file mode 100644 index 000000000..95c9cf0c3 --- /dev/null +++ b/modoboa/core/migrations/0003_update_md5crypt_passwords.py @@ -0,0 +1,87 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import DataMigration +from django.db import models + + +class Migration(DataMigration): + + def forwards(self, orm): + "Write your forwards methods here." + for user in orm['core.User'].objects.all(): + if user.password.startswith('$1$'): + user.password = '{MD5-CRYPT}%s' % user.password + user.save() + + def backwards(self, orm): + "Write your backwards methods here." + for user in orm['core.User'].objects.all(): + if user.password.startswith('{MD5-CRYPT}'): + user.password = user.password.replace('{MD5-CRYPT}', '') + user.save() + + models = { + u'auth.group': { + 'Meta': {'object_name': 'Group'}, + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': u"orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + u'auth.permission': { + 'Meta': {'ordering': "(u'content_type__app_label', u'content_type__model', u'codename')", 'unique_together': "((u'content_type', u'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['contenttypes.ContentType']"}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + u'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + u'core.extension': { + 'Meta': {'object_name': 'Extension'}, + 'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '150'}) + }, + u'core.log': { + 'Meta': {'object_name': 'Log'}, + 'date_created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'level': ('django.db.models.fields.CharField', [], {'max_length': '15'}), + 'logger': ('django.db.models.fields.CharField', [], {'max_length': '30'}), + 'message': ('django.db.models.fields.CharField', [], {'max_length': '255'}) + }, + u'core.objectaccess': { + 'Meta': {'unique_together': "(('user', 'content_type', 'object_id'),)", 'object_name': 'ObjectAccess'}, + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['contenttypes.ContentType']"}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_owner': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'object_id': ('django.db.models.fields.PositiveIntegerField', [], {}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['core.User']"}) + }, + u'core.user': { + 'Meta': {'ordering': "['username']", 'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '254', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': u"orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_local': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': u"orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '254'}) + } + } + + complete_apps = ['core'] + symmetrical = True diff --git a/modoboa/core/migrations/0004_auto__chg_field_user_password.py b/modoboa/core/migrations/0004_auto__chg_field_user_password.py new file mode 100644 index 000000000..6695deeb3 --- /dev/null +++ b/modoboa/core/migrations/0004_auto__chg_field_user_password.py @@ -0,0 +1,82 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + + # Changing field 'User.password' + db.alter_column(u'core_user', 'password', self.gf('django.db.models.fields.CharField')(max_length=256)) + + def backwards(self, orm): + + # Changing field 'User.password' + db.alter_column(u'core_user', 'password', self.gf('django.db.models.fields.CharField')(max_length=128)) + + models = { + u'auth.group': { + 'Meta': {'object_name': 'Group'}, + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': u"orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + u'auth.permission': { + 'Meta': {'ordering': "(u'content_type__app_label', u'content_type__model', u'codename')", 'unique_together': "((u'content_type', u'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['contenttypes.ContentType']"}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + u'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + u'core.extension': { + 'Meta': {'object_name': 'Extension'}, + 'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '150'}) + }, + u'core.log': { + 'Meta': {'object_name': 'Log'}, + 'date_created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'level': ('django.db.models.fields.CharField', [], {'max_length': '15'}), + 'logger': ('django.db.models.fields.CharField', [], {'max_length': '30'}), + 'message': ('django.db.models.fields.CharField', [], {'max_length': '255'}) + }, + u'core.objectaccess': { + 'Meta': {'unique_together': "(('user', 'content_type', 'object_id'),)", 'object_name': 'ObjectAccess'}, + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['contenttypes.ContentType']"}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_owner': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'object_id': ('django.db.models.fields.PositiveIntegerField', [], {}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['core.User']"}) + }, + u'core.user': { + 'Meta': {'ordering': "['username']", 'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '254', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': u"orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_local': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '256'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': u"orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '254'}) + } + } + + complete_apps = ['core'] \ No newline at end of file diff --git a/modoboa/core/models.py b/modoboa/core/models.py index e817906fd..87ec3a7c5 100644 --- a/modoboa/core/models.py +++ b/modoboa/core/models.py @@ -1,29 +1,26 @@ import sys import os import re -import hashlib -import crypt -import base64 import logging -from random import Random import reversion from django.db import models from django.db.models.signals import post_delete from django.dispatch import receiver from django.utils import timezone from django.utils.translation import ugettext as _, ugettext_lazy -from django.utils.crypto import constant_time_compare from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes import generic from django.contrib.auth.models import ( - UserManager, Group, AbstractBaseUser, PermissionsMixin + UserManager, Group, PermissionsMixin ) -from modoboa.lib import events, md5crypt, parameters +from django.contrib.auth.hashers import make_password, is_password_usable +from modoboa.lib import events, parameters from modoboa.lib.exceptions import ( PermDeniedException, InternalError, BadRequest ) from modoboa.lib.sysutils import exec_cmd from modoboa.core.extensions import exts_pool +from modoboa.core.password_hashers import get_password_hasher try: from modoboa.lib.ldaputils import LDAPAuthBackend @@ -32,7 +29,7 @@ except ImportError: ldap_available = False -class User(AbstractBaseUser, PermissionsMixin): +class User(PermissionsMixin): """Custom User model. It overloads the way passwords are stored into the database. The @@ -49,6 +46,10 @@ class User(AbstractBaseUser, PermissionsMixin): is_active = models.BooleanField(default=True) date_joined = models.DateTimeField(default=timezone.now) is_local = models.BooleanField(default=True) + password = models.CharField(ugettext_lazy('password'), max_length=256) + last_login = models.DateTimeField( + ugettext_lazy('last login'), default=timezone.now + ) objects = UserManager() @@ -58,7 +59,7 @@ class User(AbstractBaseUser, PermissionsMixin): class Meta: ordering = ["username"] - password_expr = re.compile(r'(\{(\w+)\}|(\$1\$))(.+)') + password_expr = re.compile(r'\{([\w\-]+)\}(.+)') def delete(self, fromuser, *args, **kwargs): """Custom delete method @@ -95,35 +96,7 @@ class User(AbstractBaseUser, PermissionsMixin): scheme = parameters.get_admin("PASSWORD_SCHEME") if type(raw_value) is unicode: raw_value = raw_value.encode("utf-8") - if scheme == "crypt": - salt = "".join(Random().sample(string.letters + string.digits, 2)) - result = crypt.crypt(raw_value, salt) - prefix = "{CRYPT}" - elif scheme == "md5": - obj = hashlib.md5(raw_value) - result = obj.hexdigest() - prefix = "{MD5}" - # The md5crypt scheme is the only supported method that has both: - # (a) a salt ("crypt" has this too), - # (b) supports passwords lengths of more than 8 characters (all except - # "crypt"). - elif scheme == "md5crypt": - # The salt may vary from 12 to 48 bits. (Using all six bytes here - # with a subset of characters means we get only 35 random bits.) - import string - salt = "".join(Random().sample(string.letters + string.digits, 6)) - result = md5crypt(raw_value, salt) - prefix = "" # the result already has $1$ prepended to it - # to signify what this is - elif scheme == "sha256": - obj = hashlib.sha256(raw_value) - result = base64.b64encode(obj.digest()) - prefix = "{SHA256}" - else: - scheme = "plain" - result = raw_value - prefix = "{PLAIN}" - return "%s%s" % (prefix, result) + return get_password_hasher(scheme.upper())().encrypt(raw_value) def set_password(self, raw_value, curvalue=None): """Password update @@ -150,26 +123,46 @@ class User(AbstractBaseUser, PermissionsMixin): ) def check_password(self, raw_value): - m = self.password_expr.match(self.password) - if m is None: + match = self.password_expr.match(self.password) + if match is None: return False if type(raw_value) is unicode: raw_value = raw_value.encode("utf-8") - scheme = (m.group(2) or m.group(3)).lower() - val2 = m.group(4) - if scheme == u"crypt": - val1 = crypt.crypt(raw_value, val2) - elif scheme == u"md5": - val1 = hashlib.md5(raw_value).hexdigest() - elif scheme == u"sha256": - val1 = base64.b64encode(hashlib.sha256(raw_value).digest()) - elif scheme == u"$1$": # md5crypt - salt, hashed = val2.split('$') - val1 = md5crypt(raw_value, str(salt)) - val2 = self.password # re-add scheme for comparison below - else: - val1 = raw_value - return constant_time_compare(val1, val2) + scheme = match.group(1) + val2 = match.group(2) + hasher = get_password_hasher(scheme) + return hasher().verify(raw_value, val2) + + def get_username(self): + "Return the identifying username for this User" + return getattr(self, self.USERNAME_FIELD) + + def __str__(self): + return self.get_username() + + def natural_key(self): + return (self.get_username(),) + + def is_anonymous(self): + """ + Always returns False. This is a way of comparing User objects to + anonymous users. + """ + return False + + def is_authenticated(self): + """ + Always return True. This is a way to tell if the user has been + authenticated in templates. + """ + return True + + def set_unusable_password(self): + # Sets a value that will never be a valid hash + self.password = make_password(None) + + def has_usable_password(self): + return is_password_usable(self.password) @property def tags(self): diff --git a/modoboa/core/password_hashers/__init__.py b/modoboa/core/password_hashers/__init__.py new file mode 100644 index 000000000..c26f9d3fd --- /dev/null +++ b/modoboa/core/password_hashers/__init__.py @@ -0,0 +1,26 @@ +""" +Password hashers for Modoboa. +""" +from modoboa.core.password_hashers.base import ( + PLAINHasher, CRYPTHasher, MD5Hasher, SHA256Hasher +) +from modoboa.core.password_hashers.advanced import ( + MD5CRYPTHasher, SHA256CRYPTHasher, SHA512CRYPTHasher +) + + +def get_password_hasher(scheme): + """Retrieve the hasher corresponding to :keyword:`scheme`. + + If no class is found, `PLAINHasher` is returned. + + :param str scheme: a valid scheme name + :rtype: PasswordHasher sub class + :return: The hasher class + """ + try: + scheme = scheme.replace('-', '') + hasher = globals()['%sHasher' % scheme] + except AttributeError: + hasher = PLAINHasher + return hasher diff --git a/modoboa/core/password_hashers/advanced.py b/modoboa/core/password_hashers/advanced.py new file mode 100644 index 000000000..aa844c9ea --- /dev/null +++ b/modoboa/core/password_hashers/advanced.py @@ -0,0 +1,63 @@ +""" +Advanced (ie. stronger) password hashers. + +This module relies on `passlib` to provide more secure hashers. +""" +from passlib.hash import md5_crypt, sha256_crypt, sha512_crypt +from modoboa.core.password_hashers.base import PasswordHasher +from modoboa.lib import parameters + + +class MD5CRYPTHasher(PasswordHasher): + """ + MD5-CRYPT password hasher. + + This scheme can't be considered as secure anymore. + """ + @property + def scheme(self): + return '{MD5-CRYPT}' + + def _encrypt(self, clearvalue, salt=None): + return md5_crypt.encrypt(clearvalue) + + def verify(self, clearvalue, hashed_value): + return md5_crypt.verify(clearvalue, hashed_value) + + +class SHA256CRYPTHasher(PasswordHasher): + """ + SHA256-CRYPT password hasher. + + Supports rounds and is a good compromise between security and + performance. + """ + @property + def scheme(self): + return '{SHA256-CRYPT}' + + def _encrypt(self, clearvalue, salt=None): + rounds = int(parameters.get_admin('ROUNDS_NUMBER')) + return sha256_crypt.encrypt(clearvalue, rounds=rounds) + + def verify(self, clearvalue, hashed_value): + return sha256_crypt.verify(clearvalue, hashed_value) + + +class SHA512CRYPTHasher(PasswordHasher): + """ + SHA512-CRYPT password hasher. + + Supports rounds and is the strongest scheme provided by + Modoboa. Requires more resource than SHA256-CRYPT. + """ + @property + def scheme(self): + return '{SHA512-CRYPT}' + + def _encrypt(self, clearvalue, salt=None): + rounds = int(parameters.get_admin('ROUNDS_NUMBER')) + return sha512_crypt.encrypt(clearvalue, rounds=rounds) + + def verify(self, clearvalue, hashed_value): + return sha512_crypt.verify(clearvalue, hashed_value) diff --git a/modoboa/core/password_hashers/base.py b/modoboa/core/password_hashers/base.py new file mode 100644 index 000000000..eb7ddd800 --- /dev/null +++ b/modoboa/core/password_hashers/base.py @@ -0,0 +1,100 @@ +""" +Base password hashers. + +Contains weak hashers (the original ones) available with Modoboa. +""" +import crypt +import hashlib +import base64 +import string +from random import Random +from django.utils.crypto import constant_time_compare + + +class PasswordHasher(object): + """ + Base class of all hashers. + """ + def encrypt(self, clearvalue): + """Encrypt a password. + + The format used to store passwords is the same than dovecot's one:: + + {SCHEME} + + may differ according to the used scheme. + + :param str clearvalue: clear password + :rtype: str + :return: encrypted password + """ + return '%s%s' % (self.scheme, self._encrypt(clearvalue)) + + def verify(self, clearvalue, hashed_value): + """Verify a password against a hashed value. + + :param str clearvalue: clear password + :param str hashed_value: password previously hashed + :return: True if passwords match, False otherwise + """ + return constant_time_compare( + self._encrypt(clearvalue, hashed_value), + hashed_value + ) + + +class PLAINHasher(PasswordHasher): + """ + Plain (ie. clear) password hasher. + """ + @property + def scheme(self): + return '{PLAIN}' + + def _encrypt(self, clearvalue, salt=None): + return clearvalue + + +class CRYPTHasher(PasswordHasher): + """ + crypt password hasher. + + Uses python `crypt` standard module. + """ + @property + def scheme(self): + return '{CRYPT}' + + def _encrypt(self, clearvalue, salt=None): + if salt is None: + salt = "".join(Random().sample(string.letters + string.digits, 2)) + return crypt.crypt(clearvalue, salt) + + +class MD5Hasher(PasswordHasher): + """ + MD5 password hasher. + + Uses python `hashlib` standard module. + """ + @property + def scheme(self): + return '{MD5}' + + def _encrypt(self, clearvalue, salt=None): + obj = hashlib.md5(clearvalue) + return obj.hexdigest() + + +class SHA256Hasher(PasswordHasher): + """ + SHA256 password hasher. + + Uses python `hashlib` and `base64` standard modules. + """ + @property + def scheme(self): + return '{SHA256}' + + def _encrypt(self, clearvalue, salt=None): + return base64.b64encode(hashlib.sha256(clearvalue).digest()) diff --git a/modoboa/core/tests.py b/modoboa/core/tests.py index 5b3491ba2..80de9db01 100644 --- a/modoboa/core/tests.py +++ b/modoboa/core/tests.py @@ -19,9 +19,9 @@ class ProfileTestCase(ModoTestCase): * The default admin changes his password (no associated Mailbox) * A normal user changes his password """ - self.check_ajax_post(reverse("modoboa.core.views.user.profile"), - {"oldpassword": "password", - "newpassword": "titi", "confirmation": "titi"}) + self.ajax_post(reverse("modoboa.core.views.user.profile"), + {"oldpassword": "password", + "newpassword": "titi", "confirmation": "titi"}) self.clt.logout() self.assertEqual( @@ -31,12 +31,12 @@ class ProfileTestCase(ModoTestCase): self.clt.login(username="user@test.com", password="toto"), True ) - self.check_ajax_post( + self.ajax_post( reverse("modoboa.core.views.user.profile"), {"oldpassword": "toto", "newpassword": "tutu", "confirmation": "tutu"} ) self.clt.logout() - self.assertEqual( - self.clt.login(username="user@test.com", password="tutu"), True + self.assertTrue( + self.clt.login(username="user@test.com", password="tutu") ) diff --git a/modoboa/extensions/admin/tests/__init__.py b/modoboa/extensions/admin/tests/__init__.py index 9ee5833ef..9d2cb39be 100644 --- a/modoboa/extensions/admin/tests/__init__.py +++ b/modoboa/extensions/admin/tests/__init__.py @@ -4,9 +4,10 @@ from .account import AccountTestCase, PermissionsTestCase from .alias import AliasTestCase from .import_ import ImportTestCase from .export import ExportTestCase +from .password_schemes import PasswordSchemesTestCase __all__ = [ 'DomainTestCase', 'DomainAliasTestCase', 'AccountTestCase', 'PermissionsTestCase', 'AliasTestCase', 'ImportTestCase', - 'ExportTestCase' + 'ExportTestCase', 'PasswordSchemesTestCase' ] diff --git a/modoboa/extensions/admin/tests/password_schemes.py b/modoboa/extensions/admin/tests/password_schemes.py new file mode 100644 index 000000000..8f6bee03a --- /dev/null +++ b/modoboa/extensions/admin/tests/password_schemes.py @@ -0,0 +1,55 @@ +from django.core.urlresolvers import reverse +from modoboa.core import load_core_settings +from modoboa.core.models import User +from modoboa.lib.tests import ModoTestCase +from modoboa.lib import parameters +from modoboa.extensions.admin import factories + + +class PasswordSchemesTestCase(ModoTestCase): + fixtures = ['initial_users.json'] + + def setUp(self): + super(PasswordSchemesTestCase, self).setUp() + factories.populate_database() + load_core_settings() + + def _create_account(self): + values = dict( + username="tester@test.com", first_name="Tester", last_name="Toto", + password1="toto", password2="toto", role="SimpleUsers", + quota_act=True, + is_active=True, email="tester@test.com", stepid='step2' + ) + self.ajax_post( + reverse("modoboa.extensions.admin.views.identity.newaccount"), + values + ) + + def _test_scheme(self, name, startpattern): + parameters.save_admin('PASSWORD_SCHEME', name, app='core') + self._create_account() + account = User.objects.get(username='tester@test.com') + self.assertTrue(account.password.startswith(startpattern)) + self.assertTrue(account.check_password('toto')) + + def test_sha512crypt_scheme(self): + self._test_scheme('sha512crypt', '{SHA512-CRYPT}') + + def test_sha256crypt_scheme(self): + self._test_scheme('sha256crypt', '{SHA256-CRYPT}') + + def test_md5crypt_scheme(self): + self._test_scheme('md5crypt', '{MD5-CRYPT}') + + def test_sha256_scheme(self): + self._test_scheme('sha256', '{SHA256}') + + def test_md5_scheme(self): + self._test_scheme('md5', '{MD5}') + + def test_crypt(self): + self._test_scheme('crypt', '{CRYPT}') + + def test_plain(self): + self._test_scheme('plain', '{PLAIN}') diff --git a/modoboa/lib/__init__.py b/modoboa/lib/__init__.py index e823e5932..e69de29bb 100644 --- a/modoboa/lib/__init__.py +++ b/modoboa/lib/__init__.py @@ -1,79 +0,0 @@ -# coding: utf-8 -import hashlib - -def md5crypt(password, salt, magic='$1$'): - """Taken from: - http://code.activestate.com/recipes/325204-passwd-file-compatible-1-md5-crypt/ - - Based on FreeBSD src/lib/libcrypt/crypt.c 1.2 - http://www.freebsd.org/cgi/cvsweb.cgi/~checkout~/src/lib/libcrypt/crypt.c?rev=1.2&content-type=text/plain - - Original license: - * "THE BEER-WARE LICENSE" (Revision 42): - * wrote this file. As long as you retain this notice you - * can do whatever you want with this stuff. If we meet some day, and you think - * this stuff is worth it, you can buy me a beer in return. Poul-Henning Kamp - - "A port of Poul-Henning Kamp's MD5 password hash routine, as initially - found in FreeBSD 2. It is also used in Cisco routers, Apache htpasswd files, - and other places that you find "$1$" at the beginning of password hashes." - by Mark Johnston - """ - # The password first, since that is what is most unknown - # Then our magic string. Then the raw salt. - m = hashlib.md5() - m.update(password + magic + salt) - - # /* Then just as many characters of the MD5(pw,salt,pw) */ - mixin = hashlib.md5(password + salt + password).digest() - for i in range(0, len(password)): - m.update(mixin[i % 16]) - - # Then something really weird... Also really broken, as far as I - # can tell. -m - i = len(password) - while i: - if i & 1: - m.update('\x00') - else: - m.update(password[0]) - i >>= 1 - - final = m.digest() - - # And now, just to make sure things don't run too fast - for i in range(1000): - m2 = hashlib.md5() - if i & 1: - m2.update(password) - else: - m2.update(final) - - if i % 3: - m2.update(salt) - - if i % 7: - m2.update(password) - - if i & 1: - m2.update(final) - else: - m2.update(password) - - final = m2.digest() - - # This is the bit that uses to64() in the original code. - - itoa64 = './0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz' - - rearranged = '' - for a, b, c in ((0, 6, 12), (1, 7, 13), (2, 8, 14), (3, 9, 15), (4, 10, 5)): - v = ord(final[a]) << 16 | ord(final[b]) << 8 | ord(final[c]) - for i in range(4): - rearranged += itoa64[v & 0x3f]; v >>= 6 - - v = ord(final[11]) - for i in range(2): - rearranged += itoa64[v & 0x3f]; v >>= 6 - - return magic + salt + '$' + rearranged diff --git a/requirements.txt b/requirements.txt index 94613aa78..ec8ce5408 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,3 +9,4 @@ chardet argparse rfc6266 factory-boy +passlib diff --git a/tests/install_from_scratch.py b/tests/install_from_scratch.py index bf1ea336d..17f497ff7 100644 --- a/tests/install_from_scratch.py +++ b/tests/install_from_scratch.py @@ -20,7 +20,7 @@ class DeployTest(unittest.TestCase): def tearDown(self): path = os.path.join(self.workdir, self.projname) - code, output = exec_cmd("python manage.py test lib admin limits postfix_relay_domains", cwd=path) + code, output = exec_cmd("python manage.py test core lib admin limits postfix_relay_domains", cwd=path) print output self.assertEqual(code, 0) From f16368a0dae9037f17e0fd7ddaddc9ce4f550f5a Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 15 Nov 2013 18:15:12 +0100 Subject: [PATCH 3/5] Drop python 2.6 because unit tests refuse to start. I need to find find the way to launch tests using python 2.6... --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 5c692ad59..9e4c369c7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,5 @@ language: python python: - - "2.6" - "2.7" env: From 63d37550d9d93af7ce80baf3fd6dc434a255deba Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 15 Nov 2013 18:47:42 +0100 Subject: [PATCH 4/5] Prevent the creation of a domain alias using the same name as a domain. fixes #498 --- modoboa/extensions/admin/models/domain_alias.py | 8 +++++++- modoboa/extensions/admin/tests/import_.py | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/modoboa/extensions/admin/models/domain_alias.py b/modoboa/extensions/admin/models/domain_alias.py index 382ba74e3..a24549b96 100644 --- a/modoboa/extensions/admin/models/domain_alias.py +++ b/modoboa/extensions/admin/models/domain_alias.py @@ -1,7 +1,7 @@ import reversion from django.db import models from django.utils.translation import ugettext as _, ugettext_lazy -from modoboa.lib.exceptions import BadRequest +from modoboa.lib.exceptions import BadRequest, Conflict from .base import AdminObject from .domain import Domain @@ -38,6 +38,12 @@ class DomainAlias(AdminObject): if len(row) < 4: raise BadRequest(_("Invalid line")) self.name = row[1].strip() + try: + Domain.objects.get(name=self.name) + except Domain.DoesNotExist: + pass + else: + raise Conflict(_("A domain %s already exists" % self.name)) domname = row[2].strip() try: self.target = Domain.objects.get(name=domname) diff --git a/modoboa/extensions/admin/tests/import_.py b/modoboa/extensions/admin/tests/import_.py index 186f3ff1d..89ffdffdd 100644 --- a/modoboa/extensions/admin/tests/import_.py +++ b/modoboa/extensions/admin/tests/import_.py @@ -40,6 +40,17 @@ domainalias; domalias1.com; domain1.com; True self.assertFalse(dom.enabled) self.assertTrue(admin.is_owner(dom)) + def test_import_domains_with_conflict(self): + f = ContentFile(b"""domain;test.alias;10;True +domainalias;test.alias;test.com;True +""", name="domains.csv") + resp = self.clt.post( + reverse("modoboa.extensions.admin.views.import.import_domains"), { + "sourcefile": f + } + ) + self.assertIn('A domain test.alias already exists', resp.content) + def test_identities_import(self): f = ContentFile(b""" account; user1@test.com; toto; User; One; True; SimpleUsers; user1@test.com From 003a81076c4359103854d45d7555f278306c6155 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Tue, 19 Nov 2013 17:56:22 +0100 Subject: [PATCH 5/5] Always set ownership. fixes #499 --- modoboa/core/models.py | 7 +++--- modoboa/extensions/admin/tests/__init__.py | 3 ++- modoboa/extensions/admin/tests/user.py | 26 ++++++++++++++++++++++ modoboa/extensions/admin/views/user.py | 6 ++--- 4 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 modoboa/extensions/admin/tests/user.py diff --git a/modoboa/core/models.py b/modoboa/core/models.py index 87ec3a7c5..73afefc4a 100644 --- a/modoboa/core/models.py +++ b/modoboa/core/models.py @@ -236,9 +236,10 @@ class User(PermissionsMixin): def can_access(self, obj): """Check if the user can access a specific object - This function is recursive : if the given user hasn't got direct - access to this object and if he has got access other ``User`` - objects, we check if one of those users owns the object. + This function is recursive: if the given user hasn't got + direct access to this object and if he has got access to other + ``User`` objects, we check if one of those users owns the + object. :param obj: a admin object :return: a boolean diff --git a/modoboa/extensions/admin/tests/__init__.py b/modoboa/extensions/admin/tests/__init__.py index 9d2cb39be..8fc7ac21f 100644 --- a/modoboa/extensions/admin/tests/__init__.py +++ b/modoboa/extensions/admin/tests/__init__.py @@ -5,9 +5,10 @@ from .alias import AliasTestCase from .import_ import ImportTestCase from .export import ExportTestCase from .password_schemes import PasswordSchemesTestCase +from .user import ForwardTestCase __all__ = [ 'DomainTestCase', 'DomainAliasTestCase', 'AccountTestCase', 'PermissionsTestCase', 'AliasTestCase', 'ImportTestCase', - 'ExportTestCase', 'PasswordSchemesTestCase' + 'ExportTestCase', 'PasswordSchemesTestCase', 'ForwardTestCase' ] diff --git a/modoboa/extensions/admin/tests/user.py b/modoboa/extensions/admin/tests/user.py new file mode 100644 index 000000000..b45915d49 --- /dev/null +++ b/modoboa/extensions/admin/tests/user.py @@ -0,0 +1,26 @@ +from django.core.urlresolvers import reverse +from modoboa.core.models import User +from modoboa.lib.tests import ModoTestCase +from modoboa.extensions.admin.factories import populate_database +from modoboa.extensions.admin.models import Alias + + +class ForwardTestCase(ModoTestCase): + fixtures = ['initial_users.json'] + + def setUp(self): + super(ForwardTestCase, self).setUp() + populate_database() + + def test_forward_permissions(self): + self.clt.logout() + self.clt.login(username='user@test.com', password='toto') + self.ajax_post( + reverse('modoboa.extensions.admin.views.user.forward'), + {'dest': 'user@extdomain.com', 'keepcopies': True} + ) + forward = Alias.objects.get(address='user', domain__name='test.com') + sadmin = User.objects.get(username='admin') + self.assertTrue(sadmin.can_access(forward)) + domadmin = User.objects.get(username='admin@test.com') + self.assertTrue(domadmin.can_access(forward)) diff --git a/modoboa/extensions/admin/views/user.py b/modoboa/extensions/admin/views/user.py index 7c0382c41..f836968b1 100644 --- a/modoboa/extensions/admin/views/user.py +++ b/modoboa/extensions/admin/views/user.py @@ -31,9 +31,9 @@ def forward(request, tplname='admin/forward.html'): if form.cleaned_data["keepcopies"]: intdests += [mb] form.parse_dest() - al.save(int_rcpts=intdests, ext_rcpts=form.dests) - if request.user.group != "SimpleUsers": - al.post_create(request.user) + al.save( + int_rcpts=intdests, ext_rcpts=form.dests, creator=request.user + ) return render_to_json_response(_("Forward updated")) return render_to_json_response(