More various fixes. Everything minus testing done

This commit is contained in:
sydneyli 2018-05-01 12:47:03 -07:00
parent 0e0db9ea44
commit 81a472b29a
11 changed files with 152 additions and 55 deletions

View file

@ -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

View file

@ -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."""

View file

@ -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.

View file

@ -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:

View file

@ -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")

View file

@ -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()

View file

@ -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()

View file

@ -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

View file

@ -8,6 +8,7 @@ install_requires = [
'certbot>0.23.0',
'setuptools',
'six',
'zope.component',
'zope.interface',
]

View file

@ -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"

View file

@ -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 ${@}
}