diff --git a/certbot-postfix/Dockerfile b/certbot-postfix/Dockerfile index d59c72ff5..af478ed5f 100644 --- a/certbot-postfix/Dockerfile +++ b/certbot-postfix/Dockerfile @@ -20,7 +20,6 @@ COPY docker-files/rsyslog.conf /etc/rsyslog.conf COPY certbot_postfix/ certbot_postfix/ COPY setup.py setup.py COPY requirements.txt requirements.txt -RUN pip install --no-cache-dir -e /opt/certbot/src/acme -e /opt/certbot/src -e . RUN pip install --no-cache-dir --editable . RUN pip install -r requirements.txt diff --git a/certbot-postfix/certbot_postfix/constants.py b/certbot-postfix/certbot_postfix/constants.py index 72454dbdf..4f06e1a05 100644 --- a/certbot-postfix/certbot_postfix/constants.py +++ b/certbot-postfix/certbot_postfix/constants.py @@ -11,32 +11,50 @@ MINIMUM_VERSION = (2, 11,) # Certbot will default to the first value in the tuple, but will # not override "more secure" settings. -ACCEPTABLE_SECURITY_LEVELS = ("may", "encrypt") +ACCEPTABLE_SERVER_SECURITY_LEVELS = ("may", "encrypt") +ACCEPTABLE_CLIENT_SECURITY_LEVELS = ("may", "encrypt", + "dane", "dane-only", + "fingerprint", + "verify", "secure") ACCEPTABLE_CIPHER_LEVELS = ("medium", "high") +EXCLUDE_CIPHERS = ("aNULL, eNULL, EXPORT, DES, RC4, MD5, PSK, aECDH, " + "EDH-DSS-DES-CBC3-SHA, EDH-RSA-DES-CBC3-SHA, KRB5-DES, CBC3-SHA") + TLS_VERSIONS = ("SSLv2", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2") # Should NOT use SSLv2/3. ACCEPTABLE_TLS_VERSIONS = ("TLSv1", "TLSv1.1", "TLSv1.2") +# Variables associated with enabling opportunistic TLS. +TLS_SERVER_VARS = { + "smtpd_tls_auth_only": "yes", + "smtpd_tls_security_level": ACCEPTABLE_SERVER_SECURITY_LEVELS, +} # Default variables for a secure MTA server [receiver]. DEFAULT_SERVER_VARS = { "smtpd_tls_mandatory_protocols": "!SSLv2, !SSLv3", "smtpd_tls_protocols": "!SSLv2, !SSLv3", - "smtpd_tls_security_level": ACCEPTABLE_SECURITY_LEVELS, "smtpd_tls_ciphers": ACCEPTABLE_CIPHER_LEVELS, + "smtpd_tls_mandatory_ciphers": ACCEPTABLE_CIPHER_LEVELS, + "smtpd_exclude_ciphers": EXCLUDE_CIPHERS, "smtpd_tls_eecdh_grade": "strong", } # Default variables for a secure MTA client [sender]. DEFAULT_CLIENT_VARS = { - "smtp_tls_security_level": ACCEPTABLE_SECURITY_LEVELS, + "smtp_tls_security_level": ACCEPTABLE_CLIENT_SECURITY_LEVELS, "smtp_tls_ciphers": ACCEPTABLE_CIPHER_LEVELS, + "smtp_exclude_ciphers": EXCLUDE_CIPHERS, + "smtp_tls_mandatory_ciphers": ACCEPTABLE_CIPHER_LEVELS, } CLI_DEFAULTS = dict( config_dir="/etc/postfix", ctl="postfix", config_utility="postconf", + tls_only=False, + ignore_master_overrides=False, + server_only=False, policy_file=POLICY_FILENAME, ) """CLI defaults.""" diff --git a/certbot-postfix/certbot_postfix/installer.py b/certbot-postfix/certbot_postfix/installer.py index e7707359f..f3e2df473 100644 --- a/certbot-postfix/certbot_postfix/installer.py +++ b/certbot-postfix/certbot_postfix/installer.py @@ -1,10 +1,9 @@ -"""Certbot installer plugin for Postfix.""" +"""certbot installer plugin for postfix.""" import logging import os -from functools import partial - import zope.interface +import zope.component import six from certbot import errors @@ -52,6 +51,12 @@ class Installer(plugins_common.Installer): help="Path to the 'postconf' executable.") add("policy-file", default=constants.CLI_DEFAULTS["policy_file"], help="Name of the policy file that we should write to in config-dir.") + add("tls-only", default=constants.CLI_DEFAULTS["tls_only"], + help="Only set params to enable opportunistic TLS and install certificates.") + add("server-only", default=constants.CLI_DEFAULTS["server_only"], + help="Only set server params (prefixed with smtpd*)") + add("ignore-master-overrides", default=constants.CLI_DEFAULTS["ignore_master_overrides"], + help="Ignore errors reporting overridden TLS parameters in master.cf.") def _verify_setup(self): # TODO (sydneyli): do this @@ -74,9 +79,10 @@ class Installer(plugins_common.Installer): self.postfix = None self.policy_file = None self._enhance_func = {"starttls-policy": self._enable_policy_list} - # Since we only need to enable policy once for all domains, + # Since we only need to enable TLS or the STARTTLS policy once for all domains, # keep track of whether this enhancement was already called. self._starttls_policy_enabled = False + self._tls_enabled = False def _ensure_ca_certificates_exist(self): # TODO (sydneyli): This might block starttls-everywhere @@ -109,7 +115,11 @@ class Installer(plugins_common.Installer): # Set up CLI tools self.postfix = util.PostfixUtil(self.conf('config-dir')) - self.postconf = postconf.ConfigMain(self.conf('config-utility')) + report_master_overrides = util.report_master_overrides + if self.conf('ignore-master-overrides'): + report_master_overrides = None + self.postconf = postconf.ConfigMain(self.conf('config-utility'), + report_master_overrides) # Ensure current configuration is valid. self.config_test() @@ -118,6 +128,7 @@ class Installer(plugins_common.Installer): self._check_version() self._lock_config_dir() self.policy_file = os.path.join(self.conf('config-dir'), self.conf('policy-file')) + self.install_ssl_dhparams() def config_test(self): """Test to see that the current Postfix configuration is valid. @@ -186,17 +197,32 @@ class Installer(plugins_common.Installer): return certbot_util.get_filtered_names(self.postconf.get(var) for var in ('mydomain', 'myhostname', 'myorigin',)) + def _set_vars(self, var_dict): """Sets all parameters in var_dict to config file. """ for param, acceptable in six.iteritems(var_dict): - if isinstance(acceptable, tuple): - if self.postconf.get(param) not in acceptable: - self.postconf.set(param, acceptable[0], - partial(util.report_master_overrides, acceptable_overrides=acceptable)) - else: - self.postconf.set(param, acceptable, - partial(util.report_master_overrides, acceptable_overrides=acceptable)) + if util.is_acceptable_value(param, self.postconf.get(param), acceptable): + if isinstance(acceptable, tuple): + self.postconf.set(param, acceptable[0], acceptable) + else: + self.postconf.set(param, acceptable, acceptable) + + def _confirm_changes(self): + """Confirming outstanding updates for configuration parameters. + + :raises errors.PluginError: when user rejects the configuration changes. + """ + updates = self.postconf.get_changes() + output_string = "Postfix TLS configuration parameters to update in main.cf:\n" + for name, value in six.iteritems(updates): + output_string += "{0} = {1}\n".format(name, value) + output_string += "Is this okay?\n" + if not zope.component.getUtility(interfaces.IDisplay).yesno(output_string): + raise errors.PluginError( + "Manually rejected configuration changes.\n" + "Try using --tls-only or --server-only to change a particular" + "subset of configuration parameters.") def deploy_cert(self, domain, cert_path, key_path, chain_path, fullchain_path): @@ -213,15 +239,22 @@ class Installer(plugins_common.Installer): """ # pylint: disable=unused-argument + if self._tls_enabled: + return + self._tls_enabled = True self.save_notes.append("Configuring TLS for {0}".format(domain)) - self.postconf.set("smtpd_tls_cert_file", cert_path, - check_override=util.report_master_overrides) - self.postconf.set("smtpd_tls_key_file", key_path, - check_override=util.report_master_overrides) - self._set_vars(constants.DEFAULT_SERVER_VARS) - self._set_vars(constants.DEFAULT_CLIENT_VARS) - self.postconf.set("smtp_tls_CApath", constants.CA_CERTS_PATH, - check_override=util.report_master_overrides) + self.postconf.set("smtpd_tls_cert_file", cert_path) + self.postconf.set("smtpd_tls_key_file", key_path) + # self.postconf.set("smtpd_tls_security_level", ACCEPTABLE_SECURITY_LEVELS) + self._set_vars(constants.TLS_SERVER_VARS) + if not self.conf('tls_only'): + self._set_vars(constants.DEFAULT_SERVER_VARS) + # Despite the name, this option also supports 2048-bit DH params. + # http://www.postfix.org/FORWARD_SECRECY_README.html#server_fs + self.postconf.set("smtpd_tls_dh1024_param_file", self.ssl_dhparams) + if not self.conf('server_only'): + self._set_vars(constants.DEFAULT_CLIENT_VARS) + self._confirm_changes() def _enable_policy_list(self, domain, options): # pylint: disable=unused-argument @@ -240,6 +273,7 @@ class Installer(plugins_common.Installer): util.write_domainwise_tls_policies(policy, self.policy_file) policy_cf_entry = "texthash:" + self.policy_file self.postconf.set("smtp_tls_policy_maps", policy_cf_entry) + self.postconf.set("smtp_tls_CApath", constants.CA_CERTS_PATH) def enhance(self, domain, enhancement, options=None): """Raises an exception for request for unsupported enhancement. diff --git a/certbot-postfix/certbot_postfix/postconf.py b/certbot-postfix/certbot_postfix/postconf.py index a28a1b1e7..d327b23f0 100644 --- a/certbot-postfix/certbot_postfix/postconf.py +++ b/certbot-postfix/certbot_postfix/postconf.py @@ -9,8 +9,9 @@ class ConfigMain(util.PostfixUtilBase): """A parser for Postfix's main.cf file.""" - def __init__(self, executable, config_dir=None): + def __init__(self, executable, handle_overrides, config_dir=None): super(ConfigMain, self).__init__(executable, config_dir) + self._handle_overrides = handle_overrides self._db = {} # List of current master.cf overrides from Postfix config. Dictionary # of parameter name => list of tuples (service name, paramter value) @@ -65,22 +66,25 @@ class ConfigMain(util.PostfixUtilBase): return self._master_db[name] return None - def set(self, name, value, check_override=None): + def set(self, name, value, acceptable_overrides=None): """Sets parameter `name` to `value`. If `name` is overridden by a particular service in `master.cf`, calls - `check_override` on `name`, and the set of overrides. + `self.handle_overrides` on `name`, and the set of overrides. Note that this function does not flush these parameter values to main.cf; To do that, use `flush`. :param str name: The name of the parameter to set. :param str value: The value of the parameter. + :param list acceptable_overrides: + If the master configuration file overrides `value` with a value in + acceptable_overrides, no need to call `_handle_overrides`. """ if name not in self._db: raise KeyError("Parameter name %s is not a valid Postfix parameter name.", name) # Check to see if this parameter is overridden by master. overrides = self.get_master_overrides(name) - if check_override is not None and overrides is not None: - check_override(name, overrides) + if overrides is not None: + self._handle_overrides(name, overrides, acceptable_overrides) if value != self._db[name]: # _db contains the "original" state of parameters. We only care about # writes if they cause a delta from the original state. @@ -107,6 +111,11 @@ class ConfigMain(util.PostfixUtilBase): self._db[name] = value self._updated = {} + def get_changes(self): + """ Return queued changes to main.cf. + """ + return self._updated + def _parse_main_output(output): """Parses the raw output from Postconf about main.cf. Expects the output to look like: diff --git a/certbot-postfix/certbot_postfix/tests/installer_test.py b/certbot-postfix/certbot_postfix/tests/installer_test.py index a6f55bf5a..a4ef75012 100644 --- a/certbot-postfix/certbot_postfix/tests/installer_test.py +++ b/certbot-postfix/certbot_postfix/tests/installer_test.py @@ -20,12 +20,17 @@ class InstallerTest(certbot_test_util.ConfigTestCase): self.config.postfix_config_dir = self.tempdir self.config.postfix_config_utility = "postconf" self.config.postfix_policy_file = os.path.join(self.tempdir, "config.json") + self.config.config_dir = self.tempdir shutil.copyfile(_config_file, self.config.postfix_policy_file) self.mock_postfix = MockPostfix() self.mock_postconf = MockPostconf(self.tempdir, {"mail_version": "3.1.4"}) + def test_confirm_changes(self): + pass + def test_add_parser_arguments(self): - options = set(('ctl', 'config-dir', 'config-utility', 'policy-file',)) + options = set(('ctl', 'config-dir', 'config-utility', 'policy-file', + 'tls-only', 'server-only', 'ignore-master-overrides')) mock_add = mock.MagicMock() from certbot_postfix import installer @@ -92,6 +97,8 @@ class InstallerTest(certbot_test_util.ConfigTestCase): :param str domain: domain to deploy cert for """ + # pylint: disable=protected-access + installer._confirm_changes = lambda: "noop" installer.deploy_cert(domain, "foo", "bar", "baz", "qux") self._mock_postfix_and_call(deploy_cert, "example.org") diff --git a/certbot-postfix/certbot_postfix/tests/postconf_test.py b/certbot-postfix/certbot_postfix/tests/postconf_test.py index 05a3e21f5..29512f0eb 100644 --- a/certbot-postfix/certbot_postfix/tests/postconf_test.py +++ b/certbot-postfix/certbot_postfix/tests/postconf_test.py @@ -1,20 +1,16 @@ """Tests for certbot_postfix.postconf.""" import mock -import os -import pkg_resources -import shutil import unittest -from certbot.tests import util as test_util - class PostConfTest(unittest.TestCase): """Tests for certbot_postfix.util.PostConf.""" def setUp(self): from certbot_postfix.postconf import ConfigMain super(PostConfTest, self).setUp() with mock.patch('certbot_postfix.util.PostfixUtilBase._get_output') as mock_call: - with mock.patch('certbot_postfix.postconf.ConfigMain._get_output_master') as mock_master_call: + with mock.patch('certbot_postfix.postconf.ConfigMain._get_output_master') as \ + mock_master_call: with mock.patch('certbot_postfix.postconf.util.verify_exe_exists') as verify_exe: verify_exe.return_value = True mock_call.return_value = ('default_parameter = value\n' @@ -22,9 +18,9 @@ class PostConfTest(unittest.TestCase): 'overridden_by_master = default\n') mock_master_call.return_value = ( 'service/type/overridden_by_master = master_value\n' - 'service2/type/overridden_by_master = master_value2' + 'service2/type/overridden_by_master = master_value2\n' ) - self.config = ConfigMain('postconf', '') + self.config = ConfigMain('postconf', lambda x, y, z: None) @mock.patch('certbot_postfix.util.PostfixUtilBase._get_output') def test_read_defalut(self, mock_get_output): @@ -63,13 +59,13 @@ class PostConfTest(unittest.TestCase): expected_overrides = [ ('service/type', 'master_value'), ('service2/type', 'master_value2')] - def _check_overrides(name, overrides): + def _check_overrides(name, overrides, acceptable_overrides): + # pylint: disable=unused-argument self.assertEqual('overridden_by_master', name) self.assertEqual(expected_overrides, overrides) - self.config.set('overridden_by_master', 'new_value', check_override=_check_overrides) - self.assertEqual(self.config.get_master_overrides('overridden_by_master'), - [('service/type', 'master_value'), - ('service2/type', 'master_value2')]) + # pylint: disable=protected-access + self.config._handle_overrides = _check_overrides + self.config.set('overridden_by_master', 'new_value') if __name__ == '__main__': # pragma: no cover unittest.main() diff --git a/certbot-postfix/certbot_postfix/tests/util_test.py b/certbot-postfix/certbot_postfix/tests/util_test.py index 257a26a69..3b7129d6e 100644 --- a/certbot-postfix/certbot_postfix/tests/util_test.py +++ b/certbot-postfix/certbot_postfix/tests/util_test.py @@ -96,5 +96,11 @@ class VerifyExeExistsTest(unittest.TestCase): mock_path_surgery.return_value = True self._call('foo') +class WriteDomainwiseTlsPoliciesTest(unittest.TestCase): + pass + +class ReportMasterOverridesTest(unittest.TestCase): + pass + if __name__ == '__main__': # pragma: no cover unittest.main() diff --git a/certbot-postfix/certbot_postfix/util.py b/certbot-postfix/certbot_postfix/util.py index ba4ac9468..9a769aa4f 100644 --- a/certbot-postfix/certbot_postfix/util.py +++ b/certbot-postfix/certbot_postfix/util.py @@ -263,8 +263,37 @@ def report_master_overrides(name, overrides, acceptable_overrides=None): another service is overriding our parameter with a more secure option, we don't have to warn. If this is set to None, warnings are reported for *all* overrides! """ + error_string = "" for override in overrides: - if acceptable_overrides is None or override not in acceptable_overrides: - logger.warning("Parameter {0} is overridden as {1} for service {2} in " + - "master configuration file!", name, override[1], override[0]) + service, value = override + # If this override is acceptable: + if acceptable_overrides is not None and \ + _is_acceptable_value(name, value, acceptable_overrides): + continue + error_string += " {1}: {2}\n".format(service, value) + if len(error_string) > 0: + raise errors.PluginError("{0} is overridden with less secure options by the " + "following services in master.cf:\n" + error_string) + +def is_acceptable_value(parameter, value, acceptable): + # If it's a tuple, there's multiple acceptable options. + # Only set a param if it's not acceptable. + if isinstance(acceptable, tuple): + if value not in acceptable: + return False + # Check if param value is a comma-separated list of protocols. + elif 'protocols' in parameter: + return _has_acceptable_tls_versions(value) + # Otherwise, just check whether the value is equal to acceptable. + return value == acceptable + +def _has_acceptable_tls_versions(parameter_string): + """ + Checks to see if the comma-separated list of TLS protocols to exclude is acceptable. + Sample string: "!SSLv2, !SSLv3" + """ + for bad_version in ("SSLv2", "SSLv3"): # TODO: subtract acceptable from tls-verions constant + if "!" + bad_version not in parameter_string: + return False + return True diff --git a/certbot-postfix/setup.py b/certbot-postfix/setup.py index 5377200f5..22757d31e 100644 --- a/certbot-postfix/setup.py +++ b/certbot-postfix/setup.py @@ -8,6 +8,7 @@ install_requires = [ 'certbot>0.23.0', 'setuptools', 'six', + 'zope.component', 'zope.interface', ] diff --git a/certbot-postfix/tests/run_tests.sh b/certbot-postfix/tests/run_tests.sh index 3178d2c86..362071dca 100755 --- a/certbot-postfix/tests/run_tests.sh +++ b/certbot-postfix/tests/run_tests.sh @@ -12,8 +12,6 @@ BASE_IMAGE=certbot_local docker network create -d bridge $NETWORKNAME || true # Build with all the changes. -# TODO: when changes from this branch land in master, -# we no longer need to re-build the base image. docker build -t $BASE_IMAGE -f ../Dockerfile ../ docker build -t $IMAGE_NAME . @@ -22,10 +20,10 @@ docker stop $SENDNAME || true docker stop $RCPTNAME || true docker run --rm --network=$NETWORKNAME \ - -itd --name $SENDNAME -h $SENDNAME -p 25 $IMAGE_NAME + -d --name $SENDNAME -h $SENDNAME $IMAGE_NAME docker run --rm --network=$NETWORKNAME \ - -itd --name $RCPTNAME -h $RCPTNAME -p 25 $IMAGE_NAME + -d --name $RCPTNAME -h $RCPTNAME $IMAGE_NAME docker_do() { docker exec ${1} /bin/sh -c ". ./tests/setup.sh && ${2}" @@ -48,13 +46,13 @@ both_do "setup && install_certs valid" echo "Regular mail over TLS..." sender_do "echo -e 'Subject: Subject\n\nbody' | sendmail root@${RCPTNAME}" sleep 1 -recipient_do "cat /var/mail/root | grep \"TLS\"" +recipient_do "grep \"TLS\" /var/mail/root" echo "Mail NOT sent over TLS..." recipient_do "rm /var/mail/root" recipient_do uninstall_certs sender_do "echo -e 'Subject: Subject\n\nbody' | sendmail root@${RCPTNAME}" -recipient_do "[ -f /var/mail/root ] && ! (cat /var/mail/root | grep \"TLS\")" +recipient_do "[ -f /var/mail/root ] && ! (grep \"TLS\" /var/mail/root)" echo "Mail NOT sent over TLS if policy configured poorly..." sender_do "install_certs valid --starttls-policy /opt/certbot-postfix/testdata/recipient_policy.json" @@ -64,7 +62,7 @@ sender_do "mailq | grep \"TLS is required, but was not offered\"" echo "Mail NOT sent over TLS if cert name wrong..." recipient_do "install_certs evil" sender_do "echo -e 'Subject: Subject\n\nbody' | sendmail root@${RCPTNAME}" -sender_do "mailq | grep \"Server certificate not verified\"" +sender_do "mailq | grep \"Server certificate not trusted\"" echo "Mail NOT sent over TLS if certs root not trusted..." recipient_do "install_certs self-signed" @@ -75,5 +73,5 @@ echo "Mail sent over TLS if policy configured properly..." recipient_do "install_certs valid" sender_do "echo -e 'Subject: Subject\n\nbody' | sendmail root@${RCPTNAME}" sleep 1 -recipient_do "cat /var/mail/root | grep \"TLS\"" +recipient_do "grep \"TLS\" /var/mail/root" diff --git a/certbot-postfix/tests/setup.sh b/certbot-postfix/tests/setup.sh index 7eb4fa689..eb94e81bc 100755 --- a/certbot-postfix/tests/setup.sh +++ b/certbot-postfix/tests/setup.sh @@ -28,7 +28,7 @@ install_certs() { # Install certs via certbot! cert_name=$1 shift - certbot install --installer certbot-postfix:postfix \ + certbot install --installer postfix \ --cert-path /etc/certificates/$cert_name.crt --key-path /etc/certificates/$cert_name.key \ -d recipient.com ${@} }