diff --git a/certbot-postfix/certbot_postfix/constants.py b/certbot-postfix/certbot_postfix/constants.py index 8721ef5f2..a8e6dd811 100644 --- a/certbot-postfix/certbot_postfix/constants.py +++ b/certbot-postfix/certbot_postfix/constants.py @@ -14,25 +14,27 @@ ACCEPTABLE_CLIENT_SECURITY_LEVELS = ("may", "encrypt", "verify", "secure") ACCEPTABLE_CIPHER_LEVELS = ("medium", "high") +# Exporting certain ciphers to prevent logjam: https://weakdh.org/sysadmin.html 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_auth_only": "yes", "smtpd_tls_mandatory_protocols": "!SSLv2, !SSLv3", "smtpd_tls_protocols": "!SSLv2, !SSLv3", "smtpd_tls_ciphers": ACCEPTABLE_CIPHER_LEVELS, "smtpd_tls_mandatory_ciphers": ACCEPTABLE_CIPHER_LEVELS, - "smtpd_exclude_ciphers": EXCLUDE_CIPHERS, + "smtpd_tls_exclude_ciphers": EXCLUDE_CIPHERS, "smtpd_tls_eecdh_grade": "strong", } @@ -40,7 +42,7 @@ DEFAULT_SERVER_VARS = { DEFAULT_CLIENT_VARS = { "smtp_tls_security_level": ACCEPTABLE_CLIENT_SECURITY_LEVELS, "smtp_tls_ciphers": ACCEPTABLE_CIPHER_LEVELS, - "smtp_exclude_ciphers": EXCLUDE_CIPHERS, + "smtp_tls_exclude_ciphers": EXCLUDE_CIPHERS, "smtp_tls_mandatory_ciphers": ACCEPTABLE_CIPHER_LEVELS, } diff --git a/certbot-postfix/certbot_postfix/installer.py b/certbot-postfix/certbot_postfix/installer.py index e599e32d4..efaeb1e7b 100644 --- a/certbot-postfix/certbot_postfix/installer.py +++ b/certbot-postfix/certbot_postfix/installer.py @@ -44,11 +44,11 @@ class Installer(plugins_common.Installer): "default configuration paths.") add("config-utility", default=constants.CLI_DEFAULTS["config_utility"], help="Path to the 'postconf' executable.") - add("tls-only", default=constants.CLI_DEFAULTS["tls_only"], + add("tls-only", action="store_true", 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"], + add("server-only", action="store_true", 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"], + add("ignore-master-overrides", action="store_true", default=constants.CLI_DEFAULTS["ignore_master_overrides"], help="Ignore errors reporting overridden TLS parameters in master.cf.") def __init__(self, *args, **kwargs): @@ -86,11 +86,9 @@ class Installer(plugins_common.Installer): # Set up CLI tools self.postfix = util.PostfixUtil(self.conf('config-dir')) - 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) + self.conf('ignore-master-overrides'), + self.conf('config-dir')) # Ensure current configuration is valid. self.config_test() @@ -113,7 +111,7 @@ class Installer(plugins_common.Installer): """ if self._get_version() < constants.MINIMUM_VERSION: version_string = '.'.join([str(n) for n in constants.MINIMUM_VERSION]) - raise errors.NotSupportedError('Postfix version must be at least %s', version_string) + raise errors.NotSupportedError('Postfix version must be at least %s' % version_string) def _lock_config_dir(self): """Stop two Postfix plugins from modifying the config at once. @@ -126,7 +124,7 @@ class Installer(plugins_common.Installer): except (OSError, errors.LockError): logger.debug("Encountered error:", exc_info=True) raise errors.PluginError( - "Unable to lock %s", self.conf('config-dir')) + "Unable to lock %s" % self.conf('config-dir')) def more_info(self): """Human-readable string to help the user. @@ -188,7 +186,8 @@ class Installer(plugins_common.Installer): 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, default=False): + if not zope.component.getUtility(interfaces.IDisplay).yesno(output_string, + force_interactive=True, default=True): raise errors.PluginError( "Manually rejected configuration changes.\n" "Try using --tls-only or --server-only to change a particular" @@ -215,7 +214,6 @@ class Installer(plugins_common.Installer): self.save_notes.append("Configuring TLS for {0}".format(domain)) 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) @@ -266,7 +264,9 @@ class Installer(plugins_common.Installer): def recovery_routine(self): super(Installer, self).recovery_routine() - self.postconf = postconf.ConfigMain(self.conf('config-utility'), self.conf('config-dir')) + self.postconf = postconf.ConfigMain(self.conf('config-utility'), + self.conf('ignore-master-overrides'), + self.conf('config-dir')) def rollback_checkpoints(self, rollback=1): """Rollback saved checkpoints. @@ -278,7 +278,9 @@ class Installer(plugins_common.Installer): """ super(Installer, self).rollback_checkpoints(rollback) - self.postconf = postconf.ConfigMain(self.conf('config-utility'), self.conf('config-dir')) + self.postconf = postconf.ConfigMain(self.conf('config-utility'), + self.conf('ignore-master-overrides'), + self.conf('config-dir')) def restart(self): """Restart or refresh the server content. diff --git a/certbot-postfix/certbot_postfix/postconf.py b/certbot-postfix/certbot_postfix/postconf.py index da1e34da5..2f17ed069 100644 --- a/certbot-postfix/certbot_postfix/postconf.py +++ b/certbot-postfix/certbot_postfix/postconf.py @@ -8,9 +8,11 @@ from certbot_postfix import util class ConfigMain(util.PostfixUtilBase): """A parser for Postfix's main.cf file.""" - def __init__(self, executable, handle_overrides, config_dir=None): + def __init__(self, executable, ignore_master_overrides=False, config_dir=None): super(ConfigMain, self).__init__(executable, config_dir) - self._handle_overrides = handle_overrides + # Whether to ignore overrides from master. + self._ignore_master_overrides = ignore_master_overrides + print(self._ignore_master_overrides) # List of all current Postfix parameters, from `postconf` command. self._db = {} # List of current master.cf overrides from Postfix config. Dictionary @@ -40,8 +42,9 @@ class ConfigMain(util.PostfixUtilBase): def get_default(self, name): """Retrieves default value of parameter `name` from postfix parameters. - :param str name: The name of the parameter to fetch. - :rtype str: The default value of parameter `name`. + :param str name: The name of the parameter to fetch. + :returns: The default value of parameter `name`. + :rtype: str """ out = self._get_output(['-d', name]) _, value = next(_parse_main_output(out), (None, None)) @@ -50,7 +53,8 @@ class ConfigMain(util.PostfixUtilBase): def get(self, name): """Retrieves working value of parameter `name` from postfix parameters. :param str name: The name of the parameter to fetch. - :rtype str: The value of parameter `name`. + :returns: The value of parameter `name`. + :rtype: str """ if name in self._updated: return self._updated[name] @@ -59,9 +63,9 @@ class ConfigMain(util.PostfixUtilBase): def get_master_overrides(self, name): """Retrieves list of overrides for parameter `name` in postfix's Master config file. - :returns: List of tuples (service, value), meaning that parameter `name` - is overridden as `value` for `service`. - :rtype `list` of `tuple` of `str: + :returns: List of tuples (service, value), meaning that parameter `name` + is overridden as `value` for `service`. + :rtype: `list` of `tuple` of `str` """ if name in self._master_db: return self._master_db[name] @@ -69,23 +73,23 @@ class ConfigMain(util.PostfixUtilBase): 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 - `self.handle_overrides` on `name`, and the set of overrides. + If `name` is overridden by a particular service in `master.cf`, reports any + these parameter conflicts as long as `self._ignore_master_overrides` is not set. 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`. + :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 overrides is not None: - self._handle_overrides(name, overrides, acceptable_overrides) + if not self._ignore_master_overrides and overrides is not None: + util.report_master_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. @@ -97,7 +101,7 @@ class ConfigMain(util.PostfixUtilBase): def flush(self): """Flushes all parameter changes made using "self.set" to "main.cf". - :raises error.PluginError: When we can't flush to main.cf. + :raises error.PluginError: When we can't flush to main.cf. """ if len(self._updated) == 0: return @@ -127,8 +131,7 @@ def _parse_main_output(output): :param str output: data postconf wrote to stdout about main.cf :returns: generator providing key-value pairs from main.cf - :rtype: generator - + :rtype: Iterator[tuple(str, str)] """ for line in output.splitlines(): name, _, value = line.partition(" =") diff --git a/certbot-postfix/certbot_postfix/tests/installer_test.py b/certbot-postfix/certbot_postfix/tests/installer_test.py index 1b4532ff8..8243832ff 100644 --- a/certbot-postfix/certbot_postfix/tests/installer_test.py +++ b/certbot-postfix/certbot_postfix/tests/installer_test.py @@ -21,12 +21,12 @@ DEFAULT_MAIN_CF = { "smtpd_tls_mandatory_protocols": "", "smtpd_tls_protocols": "", "smtpd_tls_ciphers": "", - "smtpd_exclude_ciphers": "", + "smtpd_tls_exclude_ciphers": "", "smtpd_tls_mandatory_ciphers": "", "smtpd_tls_eecdh_grade": "medium", "smtp_tls_security_level": "", "smtp_tls_ciphers": "", - "smtp_exclude_ciphers": "", + "smtp_tls_exclude_ciphers": "", "smtp_tls_mandatory_ciphers": "", "mail_version": "3.2.3" } @@ -50,14 +50,14 @@ class InstallerTest(certbot_test_util.ConfigTestCase): self.config.postfix_server_only = False self.config.config_dir = self.tempdir - @mock.patch('certbot_postfix.installer.util.is_acceptable_value') + @mock.patch("certbot_postfix.installer.util.is_acceptable_value") def test_set_vars(self, mock_is_acceptable_value): mock_is_acceptable_value.return_value = True with create_installer(self.config) as installer: installer.prepare() mock_is_acceptable_value.return_value = False - @mock.patch('certbot_postfix.installer.util.is_acceptable_value') + @mock.patch("certbot_postfix.installer.util.is_acceptable_value") def test_acceptable_value(self, mock_is_acceptable_value): mock_is_acceptable_value.return_value = True with create_installer(self.config) as installer: @@ -70,8 +70,8 @@ class InstallerTest(certbot_test_util.ConfigTestCase): with create_installer(self.config) as installer: installer.prepare() self.assertRaises(errors.PluginError, installer.deploy_cert, - 'example.com', 'cert_path', 'key_path', - 'chain_path', 'fullchain_path') + "example.com", "cert_path", "key_path", + "chain_path", "fullchain_path") @certbot_test_util.patch_get_utility() def test_save(self, mock_util): @@ -80,7 +80,7 @@ class InstallerTest(certbot_test_util.ConfigTestCase): installer.prepare() installer.postconf.flush = mock.Mock() installer.reverter = mock.Mock() - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") installer.save() self.assertEqual(installer.save_notes, []) @@ -94,9 +94,9 @@ class InstallerTest(certbot_test_util.ConfigTestCase): installer.prepare() installer.postconf.flush = mock.Mock() installer.reverter = mock.Mock() - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") - installer.save(title='new_file!') + installer.save(title="new_file!") self.assertEqual(installer.reverter.finalize_checkpoint.call_count, 1) @certbot_test_util.patch_get_utility() @@ -104,7 +104,7 @@ class InstallerTest(certbot_test_util.ConfigTestCase): mock_util().yesno.return_value = True with create_installer(self.config) as installer: installer.prepare() - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") installer.rollback_checkpoints() self.assertEqual(installer.postconf.get_changes(), {}) @@ -114,7 +114,7 @@ class InstallerTest(certbot_test_util.ConfigTestCase): mock_util().yesno.return_value = True with create_installer(self.config) as installer: installer.prepare() - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") installer.recovery_routine() self.assertEqual(installer.postconf.get_changes(), {}) @@ -126,8 +126,8 @@ class InstallerTest(certbot_test_util.ConfigTestCase): self.assertEqual(installer.postfix.restart.call_count, 1) def test_add_parser_arguments(self): - options = set(('ctl', 'config-dir', 'config-utility', - 'tls-only', 'server-only', 'ignore-master-overrides')) + options = set(("ctl", "config-dir", "config-utility", + "tls-only", "server-only", "ignore-master-overrides")) mock_add = mock.MagicMock() from certbot_postfix import installer @@ -147,7 +147,7 @@ class InstallerTest(certbot_test_util.ConfigTestCase): installer.prepare) def test_old_version(self): - with create_installer(self.config, main_cf=_main_cf_with({'mail_version': '0.0.1'}))\ + with create_installer(self.config, main_cf=_main_cf_with({"mail_version": "0.0.1"}))\ as installer: self.assertRaises(errors.NotSupportedError, installer.prepare) @@ -183,15 +183,15 @@ class InstallerTest(certbot_test_util.ConfigTestCase): installer.prepare() # pylint: disable=protected-access - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") changes = installer.postconf.get_changes() expected = {} expected.update(constants.TLS_SERVER_VARS) expected.update(constants.DEFAULT_SERVER_VARS) expected.update(constants.DEFAULT_CLIENT_VARS) - self.assertEqual(changes['smtpd_tls_key_file'], 'key_path') - self.assertEqual(changes['smtpd_tls_cert_file'], 'cert_path') + self.assertEqual(changes["smtpd_tls_key_file"], "key_path") + self.assertEqual(changes["smtpd_tls_cert_file"], "cert_path") for name, value in six.iteritems(expected): if isinstance(value, tuple): self.assertEqual(changes[name], value[0]) @@ -203,20 +203,20 @@ class InstallerTest(certbot_test_util.ConfigTestCase): mock_util().yesno.return_value = True with create_installer(self.config) as installer: installer.prepare() - installer.conf = lambda x: x == 'tls_only' + installer.conf = lambda x: x == "tls_only" installer.postconf.set = mock.Mock() - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") - self.assertEqual(installer.postconf.set.call_count, 8) + self.assertEqual(installer.postconf.set.call_count, 7) @certbot_test_util.patch_get_utility() def test_server_only(self, mock_util): mock_util().yesno.return_value = True with create_installer(self.config) as installer: installer.prepare() - installer.conf = lambda x: x == 'server_only' + installer.conf = lambda x: x == "server_only" installer.postconf.set = mock.Mock() - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") self.assertEqual(installer.postconf.set.call_count, 11) @@ -227,9 +227,9 @@ class InstallerTest(certbot_test_util.ConfigTestCase): installer.prepare() installer.conf = lambda x: True installer.postconf.set = mock.Mock() - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") - self.assertEqual(installer.postconf.set.call_count, 4) + self.assertEqual(installer.postconf.set.call_count, 3) @certbot_test_util.patch_get_utility() def test_deploy_twice(self, mock_util): @@ -238,12 +238,12 @@ class InstallerTest(certbot_test_util.ConfigTestCase): with create_installer(self.config) as installer: installer.prepare() from certbot_postfix.postconf import ConfigMain - with mock.patch.object(ConfigMain, 'set', wraps=installer.postconf.set) as fake_set: - installer.deploy_cert('example.com', "cert_path", "key_path", + with mock.patch.object(ConfigMain, "set", wraps=installer.postconf.set) as fake_set: + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") self.assertEqual(fake_set.call_count, 15) fake_set.reset_mock() - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") fake_set.assert_not_called() @@ -252,14 +252,14 @@ class InstallerTest(certbot_test_util.ConfigTestCase): # Should not overwrite "more-secure" parameters mock_util().yesno.return_value = True more_secure = { - 'smtpd_tls_security_level': 'encrypt', - 'smtpd_tls_protocols': '!SSLv3, !SSLv2, !TLSv1', - 'smtpd_tls_eecdh_grade': 'strong' + "smtpd_tls_security_level": "encrypt", + "smtpd_tls_protocols": "!SSLv3, !SSLv2, !TLSv1", + "smtpd_tls_eecdh_grade": "strong" } with create_installer(self.config,\ main_cf=_main_cf_with(more_secure)) as installer: installer.prepare() - installer.deploy_cert('example.com', "cert_path", "key_path", + installer.deploy_cert("example.com", "cert_path", "key_path", "chain_path", "fullchain_path") for param in more_secure.keys(): self.assertFalse(param in installer.postconf.get_changes()) @@ -286,10 +286,10 @@ def create_installer(config, main_cf=DEFAULT_MAIN_CF): """ from certbot_postfix.postconf import ConfigMain from certbot_postfix import installer - def _mock_init_postconf(postconf, executable, handle_overrides, config_dir=None): + def _mock_init_postconf(postconf, executable, ignore_master_overrides=False, config_dir=None): # pylint: disable=protected-access super(ConfigMain, postconf).__init__(executable, config_dir) - postconf._handle_overrides = handle_overrides + postconf._ignore_master_overrides = ignore_master_overrides postconf._db = main_cf postconf._master_db = {} postconf._updated = {} @@ -302,6 +302,6 @@ def create_installer(config, main_cf=DEFAULT_MAIN_CF): return_value=mock.Mock()): yield installer.Installer(config, "postfix") -if __name__ == '__main__': +if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-postfix/certbot_postfix/tests/postconf_test.py b/certbot-postfix/certbot_postfix/tests/postconf_test.py index fb053b733..35c9637cc 100644 --- a/certbot-postfix/certbot_postfix/tests/postconf_test.py +++ b/certbot-postfix/certbot_postfix/tests/postconf_test.py @@ -25,7 +25,7 @@ class PostConfTest(unittest.TestCase): 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): + def test_read_default(self, mock_get_output): mock_get_output.return_value = 'param = default_value' self.assertEqual(self.config.get_default('param'), 'default_value') diff --git a/certbot-postfix/certbot_postfix/tests/util_test.py b/certbot-postfix/certbot_postfix/tests/util_test.py index 0077b48c9..8673538a7 100644 --- a/certbot-postfix/certbot_postfix/tests/util_test.py +++ b/certbot-postfix/certbot_postfix/tests/util_test.py @@ -170,14 +170,26 @@ class TestUtils(unittest.TestCase): def test_is_acceptable_protocols(self): from certbot_postfix.util import is_acceptable_value - self.assertFalse(is_acceptable_value('something_protocols_lol', + # SSLv2 and SSLv3 are both not supported, unambiguously + self.assertFalse(is_acceptable_value('tls_protocols_lol', 'SSLv2, SSLv3', '')) - self.assertFalse(is_acceptable_value('something_protocols_lol', + self.assertFalse(is_acceptable_value('tls_protocols_lol', '!SSLv2, !TLSv1', '')) - self.assertTrue(is_acceptable_value('something_protocols_lol', + self.assertFalse(is_acceptable_value('tls_protocols_lol', + '!SSLv2, SSLv3, !SSLv3, ', '')) + self.assertTrue(is_acceptable_value('tls_protocols_lol', '!SSLv2, !SSLv3', '')) - self.assertTrue(is_acceptable_value('something_protocols_lol', + self.assertTrue(is_acceptable_value('tls_protocols_lol', '!SSLv3, !TLSv1, !SSLv2', '')) + # TLSv1.2 is supported unambiguously + self.assertFalse(is_acceptable_value('tls_protocols_lol', + 'TLSv1, TLSv1.1,', '')) + self.assertFalse(is_acceptable_value('tls_protocols_lol', + 'TLSv1.2, !TLSv1.2,', '')) + self.assertTrue(is_acceptable_value('tls_protocols_lol', + 'TLSv1.2, ', '')) + self.assertTrue(is_acceptable_value('tls_protocols_lol', + 'TLSv1, TLSv1.1, TLSv1.2', '')) 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 00c4725c5..afa30f4a6 100644 --- a/certbot-postfix/certbot_postfix/util.py +++ b/certbot-postfix/certbot_postfix/util.py @@ -1,5 +1,6 @@ """Utility functions for use in the Postfix installer.""" import logging +import re import subprocess from certbot import errors @@ -221,7 +222,7 @@ def report_master_overrides(name, overrides, acceptable_overrides=None): is_acceptable_value(name, value, acceptable_overrides): continue error_string += " {0}: {1}\n".format(service, value) - if len(error_string) > 0: + if error_string: raise errors.PluginError("{0} is overridden with less secure options by the " "following services in master.cf:\n".format(name) + error_string) @@ -234,21 +235,50 @@ def is_acceptable_value(parameter, value, acceptable): if isinstance(acceptable, tuple): return value in acceptable # Check if param value is a comma-separated list of protocols. - elif 'protocols' in parameter: + elif 'tls_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" + Checks to see if the list of TLS protocols is acceptable. + This means TLSv1.2 is supported, and neither SSLv2 nor SSLv3 are supported. + + Should be a string of protocol names delimited by commas, spaces, or colons. + + Postfix's documents suggest listing protocols to exclude, like "!SSLv2, !SSLv3". + Listing the protocols to include, like "TLSv1, TLSv1.1, TLSv1.2" is okay as well, + though not recommended + + When these two modes are interspersed, the presence of a single non-negated protocol name + (i.e. "TLSv1" rather than "!TLSv1") automatically excludes all other unnamed protocols. + + In addition, the presence of a protocol name overrides any exclusion, so "SSLv3, !SSLv3" + supports SSLv3. This behavior isn't explicitly documented, so this method should return False + if it encounters contradicting statements about TLSv1.2, SSLv2, or SSLv3. """ + if not parameter_string: + return False bad_versions = list(constants.TLS_VERSIONS) for version in constants.ACCEPTABLE_TLS_VERSIONS: del bad_versions[bad_versions.index(version)] - for bad_version in bad_versions: - if "!" + bad_version not in parameter_string: + supported_version_list = re.split("[, :]+", parameter_string) + # The presence of any non-"!" protocol listing excludes the others by default. + inclusion_list = False + for version in supported_version_list: + if not version: + continue + if version in bad_versions: # short-circuit if we recognize any bad version return False + if version[0] != "!": + inclusion_list = True + if inclusion_list: # For any inclusion list, we still require TLS 1.2. + if "TLSv1.2" not in supported_version_list or "!TLSv1.2" in supported_version_list: + return False + else: + for bad_version in bad_versions: + if "!" + bad_version not in supported_version_list: + return False return True diff --git a/certbot-postfix/setup.py b/certbot-postfix/setup.py index 22757d31e..85f12e100 100644 --- a/certbot-postfix/setup.py +++ b/certbot-postfix/setup.py @@ -12,6 +12,11 @@ install_requires = [ 'zope.interface', ] +docs_extras = [ + 'Sphinx>=1.0', # autodoc_member_order = 'bysource', autodoc_default_flags + 'sphinx_rtd_theme', +] + setup( name='certbot-postfix', version=version, @@ -45,6 +50,9 @@ setup( packages=find_packages(), include_package_data=True, install_requires=install_requires, + extras_require={ + 'docs': docs_extras, + }, entry_points={ 'certbot.plugins': [ 'postfix = certbot_postfix:Installer',