From 213a36371649c0c97828128c2f6e88d329a837f0 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 12 Jan 2018 17:42:04 +0100 Subject: [PATCH 1/8] Per domain DKIM key generation. see #1325 --- modoboa/admin/app_settings.py | 37 +++++++++++ modoboa/admin/constants.py | 6 ++ modoboa/admin/forms/domain.py | 21 +++++- modoboa/admin/management/commands/modo.py | 2 + .../commands/subcommands/_manage_dkim_keys.py | 51 ++++++++++++++ .../migrations/0012_auto_20180112_1658.py | 40 +++++++++++ modoboa/admin/models/domain.py | 21 ++++++ modoboa/admin/static/admin/js/admin.js | 20 ++++++ .../templates/admin/_domain_dkim_key.html | 23 +++++++ .../admin/templates/admin/domain_detail.html | 66 +++++++++++++------ .../templates/admin/domain_general_form.html | 3 + modoboa/admin/tests/test_domain.py | 50 +++++++++++++- .../migrations/0015_auto_20180112_1642.py | 20 ++++++ modoboa/static/css/custom.css | 2 +- 14 files changed, 338 insertions(+), 24 deletions(-) create mode 100644 modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py create mode 100644 modoboa/admin/migrations/0012_auto_20180112_1658.py create mode 100644 modoboa/admin/templates/admin/_domain_dkim_key.html create mode 100644 modoboa/core/migrations/0015_auto_20180112_1642.py diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py index 95004aeab..3723df9b4 100644 --- a/modoboa/admin/app_settings.py +++ b/modoboa/admin/app_settings.py @@ -13,6 +13,8 @@ from modoboa.lib.form_utils import YesNoField, SeparatorField from modoboa.lib.sysutils import exec_cmd from modoboa.parameters import forms as param_forms +from . import constants + class AdminParametersForm(param_forms.AdminParametersForm): app = "admin" @@ -55,6 +57,25 @@ class AdminParametersForm(param_forms.AdminParametersForm): ) ) + dkim_keys_storage_dir = forms.CharField( + label=ugettext_lazy("DKIM keys storage directory"), + initial="", + help_text=ugettext_lazy( + "Absolute path of the directory where DKIM private keys will " + "be stored. Make sure this directory belongs to root user " + "and is not readable by the outside world." + ) + ) + + dkim_default_key_length = forms.ChoiceField( + label=ugettext_lazy("Default DKIM key length"), + initial=1024, + choices=constants.DKIM_KEY_LENGTHS, + help_text=ugettext_lazy( + "Default length for newly generated DKIM keys." + ) + ) + mbsep = SeparatorField(label=ugettext_lazy("Mailboxes")) handle_mailboxes = YesNoField( @@ -173,6 +194,22 @@ class AdminParametersForm(param_forms.AdminParametersForm): ) return self.cleaned_data["default_mailbox_quota"] + def clean_dkim_keys_storage_dir(self): + """Check that directory exists.""" + storage_dir = self.cleaned_data.get("dkim_keys_storage_dir", "") + if storage_dir: + if not os.path.isdir(storage_dir): + raise forms.ValidationError( + ugettext_lazy("Directory not found") + ) + code, output = exec_cmd("which openssl") + if code: + raise forms.ValidationError( + ugettext_lazy( + "openssl not found, please make sure it is installed") + ) + return storage_dir + def clean(self): """Check MX options.""" cleaned_data = super(AdminParametersForm, self).clean() diff --git a/modoboa/admin/constants.py b/modoboa/admin/constants.py index 42fdc1277..372f1ef20 100644 --- a/modoboa/admin/constants.py +++ b/modoboa/admin/constants.py @@ -99,3 +99,9 @@ DOMAIN_TYPES = [ ("domain", _("Domain")), ("relaydomain", _("Relay domain")), ] + +DKIM_KEY_LENGTHS = [ + (1024, "1024"), + (2048, "2048"), + (4096, "4096"), +] diff --git a/modoboa/admin/forms/domain.py b/modoboa/admin/forms/domain.py index 558b6e89f..ddd4bf070 100644 --- a/modoboa/admin/forms/domain.py +++ b/modoboa/admin/forms/domain.py @@ -43,12 +43,16 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm): "press ENTER to add a new input." ) ) + dkim_key_selector = forms.CharField( + label=ugettext_lazy("Key selector"), required=False) class Meta: model = Domain fields = ( "name", "type", "quota", "default_mailbox_quota", "aliases", - "enabled", "enable_dns_checks") + "enabled", "enable_dns_checks", "enable_dkim", + "dkim_key_selector", "dkim_key_length" + ) def __init__(self, user, *args, **kwargs): self.oldname = None @@ -93,6 +97,17 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm): return name + def clean_enable_dkim(self): + """Check prerequisites.""" + enabled = self.cleaned_data.get("enable_dkim") + if not enabled: + return enabled + storage_dir = param_tools.get_global_parameter("dkim_keys_storage_dir") + if not storage_dir: + raise forms.ValidationError( + _("DKIM keys storage directory not configured")) + return enabled + def clean(self): """Custom fields validation. @@ -123,6 +138,10 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm): "default_mailbox_quota"] if default_mailbox_quota == 0: self.add_error("default_mailbox_quota", msg) + if self.cleaned_data["enable_dkim"]: + if not self.cleaned_data.get("dkim_key_selector"): + self.add_error( + "dkim_key_selector", _("This field is required.")) self.aliases = [] copied_data = cleaned_data.copy() for k in copied_data.keys(): diff --git a/modoboa/admin/management/commands/modo.py b/modoboa/admin/management/commands/modo.py index 5088ace23..d395c4c19 100644 --- a/modoboa/admin/management/commands/modo.py +++ b/modoboa/admin/management/commands/modo.py @@ -7,6 +7,7 @@ from subcommand.base import SubcommandCommand from .subcommands._mx import CheckMXRecords from .subcommands._export import ExportCommand from .subcommands._import import ImportCommand +from .subcommands._manage_dkim_keys import ManageDKIMKeys from .subcommands._repair import Repair @@ -22,5 +23,6 @@ class Command(SubcommandCommand): "export": ExportCommand, "import": ImportCommand, "check_mx": CheckMXRecords, + "manage_dkim_keys": ManageDKIMKeys, "repair": Repair, } diff --git a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py new file mode 100644 index 000000000..1810a96af --- /dev/null +++ b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py @@ -0,0 +1,51 @@ +"""Management command to create DKIM keys.""" + +from __future__ import print_function, unicode_literals + +import os + +from django.core.management.base import BaseCommand + +from modoboa.lib import sysutils +from modoboa.parameters import tools as param_tools + +from .... import models + + +class ManageDKIMKeys(BaseCommand): + """Command class.""" + + def create_new_dkim_key(self, domain): + """Create a new DKIM key.""" + storage_dir = param_tools.get_global_parameter("dkim_keys_storage_dir") + pkey_path = os.path.join(storage_dir, "{}.pem".format(domain.name)) + key_size = ( + domain.dkim_key_length if domain.dkim_key_length + else self.default_key_length) + code, output = sysutils.exec_cmd( + "openssl genrsa -out {} {}".format(pkey_path, key_size)) + if code: + print("Failed to generate DKIM private key for domain {}: {}" + .format(domain.name, output)) + domain.dkim_private_key_path = pkey_path + code, output = sysutils.exec_cmd( + "openssl rsa -in {} -pubout".format(pkey_path)) + if code: + print("Failed to generate DKIM public key for domain {}: {}" + .format(domain.name, output)) + public_key = "" + for cpt, line in enumerate(output.splitlines()): + if cpt == 0 or line.startswith("-----"): + continue + public_key += line + domain.dkim_public_key = public_key + domain.save(update_fields=["dkim_public_key", "dkim_private_key_path"]) + + def handle(self, *args, **options): + """Entry point.""" + self.default_key_length = param_tools.get_global_parameter( + "dkim_default_key_length") + qset = models.Domain.objects.filter( + enable_dkim=True, dkim_private_key_path="") + for domain in qset: + self.create_new_dkim_key(domain) diff --git a/modoboa/admin/migrations/0012_auto_20180112_1658.py b/modoboa/admin/migrations/0012_auto_20180112_1658.py new file mode 100644 index 000000000..ba9f1f071 --- /dev/null +++ b/modoboa/admin/migrations/0012_auto_20180112_1658.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.8 on 2018-01-12 15:58 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('admin', '0011_domain_transport'), + ] + + operations = [ + migrations.AddField( + model_name='domain', + name='dkim_key_length', + field=models.PositiveIntegerField(blank=True, choices=[(1024, '1024'), (2048, '2048'), (4096, '4096')], null=True, verbose_name='Key length'), + ), + migrations.AddField( + model_name='domain', + name='dkim_key_selector', + field=models.CharField(default='default', max_length=30, verbose_name='Key selector'), + ), + migrations.AddField( + model_name='domain', + name='dkim_private_key_path', + field=models.CharField(blank=True, max_length=254), + ), + migrations.AddField( + model_name='domain', + name='dkim_public_key', + field=models.TextField(blank=True), + ), + migrations.AddField( + model_name='domain', + name='enable_dkim', + field=models.BooleanField(default=False, help_text='If you activate this feature, a DKIM key will be generated for this domain.', verbose_name='Enable DKIM signing'), + ), + ] diff --git a/modoboa/admin/models/domain.py b/modoboa/admin/models/domain.py index 04c1f3518..30a9e0219 100644 --- a/modoboa/admin/models/domain.py +++ b/modoboa/admin/models/domain.py @@ -58,6 +58,21 @@ class Domain(AdminObject): transport = models.OneToOneField( "transport.Transport", null=True, on_delete=models.SET_NULL) + enable_dkim = models.BooleanField( + ugettext_lazy("Enable DKIM signing"), + help_text=ugettext_lazy( + "If you activate this feature, a DKIM key will be " + "generated for this domain."), + default=False + ) + dkim_key_selector = models.CharField(max_length=30, default="default") + dkim_key_length = models.PositiveIntegerField( + ugettext_lazy("Key length"), choices=constants.DKIM_KEY_LENGTHS, + blank=True, null=True + ) + dkim_public_key = models.TextField(blank=True) + dkim_private_key_path = models.CharField(max_length=254, blank=True) + class Meta: permissions = ( ("view_domain", "View domain"), @@ -187,6 +202,12 @@ class Domain(AdminObject): return 0 return int(self.used_quota / float(self.quota) * 100) + @property + def bind_format_dkim_public_key(self): + """Return DKIM public key using Bind format.""" + return "{}._domainkey 10800 IN TXT \"v=DKIM1; k=rsa; p={}\"".format( + self.dkim_key_selector, self.dkim_public_key) + def add_admin(self, account): """Add a new administrator to this domain. diff --git a/modoboa/admin/static/admin/js/admin.js b/modoboa/admin/static/admin/js/admin.js index 47ce73aab..b198b9ab7 100644 --- a/modoboa/admin/static/admin/js/admin.js +++ b/modoboa/admin/static/admin/js/admin.js @@ -223,6 +223,21 @@ Domains.prototype = { "disabled", $target.val() === "False"); }, + /** + * Hide or show password inputs depending on value. + * + * @param {bool} value - visibility flag + */ + toggleDKIMFields: function (value) { + if (value) { + $('#id_dkim_key_selector').parents(".form-group").show(); + $('#id_dkim_key_length').parents(".form-group").show(); + } else { + $('#id_dkim_key_selector').parents(".form-group").hide(); + $('#id_dkim_key_length').parents(".form-group").hide(); + } + }, + /** * Initialize the main form contained in the domain edition modal. * @@ -239,6 +254,11 @@ Domains.prototype = { return true; } }); + $('#id_enable_dkim').click($.proxy(function (e) { + var $this = $(e.target); + this.toggleDKIMFields($this.prop('checked')); + }, this)); + this.toggleDKIMFields($('#id_enable_dkim').prop('checked')); }, optionsform_init: function() { diff --git a/modoboa/admin/templates/admin/_domain_dkim_key.html b/modoboa/admin/templates/admin/_domain_dkim_key.html new file mode 100644 index 000000000..d867ac509 --- /dev/null +++ b/modoboa/admin/templates/admin/_domain_dkim_key.html @@ -0,0 +1,23 @@ +{% load i18n %} + +
+
+
+ +

{% blocktrans trimmed with domain=domain.name %}DKIM public key for {{ domain }}{% endblocktrans %}

+
+
+
+ {% trans "Raw format" %}
+ {{ domain.dkim_public_key }} +
+
+ {% trans "Bind/named format" %}
+ {{ domain.bind_format_dkim_public_key }} +
+
+
+ {% trans "Close" %} +
+
+
diff --git a/modoboa/admin/templates/admin/domain_detail.html b/modoboa/admin/templates/admin/domain_detail.html index a4e7e21bd..96dafeb9c 100644 --- a/modoboa/admin/templates/admin/domain_detail.html +++ b/modoboa/admin/templates/admin/domain_detail.html @@ -25,14 +25,6 @@

{% trans "Summary" %}

- {% if enable_mx_checks or enable_dnsbl_checks %} - - - - - {% endif %} @@ -94,6 +86,33 @@
+
+
+

{% trans "DNS" %}

+
+
{% trans "DNS status" %} - {% include "admin/_domain_dnschecks_status.html" %} -
{% trans "Creation date" %} {{ domain.creation }}
+ {% if enable_mx_checks or enable_dnsbl_checks %} + + + + + {% endif %} + + + + +
{% trans "Status" %} + {% include "admin/_domain_dnschecks_status.html" %} +
{% trans "DKIM key" %} + {% if domain.dkim_public_key %} + + {% else %} + {% trans "Not generated" %} + {% endif %} +
+

{% trans "Administrators" %}

@@ -101,19 +120,19 @@ - {% for da in domain.admins %} - - - - - - {% empty %} - - {% endfor %} + {% for da in domain.admins %} + + + + + + {% empty %} + + {% endfor %}
{% trans "Username" %}{% trans "Name" %}
{{ da.username }}{{ da.fullname }} - {% if perms.admin.change_domain %} - - {% endif %} -
{% trans "No domain administrator defined yet." %}
{{ da.username }}{{ da.fullname }} + {% if perms.admin.change_domain %} + + {% endif %} +
{% trans "No domain administrator defined yet." %}
@@ -123,4 +142,9 @@ {% endfor %} + {% if domain.dkim_public_key %} + + {% endif %} {% endblock %} diff --git a/modoboa/admin/templates/admin/domain_general_form.html b/modoboa/admin/templates/admin/domain_general_form.html index 4caca3a92..f95838700 100644 --- a/modoboa/admin/templates/admin/domain_general_form.html +++ b/modoboa/admin/templates/admin/domain_general_form.html @@ -6,3 +6,6 @@ {% render_fields_group form "aliases" %} {% render_field form.enabled %} {% render_field form.enable_dns_checks %} +{% render_field form.enable_dkim %} +{% render_field form.dkim_key_selector %} +{% render_field form.dkim_key_length %} diff --git a/modoboa/admin/tests/test_domain.py b/modoboa/admin/tests/test_domain.py index da61d7cea..a161ffde5 100644 --- a/modoboa/admin/tests/test_domain.py +++ b/modoboa/admin/tests/test_domain.py @@ -4,9 +4,14 @@ from __future__ import unicode_literals +import os +import shutil +import tempfile + import dns.resolver from mock import patch +from django.core.management import call_command from django.urls import reverse from modoboa.core import factories as core_factories @@ -213,7 +218,8 @@ class DomainTestCase(ModoTestCase): mock_query.side_effect = utils.mock_dns_query_result mock_getaddrinfo.side_effect = utils.mock_ip_query_result values = { - "name": "no-mx.example.com", "quota": 0, "default_mailbox_quota": 0, + "name": "no-mx.example.com", "quota": 0, + "default_mailbox_quota": 0, "create_dom_admin": True, "dom_admin_username": "toto", "create_aliases": True, "type": "domain", "stepid": "step3", "with_mailbox": True @@ -347,3 +353,45 @@ class DomainTestCase(ModoTestCase): content = response.content.decode('utf-8') self.assertNotIn("Global statistics", content) self.assertIn("Per-domain statistics", content) + + +class DKIMTestCase(ModoTestCase): + """Test case for DKIM.""" + + @classmethod + def setUpTestData(cls): # noqa: N802 + """Create test data.""" + super(DKIMTestCase, cls).setUpTestData() + factories.populate_database() + + def setUp(self): + super(DKIMTestCase, self).setUp() + self.workdir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.workdir) + + def test_dkim_key_creation(self): + """Test DKIM key creation.""" + values = { + "name": "pouet.com", "quota": 1000, "default_mailbox_quota": 100, + "create_dom_admin": False, "type": "domain", "stepid": "step3", + "enable_dkim": True, "dkim_key_selector": "" + } + response = self.ajax_post(reverse("admin:domain_add"), values, 400) + self.assertEqual( + response["form_errors"]["enable_dkim"][0], + "DKIM keys storage directory not configured") + + self.set_global_parameter("dkim_keys_storage_dir", self.workdir) + response = self.ajax_post(reverse("admin:domain_add"), values, 400) + self.assertEqual( + response["form_errors"]["dkim_key_selector"][0], + "This field is required.") + + values["dkim_key_selector"] = "default" + self.ajax_post(reverse("admin:domain_add"), values) + + call_command("modo", "manage_dkim_keys") + key_path = os.path.join(self.workdir, "{}.pem".format(values["name"])) + self.assertTrue(os.path.exists(key_path)) diff --git a/modoboa/core/migrations/0015_auto_20180112_1642.py b/modoboa/core/migrations/0015_auto_20180112_1642.py new file mode 100644 index 000000000..d253aaa52 --- /dev/null +++ b/modoboa/core/migrations/0015_auto_20180112_1642.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.8 on 2018-01-12 15:42 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0014_auto_20171010_1746'), + ] + + operations = [ + migrations.AlterField( + model_name='user', + name='language', + field=models.CharField(choices=[('cs', '\u010de\u0161tina'), ('de', 'deutsch'), ('en', 'english'), ('el_GR', '\u03b5\u03bb\u03bb\u03b7\u03bd\u03b9\u03ba\u03ac'), ('es', 'espa\xf1ol'), ('fr', 'fran\xe7ais'), ('it', 'italiano'), ('ja_JP', '\u65e5\u672c\u306e'), ('nl', 'nederlands'), ('pt_PT', 'portugu\xeas'), ('pt_BR', 'portugu\xeas (BR)'), ('pl_PL', 'polski'), ('ru', '\u0440\u0443\u0441\u0441\u043a\u0438\u0439'), ('sv', 'svenska')], default='en', help_text='Prefered language to display pages.', max_length=10, verbose_name='language'), + ), + ] diff --git a/modoboa/static/css/custom.css b/modoboa/static/css/custom.css index 63252714a..1db572383 100644 --- a/modoboa/static/css/custom.css +++ b/modoboa/static/css/custom.css @@ -298,4 +298,4 @@ table td[name="actions"] { /* Dashboard */ .slogan { padding-top: 15px; -} \ No newline at end of file +} From 32c1fdee0edeb723eea799c612e2de739dbb10ed Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 12 Jan 2018 18:59:19 +0100 Subject: [PATCH 2/8] Fixed unit test. --- modoboa/admin/app_settings.py | 3 ++- modoboa/core/tests/test_views.py | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py index 3723df9b4..fa063105c 100644 --- a/modoboa/admin/app_settings.py +++ b/modoboa/admin/app_settings.py @@ -64,7 +64,8 @@ class AdminParametersForm(param_forms.AdminParametersForm): "Absolute path of the directory where DKIM private keys will " "be stored. Make sure this directory belongs to root user " "and is not readable by the outside world." - ) + ), + required=False ) dkim_default_key_length = forms.ChoiceField( diff --git a/modoboa/core/tests/test_views.py b/modoboa/core/tests/test_views.py index 0c433c16b..7e06ad910 100644 --- a/modoboa/core/tests/test_views.py +++ b/modoboa/core/tests/test_views.py @@ -159,6 +159,8 @@ class SettingsTestCase(ModoTestCase): "admin-domains_must_have_authorized_mx": False, "core-random_password_length": "8", "admin-create_alias_on_mbox_rename": False, + "admin-dkim_keys_storage_dir": "", + "admin-dkim_default_key_length": 1024 } def test_get_settings(self): From 61187da42971647339f93ba2cff33c80e1189f33 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 12 Jan 2018 19:08:57 +0100 Subject: [PATCH 3/8] Fixed javascript issue. --- modoboa/static/js/twocols_nav.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modoboa/static/js/twocols_nav.js b/modoboa/static/js/twocols_nav.js index 85d164401..3401e0933 100644 --- a/modoboa/static/js/twocols_nav.js +++ b/modoboa/static/js/twocols_nav.js @@ -36,7 +36,7 @@ TwocolsNav.prototype = { success_cb: $.proxy(this.save_cb, this) }); }, this)); - $(document).on('submit', '#parameters_form', function(evt) { + $(document).on('submit', '#parameters_form', $.proxy(function(evt) { simple_ajax_form_post(evt, { formid: $(this).attr('id'), modal: false, @@ -46,7 +46,7 @@ TwocolsNav.prototype = { $('a[data-app="' + data.prefix + '"]').tab('show'); } }); - }); + }, this)); }, update_content: function(data) { From 9761ffff0f0f60ea1d3846d8df5bad7f177cd7c8 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 12 Jan 2018 20:34:45 +0100 Subject: [PATCH 4/8] Fixed py3 compat. --- .../admin/management/commands/subcommands/_manage_dkim_keys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py index 1810a96af..1e9b4e9b2 100644 --- a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py +++ b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py @@ -35,7 +35,7 @@ class ManageDKIMKeys(BaseCommand): .format(domain.name, output)) public_key = "" for cpt, line in enumerate(output.splitlines()): - if cpt == 0 or line.startswith("-----"): + if cpt == 0 or line.startswith(b"-----"): continue public_key += line domain.dkim_public_key = public_key From e0aa745f7e5f0413c962685daea7d12f23411053 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 12 Jan 2018 20:52:21 +0100 Subject: [PATCH 5/8] Fix py3 compat! --- .../admin/management/commands/subcommands/_manage_dkim_keys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py index 1e9b4e9b2..610b8779a 100644 --- a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py +++ b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py @@ -33,7 +33,7 @@ class ManageDKIMKeys(BaseCommand): if code: print("Failed to generate DKIM public key for domain {}: {}" .format(domain.name, output)) - public_key = "" + public_key = b"" for cpt, line in enumerate(output.splitlines()): if cpt == 0 or line.startswith(b"-----"): continue From 3cebf2db1ab7d36e4a48ef7c858abebf6ad4f5cc Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Sat, 13 Jan 2018 09:53:42 +0100 Subject: [PATCH 6/8] Improve coverage. --- modoboa/admin/app_settings.py | 4 +- .../commands/subcommands/_manage_dkim_keys.py | 11 +- modoboa/admin/tests/test_domain.py | 24 ++++ modoboa/core/tests/test_views.py | 121 +++++++++--------- 4 files changed, 93 insertions(+), 67 deletions(-) diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py index fa063105c..6ce1b66f4 100644 --- a/modoboa/admin/app_settings.py +++ b/modoboa/admin/app_settings.py @@ -201,13 +201,13 @@ class AdminParametersForm(param_forms.AdminParametersForm): if storage_dir: if not os.path.isdir(storage_dir): raise forms.ValidationError( - ugettext_lazy("Directory not found") + ugettext_lazy("Directory not found.") ) code, output = exec_cmd("which openssl") if code: raise forms.ValidationError( ugettext_lazy( - "openssl not found, please make sure it is installed") + "openssl not found, please make sure it is installed.") ) return storage_dir diff --git a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py index 610b8779a..bae55519c 100644 --- a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py +++ b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py @@ -5,6 +5,7 @@ from __future__ import print_function, unicode_literals import os from django.core.management.base import BaseCommand +from django.utils.encoding import smart_text from modoboa.lib import sysutils from modoboa.parameters import tools as param_tools @@ -26,16 +27,16 @@ class ManageDKIMKeys(BaseCommand): "openssl genrsa -out {} {}".format(pkey_path, key_size)) if code: print("Failed to generate DKIM private key for domain {}: {}" - .format(domain.name, output)) + .format(domain.name, smart_text(output))) domain.dkim_private_key_path = pkey_path code, output = sysutils.exec_cmd( "openssl rsa -in {} -pubout".format(pkey_path)) if code: print("Failed to generate DKIM public key for domain {}: {}" - .format(domain.name, output)) - public_key = b"" - for cpt, line in enumerate(output.splitlines()): - if cpt == 0 or line.startswith(b"-----"): + .format(domain.name, smart_text(output))) + public_key = "" + for cpt, line in enumerate(smart_text(output).splitlines()): + if cpt == 0 or line.startswith("-----"): continue public_key += line domain.dkim_public_key = public_key diff --git a/modoboa/admin/tests/test_domain.py b/modoboa/admin/tests/test_domain.py index a161ffde5..cabab25e3 100644 --- a/modoboa/admin/tests/test_domain.py +++ b/modoboa/admin/tests/test_domain.py @@ -10,12 +10,15 @@ import tempfile import dns.resolver from mock import patch +from testfixtures import compare from django.core.management import call_command from django.urls import reverse +from django.utils.html import escape from modoboa.core import factories as core_factories from modoboa.core.models import User +from modoboa.core.tests.test_views import SETTINGS_SAMPLE from modoboa.lib.tests import ModoTestCase from . import utils @@ -371,6 +374,21 @@ class DKIMTestCase(ModoTestCase): def tearDown(self): shutil.rmtree(self.workdir) + def test_global_settings(self): + """Check validation rules.""" + url = reverse("core:parameters") + settings = SETTINGS_SAMPLE.copy() + settings["admin-dkim_keys_storage_dir"] = "/wrong" + response = self.client.post(url, settings, format="json") + self.assertEqual(response.status_code, 400) + compare(response.json(), { + "form_errors": {"dkim_keys_storage_dir": ["Directory not found."]}, + "prefix": "admin" + }) + settings["admin-dkim_keys_storage_dir"] = self.workdir + response = self.client.post(url, settings, format="json") + self.assertEqual(response.status_code, 200) + def test_dkim_key_creation(self): """Test DKIM key creation.""" values = { @@ -395,3 +413,9 @@ class DKIMTestCase(ModoTestCase): call_command("modo", "manage_dkim_keys") key_path = os.path.join(self.workdir, "{}.pem".format(values["name"])) self.assertTrue(os.path.exists(key_path)) + + domain = Domain.objects.get(name=values["name"]) + url = reverse("admin:domain_detail", args=[domain.pk]) + response = self.client.get(url) + self.assertContains( + response, escape(domain.bind_format_dkim_public_key)) diff --git a/modoboa/core/tests/test_views.py b/modoboa/core/tests/test_views.py index 7e06ad910..c9d5a004a 100644 --- a/modoboa/core/tests/test_views.py +++ b/modoboa/core/tests/test_views.py @@ -15,6 +15,63 @@ from .. import factories from .. import models from .. import signals +SETTINGS_SAMPLE = { + "core-check_new_versions": "True", + "core-log_maximum_age": "365", + "core-ldap_auth_method": "searchbind", + "limits-deflt_domain_mailbox_aliases_limit": "0", + "core-send_statistics": "True", + "core-default_top_redirection": "user", + "core-sender_address": "noreply@modoboa.org", + "admin-auto_account_removal": "False", + "core-ldap_server_port": "389", + "core-rounds_number": "70000", + "core-ldap_bind_password": "", + "limits-deflt_domain_domain_aliases_limit": "0", + "core-secret_key": ":?j3]QPWo!.'_c4n", + "limits-deflt_domain_domain_admins_limit": "0", + "limits-enable_admin_limits": "True", + "core-ldap_bind_dn": "", + "core-enable_api_communication": "True", + "limits-deflt_user_domain_admins_limit": "0", + "limits-enable_domain_limits": "False", + "csrfmiddlewaretoken": "SGgMVZsA4TPqoiV786TMST6xgOlhAf4F", + "limits-deflt_user_mailboxes_limit": "0", + "core-password_scheme": "sha512crypt", + "core-items_per_page": "30", + "limits-deflt_user_mailbox_aliases_limit": "0", + "limits-deflt_domain_mailboxes_limit": "0", + "core-ldap_is_active_directory": "False", + "core-ldap_group_type": "posixgroup", + "limits-deflt_user_domain_aliases_limit": "0", + "core-top_notifications_check_interval": "30", + "core-ldap_password_attribute": "userPassword", + "admin-auto_create_domain_and_mailbox": "True", + "admin-enable_dnsbl_checks": "True", + "admin-default_domain_quota": "0", + "admin-default_mailbox_quota": "0", + "core-ldap_server_address": "localhost", + "core-authentication_type": "local", + "core-ldap_admin_groups": "", + "core-ldap_groups_search_base": "", + "admin-enable_mx_checks": "True", + "core-ldap_search_base": "", + "admin-valid_mxs": "", + "limits-deflt_user_domains_limit": "0", + "core-ldap_search_filter": "(mail=%(user)s)", + "core-ldap_secured": "none", + "core-ldap_user_dn_template": "", + "core-default_password": "Toto1000", + "limits-deflt_user_quota_limit": "0", + "core-hide_features_widget": "False", + "core-inactive_account_threshold": "30", + "admin-domains_must_have_authorized_mx": False, + "core-random_password_length": "8", + "admin-create_alias_on_mbox_rename": False, + "admin-dkim_keys_storage_dir": "", + "admin-dkim_default_key_length": 1024 +} + def announcement(sender, location, **kwargs): """Simpler handler.""" @@ -106,63 +163,6 @@ class DashboardTestCase(ModoTestCase): class SettingsTestCase(ModoTestCase): """Settings tests.""" - settings_sample = { - "core-check_new_versions": "True", - "core-log_maximum_age": "365", - "core-ldap_auth_method": "searchbind", - "limits-deflt_domain_mailbox_aliases_limit": "0", - "core-send_statistics": "True", - "core-default_top_redirection": "user", - "core-sender_address": "noreply@modoboa.org", - "admin-auto_account_removal": "False", - "core-ldap_server_port": "389", - "core-rounds_number": "70000", - "core-ldap_bind_password": "", - "limits-deflt_domain_domain_aliases_limit": "0", - "core-secret_key": ":?j3]QPWo!.'_c4n", - "limits-deflt_domain_domain_admins_limit": "0", - "limits-enable_admin_limits": "True", - "core-ldap_bind_dn": "", - "core-enable_api_communication": "True", - "limits-deflt_user_domain_admins_limit": "0", - "limits-enable_domain_limits": "False", - "csrfmiddlewaretoken": "SGgMVZsA4TPqoiV786TMST6xgOlhAf4F", - "limits-deflt_user_mailboxes_limit": "0", - "core-password_scheme": "sha512crypt", - "core-items_per_page": "30", - "limits-deflt_user_mailbox_aliases_limit": "0", - "limits-deflt_domain_mailboxes_limit": "0", - "core-ldap_is_active_directory": "False", - "core-ldap_group_type": "posixgroup", - "limits-deflt_user_domain_aliases_limit": "0", - "core-top_notifications_check_interval": "30", - "core-ldap_password_attribute": "userPassword", - "admin-auto_create_domain_and_mailbox": "True", - "admin-enable_dnsbl_checks": "True", - "admin-default_domain_quota": "0", - "admin-default_mailbox_quota": "0", - "core-ldap_server_address": "localhost", - "core-authentication_type": "local", - "core-ldap_admin_groups": "", - "core-ldap_groups_search_base": "", - "admin-enable_mx_checks": "True", - "core-ldap_search_base": "", - "admin-valid_mxs": "", - "limits-deflt_user_domains_limit": "0", - "core-ldap_search_filter": "(mail=%(user)s)", - "core-ldap_secured": "none", - "core-ldap_user_dn_template": "", - "core-default_password": "Toto1000", - "limits-deflt_user_quota_limit": "0", - "core-hide_features_widget": "False", - "core-inactive_account_threshold": "30", - "admin-domains_must_have_authorized_mx": False, - "core-random_password_length": "8", - "admin-create_alias_on_mbox_rename": False, - "admin-dkim_keys_storage_dir": "", - "admin-dkim_default_key_length": 1024 - } - def test_get_settings(self): """Test settings display.""" url = reverse("core:parameters") @@ -173,10 +173,11 @@ class SettingsTestCase(ModoTestCase): def test_save_settings(self): """Test settings save.""" url = reverse("core:parameters") - response = self.client.post(url, self.settings_sample, format="json") + settings = SETTINGS_SAMPLE.copy() + response = self.client.post(url, settings, format="json") self.assertEqual(response.status_code, 200) - self.settings_sample["core-rounds_number"] = "" - response = self.client.post(url, self.settings_sample, format="json") + settings["core-rounds_number"] = "" + response = self.client.post(url, settings, format="json") self.assertEqual(response.status_code, 400) compare(response.json(), { "form_errors": {"rounds_number": ["This field is required."]}, From ff2c019fc662389766da4b7cd422c3e069b74823 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 19 Jan 2018 09:56:24 +0100 Subject: [PATCH 7/8] Few fixes. --- doc/configuration.rst | 14 ++++++++++++++ doc/manual_installation/modoboa.rst | 3 +++ modoboa/admin/app_settings.py | 2 +- .../admin/migrations/0012_auto_20180112_1658.py | 2 +- modoboa/admin/models/domain.py | 2 +- 5 files changed, 20 insertions(+), 3 deletions(-) diff --git a/doc/configuration.rst b/doc/configuration.rst index dbaed6d09..628261179 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -103,6 +103,20 @@ The *admin* application exposes several parameters, they are presented below: | | | | | | | | | | +--------------------+---------------------+-------------------------------+--------------------+ +|DKIM keys storage |Admin |Path to a directory where DKIM | | +|directory | | generated keys will be stored | | +| | | | | +| | | | | +| | | | | +| | | | | ++--------------------+---------------------+-------------------------------+--------------------+ +|Default DKIM key |Admin |The default size (in bits) for |2048 | +|length | |new keys | | +| | | | | +| | | | | +| | | | | +| | | | | ++--------------------+---------------------+-------------------------------+--------------------+ |Handle mailboxes on |Admin |Rename or remove |no | |filesystem | |mailboxes on the | | | | |filesystem when they | | diff --git a/doc/manual_installation/modoboa.rst b/doc/manual_installation/modoboa.rst index d2842360b..5387a851b 100644 --- a/doc/manual_installation/modoboa.rst +++ b/doc/manual_installation/modoboa.rst @@ -190,5 +190,8 @@ following content inside:: # Public API communication 0 * * * * root $PYTHON $INSTANCE/manage.py communicate_with_public_api + # Generate DKIM keys (they will belong to the user running this job) + * * * * * root $PYTHON $INSTANCE/manage.py modo manage_dkim_keys + Now you can continue to the :ref:`webserver` section. diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py index 6ce1b66f4..453bc49c7 100644 --- a/modoboa/admin/app_settings.py +++ b/modoboa/admin/app_settings.py @@ -73,7 +73,7 @@ class AdminParametersForm(param_forms.AdminParametersForm): initial=1024, choices=constants.DKIM_KEY_LENGTHS, help_text=ugettext_lazy( - "Default length for newly generated DKIM keys." + "Default length in bits for newly generated DKIM keys." ) ) diff --git a/modoboa/admin/migrations/0012_auto_20180112_1658.py b/modoboa/admin/migrations/0012_auto_20180112_1658.py index ba9f1f071..a69e64388 100644 --- a/modoboa/admin/migrations/0012_auto_20180112_1658.py +++ b/modoboa/admin/migrations/0012_auto_20180112_1658.py @@ -20,7 +20,7 @@ class Migration(migrations.Migration): migrations.AddField( model_name='domain', name='dkim_key_selector', - field=models.CharField(default='default', max_length=30, verbose_name='Key selector'), + field=models.CharField(default='modoboa', max_length=30, verbose_name='Key selector'), ), migrations.AddField( model_name='domain', diff --git a/modoboa/admin/models/domain.py b/modoboa/admin/models/domain.py index 30a9e0219..fa0337a3a 100644 --- a/modoboa/admin/models/domain.py +++ b/modoboa/admin/models/domain.py @@ -65,7 +65,7 @@ class Domain(AdminObject): "generated for this domain."), default=False ) - dkim_key_selector = models.CharField(max_length=30, default="default") + dkim_key_selector = models.CharField(max_length=30, default="modoboa") dkim_key_length = models.PositiveIntegerField( ugettext_lazy("Key length"), choices=constants.DKIM_KEY_LENGTHS, blank=True, null=True From f6528adb719c2c3c13f0fbf5994e36f2bf4e34e2 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 19 Jan 2018 10:37:59 +0100 Subject: [PATCH 8/8] Set 2048 bits as the default length. --- modoboa/admin/app_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py index 453bc49c7..a72dd7fff 100644 --- a/modoboa/admin/app_settings.py +++ b/modoboa/admin/app_settings.py @@ -70,7 +70,7 @@ class AdminParametersForm(param_forms.AdminParametersForm): dkim_default_key_length = forms.ChoiceField( label=ugettext_lazy("Default DKIM key length"), - initial=1024, + initial=2048, choices=constants.DKIM_KEY_LENGTHS, help_text=ugettext_lazy( "Default length in bits for newly generated DKIM keys."