Merge branch 'master' into separate-integration

.gitignore

@@ -38,6 +38,7 @@ tests/letstest/venv/
 
 # pytest cache
 .cache
+.mypy_cache/
 
 # docker files
 .docker

.travis.yml

@@ -29,6 +29,10 @@ matrix:
       addons:
     - python: "2.7"
       env: TOXENV=lint
+    - python: "3.4"
+      env: TOXENV=mypy
+    - python: "3.5"
+      env: TOXENV=mypy
     - python: "2.7"
       env: TOXENV='py27-{acme,apache,certbot,dns,nginx}-oldest'
       sudo: required
@@ -103,6 +107,7 @@ notifications:
   irc:
     channels:
       - secure: "SGWZl3ownKx9xKVV2VnGt7DqkTmutJ89oJV9tjKhSs84kLijU6EYdPnllqISpfHMTxXflNZuxtGo0wTDYHXBuZL47w1O32W6nzuXdra5zC+i4sYQwYULUsyfOv9gJX8zWAULiK0Z3r0oho45U+FR5ZN6TPCidi8/eGU+EEPwaAw="
+    on_cancel: never
     on_success: never
     on_failure: always
     use_notice: true

CHANGELOG.md

@@ -2,6 +2,92 @@
 
 Certbot adheres to [Semantic Versioning](http://semver.org/).
 
+## 0.24.0 - 2018-05-02
+
+### Added
+
+* certbot now has an enhance subcommand which allows you to configure security
+  enhancements like HTTP to HTTPS redirects, OCSP stapling, and HSTS without
+  reinstalling a certificate.
+* certbot-dns-rfc2136 now allows the user to specify the port to use to reach
+  the DNS server in its credentials file.
+* acme now parses the wildcard field included in authorizations so it can be
+  used by users of the library.
+
+### Changed
+
+* certbot-dns-route53 used to wait for each DNS update to propagate before
+  sending the next one, but now it sends all updates before waiting which
+  speeds up issuance for multiple domains dramatically.
+* Certbot's official Docker images are now based on Alpine Linux 3.7 rather
+  than 3.4 because 3.4 has reached its end-of-life.
+* We've doubled the time Certbot will spend polling authorizations before
+  timing out.
+* The level of the message logged when Certbot is being used with
+  non-standard paths warning that crontabs for renewal included in Certbot
+  packages from OS package managers may not work has been reduced. This stops
+  the message from being written to stderr every time `certbot renew` runs.
+
+### Fixed
+
+* certbot-auto now works with Python 3.6.
+
+Despite us having broken lockstep, we are continuing to release new versions of
+all Certbot components during releases for the time being, however, the only
+packages with changes other than their version number were:
+
+* acme
+* certbot
+* certbot-apache
+* certbot-dns-digitalocean (only style improvements to tests)
+* certbot-dns-rfc2136
+
+More details about these changes can be found on our GitHub repo:
+https://github.com/certbot/certbot/milestone/52?closed=1
+
+## 0.23.0 - 2018-04-04
+
+### Added
+
+* Support for OpenResty was added to the Nginx plugin.
+
+### Changed
+
+* The timestamps in Certbot's logfiles now use the system's local time zone
+  rather than UTC.
+* Certbot's DNS plugins that use Lexicon now rely on Lexicon>=2.2.1 to be able
+  to create and delete multiple TXT records on a single domain.
+* certbot-dns-google's test suite now works without an internet connection.
+
+### Fixed
+
+* Removed a small window that if during which an error occurred, Certbot
+  wouldn't clean up performed challenges.
+* The parameters `default` and `ipv6only` are now removed from `listen`
+  directives when creating a new server block in the Nginx plugin.
+* `server_name` directives enclosed in quotation marks in Nginx are now properly
+  supported.
+* Resolved an issue preventing the Apache plugin from starting Apache when it's
+  not currently running on RHEL and Gentoo based systems.
+
+Despite us having broken lockstep, we are continuing to release new versions of
+all Certbot components during releases for the time being, however, the only
+packages with changes other than their version number were:
+
+* certbot
+* certbot-apache
+* certbot-dns-cloudxns
+* certbot-dns-dnsimple
+* certbot-dns-dnsmadeeasy
+* certbot-dns-google
+* certbot-dns-luadns
+* certbot-dns-nsone
+* certbot-dns-rfc2136
+* certbot-nginx
+
+More details about these changes can be found on our GitHub repo:
+https://github.com/certbot/certbot/milestone/50?closed=1
+
 ## 0.22.2 - 2018-03-19
 
 ### Fixed

Dockerfile

@@ -1,4 +1,4 @@
-FROM python:2-alpine
+FROM python:2-alpine3.7
 
 ENTRYPOINT [ "certbot" ]
 EXPOSE 80 443

acme/acme/challenges.py

@@ -1,5 +1,6 @@
 """ACME Identifier Validation Challenges."""
 import abc
+import codecs
 import functools
 import hashlib
 import logging
@@ -7,8 +8,9 @@ import socket
 
 from cryptography.hazmat.primitives import hashes  # type: ignore
 import josepy as jose
-import OpenSSL
+from OpenSSL import crypto
 import requests
+import six
 
 from acme import errors
 from acme import crypto_util
@@ -139,16 +141,16 @@ class KeyAuthorizationChallengeResponse(ChallengeResponse):
         return True
 
 
+@six.add_metaclass(abc.ABCMeta)
 class KeyAuthorizationChallenge(_TokenChallenge):
     # pylint: disable=abstract-class-little-used,too-many-ancestors
     """Challenge based on Key Authorization.
 
     :param response_cls: Subclass of `KeyAuthorizationChallengeResponse`
         that will be used to generate `response`.
-
+    :param str typ: type of the challenge
     """
-    __metaclass__ = abc.ABCMeta
-
+    typ = NotImplemented
     response_cls = NotImplemented
     thumbprint_hash_function = (
         KeyAuthorizationChallengeResponse.thumbprint_hash_function)
@@ -410,8 +412,8 @@ class TLSSNI01Response(KeyAuthorizationChallengeResponse):
 
         """
         if key is None:
-            key = OpenSSL.crypto.PKey()
-            key.generate_key(OpenSSL.crypto.TYPE_RSA, bits)
+            key = crypto.PKey()
+            key.generate_key(crypto.TYPE_RSA, bits)
         return crypto_util.gen_ss_cert(key, [
             # z_domain is too big to fit into CN, hence first dummy domain
             'dummy', self.z_domain.decode()], force_san=True), key
@@ -477,7 +479,7 @@ class TLSSNI01Response(KeyAuthorizationChallengeResponse):
             try:
                 cert = self.probe_cert(domain=domain, **kwargs)
             except errors.Error as error:
-                logger.debug(error, exc_info=True)
+                logger.debug(str(error), exc_info=True)
                 return False
 
         return self.verify_cert(cert)
@@ -506,6 +508,151 @@ class TLSSNI01(KeyAuthorizationChallenge):
         return self.response(account_key).gen_cert(key=kwargs.get('cert_key'))
 
 
+@ChallengeResponse.register
+class TLSALPN01Response(KeyAuthorizationChallengeResponse):
+    """ACME tls-alpn-01 challenge response."""
+    typ = "tls-alpn-01"
+
+    PORT = 443
+    """Verification port as defined by the protocol.
+
+    You can override it (e.g. for testing) by passing ``port`` to
+    `simple_verify`.
+
+    """
+
+    ID_PE_ACME_IDENTIFIER_V1 = b"1.3.6.1.5.5.7.1.30.1"
+    ACME_TLS_1_PROTOCOL = "acme-tls/1"
+
+    @property
+    def h(self):
+        """Hash value stored in challenge certificate"""
+        return hashlib.sha256(self.key_authorization.encode('utf-8')).digest()
+
+    def gen_cert(self, domain, key=None, bits=2048):
+        """Generate tls-alpn-01 certificate.
+
+        :param unicode domain: Domain verified by the challenge.
+        :param OpenSSL.crypto.PKey key: Optional private key used in
+            certificate generation. If not provided (``None``), then
+            fresh key will be generated.
+        :param int bits: Number of bits for newly generated key.
+
+        :rtype: `tuple` of `OpenSSL.crypto.X509` and `OpenSSL.crypto.PKey`
+
+        """
+        if key is None:
+            key = crypto.PKey()
+            key.generate_key(crypto.TYPE_RSA, bits)
+
+
+        der_value = b"DER:" + codecs.encode(self.h, 'hex')
+        acme_extension = crypto.X509Extension(self.ID_PE_ACME_IDENTIFIER_V1,
+                critical=True, value=der_value)
+
+        return crypto_util.gen_ss_cert(key, [domain], force_san=True,
+                extensions=[acme_extension]), key
+
+    def probe_cert(self, domain, host=None, port=None):
+        """Probe tls-alpn-01 challenge certificate.
+
+        :param unicode domain: domain being validated, required.
+        :param string host: IP address used to probe the certificate.
+        :param int port: Port used to probe the certificate.
+
+        """
+        if host is None:
+            host = socket.gethostbyname(domain)
+            logger.debug('%s resolved to %s', domain, host)
+        if port is None:
+            port = self.PORT
+
+        return crypto_util.probe_sni(host=host, port=port, name=domain,
+                alpn_protocols=[self.ACME_TLS_1_PROTOCOL])
+
+    def verify_cert(self, domain, cert):
+        """Verify tls-alpn-01 challenge certificate.
+
+        :param unicode domain: Domain name being validated.
+        :param OpensSSL.crypto.X509 cert: Challenge certificate.
+
+        :returns: Whether the certificate was successfully verified.
+        :rtype: bool
+
+        """
+        # pylint: disable=protected-access
+        names = crypto_util._pyopenssl_cert_or_req_all_names(cert)
+        logger.debug('Certificate %s. SANs: %s', cert.digest('sha256'), names)
+        if len(names) != 1 or names[0].lower() != domain.lower():
+            return False
+
+        for i in range(cert.get_extension_count()):
+            ext = cert.get_extension(i)
+            # FIXME: assume this is the ACME extension. Currently there is no
+            # way to get full OID of an unknown extension from pyopenssl.
+            if ext.get_short_name() == b'UNDEF':
+                data = ext.get_data()
+                return data == self.h
+
+        return False
+
+    # pylint: disable=too-many-arguments
+    def simple_verify(self, chall, domain, account_public_key,
+                      cert=None, host=None, port=None):
+        """Simple verify.
+
+        Verify ``validation`` using ``account_public_key``, optionally
+        probe tls-alpn-01 certificate and check using `verify_cert`.
+
+        :param .challenges.TLSALPN01 chall: Corresponding challenge.
+        :param str domain: Domain name being validated.
+        :param JWK account_public_key:
+        :param OpenSSL.crypto.X509 cert: Optional certificate. If not
+            provided (``None``) certificate will be retrieved using
+            `probe_cert`.
+        :param string host: IP address used to probe the certificate.
+        :param int port: Port used to probe the certificate.
+
+
+        :returns: ``True`` iff client's control of the domain has been
+            verified.
+        :rtype: bool
+
+        """
+        if not self.verify(chall, account_public_key):
+            logger.debug("Verification of key authorization in response failed")
+            return False
+
+        if cert is None:
+            try:
+                cert = self.probe_cert(domain=domain, host=host, port=port)
+            except errors.Error as error:
+                logger.debug(str(error), exc_info=True)
+                return False
+
+        return self.verify_cert(cert, domain)
+
+
+@Challenge.register  # pylint: disable=too-many-ancestors
+class TLSALPN01(KeyAuthorizationChallenge):
+    """ACME tls-alpn-01 challenge."""
+    response_cls = TLSALPN01Response
+    typ = response_cls.typ
+
+    def validation(self, account_key, **kwargs):
+        """Generate validation.
+
+        :param JWK account_key:
+        :param OpenSSL.crypto.PKey cert_key: Optional private key used
+            in certificate generation. If not provided (``None``), then
+            fresh key will be generated.
+
+        :rtype: `tuple` of `OpenSSL.crypto.X509` and `OpenSSL.crypto.PKey`
+
+        """
+        return self.response(account_key).gen_cert(key=kwargs.get('cert_key'))
+
+
 @Challenge.register  # pylint: disable=too-many-ancestors
 class DNS(_TokenChallenge):
     """ACME "dns" challenge."""

acme/acme/challenges_test.py

@@ -393,6 +393,127 @@ class TLSSNI01Test(unittest.TestCase):
         mock_gen_cert.assert_called_once_with(key=mock.sentinel.cert_key)
 
 
+class TLSALPN01ResponseTest(unittest.TestCase):
+    # pylint: disable=too-many-instance-attributes
+
+    def setUp(self):
+        from acme.challenges import TLSALPN01
+        self.chall = TLSALPN01(
+            token=jose.b64decode(b'a82d5ff8ef740d12881f6d3c2277ab2e'))
+        self.domain = u'example.com'
+        self.domain2 = u'example2.com'
+
+        self.response = self.chall.response(KEY)
+        self.jmsg = {
+            'resource': 'challenge',
+            'type': 'tls-alpn-01',
+            'keyAuthorization': self.response.key_authorization,
+        }
+
+    def test_to_partial_json(self):
+        self.assertEqual(self.jmsg, self.response.to_partial_json())
+
+    def test_from_json(self):
+        from acme.challenges import TLSALPN01Response
+        self.assertEqual(self.response, TLSALPN01Response.from_json(self.jmsg))
+
+    def test_from_json_hashable(self):
+        from acme.challenges import TLSALPN01Response
+        hash(TLSALPN01Response.from_json(self.jmsg))
+
+    def test_gen_verify_cert(self):
+        key1 = test_util.load_pyopenssl_private_key('rsa512_key.pem')
+        cert, key2 = self.response.gen_cert(self.domain, key1)
+        self.assertEqual(key1, key2)
+        self.assertTrue(self.response.verify_cert(self.domain, cert))
+
+    def test_gen_verify_cert_gen_key(self):
+        cert, key = self.response.gen_cert(self.domain)
+        self.assertTrue(isinstance(key, OpenSSL.crypto.PKey))
+        self.assertTrue(self.response.verify_cert(self.domain, cert))
+
+    def test_verify_bad_cert(self):
+        self.assertFalse(self.response.verify_cert(self.domain,
+            test_util.load_cert('cert.pem')))
+
+    def test_verify_bad_domain(self):
+        key1 = test_util.load_pyopenssl_private_key('rsa512_key.pem')
+        cert, key2 = self.response.gen_cert(self.domain, key1)
+        self.assertEqual(key1, key2)
+        self.assertFalse(self.response.verify_cert(self.domain2, cert))
+
+    def test_simple_verify_bad_key_authorization(self):
+        key2 = jose.JWKRSA.load(test_util.load_vector('rsa256_key.pem'))
+        self.response.simple_verify(self.chall, "local", key2.public_key())
+
+    @mock.patch('acme.challenges.TLSALPN01Response.verify_cert', autospec=True)
+    def test_simple_verify(self, mock_verify_cert):
+        mock_verify_cert.return_value = mock.sentinel.verification
+        self.assertEqual(
+            mock.sentinel.verification, self.response.simple_verify(
+                self.chall, self.domain, KEY.public_key(),
+                cert=mock.sentinel.cert))
+        mock_verify_cert.assert_called_once_with(
+            self.response, mock.sentinel.cert, self.domain)
+
+    @mock.patch('acme.challenges.socket.gethostbyname')
+    @mock.patch('acme.challenges.crypto_util.probe_sni')
+    def test_probe_cert(self, mock_probe_sni, mock_gethostbyname):
+        mock_gethostbyname.return_value = '127.0.0.1'
+        self.response.probe_cert('foo.com')
+        mock_gethostbyname.assert_called_once_with('foo.com')
+        mock_probe_sni.assert_called_once_with(
+            host='127.0.0.1', port=self.response.PORT, name='foo.com',
+            alpn_protocols=['acme-tls/1'])
+
+        self.response.probe_cert('foo.com', host='8.8.8.8')
+        mock_probe_sni.assert_called_with(
+            host='8.8.8.8', port=mock.ANY, name='foo.com',
+            alpn_protocols=['acme-tls/1'])
+
+    @mock.patch('acme.challenges.TLSALPN01Response.probe_cert')
+    def test_simple_verify_false_on_probe_error(self, mock_probe_cert):
+        mock_probe_cert.side_effect = errors.Error
+        self.assertFalse(self.response.simple_verify(
+            self.chall, self.domain, KEY.public_key()))
+
+
+class TLSALPN01Test(unittest.TestCase):
+
+    def setUp(self):
+        from acme.challenges import TLSALPN01
+        self.msg = TLSALPN01(
+            token=jose.b64decode('a82d5ff8ef740d12881f6d3c2277ab2e'))
+        self.jmsg = {
+            'type': 'tls-alpn-01',
+            'token': 'a82d5ff8ef740d12881f6d3c2277ab2e',
+        }
+
+    def test_to_partial_json(self):
+        self.assertEqual(self.jmsg, self.msg.to_partial_json())
+
+    def test_from_json(self):
+        from acme.challenges import TLSALPN01
+        self.assertEqual(self.msg, TLSALPN01.from_json(self.jmsg))
+
+    def test_from_json_hashable(self):
+        from acme.challenges import TLSALPN01
+        hash(TLSALPN01.from_json(self.jmsg))
+
+    def test_from_json_invalid_token_length(self):
+        from acme.challenges import TLSALPN01
+        self.jmsg['token'] = jose.encode_b64jose(b'abcd')
+        self.assertRaises(
+            jose.DeserializationError, TLSALPN01.from_json, self.jmsg)
+
+    @mock.patch('acme.challenges.TLSALPN01Response.gen_cert')
+    def test_validation(self, mock_gen_cert):
+        mock_gen_cert.return_value = ('cert', 'key')
+        self.assertEqual(('cert', 'key'), self.msg.validation(
+            KEY, cert_key=mock.sentinel.cert_key))
+        mock_gen_cert.assert_called_once_with(key=mock.sentinel.cert_key)
+
+
 class DNSTest(unittest.TestCase):
 
     def setUp(self):

acme/acme/client.py

@@ -9,17 +9,20 @@ import time
 
 import six
 from six.moves import http_client  # pylint: disable=import-error
-
 import josepy as jose
 import OpenSSL
 import re
+from requests_toolbelt.adapters.source import SourceAddressAdapter
 import requests
+from requests.adapters import HTTPAdapter
 import sys
 
 from acme import crypto_util
 from acme import errors
 from acme import jws
 from acme import messages
+# pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import Dict, List, Set, Text
 
 
 logger = logging.getLogger(__name__)
@@ -415,7 +418,7 @@ class Client(ClientBase):
         """
         # pylint: disable=too-many-locals
         assert max_attempts > 0
-        attempts = collections.defaultdict(int)
+        attempts = collections.defaultdict(int) # type: Dict[messages.AuthorizationResource, int]
         exhausted = set()
 
         # priority queue with datetime.datetime (based on Retry-After) as key,
@@ -529,7 +532,7 @@ class Client(ClientBase):
         :rtype: `list` of `OpenSSL.crypto.X509` wrapped in `.ComparableX509`
 
         """
-        chain = []
+        chain = [] # type: List[jose.ComparableX509]
         uri = certr.cert_chain_uri
         while uri is not None and len(chain) < max_length:
             response, cert = self._get_cert(uri)
@@ -856,18 +859,28 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
     :param bool verify_ssl: Whether to verify certificates on SSL connections.
     :param str user_agent: String to send as User-Agent header.
     :param float timeout: Timeout for requests.
+    :param source_address: Optional source address to bind to when making requests.
+    :type source_address: str or tuple(str, int)
     """
     def __init__(self, key, account=None, alg=jose.RS256, verify_ssl=True,
-                 user_agent='acme-python', timeout=DEFAULT_NETWORK_TIMEOUT):
+                 user_agent='acme-python', timeout=DEFAULT_NETWORK_TIMEOUT,
+                 source_address=None):
         # pylint: disable=too-many-arguments
         self.key = key
         self.account = account
         self.alg = alg
         self.verify_ssl = verify_ssl
-        self._nonces = set()
+        self._nonces = set() # type: Set[Text]
         self.user_agent = user_agent
         self.session = requests.Session()
         self._default_timeout = timeout
+        adapter = HTTPAdapter()
+
+        if source_address is not None:
+            adapter = SourceAddressAdapter(source_address)
+
+        self.session.mount("http://", adapter)
+        self.session.mount("https://", adapter)
 
     def __del__(self):
         # Try to close the session, but don't show exceptions to the
@@ -1017,7 +1030,7 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
         if response.headers.get("Content-Type") == DER_CONTENT_TYPE:
             debug_content = base64.b64encode(response.content)
         else:
-            debug_content = response.content
+            debug_content = response.content.decode("utf-8")
         logger.debug('Received response:\nHTTP %d\n%s\n\n%s',
                      response.status_code,
                      "\n".join(["{0}: {1}".format(k, v)

acme/acme/client_test.py

@@ -17,6 +17,7 @@ from acme import jws as acme_jws
 from acme import messages
 from acme import messages_test
 from acme import test_util
+from acme.magic_typing import Dict # pylint: disable=unused-import, no-name-in-module
 
 
 CERT_DER = test_util.load_vector('cert.der')
@@ -61,7 +62,8 @@ class ClientTestBase(unittest.TestCase):
         self.contact = ('mailto:cert-admin@example.com', 'tel:+12025551212')
         reg = messages.Registration(
             contact=self.contact, key=KEY.public_key())
-        self.new_reg = messages.NewRegistration(**dict(reg))
+        the_arg = dict(reg) # type: Dict
+        self.new_reg = messages.NewRegistration(**the_arg) # pylint: disable=star-args
         self.regr = messages.RegistrationResource(
             body=reg, uri='https://www.letsencrypt-demo.org/acme/reg/1')
 
@@ -1127,6 +1129,31 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase):
         self.assertRaises(requests.exceptions.RequestException,
                           self.net.post, 'uri', obj=self.obj)
 
+class ClientNetworkSourceAddressBindingTest(unittest.TestCase):
+    """Tests that if ClientNetwork has a source IP set manually, the underlying library has
+    used the provided source address."""
+
+    def setUp(self):
+        self.source_address = "8.8.8.8"
+
+    def test_source_address_set(self):
+        from acme.client import ClientNetwork
+        net = ClientNetwork(key=None, alg=None, source_address=self.source_address)
+        for adapter in net.session.adapters.values():
+            self.assertTrue(self.source_address in adapter.source_address)
+
+    def test_behavior_assumption(self):
+        """This is a test that guardrails the HTTPAdapter behavior so that if the default for
+        a Session() changes, the assumptions here aren't violated silently."""
+        from acme.client import ClientNetwork
+        # Source address not specified, so the default adapter type should be bound -- this
+        # test should fail if the default adapter type is changed by requests
+        net = ClientNetwork(key=None, alg=None)
+        session = requests.Session()
+        for scheme in session.adapters.keys():
+            client_network_adapter = net.session.adapters.get(scheme)
+            default_adapter = session.adapters.get(scheme)
+            self.assertEqual(client_network_adapter.__class__, default_adapter.__class__)
 
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

acme/acme/crypto_util.py

@@ -6,11 +6,14 @@ import os
 import re
 import socket
 
-import OpenSSL
+from OpenSSL import crypto
+from OpenSSL import SSL # type: ignore # https://github.com/python/typeshed/issues/2052
 import josepy as jose
 
-
 from acme import errors
+# pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import Callable, Union, Tuple, Optional
+# pylint: enable=unused-import, no-name-in-module
 
 
 logger = logging.getLogger(__name__)
@@ -25,7 +28,16 @@ logger = logging.getLogger(__name__)
 # https://www.openssl.org/docs/ssl/SSLv23_method.html). _serve_sni
 # should be changed to use "set_options" to disable SSLv2 and SSLv3,
 # in case it's used for things other than probing/serving!
-_DEFAULT_TLSSNI01_SSL_METHOD = OpenSSL.SSL.SSLv23_METHOD  # type: ignore
+_DEFAULT_TLSSNI01_SSL_METHOD = SSL.SSLv23_METHOD  # type: ignore
+
+
+class _DefaultCertSelection(object):
+    def __init__(self, certs):
+        self.certs = certs
+
+    def __call__(self, connection):
+        server_name = connection.get_servername()
+        return self.certs.get(server_name, None)
 
 
 class SSLSocket(object):  # pylint: disable=too-few-public-methods
@@ -35,12 +47,25 @@ class SSLSocket(object):  # pylint: disable=too-few-public-methods
     :ivar dict certs: Mapping from domain names (`bytes`) to
         `OpenSSL.crypto.X509`.
     :ivar method: See `OpenSSL.SSL.Context` for allowed values.
+    :ivar alpn_selection: Hook to select negotiated ALPN protocol for
+        connection.
+    :ivar cert_selection: Hook to select certificate for connection. If given,
+        `certs` parameter would be ignored, and therefore must be empty.
 
     """
-    def __init__(self, sock, certs, method=_DEFAULT_TLSSNI01_SSL_METHOD):
+    def __init__(self, sock, certs=None,
+            method=_DEFAULT_TLSSNI01_SSL_METHOD, alpn_selection=None,
+            cert_selection=None):
         self.sock = sock
-        self.certs = certs
+        self.alpn_selection = alpn_selection
         self.method = method
+        if not cert_selection and not certs:
+            raise ValueError("Neither cert_selection or certs specified.")
+        if cert_selection and certs:
+            raise ValueError("Both cert_selection and certs specified.")
+        if cert_selection is None:
+            cert_selection = _DefaultCertSelection(certs)
+        self.cert_selection = cert_selection
 
     def __getattr__(self, name):
         return getattr(self.sock, name)
@@ -57,18 +82,19 @@ class SSLSocket(object):  # pylint: disable=too-few-public-methods
         :type connection: :class:`OpenSSL.Connection`
 
         """
-        server_name = connection.get_servername()
-        try:
-            key, cert = self.certs[server_name]
-        except KeyError:
-            logger.debug("Server name (%s) not recognized, dropping SSL",
-                         server_name)
+        pair = self.cert_selection(connection)
+        if pair is None:
+            logger.debug("Certificate selection for server name %s failed, dropping SSL",
+                         connection.get_servername())
             return
-        new_context = OpenSSL.SSL.Context(self.method)
-        new_context.set_options(OpenSSL.SSL.OP_NO_SSLv2)
-        new_context.set_options(OpenSSL.SSL.OP_NO_SSLv3)
+        key, cert = pair
+        new_context = SSL.Context(self.method)
+        new_context.set_options(SSL.OP_NO_SSLv2)
+        new_context.set_options(SSL.OP_NO_SSLv3)
         new_context.use_privatekey(key)
         new_context.use_certificate(cert)
+        if self.alpn_selection is not None:
+            new_context.set_alpn_select_callback(self.alpn_selection)
         connection.set_context(new_context)
 
     class FakeConnection(object):
@@ -89,18 +115,20 @@ class SSLSocket(object):  # pylint: disable=too-few-public-methods
     def accept(self):  # pylint: disable=missing-docstring
         sock, addr = self.sock.accept()
 
-        context = OpenSSL.SSL.Context(self.method)
-        context.set_options(OpenSSL.SSL.OP_NO_SSLv2)
-        context.set_options(OpenSSL.SSL.OP_NO_SSLv3)
+        context = SSL.Context(self.method)
+        context.set_options(SSL.OP_NO_SSLv2)
+        context.set_options(SSL.OP_NO_SSLv3)
         context.set_tlsext_servername_callback(self._pick_certificate_cb)
+        if self.alpn_selection is not None:
+            context.set_alpn_select_callback(self.alpn_selection)
 
-        ssl_sock = self.FakeConnection(OpenSSL.SSL.Connection(context, sock))
+        ssl_sock = self.FakeConnection(SSL.Connection(context, sock))
         ssl_sock.set_accept_state()
 
         logger.debug("Performing handshake with %s", addr)
         try:
             ssl_sock.do_handshake()
-        except OpenSSL.SSL.Error as error:
+        except SSL.Error as error:
             # _pick_certificate_cb might have returned without
             # creating SSL context (wrong server name)
             raise socket.error(error)
@@ -108,8 +136,9 @@ class SSLSocket(object):  # pylint: disable=too-few-public-methods
         return ssl_sock, addr
 
 
-def probe_sni(name, host, port=443, timeout=300,
-              method=_DEFAULT_TLSSNI01_SSL_METHOD, source_address=('', 0)):
+def probe_sni(name, host, port=443, timeout=300, # pylint: disable=too-many-arguments
+              method=_DEFAULT_TLSSNI01_SSL_METHOD, source_address=('', 0),
+              alpn_protocols=None):
     """Probe SNI server for SSL certificate.
 
     :param bytes name: Byte string to send as the server name in the
@@ -121,6 +150,8 @@ def probe_sni(name, host, port=443, timeout=300,
     :param tuple source_address: Enables multi-path probing (selection
         of source interface). See `socket.creation_connection` for more
         info. Available only in Python 2.7+.
+    :param alpn_protocols: Protocols to request using ALPN.
+    :type alpn_protocols: `list` of `bytes`
 
     :raises acme.errors.Error: In case of any problems.
 
@@ -128,30 +159,41 @@ def probe_sni(name, host, port=443, timeout=300,
     :rtype: OpenSSL.crypto.X509
 
     """
-    context = OpenSSL.SSL.Context(method)
+    context = SSL.Context(method)
     context.set_timeout(timeout)
 
     socket_kwargs = {'source_address': source_address}
 
-    host_protocol_agnostic = None if host == '::' or host == '0' else host
+    host_protocol_agnostic = host
+    if host == '::' or host == '0':
+        # https://github.com/python/typeshed/pull/2136
+        # while PR is not merged, we need to ignore
+        host_protocol_agnostic = None
 
     try:
         # pylint: disable=star-args
-        logger.debug("Attempting to connect to %s:%d%s.", host_protocol_agnostic, port,
-            " from {0}:{1}".format(source_address[0], source_address[1]) if \
-            socket_kwargs else "")
-        sock = socket.create_connection((host_protocol_agnostic, port), **socket_kwargs)
+        logger.debug(
+            "Attempting to connect to %s:%d%s.", host_protocol_agnostic, port,
+            " from {0}:{1}".format(
+                source_address[0],
+                source_address[1]
+            ) if socket_kwargs else ""
+        )
+        socket_tuple = (host_protocol_agnostic, port)  # type: Tuple[Optional[str], int]
+        sock = socket.create_connection(socket_tuple, **socket_kwargs)  # type: ignore
     except socket.error as error:
         raise errors.Error(error)
 
     with contextlib.closing(sock) as client:
-        client_ssl = OpenSSL.SSL.Connection(context, client)
+        client_ssl = SSL.Connection(context, client)
         client_ssl.set_connect_state()
         client_ssl.set_tlsext_host_name(name)  # pyOpenSSL>=0.13
+        if alpn_protocols is not None:
+            client_ssl.set_alpn_protos(alpn_protocols)
         try:
             client_ssl.do_handshake()
             client_ssl.shutdown()
-        except OpenSSL.SSL.Error as error:
+        except SSL.Error as error:
             raise errors.Error(error)
     return client_ssl.get_peer_certificate()
 
@@ -164,18 +206,18 @@ def make_csr(private_key_pem, domains, must_staple=False):
         OCSP Must Staple: https://tools.ietf.org/html/rfc7633).
     :returns: buffer PEM-encoded Certificate Signing Request.
     """
-    private_key = OpenSSL.crypto.load_privatekey(
-        OpenSSL.crypto.FILETYPE_PEM, private_key_pem)
-    csr = OpenSSL.crypto.X509Req()
+    private_key = crypto.load_privatekey(
+        crypto.FILETYPE_PEM, private_key_pem)
+    csr = crypto.X509Req()
     extensions = [
-        OpenSSL.crypto.X509Extension(
+        crypto.X509Extension(
             b'subjectAltName',
             critical=False,
             value=', '.join('DNS:' + d for d in domains).encode('ascii')
         ),
     ]
     if must_staple:
-        extensions.append(OpenSSL.crypto.X509Extension(
+        extensions.append(crypto.X509Extension(
             b"1.3.6.1.5.5.7.1.24",
             critical=False,
             value=b"DER:30:03:02:01:05"))
@@ -183,8 +225,8 @@ def make_csr(private_key_pem, domains, must_staple=False):
     csr.set_pubkey(private_key)
     csr.set_version(2)
     csr.sign(private_key, 'sha256')
-    return OpenSSL.crypto.dump_certificate_request(
-        OpenSSL.crypto.FILETYPE_PEM, csr)
+    return crypto.dump_certificate_request(
+        crypto.FILETYPE_PEM, csr)
 
 def _pyopenssl_cert_or_req_all_names(loaded_cert_or_req):
     common_name = loaded_cert_or_req.get_subject().CN
@@ -221,11 +263,12 @@ def _pyopenssl_cert_or_req_san(cert_or_req):
     parts_separator = ", "
     prefix = "DNS" + part_separator
 
-    if isinstance(cert_or_req, OpenSSL.crypto.X509):
-        func = OpenSSL.crypto.dump_certificate
+    if isinstance(cert_or_req, crypto.X509):
+        # pylint: disable=line-too-long
+        func = crypto.dump_certificate # type: Union[Callable[[int, crypto.X509Req], bytes], Callable[[int, crypto.X509], bytes]]
     else:
-        func = OpenSSL.crypto.dump_certificate_request
-    text = func(OpenSSL.crypto.FILETYPE_TEXT, cert_or_req).decode("utf-8")
+        func = crypto.dump_certificate_request
+    text = func(crypto.FILETYPE_TEXT, cert_or_req).decode("utf-8")
     # WARNING: this function does not support multiple SANs extensions.
     # Multiple X509v3 extensions of the same type is disallowed by RFC 5280.
     match = re.search(r"X509v3 Subject Alternative Name:(?: critical)?\s*(.*)", text)
@@ -238,12 +281,14 @@ def _pyopenssl_cert_or_req_san(cert_or_req):
 
 
 def gen_ss_cert(key, domains, not_before=None,
-                validity=(7 * 24 * 60 * 60), force_san=True):
+                validity=(7 * 24 * 60 * 60), force_san=True, extensions=None):
     """Generate new self-signed certificate.
 
     :type domains: `list` of `unicode`
     :param OpenSSL.crypto.PKey key:
     :param bool force_san:
+    :param extensions: List of additional extensions to include in the cert.
+    :type extensions: `list` of `OpenSSL.crypto.X509Extension`
 
     If more than one domain is provided, all of the domains are put into
     ``subjectAltName`` X.509 extension and first domain is set as the
@@ -252,21 +297,24 @@ def gen_ss_cert(key, domains, not_before=None,
 
     """
     assert domains, "Must provide one or more hostnames for the cert."
-    cert = OpenSSL.crypto.X509()
+    cert = crypto.X509()
     cert.set_serial_number(int(binascii.hexlify(os.urandom(16)), 16))
     cert.set_version(2)
 
-    extensions = [
-        OpenSSL.crypto.X509Extension(
+    if extensions is None:
+        extensions = []
+
+    extensions.append(
+        crypto.X509Extension(
             b"basicConstraints", True, b"CA:TRUE, pathlen:0"),
-    ]
+    )
 
     cert.get_subject().CN = domains[0]
     # TODO: what to put into cert.get_subject()?
     cert.set_issuer(cert.get_subject())
 
     if force_san or len(domains) > 1:
-        extensions.append(OpenSSL.crypto.X509Extension(
+        extensions.append(crypto.X509Extension(
             b"subjectAltName",
             critical=False,
             value=b", ".join(b"DNS:" + d.encode() for d in domains)
@@ -281,7 +329,7 @@ def gen_ss_cert(key, domains, not_before=None,
     cert.sign(key, "sha256")
     return cert
 
-def dump_pyopenssl_chain(chain, filetype=OpenSSL.crypto.FILETYPE_PEM):
+def dump_pyopenssl_chain(chain, filetype=crypto.FILETYPE_PEM):
     """Dump certificate chain into a bundle.
 
     :param list chain: List of `OpenSSL.crypto.X509` (or wrapped in
@@ -298,7 +346,7 @@ def dump_pyopenssl_chain(chain, filetype=OpenSSL.crypto.FILETYPE_PEM):
         if isinstance(cert, jose.ComparableX509):
             # pylint: disable=protected-access
             cert = cert.wrapped
-        return OpenSSL.crypto.dump_certificate(filetype, cert)
+        return crypto.dump_certificate(filetype, cert)
 
     # assumes that OpenSSL.crypto.dump_certificate includes ending
     # newline character

acme/acme/crypto_util_test.py

@@ -13,12 +13,12 @@ import OpenSSL
 
 from acme import errors
 from acme import test_util
+from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module
 
 
 class SSLSocketAndProbeSNITest(unittest.TestCase):
     """Tests for acme.crypto_util.SSLSocket/probe_sni."""
 
-
     def setUp(self):
         self.cert = test_util.load_comparable_cert('rsa2048_cert.pem')
         key = test_util.load_pyopenssl_private_key('rsa2048_key.pem')
@@ -33,7 +33,8 @@ class SSLSocketAndProbeSNITest(unittest.TestCase):
             # six.moves.* | pylint: disable=attribute-defined-outside-init,no-init
 
             def server_bind(self):  # pylint: disable=missing-docstring
-                self.socket = SSLSocket(socket.socket(), certs=certs)
+                self.socket = SSLSocket(socket.socket(),
+                        certs)
                 socketserver.TCPServer.server_bind(self)
 
         self.server = _TestServer(('', 0), socketserver.BaseRequestHandler)
@@ -65,6 +66,18 @@ class SSLSocketAndProbeSNITest(unittest.TestCase):
     #    self.assertRaises(errors.Error, self._probe, b'bar')
 
 
+class SSLSocketTest(unittest.TestCase):
+    """Tests for acme.crypto_util.SSLSocket."""
+
+    def test_ssl_socket_invalid_arguments(self):
+        from acme.crypto_util import SSLSocket
+        with self.assertRaises(ValueError):
+            _ = SSLSocket(None, {'sni': ('key', 'cert')},
+                    cert_selection=lambda _: None)
+        with self.assertRaises(ValueError):
+            _ = SSLSocket(None)
+
+
 class PyOpenSSLCertOrReqAllNamesTest(unittest.TestCase):
     """Test for acme.crypto_util._pyopenssl_cert_or_req_all_names."""
 
@@ -165,7 +178,7 @@ class RandomSnTest(unittest.TestCase):
 
     def setUp(self):
         self.cert_count = 5
-        self.serial_num = []
+        self.serial_num = [] # type: List[int]
         self.key = OpenSSL.crypto.PKey()
         self.key.generate_key(OpenSSL.crypto.TYPE_RSA, 2048)
 

acme/acme/magic_typing.py

@@ -0,0 +1,16 @@
+"""Shim class to not have to depend on typing module in prod."""
+import sys
+
+class TypingClass(object):
+    """Ignore import errors by getting anything"""
+    def __getattr__(self, name):
+        return None
+
+try:
+    # mypy doesn't respect modifying sys.modules
+    from typing import *  # pylint: disable=wildcard-import, unused-wildcard-import
+    # pylint: disable=unused-import
+    from typing import Collection, IO  # type: ignore
+    # pylint: enable=unused-import
+except ImportError:
+    sys.modules[__name__] = TypingClass()

acme/acme/magic_typing_test.py

@@ -0,0 +1,41 @@
+"""Tests for acme.magic_typing."""
+import sys
+import unittest
+
+import mock
+
+
+class MagicTypingTest(unittest.TestCase):
+    """Tests for acme.magic_typing."""
+    def test_import_success(self):
+        try:
+            import typing as temp_typing
+        except ImportError: # pragma: no cover
+            temp_typing = None # pragma: no cover
+        typing_class_mock = mock.MagicMock()
+        text_mock = mock.MagicMock()
+        typing_class_mock.Text = text_mock
+        sys.modules['typing'] = typing_class_mock
+        if 'acme.magic_typing' in sys.modules:
+            del sys.modules['acme.magic_typing'] # pragma: no cover
+        from acme.magic_typing import Text # pylint: disable=no-name-in-module
+        self.assertEqual(Text, text_mock)
+        del sys.modules['acme.magic_typing']
+        sys.modules['typing'] = temp_typing
+
+    def test_import_failure(self):
+        try:
+            import typing as temp_typing
+        except ImportError: # pragma: no cover
+            temp_typing = None # pragma: no cover
+        sys.modules['typing'] = None
+        if 'acme.magic_typing' in sys.modules:
+            del sys.modules['acme.magic_typing'] # pragma: no cover
+        from acme.magic_typing import Text # pylint: disable=no-name-in-module
+        self.assertTrue(Text is None)
+        del sys.modules['acme.magic_typing']
+        sys.modules['typing'] = temp_typing
+
+
+if __name__ == '__main__':
+    unittest.main()  # pragma: no cover

acme/acme/messages.py

@@ -145,6 +145,7 @@ STATUS_PROCESSING = Status('processing')
 STATUS_VALID = Status('valid')
 STATUS_INVALID = Status('invalid')
 STATUS_REVOKED = Status('revoked')
+STATUS_READY = Status('ready')
 
 
 class IdentifierType(_Constant):
@@ -284,7 +285,7 @@ class Registration(ResourceBody):
         if phone is not None:
             details.append(cls.phone_prefix + phone)
         if email is not None:
-            details.append(cls.email_prefix + email)
+            details.extend([cls.email_prefix + mail for mail in email.split(',')])
         kwargs['contact'] = tuple(details)
         return cls(**kwargs)
 
@@ -435,6 +436,7 @@ class Authorization(ResourceBody):
     # be absent'... then acme-spec gives example with 'expires'
     # present... That's confusing!
     expires = fields.RFC3339Field('expires', omitempty=True)
+    wildcard = jose.Field('wildcard', omitempty=True)
 
     @challenges.decoder
     def challenges(value):  # pylint: disable=missing-docstring,no-self-argument

acme/acme/messages_test.py

@@ -6,6 +6,7 @@ import mock
 
 from acme import challenges
 from acme import test_util
+from acme.magic_typing import Dict # pylint: disable=unused-import, no-name-in-module
 
 
 CERT = test_util.load_comparable_cert('cert.der')
@@ -85,7 +86,7 @@ class ConstantTest(unittest.TestCase):
         from acme.messages import _Constant
 
         class MockConstant(_Constant):  # pylint: disable=missing-docstring
-            POSSIBLE_NAMES = {}
+            POSSIBLE_NAMES = {} # type: Dict
 
         self.MockConstant = MockConstant  # pylint: disable=invalid-name
         self.const_a = MockConstant('a')

acme/acme/standalone.py

@@ -16,6 +16,7 @@ import OpenSSL
 
 from acme import challenges
 from acme import crypto_util
+from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module
 
 
 logger = logging.getLogger(__name__)
@@ -42,7 +43,14 @@ class TLSServer(socketserver.TCPServer):
 
     def _wrap_sock(self):
         self.socket = crypto_util.SSLSocket(
-            self.socket, certs=self.certs, method=self.method)
+            self.socket, cert_selection=self._cert_selection,
+            alpn_selection=getattr(self, '_alpn_selection', None),
+            method=self.method)
+
+    def _cert_selection(self, connection):
+        """Callback selecting certificate for connection."""
+        server_name = connection.get_servername()
+        return self.certs.get(server_name, None)
 
     def server_bind(self):  # pylint: disable=missing-docstring
         self._wrap_sock()
@@ -66,8 +74,8 @@ class BaseDualNetworkedServers(object):
 
     def __init__(self, ServerClass, server_address, *remaining_args, **kwargs):
         port = server_address[1]
-        self.threads = []
-        self.servers = []
+        self.threads = [] # type: List[threading.Thread]
+        self.servers = [] # type: List[ACMEServerMixin]
 
         # Must try True first.
         # Ubuntu, for example, will fail to bind to IPv4 if we've already bound
@@ -82,9 +90,22 @@ class BaseDualNetworkedServers(object):
                 new_address = (server_address[0],) + (port,) + server_address[2:]
                 new_args = (new_address,) + remaining_args
                 server = ServerClass(*new_args, **kwargs) # pylint: disable=star-args
-            except socket.error:
-                logger.debug("Failed to bind to %s:%s using %s", new_address[0],
+                logger.debug(
+                    "Successfully bound to %s:%s using %s", new_address[0],
                     new_address[1], "IPv6" if ip_version else "IPv4")
+            except socket.error:
+                if self.servers:
+                    # Already bound using IPv6.
+                    logger.debug(
+                        "Certbot wasn't able to bind to %s:%s using %s, this " +
+                        "is often expected due to the dual stack nature of " +
+                        "IPv6 socket implementations.",
+                        new_address[0], new_address[1],
+                        "IPv6" if ip_version else "IPv4")
+                else:
+                    logger.debug(
+                        "Failed to bind to %s:%s using %s", new_address[0],
+                        new_address[1], "IPv6" if ip_version else "IPv4")
             else:
                 self.servers.append(server)
                 # If two servers are set up and port 0 was passed in, ensure we always
@@ -133,6 +154,45 @@ class TLSSNI01DualNetworkedServers(BaseDualNetworkedServers):
         BaseDualNetworkedServers.__init__(self, TLSSNI01Server, *args, **kwargs)
 
 
+class BadALPNProtos(Exception):
+    """Error raised when cannot negotiate ALPN protocol."""
+    pass
+
+
+class TLSALPN01Server(TLSServer, ACMEServerMixin):
+    """TLSALPN01 Server."""
+
+    ACME_TLS_1_PROTOCOL = b"acme-tls/1"
+
+    def __init__(self, server_address, certs, challenge_certs, ipv6=False):
+        TLSServer.__init__(
+            self, server_address, BaseRequestHandlerWithLogging, certs=certs,
+            ipv6=ipv6)
+        self.challenge_certs = challenge_certs
+
+    def _cert_selection(self, connection):
+        # TODO: We would like to serve challenge cert only if asked for it via
+        # ALPN. To do this, we need to retrieve the list of protos from client
+        # hello, but this is currently impossible with openssl [0], and ALPN
+        # negotiation is done after cert selection.
+        # Therefore, currently we always return challenge cert, and terminate
+        # handshake in alpn_selection() if ALPN protos are not what we expect.
+        # [0] https://github.com/openssl/openssl/issues/4952
+        server_name = connection.get_servername()
+        logger.debug("Serving challenge cert for server name %s", server_name)
+        return self.challenge_certs.get(server_name, None)
+
+    def _alpn_selection(self, _connection, alpn_protos):
+        """Callback to select alpn protocol."""
+        if len(alpn_protos) == 1 and alpn_protos[0] == self.ACME_TLS_1_PROTOCOL:
+            logger.debug("Agreed on %s ALPN", self.ACME_TLS_1_PROTOCOL)
+            return self.ACME_TLS_1_PROTOCOL
+        # Raising an exception causes openssl to terminate handshake and
+        # send fatal tls alert.
+        logger.debug("Cannot agree on ALPN proto. Got: %s", str(alpn_protos))
+        raise BadALPNProtos("Got: %s" % str(alpn_protos))
+
+
 class BaseRequestHandlerWithLogging(socketserver.BaseRequestHandler):
     """BaseRequestHandler with logging."""
 
@@ -189,7 +249,7 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
     def __init__(self, *args, **kwargs):
         self.simple_http_resources = kwargs.pop("simple_http_resources", set())
-        socketserver.BaseRequestHandler.__init__(self, *args, **kwargs)
+        BaseHTTPServer.BaseHTTPRequestHandler.__init__(self, *args, **kwargs)
 
     def log_message(self, format, *args):  # pylint: disable=redefined-builtin
         """Log arbitrary message."""
@@ -262,7 +322,7 @@ def simple_tls_sni_01_server(cli_args, forever=True):
 
     certs = {}
 
-    _, hosts, _ = next(os.walk('.'))
+    _, hosts, _ = next(os.walk('.')) # type: ignore # https://github.com/python/mypy/issues/465
     for host in hosts:
         with open(os.path.join(host, "cert.pem")) as cert_file:
             cert_contents = cert_file.read()

acme/acme/standalone_test.py

@@ -10,6 +10,7 @@ import unittest
 from six.moves import http_client  # pylint: disable=import-error
 from six.moves import socketserver  # type: ignore  # pylint: disable=import-error
 
+from OpenSSL import SSL # type: ignore # https://github.com/python/typeshed/issues/2052
 import josepy as jose
 import mock
 import requests
@@ -18,6 +19,7 @@ from acme import challenges
 from acme import crypto_util
 from acme import errors
 from acme import test_util
+from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module
 
 
 class TLSServerTest(unittest.TestCase):
@@ -72,7 +74,7 @@ class HTTP01ServerTest(unittest.TestCase):
     def setUp(self):
         self.account_key = jose.JWK.load(
             test_util.load_vector('rsa1024_key.pem'))
-        self.resources = set()
+        self.resources = set() # type: Set
 
         from acme.standalone import HTTP01Server
         self.server = HTTP01Server(('', 0), resources=self.resources)
@@ -118,6 +120,62 @@ class HTTP01ServerTest(unittest.TestCase):
         self.assertFalse(self._test_http01(add=False))
 
 
+@unittest.skipUnless(
+        hasattr(SSL.Connection, "set_alpn_protos") and
+        hasattr(SSL.Context, "set_alpn_select_callback"),
+        "pyOpenSSL too old")
+class TLSALPN01ServerTest(unittest.TestCase):
+    """Test for acme.standalone.TLSALPN01Server."""
+
+    def setUp(self):
+        self.certs = {b'localhost': (
+            test_util.load_pyopenssl_private_key('rsa2048_key.pem'),
+            test_util.load_cert('rsa2048_cert.pem'),
+        )}
+        # Use different certificate for challenge.
+        self.challenge_certs = {b'localhost': (
+            test_util.load_pyopenssl_private_key('rsa1024_key.pem'),
+            test_util.load_cert('rsa1024_cert.pem'),
+        )}
+        from acme.standalone import TLSALPN01Server
+        self.server = TLSALPN01Server(("", 0), certs=self.certs,
+                challenge_certs=self.challenge_certs)
+        # pylint: disable=no-member
+        self.thread = threading.Thread(target=self.server.serve_forever)
+        self.thread.start()
+
+    def tearDown(self):
+        self.server.shutdown()  # pylint: disable=no-member
+        self.thread.join()
+
+    #TODO: This is not implemented yet, see comments in standalone.py
+    #def test_certs(self):
+    #    host, port = self.server.socket.getsockname()[:2]
+    #    cert = crypto_util.probe_sni(
+    #        b'localhost', host=host, port=port, timeout=1)
+    #    # Expect normal cert when connecting without ALPN.
+    #    self.assertEqual(jose.ComparableX509(cert),
+    #                     jose.ComparableX509(self.certs[b'localhost'][1]))
+
+    def test_challenge_certs(self):
+        host, port = self.server.socket.getsockname()[:2]
+        cert = crypto_util.probe_sni(
+            b'localhost', host=host, port=port, timeout=1,
+            alpn_protocols=[b"acme-tls/1"])
+        #  Expect challenge cert when connecting with ALPN.
+        self.assertEqual(
+                jose.ComparableX509(cert),
+                jose.ComparableX509(self.challenge_certs[b'localhost'][1])
+        )
+
+    def test_bad_alpn(self):
+        host, port = self.server.socket.getsockname()[:2]
+        with self.assertRaises(errors.Error):
+            crypto_util.probe_sni(
+                b'localhost', host=host, port=port, timeout=1,
+                alpn_protocols=[b"bad-alpn"])
+
+
 class BaseDualNetworkedServersTest(unittest.TestCase):
     """Test for acme.standalone.BaseDualNetworkedServers."""
 
@@ -201,7 +259,7 @@ class HTTP01DualNetworkedServersTest(unittest.TestCase):
     def setUp(self):
         self.account_key = jose.JWK.load(
             test_util.load_vector('rsa1024_key.pem'))
-        self.resources = set()
+        self.resources = set() # type: Set
 
         from acme.standalone import HTTP01DualNetworkedServers
         self.servers = HTTP01DualNetworkedServers(('', 0), resources=self.resources)

acme/acme/test_util.py

@@ -10,7 +10,7 @@ import unittest
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import serialization
 import josepy as jose
-import OpenSSL
+from OpenSSL import crypto
 
 
 def vector_path(*names):
@@ -39,8 +39,8 @@ def _guess_loader(filename, loader_pem, loader_der):
 def load_cert(*names):
     """Load certificate."""
     loader = _guess_loader(
-        names[-1], OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1)
-    return OpenSSL.crypto.load_certificate(loader, load_vector(*names))
+        names[-1], crypto.FILETYPE_PEM, crypto.FILETYPE_ASN1)
+    return crypto.load_certificate(loader, load_vector(*names))
 
 
 def load_comparable_cert(*names):
@@ -51,8 +51,8 @@ def load_comparable_cert(*names):
 def load_csr(*names):
     """Load certificate request."""
     loader = _guess_loader(
-        names[-1], OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1)
-    return OpenSSL.crypto.load_certificate_request(loader, load_vector(*names))
+        names[-1], crypto.FILETYPE_PEM, crypto.FILETYPE_ASN1)
+    return crypto.load_certificate_request(loader, load_vector(*names))
 
 
 def load_comparable_csr(*names):
@@ -71,8 +71,8 @@ def load_rsa_private_key(*names):
 def load_pyopenssl_private_key(*names):
     """Load pyOpenSSL private key."""
     loader = _guess_loader(
-        names[-1], OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1)
-    return OpenSSL.crypto.load_privatekey(loader, load_vector(*names))
+        names[-1], crypto.FILETYPE_PEM, crypto.FILETYPE_ASN1)
+    return crypto.load_privatekey(loader, load_vector(*names))
 
 
 def skip_unless(condition, reason):  # pragma: no cover

acme/acme/testdata/README

@@ -10,6 +10,8 @@ and for the CSR:
 
   openssl req -key rsa2048_key.pem -new -subj '/CN=example.com' -outform DER > csr.der
 
-and for the certificate:
+and for the certificates:
 
-  openssl req -key rsa2047_key.pem -new -subj '/CN=example.com' -x509 -outform DER > cert.der
+  openssl req -key rsa2048_key.pem -new -subj '/CN=example.com' -x509 -outform DER > cert.der
+  openssl req -key rsa2048_key.pem -new -subj '/CN=example.com' -x509 > rsa2048_cert.pem
+  openssl req -key rsa1024_key.pem -new -subj '/CN=example.com' -x509 > rsa1024_cert.pem

acme/acme/testdata/rsa1024_cert.pem

@@ -0,0 +1,13 @@
+-----BEGIN CERTIFICATE-----
+MIIB/TCCAWagAwIBAgIJAOyRIBs3QT8QMA0GCSqGSIb3DQEBCwUAMBYxFDASBgNV
+BAMMC2V4YW1wbGUuY29tMB4XDTE4MDQyMzEwMzE0NFoXDTE4MDUyMzEwMzE0NFow
+FjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJ
+AoGBAJqJ87R8aVwByONxgQA9hwgvQd/QqI1r1UInXhEF2VnEtZGtUWLi100IpIqr
+Mq4qusDwNZ3g8cUPtSkvJGs89djoajMDIJP7lQUEKUYnYrI0q755Tr/DgLWSk7iW
+l5ezym0VzWUD0/xXUz8yRbNMTjTac80rS5SZk2ja2wWkYlRJAgMBAAGjUzBRMB0G
+A1UdDgQWBBSsaX0IVZ4XXwdeffVAbG7gnxSYjTAfBgNVHSMEGDAWgBSsaX0IVZ4X
+XwdeffVAbG7gnxSYjTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4GB
+ADe7SVmvGH2nkwVfONk8TauRUDkePN1CJZKFb2zW1uO9ANJ2v5Arm/OQp0BG/xnI
+Djw/aLTNVESF89oe15dkrUErtcaF413MC1Ld5lTCaJLHLGqDKY69e02YwRuxW7jY
+qarpt7k7aR5FbcfO5r4V/FK/Gvp4Dmoky8uap7SJIW6x
+-----END CERTIFICATE-----

acme/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
@@ -19,6 +17,7 @@ install_requires = [
     'pyrfc3339',
     'pytz',
     'requests[security]>=2.4.1',  # security extras added in 2.4.1
+    'requests-toolbelt>=0.3.0',
     'setuptools',
     'six>=1.9.0',  # needed for python_2_unicode_compatible
 ]

certbot-apache/certbot_apache/configurator.py

@@ -13,11 +13,13 @@ import zope.component
 import zope.interface
 
 from acme import challenges
+from acme.magic_typing import DefaultDict, Dict, List, Set  # pylint: disable=unused-import, no-name-in-module
 
 from certbot import errors
 from certbot import interfaces
 from certbot import util
 
+from certbot.achallenges import KeyAuthorizationAnnotatedChallenge  # pylint: disable=unused-import
 from certbot.plugins import common
 from certbot.plugins.util import path_surgery
 
@@ -130,10 +132,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             default=cls.OS_DEFAULTS["challenge_location"],
             help="Directory path for challenge configuration.")
         add("handle-modules", default=cls.OS_DEFAULTS["handle_mods"],
-            help="Let installer handle enabling required modules for you." +
+            help="Let installer handle enabling required modules for you. " +
                  "(Only Ubuntu/Debian currently)")
         add("handle-sites", default=cls.OS_DEFAULTS["handle_sites"],
-            help="Let installer handle enabling sites for you." +
+            help="Let installer handle enabling sites for you. " +
                  "(Only Ubuntu/Debian currently)")
         util.add_deprecated_argument(add, argument_name="ctl", nargs=1)
         util.add_deprecated_argument(
@@ -150,14 +152,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         super(ApacheConfigurator, self).__init__(*args, **kwargs)
 
         # Add name_server association dict
-        self.assoc = dict()
+        self.assoc = dict()  # type: Dict[str, obj.VirtualHost]
         # Outstanding challenges
-        self._chall_out = set()
+        self._chall_out = set()  # type: Set[KeyAuthorizationAnnotatedChallenge]
         # List of vhosts configured per wildcard domain on this run.
         # used by deploy_cert() and enhance()
-        self._wildcard_vhosts = dict()
+        self._wildcard_vhosts = dict()  # type: Dict[str, List[obj.VirtualHost]]
         # Maps enhancements to vhosts we've enabled the enhancement for
-        self._enhanced_vhosts = defaultdict(set)
+        self._enhanced_vhosts = defaultdict(set)  # type: DefaultDict[str, Set[obj.VirtualHost]]
 
         # These will be set in the prepare function
         self.parser = None
@@ -323,7 +325,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             # Returned objects are guaranteed to be ssl vhosts
             return self._choose_vhosts_wildcard(domain, create_if_no_ssl)
         else:
-            return [self.choose_vhost(domain)]
+            return [self.choose_vhost(domain, create_if_no_ssl)]
 
     def _vhosts_for_wildcard(self, domain):
         """
@@ -475,20 +477,21 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         if chain_path is not None:
             self.save_notes += "\tSSLCertificateChainFile %s\n" % chain_path
 
-    def choose_vhost(self, target_name, temp=False):
+    def choose_vhost(self, target_name, create_if_no_ssl=True):
         """Chooses a virtual host based on the given domain name.
 
         If there is no clear virtual host to be selected, the user is prompted
         with all available choices.
 
-        The returned vhost is guaranteed to have TLS enabled unless temp is
-        True. If temp is True, there is no such guarantee and the result is
-        not cached.
+        The returned vhost is guaranteed to have TLS enabled unless
+        create_if_no_ssl is set to False, in which case there is no such guarantee
+        and the result is not cached.
 
         :param str target_name: domain name
-        :param bool temp: whether the vhost is only used temporarily
+        :param bool create_if_no_ssl: If found VirtualHost doesn't have a HTTPS
+            counterpart, should one get created
 
-        :returns: ssl vhost associated with name
+        :returns: vhost associated with name
         :rtype: :class:`~certbot_apache.obj.VirtualHost`
 
         :raises .errors.PluginError: If no vhost is available or chosen
@@ -501,7 +504,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Try to find a reasonable vhost
         vhost = self._find_best_vhost(target_name)
         if vhost is not None:
-            if temp:
+            if not create_if_no_ssl:
                 return vhost
             if not vhost.ssl:
                 vhost = self.make_vhost_ssl(vhost)
@@ -510,7 +513,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             self.assoc[target_name] = vhost
             return vhost
 
-        return self._choose_vhost_from_list(target_name, temp)
+        # Negate create_if_no_ssl value to indicate if we want a SSL vhost
+        # to get created if a non-ssl vhost is selected.
+        return self._choose_vhost_from_list(target_name, temp=not create_if_no_ssl)
 
     def _choose_vhost_from_list(self, target_name, temp=False):
         # Select a vhost from a list
@@ -656,7 +661,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :rtype: set
 
         """
-        all_names = set()
+        all_names = set()  # type: Set[str]
 
         vhost_macro = []
 
@@ -797,8 +802,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         """
         # Search base config, and all included paths for VirtualHosts
-        file_paths = {}
-        internal_paths = defaultdict(set)
+        file_paths = {}  # type: Dict[str, str]
+        internal_paths = defaultdict(set)  # type: DefaultDict[str, Set[str]]
         vhs = []
         # Make a list of parser paths because the parser_paths
         # dictionary may be modified during the loop.
@@ -1236,7 +1241,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             if not self.parser.parsed_in_current(ssl_fp):
                 self.parser.parse_file(ssl_fp)
         except IOError:
-            logger.fatal("Error writing/reading to file in make_vhost_ssl")
+            logger.critical("Error writing/reading to file in make_vhost_ssl", exc_info=True)
             raise errors.PluginError("Unable to write/read in make_vhost_ssl")
 
         if sift:
@@ -1324,7 +1329,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         try:
             span_val = self.aug.span(vhost.path)
         except ValueError:
-            logger.fatal("Error while reading the VirtualHost %s from "
+            logger.critical("Error while reading the VirtualHost %s from "
                          "file %s", vhost.name, vhost.filep, exc_info=True)
             raise errors.PluginError("Unable to read VirtualHost from file")
         span_filep = span_val[0]
@@ -1505,7 +1510,20 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             raise errors.PluginError(
                 "Unsupported enhancement: {0}".format(enhancement))
 
-        vhosts = self.choose_vhosts(domain, create_if_no_ssl=False)
+        matched_vhosts = self.choose_vhosts(domain, create_if_no_ssl=False)
+        # We should be handling only SSL vhosts for enhancements
+        vhosts = [vhost for vhost in matched_vhosts if vhost.ssl]
+
+        if not vhosts:
+            msg_tmpl = ("Certbot was not able to find SSL VirtualHost for a "
+                        "domain {0} for enabling enhancement \"{1}\". The requested "
+                        "enhancement was not configured.")
+            msg_enhancement = enhancement
+            if options:
+                msg_enhancement += ": " + options
+            msg = msg_tmpl.format(domain, msg_enhancement)
+            logger.warning(msg)
+            raise errors.PluginError(msg)
         try:
             for vhost in vhosts:
                 func(vhost, options)
@@ -1754,7 +1772,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # There can be other RewriteRule directive lines in vhost config.
         # rewrite_args_dict keys are directive ids and the corresponding value
         # for each is a list of arguments to that directive.
-        rewrite_args_dict = defaultdict(list)
+        rewrite_args_dict = defaultdict(list)  # type: DefaultDict[str, List[str]]
         pat = r'(.*directive\[\d+\]).*'
         for match in rewrite_path:
             m = re.match(pat, match)
@@ -1848,7 +1866,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         if ssl_vhost.aliases:
             serveralias = "ServerAlias " + " ".join(ssl_vhost.aliases)
 
-        rewrite_rule_args = []
+        rewrite_rule_args = []  # type: List[str]
         if self.get_version() >= (2, 3, 9):
             rewrite_rule_args = constants.REWRITE_HTTPS_ARGS_WITH_END
         else:
@@ -2000,10 +2018,27 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :raises .errors.MisconfigurationError: If reload fails
 
         """
+        error = ""
         try:
             util.run_script(self.constant("restart_cmd"))
         except errors.SubprocessError as err:
-            raise errors.MisconfigurationError(str(err))
+            logger.info("Unable to restart apache using %s",
+                        self.constant("restart_cmd"))
+            alt_restart = self.constant("restart_cmd_alt")
+            if alt_restart:
+                logger.debug("Trying alternative restart command: %s",
+                             alt_restart)
+                # There is an alternative restart command available
+                # This usually is "restart" verb while original is "graceful"
+                try:
+                    util.run_script(self.constant(
+                        "restart_cmd_alt"))
+                    return
+                except errors.SubprocessError as secerr:
+                    error = str(secerr)
+            else:
+                error = str(err)
+            raise errors.MisconfigurationError(error)
 
     def config_test(self):  # pylint: disable=no-self-use
         """Check the configuration of Apache for errors.

certbot-apache/certbot_apache/http_01.py

@@ -2,9 +2,10 @@
 import logging
 import os
 
+from acme.magic_typing import Set  # pylint: disable=unused-import, no-name-in-module
 from certbot import errors
-
 from certbot.plugins import common
+from certbot_apache.obj import VirtualHost  # pylint: disable=unused-import
 
 logger = logging.getLogger(__name__)
 
@@ -51,7 +52,7 @@ class ApacheHttp01(common.TLSSNI01):
         self.challenge_dir = os.path.join(
             self.configurator.config.work_dir,
             "http_challenges")
-        self.moded_vhosts = set()
+        self.moded_vhosts = set()  # type: Set[VirtualHost]
 
     def perform(self):
         """Perform all HTTP-01 challenges."""

certbot-apache/certbot_apache/obj.py

@@ -1,6 +1,7 @@
 """Module contains classes used by the Apache Configurator."""
 import re
 
+from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module
 from certbot.plugins import common
 
 
@@ -140,7 +141,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
 
     def get_names(self):
         """Return a set of all names."""
-        all_names = set()
+        all_names = set()  # type: Set[str]
         all_names.update(self.aliases)
         # Strip out any scheme:// and <port> field from servername
         if self.name is not None:
@@ -251,7 +252,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
 
         # already_found acts to keep everything very conservative.
         # Don't allow multiple ip:ports in same set.
-        already_found = set()
+        already_found = set()  # type: Set[str]
 
         for addr in vhost.addrs:
             for local_addr in self.addrs:

certbot-apache/certbot_apache/override_centos.py

@@ -21,6 +21,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         version_cmd=['apachectl', '-v'],
         apache_cmd="apachectl",
         restart_cmd=['apachectl', 'graceful'],
+        restart_cmd_alt=['apachectl', 'restart'],
         conftest_cmd=['apachectl', 'configtest'],
         enmod=None,
         dismod=None,
@@ -46,10 +47,10 @@ class CentOSParser(parser.ApacheParser):
         self.sysconfig_filep = "/etc/sysconfig/httpd"
         super(CentOSParser, self).__init__(*args, **kwargs)
 
-    def update_runtime_variables(self, *args, **kwargs):
+    def update_runtime_variables(self):
         """ Override for update_runtime_variables for custom parsing """
         # Opportunistic, works if SELinux not enforced
-        super(CentOSParser, self).update_runtime_variables(*args, **kwargs)
+        super(CentOSParser, self).update_runtime_variables()
         self.parse_sysconfig_var()
 
     def parse_sysconfig_var(self):

certbot-apache/certbot_apache/override_gentoo.py

@@ -21,6 +21,7 @@ class GentooConfigurator(configurator.ApacheConfigurator):
         version_cmd=['/usr/sbin/apache2', '-v'],
         apache_cmd="apache2ctl",
         restart_cmd=['apache2ctl', 'graceful'],
+        restart_cmd_alt=['apache2ctl', 'restart'],
         conftest_cmd=['apache2ctl', 'configtest'],
         enmod=None,
         dismod=None,

certbot-apache/certbot_apache/parser.py

@@ -9,6 +9,7 @@ import sys
 
 import six
 
+from acme.magic_typing import Dict, List, Set  # pylint: disable=unused-import, no-name-in-module
 from certbot import errors
 
 logger = logging.getLogger(__name__)
@@ -38,9 +39,9 @@ class ApacheParser(object):
         # issues with aug.load() after adding new files / defines to parse tree
         self.configurator = configurator
 
-        self.modules = set()
-        self.parser_paths = {}
-        self.variables = {}
+        self.modules = set()  # type: Set[str]
+        self.parser_paths = {}  # type: Dict[str, List[str]]
+        self.variables = {}  # type: Dict[str, str]
 
         self.aug = aug
         # Find configuration root and make sure augeas can parse it.
@@ -119,7 +120,7 @@ class ApacheParser(object):
             the iteration issue.  Else... parse and enable mods at same time.
 
         """
-        mods = set()
+        mods = set()  # type: Set[str]
         matches = self.find_dir("LoadModule")
         iterator = iter(matches)
         # Make sure prev_size != cur_size for do: while: iteration
@@ -408,7 +409,7 @@ class ApacheParser(object):
         else:
             arg_suffix = "/*[self::arg=~regexp('%s')]" % case_i(arg)
 
-        ordered_matches = []
+        ordered_matches = []  # type: List[str]
 
         # TODO: Wildcards should be included in alphabetical order
         # https://httpd.apache.org/docs/2.4/mod/core.html#include

certbot-apache/certbot_apache/tests/centos_test.py

@@ -4,6 +4,8 @@ import unittest
 
 import mock
 
+from certbot import errors
+
 from certbot_apache import obj
 from certbot_apache import override_centos
 from certbot_apache.tests import util
@@ -121,5 +123,17 @@ class MultipleVhostsTestCentOS(util.ApacheTest):
         self.assertTrue("MOCK_NOSEP" in self.config.parser.variables.keys())
         self.assertEqual("NOSEP_VAL", self.config.parser.variables["NOSEP_TWO"])
 
+    @mock.patch("certbot_apache.configurator.util.run_script")
+    def test_alt_restart_works(self, mock_run_script):
+        mock_run_script.side_effect = [None, errors.SubprocessError, None]
+        self.config.restart()
+        self.assertEquals(mock_run_script.call_count, 3)
+
+    @mock.patch("certbot_apache.configurator.util.run_script")
+    def test_alt_restart_errors(self, mock_run_script):
+        mock_run_script.side_effect = [None,
+                                       errors.SubprocessError,
+                                       errors.SubprocessError]
+        self.assertRaises(errors.MisconfigurationError, self.config.restart)
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -246,7 +246,7 @@ class MultipleVhostsTest(util.ApacheTest):
     @mock.patch("certbot_apache.display_ops.select_vhost")
     def test_choose_vhost_select_vhost_with_temp(self, mock_select):
         mock_select.return_value = self.vh_truth[0]
-        chosen_vhost = self.config.choose_vhost("none.com", temp=True)
+        chosen_vhost = self.config.choose_vhost("none.com", create_if_no_ssl=False)
         self.assertEqual(self.vh_truth[0], chosen_vhost)
 
     @mock.patch("certbot_apache.display_ops.select_vhost")
@@ -353,14 +353,11 @@ class MultipleVhostsTest(util.ApacheTest):
 
         self.config.parser.find_dir = mock_find_dir
         mock_add.reset_mock()
-
         self.config._add_dummy_ssl_directives(self.vh_truth[0])  # pylint: disable=protected-access
-        tried_to_add = []
         for a in mock_add.call_args_list:
-            tried_to_add.append(a[0][1] == "Include" and
-                                a[0][2] == self.config.mod_ssl_conf)
-        # Include shouldn't be added, as patched find_dir "finds" existing one
-        self.assertFalse(any(tried_to_add))
+            if a[0][1] == "Include" and a[0][2] == self.config.mod_ssl_conf:
+                self.fail("Include shouldn't be added, as patched find_dir 'finds' existing one") \
+                    # pragma: no cover
 
     def test_deploy_cert(self):
         self.config.parser.modules.add("ssl_module")
@@ -936,6 +933,22 @@ class MultipleVhostsTest(util.ApacheTest):
             errors.PluginError,
             self.config.enhance, "certbot.demo", "unknown_enhancement")
 
+    def test_enhance_no_ssl_vhost(self):
+        with mock.patch("certbot_apache.configurator.logger.warning") as mock_log:
+            self.assertRaises(errors.PluginError, self.config.enhance,
+                              "certbot.demo", "redirect")
+            # Check that correct logger.warning was printed
+            self.assertTrue("not able to find" in mock_log.call_args[0][0])
+            self.assertTrue("\"redirect\"" in mock_log.call_args[0][0])
+
+            mock_log.reset_mock()
+
+            self.assertRaises(errors.PluginError, self.config.enhance,
+                              "certbot.demo", "ensure-http-header", "Test")
+            # Check that correct logger.warning was printed
+            self.assertTrue("not able to find" in mock_log.call_args[0][0])
+            self.assertTrue("Test" in mock_log.call_args[0][0])
+
     @mock.patch("certbot.util.exe_exists")
     def test_ocsp_stapling(self, mock_exe):
         self.config.parser.update_runtime_variables = mock.Mock()
@@ -945,6 +958,7 @@ class MultipleVhostsTest(util.ApacheTest):
         mock_exe.return_value = True
 
         # This will create an ssl vhost for certbot.demo
+        self.config.choose_vhost("certbot.demo")
         self.config.enhance("certbot.demo", "staple-ocsp")
 
         # Get the ssl vhost for certbot.demo
@@ -971,6 +985,7 @@ class MultipleVhostsTest(util.ApacheTest):
         mock_exe.return_value = True
 
         # Checking the case with already enabled ocsp stapling configuration
+        self.config.choose_vhost("ocspvhost.com")
         self.config.enhance("ocspvhost.com", "staple-ocsp")
 
         # Get the ssl vhost for letsencrypt.demo
@@ -995,6 +1010,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.config.parser.modules.add("mod_ssl.c")
         self.config.parser.modules.add("socache_shmcb_module")
         self.config.get_version = mock.Mock(return_value=(2, 2, 0))
+        self.config.choose_vhost("certbot.demo")
 
         self.assertRaises(errors.PluginError,
                 self.config.enhance, "certbot.demo", "staple-ocsp")
@@ -1020,6 +1036,7 @@ class MultipleVhostsTest(util.ApacheTest):
         mock_exe.return_value = True
 
         # This will create an ssl vhost for certbot.demo
+        self.config.choose_vhost("certbot.demo")
         self.config.enhance("certbot.demo", "ensure-http-header",
                             "Strict-Transport-Security")
 
@@ -1039,7 +1056,8 @@ class MultipleVhostsTest(util.ApacheTest):
         # skip the enable mod
         self.config.parser.modules.add("headers_module")
 
-        # This will create an ssl vhost for certbot.demo
+        # This will create an ssl vhost for encryption-example.demo
+        self.config.choose_vhost("encryption-example.demo")
         self.config.enhance("encryption-example.demo", "ensure-http-header",
                             "Strict-Transport-Security")
 
@@ -1058,6 +1076,7 @@ class MultipleVhostsTest(util.ApacheTest):
         mock_exe.return_value = True
 
         # This will create an ssl vhost for certbot.demo
+        self.config.choose_vhost("certbot.demo")
         self.config.enhance("certbot.demo", "ensure-http-header",
                             "Upgrade-Insecure-Requests")
 
@@ -1079,7 +1098,8 @@ class MultipleVhostsTest(util.ApacheTest):
         # skip the enable mod
         self.config.parser.modules.add("headers_module")
 
-        # This will create an ssl vhost for certbot.demo
+        # This will create an ssl vhost for encryption-example.demo
+        self.config.choose_vhost("encryption-example.demo")
         self.config.enhance("encryption-example.demo", "ensure-http-header",
                             "Upgrade-Insecure-Requests")
 
@@ -1097,6 +1117,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.config.get_version = mock.Mock(return_value=(2, 2))
 
         # This will create an ssl vhost for certbot.demo
+        self.config.choose_vhost("certbot.demo")
         self.config.enhance("certbot.demo", "redirect")
 
         # These are not immediately available in find_dir even with save() and
@@ -1147,6 +1168,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.config.save()
 
         # This will create an ssl vhost for certbot.demo
+        self.config.choose_vhost("certbot.demo")
         self.config.enhance("certbot.demo", "redirect")
 
         # These are not immediately available in find_dir even with save() and
@@ -1213,6 +1235,9 @@ class MultipleVhostsTest(util.ApacheTest):
         self.config.parser.modules.add("rewrite_module")
         self.config.get_version = mock.Mock(return_value=(2, 3, 9))
 
+        # Creates ssl vhost for the domain
+        self.config.choose_vhost("red.blue.purple.com")
+
         self.config.enhance("red.blue.purple.com", "redirect")
         verify_no_redirect = ("certbot_apache.configurator."
                               "ApacheConfigurator._verify_no_certbot_redirect")
@@ -1224,7 +1249,7 @@ class MultipleVhostsTest(util.ApacheTest):
         # Skip the enable mod
         self.config.parser.modules.add("rewrite_module")
         self.config.get_version = mock.Mock(return_value=(2, 3, 9))
-
+        self.config.choose_vhost("red.blue.purple.com")
         self.config.enhance("red.blue.purple.com", "redirect")
         # Clear state about enabling redirect on this run
         # pylint: disable=protected-access
@@ -1446,6 +1471,7 @@ class MultipleVhostsTest(util.ApacheTest):
         # pylint: disable=protected-access
         self.config.parser.modules.add("mod_ssl.c")
         self.config.parser.modules.add("headers_module")
+        self.vh_truth[3].ssl = True
         self.config._wildcard_vhosts["*.certbot.demo"] = [self.vh_truth[3]]
         self.config.enhance("*.certbot.demo", "ensure-http-header",
                             "Upgrade-Insecure-Requests")
@@ -1453,6 +1479,7 @@ class MultipleVhostsTest(util.ApacheTest):
 
     @mock.patch("certbot_apache.configurator.ApacheConfigurator._choose_vhosts_wildcard")
     def test_enhance_wildcard_no_install(self, mock_choose):
+        self.vh_truth[3].ssl = True
         mock_choose.return_value = [self.vh_truth[3]]
         self.config.parser.modules.add("mod_ssl.c")
         self.config.parser.modules.add("headers_module")

certbot-apache/certbot_apache/tests/debian_test.py

@@ -161,6 +161,8 @@ class MultipleVhostsTestDebian(util.ApacheTest):
         self.config.parser.modules.add("mod_ssl.c")
         self.config.get_version = mock.Mock(return_value=(2, 4, 7))
         mock_exe.return_value = True
+        # This will create an ssl vhost for certbot.demo
+        self.config.choose_vhost("certbot.demo")
         self.config.enhance("certbot.demo", "staple-ocsp")
         self.assertTrue("socache_shmcb_module" in self.config.parser.modules)
 
@@ -172,6 +174,7 @@ class MultipleVhostsTestDebian(util.ApacheTest):
         mock_exe.return_value = True
 
         # This will create an ssl vhost for certbot.demo
+        self.config.choose_vhost("certbot.demo")
         self.config.enhance("certbot.demo", "ensure-http-header",
                             "Strict-Transport-Security")
         self.assertTrue("headers_module" in self.config.parser.modules)
@@ -183,6 +186,7 @@ class MultipleVhostsTestDebian(util.ApacheTest):
         mock_exe.return_value = True
         self.config.get_version = mock.Mock(return_value=(2, 2))
         # This will create an ssl vhost for certbot.demo
+        self.config.choose_vhost("certbot.demo")
         self.config.enhance("certbot.demo", "redirect")
         self.assertTrue("rewrite_module" in self.config.parser.modules)
 

certbot-apache/certbot_apache/tests/gentoo_test.py

@@ -4,6 +4,8 @@ import unittest
 
 import mock
 
+from certbot import errors
+
 from certbot_apache import override_gentoo
 from certbot_apache import obj
 from certbot_apache.tests import util
@@ -123,5 +125,11 @@ class MultipleVhostsTestGentoo(util.ApacheTest):
         self.assertEquals(len(self.config.parser.modules), 4)
         self.assertTrue("mod_another.c" in self.config.parser.modules)
 
+    @mock.patch("certbot_apache.configurator.util.run_script")
+    def test_alt_restart_works(self, mock_run_script):
+        mock_run_script.side_effect = [None, errors.SubprocessError, None]
+        self.config.restart()
+        self.assertEquals(mock_run_script.call_count, 3)
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot-apache/certbot_apache/tests/http_01_test.py

@@ -4,47 +4,26 @@ import os
 import unittest
 
 from acme import challenges
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 
 from certbot import achallenges
 from certbot import errors
 
 from certbot.tests import acme_util
-
 from certbot_apache.tests import util
 
 
 NUM_ACHALLS = 3
 
 
-class ApacheHttp01TestMeta(type):
-    """Generates parmeterized tests for testing perform."""
-    def __new__(mcs, name, bases, class_dict):
-
-        def _gen_test(num_achalls, minor_version):
-            def _test(self):
-                achalls = self.achalls[:num_achalls]
-                vhosts = self.vhosts[:num_achalls]
-                self.config.version = (2, minor_version)
-                self.common_perform_test(achalls, vhosts)
-            return _test
-
-        for i in range(1, NUM_ACHALLS + 1):
-            for j in (2, 4):
-                test_name = "test_perform_{0}_{1}".format(i, j)
-                class_dict[test_name] = _gen_test(i, j)
-        return type.__new__(mcs, name, bases, class_dict)
-
-
 class ApacheHttp01Test(util.ApacheTest):
     """Test for certbot_apache.http_01.ApacheHttp01."""
 
-    __metaclass__ = ApacheHttp01TestMeta
-
     def setUp(self, *args, **kwargs):
         super(ApacheHttp01Test, self).setUp(*args, **kwargs)
 
         self.account_key = self.rsa512jwk
-        self.achalls = []
+        self.achalls = []  # type: List[achallenges.KeyAuthorizationAnnotatedChallenge]
         vh_truth = util.get_vh_truth(
             self.temp_dir, "debian_apache_2_4/multiple_vhosts")
         # Takes the vhosts for encryption-example.demo, certbot.demo, and
@@ -71,7 +50,7 @@ class ApacheHttp01Test(util.ApacheTest):
         self.assertFalse(self.http.perform())
 
     @mock.patch("certbot_apache.configurator.ApacheConfigurator.enable_mod")
-    def test_enable_modules_22(self, mock_enmod):
+    def test_enable_modules_apache_2_2(self, mock_enmod):
         self.config.version = (2, 2)
         self.config.parser.modules.remove("authz_host_module")
         self.config.parser.modules.remove("mod_authz_host.c")
@@ -80,7 +59,7 @@ class ApacheHttp01Test(util.ApacheTest):
         self.assertEqual(enmod_calls[0][0][0], "authz_host")
 
     @mock.patch("certbot_apache.configurator.ApacheConfigurator.enable_mod")
-    def test_enable_modules_24(self, mock_enmod):
+    def test_enable_modules_apache_2_4(self, mock_enmod):
         self.config.parser.modules.remove("authz_core_module")
         self.config.parser.modules.remove("mod_authz_core.c")
 
@@ -137,6 +116,31 @@ class ApacheHttp01Test(util.ApacheTest):
         self.config.config.http01_port = 12345
         self.assertRaises(errors.PluginError, self.http.perform)
 
+    def test_perform_1_achall_apache_2_2(self):
+        self.combinations_perform_test(num_achalls=1, minor_version=2)
+
+    def test_perform_1_achall_apache_2_4(self):
+        self.combinations_perform_test(num_achalls=1, minor_version=4)
+
+    def test_perform_2_achall_apache_2_2(self):
+        self.combinations_perform_test(num_achalls=2, minor_version=2)
+
+    def test_perform_2_achall_apache_2_4(self):
+        self.combinations_perform_test(num_achalls=2, minor_version=4)
+
+    def test_perform_3_achall_apache_2_2(self):
+        self.combinations_perform_test(num_achalls=3, minor_version=2)
+
+    def test_perform_3_achall_apache_2_4(self):
+        self.combinations_perform_test(num_achalls=3, minor_version=4)
+
+    def combinations_perform_test(self, num_achalls, minor_version):
+        """Test perform with the given achall count and Apache version."""
+        achalls = self.achalls[:num_achalls]
+        vhosts = self.vhosts[:num_achalls]
+        self.config.version = (2, minor_version)
+        self.common_perform_test(achalls, vhosts)
+
     def common_perform_test(self, achalls, vhosts):
         """Tests perform with the given achalls."""
         challenge_dir = self.http.challenge_dir

certbot-apache/certbot_apache/tests/util.py

@@ -87,7 +87,6 @@ class ParserTest(ApacheTest):
 def get_apache_configurator(  # pylint: disable=too-many-arguments, too-many-locals
         config_path, vhost_path,
         config_dir, work_dir, version=(2, 4, 7),
-        conf=None,
         os_info="generic",
         conf_vhost_path=None):
     """Create an Apache Configurator with the specified options.
@@ -133,10 +132,6 @@ def get_apache_configurator(  # pylint: disable=too-many-arguments, too-many-loc
                         config_class = configurator.ApacheConfigurator
                     config = config_class(config=mock_le_config, name="apache",
                         version=version)
-                    # This allows testing scripts to set it a bit more
-                    # quickly
-                    if conf is not None:
-                        config.conf = conf  # pragma: no cover
 
                     config.prepare()
     return config

certbot-apache/certbot_apache/tls_sni_01.py

@@ -3,6 +3,7 @@
 import os
 import logging
 
+from acme.magic_typing import Set  # pylint: disable=unused-import, no-name-in-module
 from certbot.plugins import common
 from certbot.errors import PluginError, MissingCommandlineFlag
 
@@ -93,7 +94,7 @@ class ApacheTlsSni01(common.TLSSNI01):
         :rtype: set
 
         """
-        addrs = set()
+        addrs = set()  # type: Set[obj.Addr]
         config_text = "<IfModule mod_ssl.c>\n"
 
         for achall in self.achalls:
@@ -123,7 +124,8 @@ class ApacheTlsSni01(common.TLSSNI01):
             self.configurator.config.tls_sni_01_port)))
 
         try:
-            vhost = self.configurator.choose_vhost(achall.domain, temp=True)
+            vhost = self.configurator.choose_vhost(achall.domain,
+                                                   create_if_no_ssl=False)
         except (PluginError, MissingCommandlineFlag):
             # We couldn't find the virtualhost for this domain, possibly
             # because it's a new vhost that's not configured yet

certbot-apache/local-oldest-requirements.txt

@@ -1,2 +1,2 @@
-acme[dev]==0.21.1
+-e acme[dev]
 certbot[dev]==0.21.1

certbot-apache/setup.py

@@ -1,15 +1,13 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
-    'acme>=0.21.1',
+    'acme>0.24.0',
     'certbot>=0.21.1',
     'mock',
     'python-augeas',

certbot-auto

@@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then
 fi
 VENV_BIN="$VENV_PATH/bin"
 BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt"
-LE_AUTO_VERSION="0.22.2"
+LE_AUTO_VERSION="0.24.0"
 BASENAME=$(basename $0)
 USAGE="Usage: $BASENAME [OPTIONS]
 A self-updating wrapper script for the Certbot ACME client. When run, updates
@@ -1089,7 +1089,7 @@ cryptography==2.0.2 \
     --hash=sha256:01e6e60654df64cca53733cda39446d67100c819c181d403afb120e0d2a71e1b \
     --hash=sha256:d46f4e5d455cb5563685c52ef212696f0a6cc1ea627603218eabbd8a095291d8 \
     --hash=sha256:3780b2663ee7ebb37cb83263326e3cd7f8b2ea439c448539d4b87de12c8d06ab
-enum34==1.1.2 \
+enum34==1.1.2 ; python_version < '3.4' \
     --hash=sha256:2475d7fcddf5951e92ff546972758802de5260bf409319a9f1934e6bbc8b1dc7 \
     --hash=sha256:35907defb0f992b75ab7788f65fedc1cf20ffa22688e0e6f6f12afc06b3ea501
 funcsigs==1.0.2 \
@@ -1199,18 +1199,18 @@ letsencrypt==0.7.0 \
     --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \
     --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9
 
-certbot==0.22.2 \
-    --hash=sha256:c8c63bdf0fed6258bdbc892454314ec37bcd1c35a7f62524a083d93ccdfc420d \
-    --hash=sha256:e6e3639293e78397f31f7d99e3c63aff82d91e2b0d50d146ee3c77f830464bef
-acme==0.22.2 \
-    --hash=sha256:59a55244612ee305d2caa6bb4cddd400fb60ec841bf011ed29a2899832a682c2 \
-    --hash=sha256:0ecd0ea369f53d5bc744d6e72717f9af2e1ceb558d109dbd433148851027adb4
-certbot-apache==0.22.2 \
-    --hash=sha256:b5340d4b9190358fde8eb6a5be0def37e32014b5142ee79ef5d2319ccbbde754 \
-    --hash=sha256:3cd26912bb5732d917ddf7aad2fe870090d4ece9a408b2c2de8e9723ec99c759
-certbot-nginx==0.22.2 \
-    --hash=sha256:91feef0d879496835d355e82841f92e5ecb5abbf6f23ea0ee5bbb8f5a92b278a \
-    --hash=sha256:b10bf04c1a20cf878d5e0d1877deb0e0780bc31b0ffda08ce7199bbc39d0753b
+certbot==0.24.0 \
+    --hash=sha256:a3fc41fde4f0dbb35f7ebec2f9e00339580b3f4298850411eac0719223073b27 \
+    --hash=sha256:a072d4528bb3ac4184f5c961a96931795ddfe4b7cb0f3a98954bdd4cce5f6d70
+acme==0.24.0 \
+    --hash=sha256:b92b16102051f447abb52917638fbfb34b646ac07267fee85961b360a0149e32 \
+    --hash=sha256:d655e0627c0830114ab3f6732d8bf2f4a2c36f602e0cde10988684e229b501cb
+certbot-apache==0.24.0 \
+    --hash=sha256:fe54db3e7e09ffe1139041c23ff5123e80aa1526d6fcd40b2a593d005cfcf152 \
+    --hash=sha256:686c6c0af5ae8d06e37cc762de7ffa0dc5c3b1ba06ff7653ef61713fa016f891
+certbot-nginx==0.24.0 \
+    --hash=sha256:d44c419f72c2cc30de3b138a2cf92e0531696dcb048f287036e229dce2131c00 \
+    --hash=sha256:3283d1db057261f05537fa408baee20e0ab9e81c3d55cfba70afe3805cd6f941
 
 UNLIKELY_EOF
     # -------------------------------------------------------------------------

certbot-compatibility-test/certbot_compatibility_test/test_driver.py

@@ -15,6 +15,7 @@ from six.moves import xrange  # pylint: disable=import-error,redefined-builtin
 from acme import challenges
 from acme import crypto_util
 from acme import messages
+from acme.magic_typing import List, Tuple  # pylint: disable=unused-import, no-name-in-module
 from certbot import achallenges
 from certbot import errors as le_errors
 from certbot.tests import acme_util
@@ -52,9 +53,8 @@ def test_authenticator(plugin, config, temp_dir):
 
     try:
         responses = plugin.perform(achalls)
-    except le_errors.Error as error:
-        logger.error("Performing challenges on %s caused an error:", config)
-        logger.exception(error)
+    except le_errors.Error:
+        logger.error("Performing challenges on %s caused an error:", config, exc_info=True)
         return False
 
     success = True
@@ -82,9 +82,8 @@ def test_authenticator(plugin, config, temp_dir):
     if success:
         try:
             plugin.cleanup(achalls)
-        except le_errors.Error as error:
-            logger.error("Challenge cleanup for %s caused an error:", config)
-            logger.exception(error)
+        except le_errors.Error:
+            logger.error("Challenge cleanup for %s caused an error:", config, exc_info=True)
             success = False
 
         if _dirs_are_unequal(config, backup):
@@ -147,9 +146,8 @@ def test_deploy_cert(plugin, temp_dir, domains):
         try:
             plugin.deploy_cert(domain, cert_path, util.KEY_PATH, cert_path, cert_path)
             plugin.save()  # Needed by the Apache plugin
-        except le_errors.Error as error:
-            logger.error("**** Plugin failed to deploy certificate for %s:", domain)
-            logger.exception(error)
+        except le_errors.Error:
+            logger.error("**** Plugin failed to deploy certificate for %s:", domain, exc_info=True)
             return False
 
     if not _save_and_restart(plugin, "deployed"):
@@ -179,7 +177,7 @@ def test_enhancements(plugin, domains):
                      "enhancements")
         return False
 
-    domains_and_info = [(domain, []) for domain in domains]
+    domains_and_info = [(domain, []) for domain in domains]  # type: List[Tuple[str, List[bool]]]
 
     for domain, info in domains_and_info:
         try:
@@ -192,10 +190,9 @@ def test_enhancements(plugin, domains):
             # Don't immediately fail because a redirect may already be enabled
             logger.warning("*** Plugin failed to enable redirect for %s:", domain)
             logger.warning("%s", error)
-        except le_errors.Error as error:
+        except le_errors.Error:
             logger.error("*** An error occurred while enabling redirect for %s:",
-                         domain)
-            logger.exception(error)
+                         domain, exc_info=True)
 
     if not _save_and_restart(plugin, "enhanced"):
         return False
@@ -222,9 +219,8 @@ def _save_and_restart(plugin, title=None):
         plugin.save(title)
         plugin.restart()
         return True
-    except le_errors.Error as error:
-        logger.error("*** Plugin failed to save and restart server:")
-        logger.exception(error)
+    except le_errors.Error:
+        logger.error("*** Plugin failed to save and restart server:", exc_info=True)
         return False
 
 
@@ -232,9 +228,8 @@ def test_rollback(plugin, config, backup):
     """Tests the rollback checkpoints function"""
     try:
         plugin.rollback_checkpoints(1337)
-    except le_errors.Error as error:
-        logger.error("*** Plugin raised an exception during rollback:")
-        logger.exception(error)
+    except le_errors.Error:
+        logger.error("*** Plugin raised an exception during rollback:", exc_info=True)
         return False
 
     if _dirs_are_unequal(config, backup):
@@ -263,21 +258,21 @@ def _dirs_are_unequal(dir1, dir2):
             logger.error("The following files and directories are only "
                          "present in one directory")
             if dircmp.left_only:
-                logger.error(dircmp.left_only)
+                logger.error(str(dircmp.left_only))
             else:
-                logger.error(dircmp.right_only)
+                logger.error(str(dircmp.right_only))
             return True
         elif dircmp.common_funny or dircmp.funny_files:
             logger.error("The following files and directories could not be "
                          "compared:")
             if dircmp.common_funny:
-                logger.error(dircmp.common_funny)
+                logger.error(str(dircmp.common_funny))
             else:
-                logger.error(dircmp.funny_files)
+                logger.error(str(dircmp.funny_files))
             return True
         elif dircmp.diff_files:
             logger.error("The following files differ:")
-            logger.error(dircmp.diff_files)
+            logger.error(str(dircmp.diff_files))
             return True
 
         for subdir in dircmp.subdirs.itervalues():
@@ -354,9 +349,8 @@ def main():
                     success = test_authenticator(plugin, config, temp_dir)
                 if success and args.install:
                     success = test_installer(args, plugin, config, temp_dir)
-            except errors.Error as error:
-                logger.error("Tests on %s raised:", config)
-                logger.exception(error)
+            except errors.Error:
+                logger.error("Tests on %s raised:", config, exc_info=True)
                 success = False
 
             if success:

certbot-compatibility-test/certbot_compatibility_test/util.py

@@ -26,12 +26,12 @@ def create_le_config(parent_dir):
     config = copy.deepcopy(constants.CLI_DEFAULTS)
 
     le_dir = os.path.join(parent_dir, "certbot")
-    config["config_dir"] = os.path.join(le_dir, "config")
-    config["work_dir"] = os.path.join(le_dir, "work")
-    config["logs_dir"] = os.path.join(le_dir, "logs_dir")
-    os.makedirs(config["config_dir"])
-    os.mkdir(config["work_dir"])
-    os.mkdir(config["logs_dir"])
+    os.mkdir(le_dir)
+    for dir_name in ("config", "logs", "work"):
+        full_path = os.path.join(le_dir, dir_name)
+        os.mkdir(full_path)
+        full_name = dir_name + "_dir"
+        config[full_name] = full_path
 
     config["domains"] = None
 

certbot-compatibility-test/certbot_compatibility_test/validator.py

@@ -33,7 +33,7 @@ class Validator(object):
         try:
             presented_cert = crypto_util.probe_sni(name, host, port)
         except acme_errors.Error as error:
-            logger.exception(error)
+            logger.exception(str(error))
             return False
 
         return presented_cert.digest("sha256") == cert.digest("sha256")
@@ -86,8 +86,7 @@ class Validator(object):
             return False
 
         try:
-            _, max_age_value = max_age[0]
-            max_age_value = int(max_age_value)
+            max_age_value = int(max_age[0][1])
         except ValueError:
             logger.error("Server responded with invalid HSTS header field")
             return False

certbot-compatibility-test/setup.py

@@ -4,7 +4,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 install_requires = [
     'certbot',

certbot-dns-cloudflare/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-cloudxns/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-digitalocean/certbot_dns_digitalocean/dns_digitalocean_test.py

@@ -50,7 +50,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
 
 
 class DigitalOceanClientTest(unittest.TestCase):
-    id = 1
+
+    id_num = 1
     record_prefix = "_acme-challenge"
     record_name = record_prefix + "." + DOMAIN
     record_content = "bar"
@@ -70,7 +71,7 @@ class DigitalOceanClientTest(unittest.TestCase):
 
         domain_mock = mock.MagicMock()
         domain_mock.name = DOMAIN
-        domain_mock.create_new_domain_record.return_value = {'domain_record': {'id': self.id}}
+        domain_mock.create_new_domain_record.return_value = {'domain_record': {'id': self.id_num}}
 
         self.manager.get_all_domains.return_value = [wrong_domain_mock, domain_mock]
 

certbot-dns-digitalocean/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-dnsimple/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-dnsmadeeasy/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-google/MANIFEST.in

@@ -1,3 +1,4 @@
 include LICENSE.txt
 include README.rst
 recursive-include docs *
+recursive-include certbot_dns_google/testdata *

certbot-dns-google/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-luadns/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-nsone/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py

@@ -21,8 +21,9 @@ Credentials
 -----------
 
 Use of this plugin requires a configuration file containing the target DNS
-server that supports RFC 2136 Dynamic Updates, the name of the TSIG key, the
-TSIG key secret itself and the algorithm used if it's different to HMAC-MD5.
+server and optional port that supports RFC 2136 Dynamic Updates, the name
+of the TSIG key, the TSIG key secret itself and the algorithm used if it's
+different to HMAC-MD5.
 
 .. code-block:: ini
    :name: credentials.ini
@@ -30,6 +31,8 @@ TSIG key secret itself and the algorithm used if it's different to HMAC-MD5.
 
    # Target DNS server
    dns_rfc2136_server = 192.0.2.1
+   # Target DNS port
+   dns_rfc2136_port = 53
    # TSIG key name
    dns_rfc2136_name = keyname.
    # TSIG key secret

certbot-dns-rfc2136/certbot_dns_rfc2136/dns_rfc2136.py

@@ -36,6 +36,8 @@ class Authenticator(dns_common.DNSAuthenticator):
       'HMAC-SHA512': dns.tsig.HMAC_SHA512
     }
 
+    PORT = 53
+
     description = 'Obtain certificates using a DNS TXT record (if you are using BIND for DNS).'
     ttl = 120
 
@@ -78,6 +80,7 @@ class Authenticator(dns_common.DNSAuthenticator):
 
     def _get_rfc2136_client(self):
         return _RFC2136Client(self.credentials.conf('server'),
+                              int(self.credentials.conf('port') or self.PORT),
                               self.credentials.conf('name'),
                               self.credentials.conf('secret'),
                               self.ALGORITHMS.get(self.credentials.conf('algorithm'),
@@ -88,8 +91,9 @@ class _RFC2136Client(object):
     """
     Encapsulates all communication with the target DNS server.
     """
-    def __init__(self, server, key_name, key_secret, key_algorithm):
+    def __init__(self, server, port, key_name, key_secret, key_algorithm):
         self.server = server
+        self.port = port
         self.keyring = dns.tsigkeyring.from_text({
             key_name: key_secret
         })
@@ -118,7 +122,7 @@ class _RFC2136Client(object):
         update.add(rel, record_ttl, dns.rdatatype.TXT, record_content)
 
         try:
-            response = dns.query.tcp(update, self.server)
+            response = dns.query.tcp(update, self.server, port=self.port)
         except Exception as e:
             raise errors.PluginError('Encountered error adding TXT record: {0}'
                                      .format(e))
@@ -153,7 +157,7 @@ class _RFC2136Client(object):
         update.delete(rel, dns.rdatatype.TXT, record_content)
 
         try:
-            response = dns.query.tcp(update, self.server)
+            response = dns.query.tcp(update, self.server, port=self.port)
         except Exception as e:
             raise errors.PluginError('Encountered error deleting TXT record: {0}'
                                      .format(e))
@@ -202,7 +206,7 @@ class _RFC2136Client(object):
         request.flags ^= dns.flags.RD
 
         try:
-            response = dns.query.udp(request, self.server)
+            response = dns.query.udp(request, self.server, port=self.port)
             rcode = response.rcode()
 
             # Authoritative Answer bit should be set

certbot-dns-rfc2136/certbot_dns_rfc2136/dns_rfc2136_test.py

@@ -14,6 +14,7 @@ from certbot.plugins.dns_test_common import DOMAIN
 from certbot.tests import util as test_util
 
 SERVER = '192.0.2.1'
+PORT = 53
 NAME = 'a-tsig-key.'
 SECRET = 'SSB3b25kZXIgd2hvIHdpbGwgYm90aGVyIHRvIGRlY29kZSB0aGlzIHRleHQK'
 VALID_CONFIG = {"rfc2136_server": SERVER, "rfc2136_name": NAME, "rfc2136_secret": SECRET}
@@ -74,7 +75,7 @@ class RFC2136ClientTest(unittest.TestCase):
     def setUp(self):
         from certbot_dns_rfc2136.dns_rfc2136 import _RFC2136Client
 
-        self.rfc2136_client = _RFC2136Client(SERVER, NAME, SECRET, dns.tsig.HMAC_MD5)
+        self.rfc2136_client = _RFC2136Client(SERVER, PORT, NAME, SECRET, dns.tsig.HMAC_MD5)
 
     @mock.patch("dns.query.tcp")
     def test_add_txt_record(self, query_mock):
@@ -84,7 +85,7 @@ class RFC2136ClientTest(unittest.TestCase):
 
         self.rfc2136_client.add_txt_record("bar", "baz", 42)
 
-        query_mock.assert_called_with(mock.ANY, SERVER)
+        query_mock.assert_called_with(mock.ANY, SERVER, port=PORT)
         self.assertTrue("bar. 42 IN TXT \"baz\"" in str(query_mock.call_args[0][0]))
 
     @mock.patch("dns.query.tcp")
@@ -117,7 +118,7 @@ class RFC2136ClientTest(unittest.TestCase):
 
         self.rfc2136_client.del_txt_record("bar", "baz")
 
-        query_mock.assert_called_with(mock.ANY, SERVER)
+        query_mock.assert_called_with(mock.ANY, SERVER, port=PORT)
         self.assertTrue("bar. 0 NONE TXT \"baz\"" in str(query_mock.call_args[0][0]))
 
     @mock.patch("dns.query.tcp")
@@ -169,7 +170,7 @@ class RFC2136ClientTest(unittest.TestCase):
         # _query_soa | pylint: disable=protected-access
         result = self.rfc2136_client._query_soa(DOMAIN)
 
-        query_mock.assert_called_with(mock.ANY, SERVER)
+        query_mock.assert_called_with(mock.ANY, SERVER, port=PORT)
         self.assertTrue(result == True)
 
     @mock.patch("dns.query.udp")
@@ -179,7 +180,7 @@ class RFC2136ClientTest(unittest.TestCase):
         # _query_soa | pylint: disable=protected-access
         result = self.rfc2136_client._query_soa(DOMAIN)
 
-        query_mock.assert_called_with(mock.ANY, SERVER)
+        query_mock.assert_called_with(mock.ANY, SERVER, port=PORT)
         self.assertTrue(result == False)
 
     @mock.patch("dns.query.udp")

certbot-dns-rfc2136/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-route53/certbot_dns_route53/dns_route53.py

@@ -11,6 +11,8 @@ from certbot import errors
 from certbot import interfaces
 from certbot.plugins import dns_common
 
+from acme.magic_typing import DefaultDict, List, Dict # pylint: disable=unused-import, no-name-in-module
+
 logger = logging.getLogger(__name__)
 
 INSTRUCTIONS = (
@@ -34,7 +36,7 @@ class Authenticator(dns_common.DNSAuthenticator):
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
         self.r53 = boto3.client("route53")
-        self._resource_records = collections.defaultdict(list)
+        self._resource_records = collections.defaultdict(list) # type: DefaultDict[str, List[Dict[str, str]]]
 
     def more_info(self):  # pylint: disable=missing-docstring,no-self-use
         return "Solve a DNS01 challenge using AWS Route53"
@@ -42,14 +44,26 @@ class Authenticator(dns_common.DNSAuthenticator):
     def _setup_credentials(self):
         pass
 
-    def _perform(self, domain, validation_domain_name, validation):
-        try:
-            change_id = self._change_txt_record("UPSERT", validation_domain_name, validation)
+    def _perform(self, domain, validation_domain_name, validation): # pylint: disable=missing-docstring
+        pass
 
-            self._wait_for_change(change_id)
+    def perform(self, achalls):
+        self._attempt_cleanup = True
+
+        try:
+            change_ids = [
+                self._change_txt_record("UPSERT",
+                  achall.validation_domain_name(achall.domain),
+                  achall.validation(achall.account_key))
+                for achall in achalls
+            ]
+
+            for change_id in change_ids:
+                self._wait_for_change(change_id)
         except (NoCredentialsError, ClientError) as e:
             logger.debug('Encountered error during perform: %s', e, exc_info=True)
             raise errors.PluginError("\n".join([str(e), INSTRUCTIONS]))
+        return [achall.response(achall.account_key) for achall in achalls]
 
     def _cleanup(self, domain, validation_domain_name, validation):
         try:

certbot-dns-route53/local-oldest-requirements.txt

@@ -1,2 +1,2 @@
-acme[dev]==0.21.1
+-e acme[dev]
 certbot[dev]==0.21.1

certbot-dns-route53/setup.py

@@ -1,14 +1,12 @@
-import sys
-
-from distutils.core import setup
+from setuptools import setup
 from setuptools import find_packages
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
-    'acme>=0.21.1',
+    'acme>0.24.0',
     'certbot>=0.21.1',
     'boto3',
     'mock',

certbot-nginx/certbot_nginx/configurator.py

@@ -28,6 +28,9 @@ from certbot_nginx import nginxparser
 from certbot_nginx import parser
 from certbot_nginx import tls_sni_01
 from certbot_nginx import http_01
+from certbot_nginx import obj # pylint: disable=unused-import
+from acme.magic_typing import List, Dict, Set # pylint: disable=unused-import, no-name-in-module
+
 
 
 logger = logging.getLogger(__name__)
@@ -98,8 +101,8 @@ class NginxConfigurator(common.Installer):
 
         # List of vhosts configured per wildcard domain on this run.
         # used by deploy_cert() and enhance()
-        self._wildcard_vhosts = {}
-        self._wildcard_redirect_vhosts = {}
+        self._wildcard_vhosts = {} # type: Dict[str, List[obj.VirtualHost]]
+        self._wildcard_redirect_vhosts = {} # type: Dict[str, List[obj.VirtualHost]]
 
         # Add number of outstanding challenges
         self._chall_out = 0
@@ -286,14 +289,15 @@ class NginxConfigurator(common.Installer):
         if not vhosts:
             if create_if_no_match:
                 # result will not be [None] because it errors on failure
-                vhosts = [self._vhost_from_duplicated_default(target_name)]
+                vhosts = [self._vhost_from_duplicated_default(target_name, True,
+                    str(self.config.tls_sni_01_port))]
             else:
                 # No matches. Raise a misconfiguration error.
                 raise errors.MisconfigurationError(
                             ("Cannot find a VirtualHost matching domain %s. "
                              "In order for Certbot to correctly perform the challenge "
                              "please add a corresponding server_name directive to your "
-                             "nginx configuration: "
+                             "nginx configuration for every domain on your certificate: "
                              "https://nginx.org/en/docs/http/server_names.html") % (target_name))
         # Note: if we are enhancing with ocsp, vhost should already be ssl.
         for vhost in vhosts:
@@ -329,9 +333,12 @@ class NginxConfigurator(common.Installer):
                     ipv6only_present = True
         return (ipv6_active, ipv6only_present)
 
-    def _vhost_from_duplicated_default(self, domain, port=None):
+    def _vhost_from_duplicated_default(self, domain, allow_port_mismatch, port):
+        """if allow_port_mismatch is False, only server blocks with matching ports will be
+           used as a default server block template.
+        """
         if self.new_vhost is None:
-            default_vhost = self._get_default_vhost(port)
+            default_vhost = self._get_default_vhost(domain, allow_port_mismatch, port)
             self.new_vhost = self.parser.duplicate_vhost(default_vhost,
                 remove_singleton_listen_params=True)
             self.new_vhost.names = set()
@@ -347,24 +354,29 @@ class NginxConfigurator(common.Installer):
             name_block[0].append(name)
         self.parser.update_or_add_server_directives(vhost, name_block)
 
-    def _get_default_vhost(self, port):
+    def _get_default_vhost(self, domain, allow_port_mismatch, port):
+        """Helper method for _vhost_from_duplicated_default; see argument documentation there"""
         vhost_list = self.parser.get_vhosts()
         # if one has default_server set, return that one
-        default_vhosts = []
+        all_default_vhosts = []
+        port_matching_vhosts = []
         for vhost in vhost_list:
             for addr in vhost.addrs:
                 if addr.default:
-                    if port is None or self._port_matches(port, addr.get_port()):
-                        default_vhosts.append(vhost)
-                        break
+                    all_default_vhosts.append(vhost)
+                    if self._port_matches(port, addr.get_port()):
+                        port_matching_vhosts.append(vhost)
+                    break
 
-        if len(default_vhosts) == 1:
-            return default_vhosts[0]
+        if len(port_matching_vhosts) == 1:
+            return port_matching_vhosts[0]
+        elif len(all_default_vhosts) == 1 and allow_port_mismatch:
+            return all_default_vhosts[0]
 
         # TODO: present a list of vhosts for user to choose from
 
         raise errors.MisconfigurationError("Could not automatically find a matching server"
-            " block. Set the `server_name` directive to use the Nginx installer.")
+            " block for %s. Set the `server_name` directive to use the Nginx installer." % domain)
 
     def _get_ranked_matches(self, target_name):
         """Returns a ranked list of vhosts that match target_name.
@@ -468,7 +480,7 @@ class NginxConfigurator(common.Installer):
             matches = self._get_redirect_ranked_matches(target_name, port)
             vhosts = [x for x in [self._select_best_name_match(matches)]if x is not None]
         if not vhosts and create_if_no_match:
-            vhosts = [self._vhost_from_duplicated_default(target_name, port=port)]
+            vhosts = [self._vhost_from_duplicated_default(target_name, False, port)]
         return vhosts
 
     def _port_matches(self, test_port, matching_port):
@@ -528,7 +540,7 @@ class NginxConfigurator(common.Installer):
         :rtype: set
 
         """
-        all_names = set()
+        all_names = set() # type: Set[str]
 
         for vhost in self.parser.get_vhosts():
             all_names.update(vhost.names)
@@ -824,7 +836,7 @@ class NginxConfigurator(common.Installer):
             self.parser.add_server_directives(vhost,
                                               stapling_directives)
         except errors.MisconfigurationError as error:
-            logger.debug(error)
+            logger.debug(str(error))
             raise errors.PluginError("An error occurred while enabling OCSP "
                                      "stapling for {0}.".format(vhost.names))
 
@@ -892,7 +904,7 @@ class NginxConfigurator(common.Installer):
                 universal_newlines=True)
             text = proc.communicate()[1]  # nginx prints output to stderr
         except (OSError, ValueError) as error:
-            logger.debug(error, exc_info=True)
+            logger.debug(str(error), exc_info=True)
             raise errors.PluginError(
                 "Unable to run %s -V" % self.conf('ctl'))
 
@@ -914,7 +926,7 @@ class NginxConfigurator(common.Installer):
             raise errors.PluginError("Nginx build doesn't support SNI")
 
         product_name, product_version = version_matches[0]
-        if product_name is not 'nginx':
+        if product_name != 'nginx':
             logger.warning("NGINX derivative %s is not officially supported by"
                            " certbot", product_name)
 

certbot-nginx/certbot_nginx/constants.py

@@ -1,9 +1,14 @@
 """nginx plugin constants."""
 import pkg_resources
+import platform
 
+if platform.system() in ('FreeBSD', 'Darwin'):
+    server_root_tmp = "/usr/local/etc/nginx"
+else:
+    server_root_tmp = "/etc/nginx"
 
 CLI_DEFAULTS = dict(
-    server_root="/etc/nginx",
+    server_root=server_root_tmp,
     ctl="nginx",
 )
 """CLI defaults."""

certbot-nginx/certbot_nginx/http_01.py

@@ -10,6 +10,7 @@ from certbot.plugins import common
 
 from certbot_nginx import obj
 from certbot_nginx import nginxparser
+from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module
 
 
 logger = logging.getLogger(__name__)
@@ -113,7 +114,7 @@ class NginxHttp01(common.ChallengePerformer):
         :returns: list of :class:`certbot_nginx.obj.Addr` to apply
         :rtype: list
         """
-        addresses = []
+        addresses = [] # type: List[obj.Addr]
         default_addr = "%s" % self.configurator.config.http01_port
         ipv6_addr = "[::]:{0}".format(
             self.configurator.config.http01_port)

certbot-nginx/certbot_nginx/nginxparser.py

@@ -248,7 +248,7 @@ class UnspacedList(list):
         """Recurse through the parse tree to figure out if any sublists are dirty"""
         if self.dirty:
             return True
-        return any((isinstance(x, list) and x.is_dirty() for x in self))
+        return any((isinstance(x, UnspacedList) and x.is_dirty() for x in self))
 
     def _spaced_position(self, idx):
         "Convert from indexes in the unspaced list to positions in the spaced one"

certbot-nginx/certbot_nginx/parser.py

@@ -13,7 +13,7 @@ from certbot import errors
 
 from certbot_nginx import obj
 from certbot_nginx import nginxparser
-
+from acme.magic_typing import Union, Dict, Set, Any, List, Tuple # pylint: disable=unused-import, no-name-in-module
 
 logger = logging.getLogger(__name__)
 
@@ -28,7 +28,7 @@ class NginxParser(object):
     """
 
     def __init__(self, root):
-        self.parsed = {}
+        self.parsed = {} # type: Dict[str, Union[List, nginxparser.UnspacedList]]
         self.root = os.path.abspath(root)
         self.config_root = self._find_config_root()
 
@@ -90,7 +90,7 @@ class NginxParser(object):
         """
         servers = self._get_raw_servers()
 
-        addr_to_ssl = {}
+        addr_to_ssl = {} # type: Dict[Tuple[str, str], bool]
         for filename in servers:
             for server, _ in servers[filename]:
                 # Parse the server block to save addr info
@@ -104,9 +104,10 @@ class NginxParser(object):
 
     def _get_raw_servers(self):
         # pylint: disable=cell-var-from-loop
+        # type: () -> Dict
         """Get a map of unparsed all server blocks
         """
-        servers = {}
+        servers = {} # type: Dict[str, Union[List, nginxparser.UnspacedList]]
         for filename in self.parsed:
             tree = self.parsed[filename]
             servers[filename] = []
@@ -565,7 +566,7 @@ def _update_or_add_directives(directives, insert_at_top, block):
 
 
 INCLUDE = 'include'
-REPEATABLE_DIRECTIVES = set(['server_name', 'listen', INCLUDE, 'rewrite'])
+REPEATABLE_DIRECTIVES = set(['server_name', 'listen', INCLUDE, 'rewrite', 'add_header'])
 COMMENT = ' managed by Certbot'
 COMMENT_BLOCK = [' ', '#', COMMENT]
 
@@ -727,9 +728,9 @@ def _parse_server_raw(server):
     :rtype: dict
 
     """
-    parsed_server = {'addrs': set(),
-                     'ssl': False,
-                     'names': set()}
+    addrs = set() # type: Set[obj.Addr]
+    ssl = False # type: bool
+    names = set() # type: Set[str]
 
     apply_ssl_to_all_addrs = False
 
@@ -739,17 +740,21 @@ def _parse_server_raw(server):
         if directive[0] == 'listen':
             addr = obj.Addr.fromstring(" ".join(directive[1:]))
             if addr:
-                parsed_server['addrs'].add(addr)
+                addrs.add(addr)
                 if addr.ssl:
-                    parsed_server['ssl'] = True
+                    ssl = True
         elif directive[0] == 'server_name':
-            parsed_server['names'].update(directive[1:])
+            names.update(x.strip('"\'') for x in directive[1:])
         elif _is_ssl_on_directive(directive):
-            parsed_server['ssl'] = True
+            ssl = True
             apply_ssl_to_all_addrs = True
 
     if apply_ssl_to_all_addrs:
-        for addr in parsed_server['addrs']:
+        for addr in addrs:
             addr.ssl = True
 
-    return parsed_server
+    return {
+        'addrs': addrs,
+        'ssl': ssl,
+        'names': names
+    }

certbot-nginx/certbot_nginx/tests/configurator_test.py

@@ -47,7 +47,7 @@ class NginxConfiguratorTest(util.NginxTest):
 
     def test_prepare(self):
         self.assertEqual((1, 6, 2), self.config.version)
-        self.assertEqual(10, len(self.config.parser.parsed))
+        self.assertEqual(11, len(self.config.parser.parsed))
 
     @mock.patch("certbot_nginx.configurator.util.exe_exists")
     @mock.patch("certbot_nginx.configurator.subprocess.Popen")
@@ -91,7 +91,8 @@ class NginxConfiguratorTest(util.NginxTest):
         self.assertEqual(names, set(
             ["155.225.50.69.nephoscale.net", "www.example.org", "another.alias",
              "migration.com", "summer.com", "geese.com", "sslon.com",
-             "globalssl.com", "globalsslsetssl.com", "ipv6.com", "ipv6ssl.com"]))
+             "globalssl.com", "globalsslsetssl.com", "ipv6.com", "ipv6ssl.com",
+             "headers.com"]))
 
     def test_supported_enhancements(self):
         self.assertEqual(['redirect', 'ensure-http-header', 'staple-ocsp'],
@@ -548,6 +549,14 @@ class NginxConfiguratorTest(util.NginxTest):
         generated_conf = self.config.parser.parsed[example_conf]
         self.assertTrue(util.contains_at_depth(generated_conf, expected, 2))
 
+    def test_multiple_headers_hsts(self):
+        headers_conf = self.config.parser.abs_path('sites-enabled/headers.com')
+        self.config.enhance("headers.com", "ensure-http-header",
+                            "Strict-Transport-Security")
+        expected = ['add_header', 'Strict-Transport-Security', '"max-age=31536000"', 'always']
+        generated_conf = self.config.parser.parsed[headers_conf]
+        self.assertTrue(util.contains_at_depth(generated_conf, expected, 2))
+
     def test_http_header_hsts_twice(self):
         self.config.enhance("www.example.com", "ensure-http-header",
                             "Strict-Transport-Security")
@@ -639,7 +648,7 @@ class NginxConfiguratorTest(util.NginxTest):
         self.assertEqual([[['server'],
                            [['listen', 'myhost', 'default_server'],
                             ['listen', 'otherhost', 'default_server'],
-                            ['server_name', 'www.example.org'],
+                            ['server_name', '"www.example.org"'],
                             [['location', '/'],
                              [['root', 'html'],
                               ['index', 'index.html', 'index.htm']]]]],
@@ -722,6 +731,13 @@ class NginxConfiguratorTest(util.NginxTest):
             "www.nomatch.com", "example/cert.pem", "example/key.pem",
             "example/chain.pem", "example/fullchain.pem")
 
+    def test_deploy_no_match_multiple_defaults_ok(self):
+        foo_conf = self.config.parser.abs_path('foo.conf')
+        self.config.parser.parsed[foo_conf][2][1][0][1][0][1] = '*:5001'
+        self.config.version = (1, 3, 1)
+        self.config.deploy_cert("www.nomatch.com", "example/cert.pem", "example/key.pem",
+            "example/chain.pem", "example/fullchain.pem")
+
     def test_deploy_no_match_add_redirect(self):
         default_conf = self.config.parser.abs_path('sites-enabled/default')
         foo_conf = self.config.parser.abs_path('foo.conf')
@@ -852,7 +868,7 @@ class NginxConfiguratorTest(util.NginxTest):
                                                 prefer_ssl=False,
                                                 no_ssl_filter_port='80')
             # Check that the dialog was called with only port 80 vhosts
-            self.assertEqual(len(mock_select_vhs.call_args[0][0]), 4)
+            self.assertEqual(len(mock_select_vhs.call_args[0][0]), 5)
 
 
 class InstallSslOptionsConfTest(util.NginxTest):

certbot-nginx/certbot_nginx/tests/parser_test.py

@@ -11,6 +11,7 @@ from certbot_nginx import nginxparser
 from certbot_nginx import obj
 from certbot_nginx import parser
 from certbot_nginx.tests import util
+from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module
 
 
 class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods
@@ -48,6 +49,7 @@ class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods
                               ['foo.conf', 'nginx.conf', 'server.conf',
                                'sites-enabled/default',
                                'sites-enabled/example.com',
+                               'sites-enabled/headers.com',
                                'sites-enabled/migration.com',
                                'sites-enabled/sslon.com',
                                'sites-enabled/globalssl.com',
@@ -76,7 +78,7 @@ class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods
         parsed = nparser._parse_files(nparser.abs_path(
             'sites-enabled/example.com.test'))
         self.assertEqual(3, len(glob.glob(nparser.abs_path('*.test'))))
-        self.assertEqual(7, len(
+        self.assertEqual(8, len(
             glob.glob(nparser.abs_path('sites-enabled/*.test'))))
         self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'],
                                         ['listen', '127.0.0.1'],
@@ -99,7 +101,7 @@ class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods
                    ([[[0], [3], [4]], [[5], [3], [0]]], [])]
 
         for mylist, result in mylists:
-            paths = []
+            paths = [] # type: List[List[int]]
             parser._do_for_subarray(mylist,
                                     lambda x: isinstance(x, list) and
                                     len(x) >= 1 and
@@ -159,7 +161,7 @@ class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods
                                                   '*.www.example.com']),
                                  [], [2, 1, 0])
 
-        self.assertEqual(12, len(vhosts))
+        self.assertEqual(13, len(vhosts))
         example_com = [x for x in vhosts if 'example.com' in x.filep][0]
         self.assertEqual(vhost3, example_com)
         default = [x for x in vhosts if 'default' in x.filep][0]

certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/default

@@ -1,7 +1,7 @@
 server {
     listen       myhost default_server;
     listen       otherhost default_server;
-    server_name www.example.org;
+    server_name "www.example.org";
 
     location / {
         root   html;

certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/headers.com

@@ -0,0 +1,4 @@
+server {
+    server_name headers.com;
+    add_header X-Content-Type-Options nosniff;
+}

certbot-nginx/setup.py

@@ -1,10 +1,8 @@
-import sys
-
 from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.23.0.dev0'
+version = '0.25.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot/__init__.py

@@ -1,4 +1,4 @@
 """Certbot client."""
 
 # version number like 1.2.3a0, must have at least 2 parts, like 1.2
-__version__ = '0.23.0.dev0'
+__version__ = '0.25.0.dev0'

certbot/account.py

@@ -16,6 +16,7 @@ import zope.component
 from acme import fields as acme_fields
 from acme import messages
 
+from certbot import constants
 from certbot import errors
 from certbot import interfaces
 from certbot import util
@@ -142,7 +143,11 @@ class AccountFileStorage(interfaces.AccountStorage):
                                    self.config.strict_permissions)
 
     def _account_dir_path(self, account_id):
-        return os.path.join(self.config.accounts_dir, account_id)
+        return self._account_dir_path_for_server_path(account_id, self.config.server_path)
+
+    def _account_dir_path_for_server_path(self, account_id, server_path):
+        accounts_dir = self.config.accounts_dir_for_server_path(server_path)
+        return os.path.join(accounts_dir, account_id)
 
     @classmethod
     def _regr_path(cls, account_dir_path):
@@ -156,22 +161,44 @@ class AccountFileStorage(interfaces.AccountStorage):
     def _metadata_path(cls, account_dir_path):
         return os.path.join(account_dir_path, "meta.json")
 
-    def find_all(self):
+    def _find_all_for_server_path(self, server_path):
+        accounts_dir = self.config.accounts_dir_for_server_path(server_path)
         try:
-            candidates = os.listdir(self.config.accounts_dir)
+            candidates = os.listdir(accounts_dir)
         except OSError:
             return []
 
         accounts = []
         for account_id in candidates:
             try:
-                accounts.append(self.load(account_id))
+                accounts.append(self._load_for_server_path(account_id, server_path))
             except errors.AccountStorageError:
                 logger.debug("Account loading problem", exc_info=True)
+
+
+        if not accounts and server_path in constants.LE_REUSE_SERVERS:
+            # find all for the next link down
+            prev_server_path = constants.LE_REUSE_SERVERS[server_path]
+            prev_accounts = self._find_all_for_server_path(prev_server_path)
+            # if we found something, link to that
+            if prev_accounts:
+                if os.path.islink(accounts_dir):
+                    os.unlink(accounts_dir)
+                else:
+                    try:
+                        os.rmdir(accounts_dir)
+                    except OSError:
+                        return []
+                prev_account_dir = self.config.accounts_dir_for_server_path(prev_server_path)
+                os.symlink(prev_account_dir, accounts_dir)
+            accounts = prev_accounts
         return accounts
 
-    def load(self, account_id):
-        account_dir_path = self._account_dir_path(account_id)
+    def find_all(self):
+        return self._find_all_for_server_path(self.config.server_path)
+
+    def _load_for_server_path(self, account_id, server_path):
+        account_dir_path = self._account_dir_path_for_server_path(account_id, server_path)
         if not os.path.isdir(account_dir_path):
             raise errors.AccountNotFound(
                 "Account at %s does not exist" % account_dir_path)
@@ -193,6 +220,9 @@ class AccountFileStorage(interfaces.AccountStorage):
                     account_id, acc.id))
         return acc
 
+    def load(self, account_id):
+        return self._load_for_server_path(account_id, self.config.server_path)
+
     def save(self, account, acme):
         self._save(account, acme, regr_only=False)
 

certbot/auth_handler.py

@@ -8,7 +8,9 @@ import zope.component
 
 from acme import challenges
 from acme import messages
-
+# pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import DefaultDict, Dict, List, Set, Collection
+# pylint: enable=unused-import, no-name-in-module
 from certbot import achallenges
 from certbot import errors
 from certbot import error_handler
@@ -117,7 +119,7 @@ class AuthHandler(object):
 
     def _solve_challenges(self, aauthzrs):
         """Get Responses for challenges from authenticators."""
-        resp = []
+        resp = []  # type: Collection[acme.challenges.ChallengeResponse]
         all_achalls = self._get_all_achalls(aauthzrs)
         try:
             if all_achalls:
@@ -133,10 +135,9 @@ class AuthHandler(object):
 
     def _get_all_achalls(self, aauthzrs):
         """Return all active challenges."""
-        all_achalls = []
+        all_achalls = []  # type: Collection[challenges.ChallengeResponse]
         for aauthzr in aauthzrs:
             all_achalls.extend(aauthzr.achalls)
-
         return all_achalls
 
     def _respond(self, aauthzrs, resp, best_effort):
@@ -146,7 +147,8 @@ class AuthHandler(object):
 
         """
         # TODO: chall_update is a dirty hack to get around acme-spec #105
-        chall_update = dict()
+        chall_update = dict() \
+        # type: Dict[int, List[achallenges.KeyAuthorizationAnnotatedChallenge]]
         self._send_responses(aauthzrs, resp, chall_update)
 
         # Check for updated status...
@@ -189,7 +191,7 @@ class AuthHandler(object):
         return active_achalls
 
     def _poll_challenges(self, aauthzrs, chall_update,
-                         best_effort, min_sleep=3, max_rounds=15):
+                         best_effort, min_sleep=3, max_rounds=30):
         """Wait for all challenge results to be determined."""
         indices_to_check = set(chall_update.keys())
         comp_indices = set()
@@ -198,7 +200,7 @@ class AuthHandler(object):
         while indices_to_check and rounds < max_rounds:
             # TODO: Use retry-after...
             time.sleep(min_sleep)
-            all_failed_achalls = set()
+            all_failed_achalls = set()  # type: Set[achallenges.KeyAuthorizationAnnotatedChallenge]
             for index in indices_to_check:
                 comp_achalls, failed_achalls = self._handle_check(
                     aauthzrs, index, chall_update[index])
@@ -424,7 +426,7 @@ def _find_smart_path(challbs, preferences, combinations):
 
     # max_cost is now equal to sum(indices) + 1
 
-    best_combo = []
+    best_combo = None
     # Set above completing all of the available challenges
     best_combo_cost = max_cost
 
@@ -479,7 +481,7 @@ def _report_no_chall_path(challbs):
         msg += (
             " You may need to use an authenticator "
             "plugin that can do challenges over DNS.")
-    logger.fatal(msg)
+    logger.critical(msg)
     raise errors.AuthorizationError(msg)
 
 
@@ -522,11 +524,11 @@ def _report_failed_challs(failed_achalls):
         :class:`certbot.achallenges.AnnotatedChallenge`.
 
     """
-    problems = dict()
+    problems = collections.defaultdict(list)\
+    # type: DefaultDict[str, List[achallenges.KeyAuthorizationAnnotatedChallenge]]
     for achall in failed_achalls:
         if achall.error:
-            problems.setdefault(achall.error.typ, []).append(achall)
-
+            problems[achall.error.typ].append(achall)
     reporter = zope.component.getUtility(interfaces.IReporter)
     for achalls in six.itervalues(problems):
         reporter.add_message(

certbot/cert_manager.py

@@ -7,6 +7,7 @@ import re
 import traceback
 import zope.component
 
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 from certbot import crypto_util
 from certbot import errors
 from certbot import interfaces
@@ -46,7 +47,7 @@ def rename_lineage(config):
     """
     disp = zope.component.getUtility(interfaces.IDisplay)
 
-    certname = _get_certnames(config, "rename")[0]
+    certname = get_certnames(config, "rename")[0]
 
     new_certname = config.new_certname
     if not new_certname:
@@ -88,7 +89,7 @@ def certificates(config):
 
 def delete(config):
     """Delete Certbot files associated with a certificate lineage."""
-    certnames = _get_certnames(config, "delete", allow_multiple=True)
+    certnames = get_certnames(config, "delete", allow_multiple=True)
     for certname in certnames:
         storage.delete_files(config, certname)
         disp = zope.component.getUtility(interfaces.IDisplay)
@@ -226,7 +227,7 @@ def match_and_check_overlaps(cli_config, acceptable_matches, match_func, rv_func
     def find_matches(candidate_lineage, return_value, acceptable_matches):
         """Returns a list of matches using _search_lineages."""
         acceptable_matches = [func(candidate_lineage) for func in acceptable_matches]
-        acceptable_matches_rv = []
+        acceptable_matches_rv = []  # type: List[str]
         for item in acceptable_matches:
             if isinstance(item, list):
                 acceptable_matches_rv += item
@@ -288,11 +289,7 @@ def human_readable_cert_info(config, cert, skip_filter_checks=False):
                          cert.privkey))
     return "".join(certinfo)
 
-###################
-# Private Helpers
-###################
-
-def _get_certnames(config, verb, allow_multiple=False):
+def get_certnames(config, verb, allow_multiple=False, custom_prompt=None):
     """Get certname from flag, interactively, or error out.
     """
     certname = config.certname
@@ -305,22 +302,32 @@ def _get_certnames(config, verb, allow_multiple=False):
         if not choices:
             raise errors.Error("No existing certificates found.")
         if allow_multiple:
+            if not custom_prompt:
+                prompt = "Which certificate(s) would you like to {0}?".format(verb)
+            else:
+                prompt = custom_prompt
             code, certnames = disp.checklist(
-                                    "Which certificate(s) would you like to {0}?".format(verb),
-                                    choices, cli_flag="--cert-name",
-                                    force_interactive=True)
+                prompt, choices, cli_flag="--cert-name", force_interactive=True)
             if code != display_util.OK:
                 raise errors.Error("User ended interaction.")
         else:
-            code, index = disp.menu("Which certificate would you like to {0}?".format(verb),
-                                    choices, cli_flag="--cert-name",
-                                    force_interactive=True)
+            if not custom_prompt:
+                prompt = "Which certificate would you like to {0}?".format(verb)
+            else:
+                prompt = custom_prompt
+
+            code, index = disp.menu(
+                prompt, choices, cli_flag="--cert-name", force_interactive=True)
 
             if code != display_util.OK or index not in range(0, len(choices)):
                 raise errors.Error("User ended interaction.")
             certnames = [choices[index]]
     return certnames
 
+###################
+# Private Helpers
+###################
+
 def _report_lines(msgs):
     """Format a results report for a category of single-line renewal outcomes"""
     return "  " + "\n  ".join(str(msg) for msg in msgs)
@@ -334,7 +341,7 @@ def _report_human_readable(config, parsed_certs):
 
 def _describe_certs(config, parsed_certs, parse_failures):
     """Print information about the certs we know about"""
-    out = []
+    out = []  # type: List[str]
 
     notify = out.append
 

certbot/cli.py

@@ -12,10 +12,14 @@ import sys
 import configargparse
 import six
 import zope.component
+import zope.interface
 
 from zope.interface import interfaces as zope_interfaces
 
 from acme import challenges
+# pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import Any, Dict, Optional
+# pylint: enable=unused-import, no-name-in-module
 
 import certbot
 
@@ -33,7 +37,7 @@ import certbot.plugins.selection as plugin_selection
 logger = logging.getLogger(__name__)
 
 # Global, to save us from a lot of argument passing within the scope of this module
-helpful_parser = None
+helpful_parser = None  # type: Optional[HelpfulArgumentParser]
 
 # For help strings, figure out how the user ran us.
 # When invoked from letsencrypt-auto, sys.argv[0] is something like:
@@ -76,6 +80,7 @@ obtain, install, and renew certificates:
     (default) run   Obtain & install a certificate in your current webserver
     certonly        Obtain or renew a certificate, but do not install it
     renew           Renew all previously obtained certificates that are near expiry
+    enhance         Add security enhancements to your existing configuration
    -d DOMAINS       Comma-separated list of domains to obtain a certificate for
 
   %s
@@ -195,17 +200,17 @@ def set_by_cli(var):
     (CLI or config file) including if the user explicitly set it to the
     default.  Returns False if the variable was assigned a default value.
     """
-    detector = set_by_cli.detector
-    if detector is None:
+    detector = set_by_cli.detector  # type: ignore
+    if detector is None and helpful_parser is not None:
         # Setup on first run: `detector` is a weird version of config in which
         # the default value of every attribute is wrangled to be boolean-false
         plugins = plugins_disco.PluginsRegistry.find_all()
         # reconstructed_args == sys.argv[1:], or whatever was passed to main()
         reconstructed_args = helpful_parser.args + [helpful_parser.verb]
-        detector = set_by_cli.detector = prepare_and_parse_args(
+        detector = set_by_cli.detector = prepare_and_parse_args(  # type: ignore
             plugins, reconstructed_args, detect_defaults=True)
         # propagate plugin requests: eg --standalone modifies config.authenticator
-        detector.authenticator, detector.installer = (
+        detector.authenticator, detector.installer = (  # type: ignore
             plugin_selection.cli_plugin_requests(detector))
 
     if not isinstance(getattr(detector, var), _Default):
@@ -219,7 +224,10 @@ def set_by_cli(var):
             return True
 
     return False
+
 # static housekeeping var
+# functions attributed are not supported by mypy
+# https://github.com/python/mypy/issues/2087
 set_by_cli.detector = None  # type: ignore
 
 
@@ -235,8 +243,10 @@ def has_default_value(option, value):
     :rtype: bool
 
     """
-    return (option in helpful_parser.defaults and
-            helpful_parser.defaults[option] == value)
+    if helpful_parser is not None:
+        return (option in helpful_parser.defaults and
+                helpful_parser.defaults[option] == value)
+    return False
 
 
 def option_was_set(option, value):
@@ -253,11 +263,12 @@ def option_was_set(option, value):
 
 
 def argparse_type(variable):
-    "Return our argparse type function for a config variable (default: str)"
+    """Return our argparse type function for a config variable (default: str)"""
     # pylint: disable=protected-access
-    for action in helpful_parser.parser._actions:
-        if action.type is not None and action.dest == variable:
-            return action.type
+    if helpful_parser is not None:
+        for action in helpful_parser.parser._actions:
+            if action.type is not None and action.dest == variable:
+                return action.type
     return str
 
 def read_file(filename, mode="rb"):
@@ -290,10 +301,12 @@ def flag_default(name):
 
 def config_help(name, hidden=False):
     """Extract the help message for an `.IConfig` attribute."""
+    # pylint: disable=no-member
     if hidden:
         return argparse.SUPPRESS
     else:
-        return interfaces.IConfig[name].__doc__
+        field = interfaces.IConfig.__getitem__(name) # type: zope.interface.interface.Attribute
+        return field.__doc__
 
 
 class HelpfulArgumentGroup(object):
@@ -415,6 +428,12 @@ VERB_HELP = [
                   os.path.join(flag_default("config_dir"), "live"))),
         "usage": "\n\n  certbot update_symlinks [options]\n\n"
     }),
+    ("enhance", {
+        "short": "Add security enhancements to your existing configuration",
+        "opts": ("Helps to harden the TLS configuration by adding security enhancements "
+                 "to already existing configuration."),
+        "usage": "\n\n  certbot enhance [options]\n\n"
+    }),
 
 ]
 # VERB_HELP is a list in order to preserve order, but a dict is sometimes useful
@@ -449,6 +468,7 @@ class HelpfulArgumentParser(object):
             "update_symlinks": main.update_symlinks,
             "certificates": main.certificates,
             "delete": main.delete,
+            "enhance": main.enhance,
         }
 
         # Get notification function for printing
@@ -465,7 +485,7 @@ class HelpfulArgumentParser(object):
         HELP_TOPICS += list(self.VERBS) + self.COMMANDS_TOPICS + ["manage"]
 
         plugin_names = list(plugins)
-        self.help_topics = HELP_TOPICS + plugin_names + [None]
+        self.help_topics = HELP_TOPICS + plugin_names + [None]  # type: ignore
 
         self.detect_defaults = detect_defaults
         self.args = args
@@ -484,8 +504,11 @@ class HelpfulArgumentParser(object):
         short_usage = self._usage_string(plugins, self.help_arg)
 
         self.visible_topics = self.determine_help_topics(self.help_arg)
-        self.groups = {}       # elements are added by .add_group()
-        self.defaults = {}     # elements are added by .parse_args()
+
+        # elements are added by .add_group()
+        self.groups = {}  # type: Dict[str, argparse._ArgumentGroup]
+        # elements are added by .parse_args()
+        self.defaults = {}  # type: Dict[str, Any]
 
         self.parser = configargparse.ArgParser(
             prog="certbot",
@@ -797,7 +820,6 @@ class HelpfulArgumentParser(object):
             if self.help_arg:
                 for v in verbs:
                     self.groups[topic].add_argument(v, help=VERB_HELP_MAP[v]["short"])
-
         return HelpfulArgumentGroup(self, topic)
 
     def add_plugin_args(self, plugins):
@@ -883,21 +905,22 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "flag to 0 disables log rotation entirely, causing "
              "Certbot to always append to the same log file.")
     helpful.add(
-        [None, "automation", "run", "certonly"], "-n", "--non-interactive", "--noninteractive",
+        [None, "automation", "run", "certonly", "enhance"],
+        "-n", "--non-interactive", "--noninteractive",
         dest="noninteractive_mode", action="store_true",
         default=flag_default("noninteractive_mode"),
         help="Run without ever asking for user input. This may require "
               "additional command line flags; the client will try to explain "
               "which ones are required if it finds one missing")
     helpful.add(
-        [None, "register", "run", "certonly"],
+        [None, "register", "run", "certonly", "enhance"],
         constants.FORCE_INTERACTIVE_FLAG, action="store_true",
         default=flag_default("force_interactive"),
         help="Force Certbot to be interactive even if it detects it's not "
              "being run in a terminal. This flag cannot be used with the "
              "renew subcommand.")
     helpful.add(
-        [None, "run", "certonly", "certificates"],
+        [None, "run", "certonly", "certificates", "enhance"],
         "-d", "--domains", "--domain", dest="domains",
         metavar="DOMAIN", action=_DomainsAction,
         default=flag_default("domains"),
@@ -913,8 +936,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "name. In the case of a name collision it will append a number "
              "like 0001 to the file path name. (default: Ask)")
     helpful.add(
-        [None, "run", "certonly", "manage", "delete", "certificates", "renew"],
-        "--cert-name", dest="certname",
+        [None, "run", "certonly", "manage", "delete", "certificates",
+         "renew", "enhance"], "--cert-name", dest="certname",
         metavar="CERTNAME", default=flag_default("certname"),
         help="Certificate name to apply. This name is used by Certbot for housekeeping "
              "and in file paths; it doesn't affect the content of the certificate itself. "
@@ -994,6 +1017,12 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "certificate already exists for the requested certificate name "
              "but does not match the requested domains, renew it now, "
              "regardless of whether it is near expiry.")
+    helpful.add(
+        "automation", "--reuse-key", dest="reuse_key",
+        action="store_true", default=flag_default("reuse_key"),
+        help="When renewing, use the same private key as the existing "
+             "certificate.")
+
     helpful.add(
         ["automation", "renew", "certonly"],
         "--allow-subset-of-names", action="store_true",
@@ -1085,7 +1114,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         dest="must_staple", default=flag_default("must_staple"),
         help=config_help("must_staple"))
     helpful.add(
-        "security", "--redirect", action="store_true", dest="redirect",
+        ["security", "enhance"],
+        "--redirect", action="store_true", dest="redirect",
         default=flag_default("redirect"),
         help="Automatically redirect all HTTP traffic to HTTPS for the newly "
              "authenticated vhost. (default: Ask)")
@@ -1095,7 +1125,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         help="Do not automatically redirect all HTTP traffic to HTTPS for the newly "
              "authenticated vhost. (default: Ask)")
     helpful.add(
-        "security", "--hsts", action="store_true", dest="hsts", default=flag_default("hsts"),
+        ["security", "enhance"],
+        "--hsts", action="store_true", dest="hsts", default=flag_default("hsts"),
         help="Add the Strict-Transport-Security header to every HTTP response."
              " Forcing browser to always use SSL for the domain."
              " Defends against SSL Stripping.")
@@ -1103,7 +1134,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         "security", "--no-hsts", action="store_false", dest="hsts",
         default=flag_default("hsts"), help=argparse.SUPPRESS)
     helpful.add(
-        "security", "--uir", action="store_true", dest="uir", default=flag_default("uir"),
+        ["security", "enhance"],
+        "--uir", action="store_true", dest="uir", default=flag_default("uir"),
         help='Add the "Content-Security-Policy: upgrade-insecure-requests"'
              ' header to every HTTP response. Forcing the browser to use'
              ' https:// for every http:// resource.')
@@ -1180,6 +1212,14 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         default=flag_default("directory_hooks"), dest="directory_hooks",
         help="Disable running executables found in Certbot's hook directories"
         " during renewal. (default: False)")
+    helpful.add(
+        "renew", "--disable-renew-updates", action="store_true",
+        default=flag_default("disable_renew_updates"), dest="disable_renew_updates",
+        help="Disable automatic updates to your server configuration that"
+        " would otherwise be done by the selected installer plugin, and triggered"
+        " when the user executes \"certbot renew\", regardless of if the certificate"
+        " is renewed. This setting does not apply to important TLS configuration"
+        " updates.")
 
     helpful.add_deprecated_argument("--agree-dev-preview", 0)
     helpful.add_deprecated_argument("--dialog", 0)
@@ -1276,14 +1316,14 @@ def _paths_parser(helpful):
         verb = helpful.help_arg
 
     cph = "Path to where certificate is saved (with auth --csr), installed from, or revoked."
-    section = ["paths", "install", "revoke", "certonly", "manage"]
+    sections = ["paths", "install", "revoke", "certonly", "manage"]
     if verb == "certonly":
-        add(section, "--cert-path", type=os.path.abspath,
+        add(sections, "--cert-path", type=os.path.abspath,
             default=flag_default("auth_cert_path"), help=cph)
     elif verb == "revoke":
-        add(section, "--cert-path", type=read_file, required=True, help=cph)
+        add(sections, "--cert-path", type=read_file, required=True, help=cph)
     else:
-        add(section, "--cert-path", type=os.path.abspath, help=cph)
+        add(sections, "--cert-path", type=os.path.abspath, help=cph)
 
     section = "paths"
     if verb in ("install", "revoke"):

certbot/client.py

@@ -4,8 +4,11 @@ import logging
 import os
 import platform
 
+
 from cryptography.hazmat.backends import default_backend
-from cryptography.hazmat.primitives.asymmetric import rsa
+# https://github.com/python/typeshed/blob/master/third_party/
+# 2/cryptography/hazmat/primitives/asymmetric/rsa.pyi
+from cryptography.hazmat.primitives.asymmetric.rsa import generate_private_key  # type: ignore
 import josepy as jose
 import OpenSSL
 import zope.component
@@ -14,6 +17,7 @@ from acme import client as acme_client
 from acme import crypto_util as acme_crypto_util
 from acme import errors as acme_errors
 from acme import messages
+from acme.magic_typing import Optional  # pylint: disable=unused-import,no-name-in-module
 
 import certbot
 
@@ -155,12 +159,16 @@ def register(config, account_storage, tos_cb=None):
         if not config.dry_run:
             logger.info("Registering without email!")
 
+    # If --dry-run is used, and there is no staging account, create one with no email.
+    if config.dry_run:
+        config.email = None
+
     # Each new registration shall use a fresh new key
-    key = jose.JWKRSA(key=jose.ComparableRSAKey(
-        rsa.generate_private_key(
+    rsa_key = generate_private_key(
             public_exponent=65537,
             key_size=config.rsa_key_size,
-            backend=default_backend())))
+            backend=default_backend())
+    key = jose.JWKRSA(key=jose.ComparableRSAKey(rsa_key))
     acme = acme_from_config_key(config, key)
     # TODO: add phone?
     regr = perform_registration(acme, config, tos_cb)
@@ -179,8 +187,9 @@ def perform_registration(acme, config, tos_cb):
     Actually register new account, trying repeatedly if there are email
     problems
 
-    :param .IConfig config: Client configuration.
     :param acme.client.Client client: ACME client object.
+    :param .IConfig config: Client configuration.
+    :param Callable tos_cb: a callback to handle Term of Service agreement.
 
     :returns: Registration Resource.
     :rtype: `acme.messages.RegistrationResource`
@@ -266,7 +275,7 @@ class Client(object):
         cert, chain = crypto_util.cert_and_chain_from_fullchain(orderr.fullchain_pem)
         return cert.encode(), chain.encode()
 
-    def obtain_certificate(self, domains):
+    def obtain_certificate(self, domains, old_keypath=None):
         """Obtains a certificate from the ACME server.
 
         `.register` must be called before `.obtain_certificate`
@@ -279,16 +288,39 @@ class Client(object):
         :rtype: tuple
 
         """
+
+        # We need to determine the key path, key PEM data, CSR path,
+        # and CSR PEM data.  For a dry run, the paths are None because
+        # they aren't permanently saved to disk.  For a lineage with
+        # --reuse-key, the key path and PEM data are derived from an
+        # existing file.
+
+        if old_keypath is not None:
+            # We've been asked to reuse a specific existing private key.
+            # Therefore, we'll read it now and not generate a new one in
+            # either case below.
+            #
+            # We read in bytes here because the type of `key.pem`
+            # created below is also bytes.
+            with open(old_keypath, "rb") as f:
+                keypath = old_keypath
+                keypem = f.read()
+            key = util.Key(file=keypath, pem=keypem) # type: Optional[util.Key]
+            logger.info("Reusing existing private key from %s.", old_keypath)
+        else:
+            # The key is set to None here but will be created below.
+            key = None
+
         # Create CSR from names
         if self.config.dry_run:
-            key = util.Key(file=None,
-                           pem=crypto_util.make_key(self.config.rsa_key_size))
+            key = key or util.Key(file=None,
+                                  pem=crypto_util.make_key(self.config.rsa_key_size))
             csr = util.CSR(file=None, form="pem",
                            data=acme_crypto_util.make_csr(
                                key.pem, domains, self.config.must_staple))
         else:
-            key = crypto_util.init_save_key(
-                self.config.rsa_key_size, self.config.key_dir)
+            key = key or crypto_util.init_save_key(self.config.rsa_key_size,
+                                                   self.config.key_dir)
             csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir)
 
         orderr = self._get_order_and_authorizations(csr.data, self.config.allow_subset_of_names)
@@ -338,9 +370,10 @@ class Client(object):
         authenticator and installer, and then create a new renewable lineage
         containing it.
 
-        :param list domains: Domains to request.
-        :param plugins: A PluginsFactory object.
-        :param str certname: Name of new cert
+        :param domains: domains to request a certificate for
+        :type domains: `list` of `str`
+        :param certname: requested name of lineage
+        :type certname: `str` or `None`
 
         :returns: A new :class:`certbot.storage.RenewableCert` instance
             referred to the enrolled cert lineage, False if the cert could not
@@ -351,17 +384,11 @@ class Client(object):
 
         if (self.config.config_dir != constants.CLI_DEFAULTS["config_dir"] or
                 self.config.work_dir != constants.CLI_DEFAULTS["work_dir"]):
-            logger.warning(
+            logger.info(
                 "Non-standard path(s), might not work with crontab installed "
                 "by your operating system package manager")
 
-        if certname:
-            new_name = certname
-        elif util.is_wildcard_domain(domains[0]):
-            # Don't make files and directories starting with *.
-            new_name = domains[0][2:]
-        else:
-            new_name = domains[0]
+        new_name = self._choose_lineagename(domains, certname)
 
         if self.config.dry_run:
             logger.debug("Dry run: Skipping creating new lineage for %s",
@@ -373,6 +400,26 @@ class Client(object):
                 key.pem, chain,
                 self.config)
 
+    def _choose_lineagename(self, domains, certname):
+        """Chooses a name for the new lineage.
+
+        :param domains: domains in certificate request
+        :type domains: `list` of `str`
+        :param certname: requested name of lineage
+        :type certname: `str` or `None`
+
+        :returns: lineage name that should be used
+        :rtype: str
+
+        """
+        if certname:
+            return certname
+        elif util.is_wildcard_domain(domains[0]):
+            # Don't make files and directories starting with *.
+            return domains[0][2:]
+        else:
+            return domains[0]
+
     def save_certificate(self, cert_pem, chain_pem,
                          cert_path, chain_path, fullchain_path):
         """Saves the certificate received from the ACME server.
@@ -451,7 +498,7 @@ class Client(object):
             # sites may have been enabled / final cleanup
             self.installer.restart()
 
-    def enhance_config(self, domains, chain_path):
+    def enhance_config(self, domains, chain_path, ask_redirect=True):
         """Enhance the configuration.
 
         :param list domains: list of domains to configure
@@ -478,8 +525,9 @@ class Client(object):
         for config_name, enhancement_name, option in enhancement_info:
             config_value = getattr(self.config, config_name)
             if enhancement_name in supported:
-                if config_name == "redirect" and config_value is None:
-                    config_value = enhancements.ask(enhancement_name)
+                if ask_redirect:
+                    if config_name == "redirect" and config_value is None:
+                        config_value = enhancements.ask(enhancement_name)
                 if config_value:
                     self.apply_enhancement(domains, enhancement_name, option)
                     enhanced = True
@@ -515,8 +563,12 @@ class Client(object):
                 try:
                     self.installer.enhance(dom, enhancement, options)
                 except errors.PluginEnhancementAlreadyPresent:
-                    logger.warning("Enhancement %s was already set.",
-                            enhancement)
+                    if enhancement == "ensure-http-header":
+                        logger.warning("Enhancement %s was already set.",
+                                options)
+                    else:
+                        logger.warning("Enhancement %s was already set.",
+                                enhancement)
                 except errors.PluginError:
                     logger.warning("Unable to set enhancement %s for %s",
                             enhancement, dom)
@@ -585,8 +637,10 @@ def validate_key_csr(privkey, csr=None):
         if csr.form == "der":
             csr_obj = OpenSSL.crypto.load_certificate_request(
                 OpenSSL.crypto.FILETYPE_ASN1, csr.data)
-            csr = util.CSR(csr.file, OpenSSL.crypto.dump_certificate(
-                OpenSSL.crypto.FILETYPE_PEM, csr_obj), "pem")
+            cert_buffer = OpenSSL.crypto.dump_certificate_request(
+                OpenSSL.crypto.FILETYPE_PEM, csr_obj
+            )
+            csr = util.CSR(csr.file, cert_buffer, "pem")
 
         # If CSR is provided, it must be readable and valid.
         if csr.data and not crypto_util.valid_csr(csr.data):

certbot/configuration.py

@@ -65,8 +65,12 @@ class NamespaceConfig(object):
 
     @property
     def accounts_dir(self):  # pylint: disable=missing-docstring
+        return self.accounts_dir_for_server_path(self.server_path)
+
+    def accounts_dir_for_server_path(self, server_path):
+        """Path to accounts directory based on server_path"""
         return os.path.join(
-            self.namespace.config_dir, constants.ACCOUNTS_DIR, self.server_path)
+            self.namespace.config_dir, constants.ACCOUNTS_DIR, server_path)
 
     @property
     def backup_dir(self):  # pylint: disable=missing-docstring

certbot/constants.py

@@ -64,6 +64,8 @@ CLI_DEFAULTS = dict(
     pref_challs=[],
     validate_hooks=True,
     directory_hooks=True,
+    reuse_key=False,
+    disable_renew_updates=False,
 
     # Subparsers
     num=None,
@@ -156,6 +158,12 @@ CONFIG_DIRS_MODE = 0o755
 ACCOUNTS_DIR = "accounts"
 """Directory where all accounts are saved."""
 
+LE_REUSE_SERVERS = {
+    'acme-staging-v02.api.letsencrypt.org/directory':
+        'acme-staging.api.letsencrypt.org/directory'
+}
+"""Servers that can reuse accounts from other servers."""
+
 BACKUP_DIR = "backups"
 """Directory (relative to `IConfig.work_dir`) where backups are kept."""
 

certbot/crypto_util.py

@@ -8,15 +8,23 @@ import hashlib
 import logging
 import os
 
-import OpenSSL
+
 import pyrfc3339
 import six
 import zope.component
+from cryptography.exceptions import InvalidSignature
 from cryptography.hazmat.backends import default_backend
-from cryptography import x509
+from cryptography.hazmat.primitives.asymmetric.ec import ECDSA
+from cryptography.hazmat.primitives.asymmetric.ec import EllipticCurvePublicKey
+from cryptography.hazmat.primitives.asymmetric.padding import PKCS1v15
+from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey
+# https://github.com/python/typeshed/tree/master/third_party/2/cryptography
+from cryptography import x509 # type: ignore
+from OpenSSL import crypto
+from OpenSSL import SSL  # type: ignore
 
 from acme import crypto_util as acme_crypto_util
-
+from acme.magic_typing import IO  # pylint: disable=unused-import, no-name-in-module
 from certbot import errors
 from certbot import interfaces
 from certbot import util
@@ -47,7 +55,7 @@ def init_save_key(key_size, key_dir, keyname="key-certbot.pem"):
     try:
         key_pem = make_key(key_size)
     except ValueError as err:
-        logger.exception(err)
+        logger.error("", exc_info=True)
         raise err
 
     config = zope.component.getUtility(interfaces.IConfig)
@@ -111,11 +119,11 @@ def valid_csr(csr):
 
     """
     try:
-        req = OpenSSL.crypto.load_certificate_request(
-            OpenSSL.crypto.FILETYPE_PEM, csr)
+        req = crypto.load_certificate_request(
+            crypto.FILETYPE_PEM, csr)
         return req.verify(req.get_pubkey())
-    except OpenSSL.crypto.Error as error:
-        logger.debug(error, exc_info=True)
+    except crypto.Error:
+        logger.debug("", exc_info=True)
         return False
 
 
@@ -129,13 +137,13 @@ def csr_matches_pubkey(csr, privkey):
     :rtype: bool
 
     """
-    req = OpenSSL.crypto.load_certificate_request(
-        OpenSSL.crypto.FILETYPE_PEM, csr)
-    pkey = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, privkey)
+    req = crypto.load_certificate_request(
+        crypto.FILETYPE_PEM, csr)
+    pkey = crypto.load_privatekey(crypto.FILETYPE_PEM, privkey)
     try:
         return req.verify(pkey)
-    except OpenSSL.crypto.Error as error:
-        logger.debug(error, exc_info=True)
+    except crypto.Error:
+        logger.debug("", exc_info=True)
         return False
 
 
@@ -145,26 +153,26 @@ def import_csr_file(csrfile, data):
     :param str csrfile: CSR filename
     :param str data: contents of the CSR file
 
-    :returns: (`OpenSSL.crypto.FILETYPE_PEM`,
+    :returns: (`crypto.FILETYPE_PEM`,
                util.CSR object representing the CSR,
                list of domains requested in the CSR)
     :rtype: tuple
 
     """
-    PEM = OpenSSL.crypto.FILETYPE_PEM
-    load = OpenSSL.crypto.load_certificate_request
+    PEM = crypto.FILETYPE_PEM
+    load = crypto.load_certificate_request
     try:
         # Try to parse as DER first, then fall back to PEM.
-        csr = load(OpenSSL.crypto.FILETYPE_ASN1, data)
-    except OpenSSL.crypto.Error:
+        csr = load(crypto.FILETYPE_ASN1, data)
+    except crypto.Error:
         try:
             csr = load(PEM, data)
-        except OpenSSL.crypto.Error:
+        except crypto.Error:
             raise errors.Error("Failed to parse CSR file: {0}".format(csrfile))
 
     domains = _get_names_from_loaded_cert_or_req(csr)
     # Internally we always use PEM, so re-encode as PEM before returning.
-    data_pem = OpenSSL.crypto.dump_certificate_request(PEM, csr)
+    data_pem = crypto.dump_certificate_request(PEM, csr)
     return PEM, util.CSR(file=csrfile, data=data_pem, form="pem"), domains
 
 
@@ -178,9 +186,9 @@ def make_key(bits):
 
     """
     assert bits >= 1024  # XXX
-    key = OpenSSL.crypto.PKey()
-    key.generate_key(OpenSSL.crypto.TYPE_RSA, bits)
-    return OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, key)
+    key = crypto.PKey()
+    key.generate_key(crypto.TYPE_RSA, bits)
+    return crypto.dump_privatekey(crypto.FILETYPE_PEM, key)
 
 
 def valid_privkey(privkey):
@@ -193,9 +201,9 @@ def valid_privkey(privkey):
 
     """
     try:
-        return OpenSSL.crypto.load_privatekey(
-            OpenSSL.crypto.FILETYPE_PEM, privkey).check()
-    except (TypeError, OpenSSL.crypto.Error):
+        return crypto.load_privatekey(
+            crypto.FILETYPE_PEM, privkey).check()
+    except (TypeError, crypto.Error):
         return False
 
 
@@ -224,13 +232,27 @@ def verify_renewable_cert_sig(renewable_cert):
     :raises errors.Error: If signature verification fails.
     """
     try:
-        with open(renewable_cert.chain, 'rb') as chain:
-            chain, _ = pyopenssl_load_certificate(chain.read())
-        with open(renewable_cert.cert, 'rb') as cert:
-            cert = x509.load_pem_x509_certificate(cert.read(), default_backend())
-        hash_name = cert.signature_hash_algorithm.name
-        OpenSSL.crypto.verify(chain, cert.signature, cert.tbs_certificate_bytes, hash_name)
-    except (IOError, ValueError, OpenSSL.crypto.Error) as e:
+        with open(renewable_cert.chain, 'rb') as chain_file:  # type: IO[bytes]
+            chain = x509.load_pem_x509_certificate(chain_file.read(), default_backend())
+        with open(renewable_cert.cert, 'rb') as cert_file:  # type: IO[bytes]
+            cert = x509.load_pem_x509_certificate(cert_file.read(), default_backend())
+        pk = chain.public_key()
+        if isinstance(pk, RSAPublicKey):
+            # https://github.com/python/typeshed/blob/master/third_party/2/cryptography/hazmat/primitives/asymmetric/rsa.pyi
+            verifier = pk.verifier(  # type: ignore
+                cert.signature, PKCS1v15(), cert.signature_hash_algorithm
+            )
+            verifier.update(cert.tbs_certificate_bytes)
+            verifier.verify()
+        elif isinstance(pk, EllipticCurvePublicKey):
+            verifier = pk.verifier(
+                cert.signature, ECDSA(cert.signature_hash_algorithm)
+            )
+            verifier.update(cert.tbs_certificate_bytes)
+            verifier.verify()
+        else:
+            raise errors.Error("Unsupported public key type")
+    except (IOError, ValueError, InvalidSignature) as e:
         error_str = "verifying the signature of the cert located at {0} has failed. \
                 Details: {1}".format(renewable_cert.cert, e)
         logger.exception(error_str)
@@ -246,11 +268,11 @@ def verify_cert_matches_priv_key(cert_path, key_path):
     :raises errors.Error: If they don't match.
     """
     try:
-        context = OpenSSL.SSL.Context(OpenSSL.SSL.SSLv23_METHOD)
+        context = SSL.Context(SSL.SSLv23_METHOD)
         context.use_certificate_file(cert_path)
         context.use_privatekey_file(key_path)
         context.check_privatekey()
-    except (IOError, OpenSSL.SSL.Error) as e:
+    except (IOError, SSL.Error) as e:
         error_str = "verifying the cert located at {0} matches the \
                 private key located at {1} has failed. \
                 Details: {2}".format(cert_path,
@@ -267,12 +289,12 @@ def verify_fullchain(renewable_cert):
     :raises errors.Error: If cert and chain do not combine to fullchain.
     """
     try:
-        with open(renewable_cert.chain) as chain:
-            chain = chain.read()
-        with open(renewable_cert.cert) as cert:
-            cert = cert.read()
-        with open(renewable_cert.fullchain) as fullchain:
-            fullchain = fullchain.read()
+        with open(renewable_cert.chain) as chain_file:  # type: IO[str]
+            chain = chain_file.read()
+        with open(renewable_cert.cert) as cert_file:  # type: IO[str]
+            cert = cert_file.read()
+        with open(renewable_cert.fullchain) as fullchain_file:  # type: IO[str]
+            fullchain = fullchain_file.read()
         if (cert + chain) != fullchain:
             error_str = "fullchain does not match cert + chain for {0}!"
             error_str = error_str.format(renewable_cert.lineagename)
@@ -294,43 +316,43 @@ def pyopenssl_load_certificate(data):
 
     openssl_errors = []
 
-    for file_type in (OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1):
+    for file_type in (crypto.FILETYPE_PEM, crypto.FILETYPE_ASN1):
         try:
-            return OpenSSL.crypto.load_certificate(file_type, data), file_type
-        except OpenSSL.crypto.Error as error:  # TODO: other errors?
+            return crypto.load_certificate(file_type, data), file_type
+        except crypto.Error as error:  # TODO: other errors?
             openssl_errors.append(error)
     raise errors.Error("Unable to load: {0}".format(",".join(
         str(error) for error in openssl_errors)))
 
 
 def _load_cert_or_req(cert_or_req_str, load_func,
-                      typ=OpenSSL.crypto.FILETYPE_PEM):
+                      typ=crypto.FILETYPE_PEM):
     try:
         return load_func(typ, cert_or_req_str)
-    except OpenSSL.crypto.Error as error:
-        logger.exception(error)
+    except crypto.Error:
+        logger.error("", exc_info=True)
         raise
 
 
 def _get_sans_from_cert_or_req(cert_or_req_str, load_func,
-                               typ=OpenSSL.crypto.FILETYPE_PEM):
+                               typ=crypto.FILETYPE_PEM):
     # pylint: disable=protected-access
     return acme_crypto_util._pyopenssl_cert_or_req_san(_load_cert_or_req(
         cert_or_req_str, load_func, typ))
 
 
-def get_sans_from_cert(cert, typ=OpenSSL.crypto.FILETYPE_PEM):
+def get_sans_from_cert(cert, typ=crypto.FILETYPE_PEM):
     """Get a list of Subject Alternative Names from a certificate.
 
     :param str cert: Certificate (encoded).
-    :param typ: `OpenSSL.crypto.FILETYPE_PEM` or `OpenSSL.crypto.FILETYPE_ASN1`
+    :param typ: `crypto.FILETYPE_PEM` or `crypto.FILETYPE_ASN1`
 
     :returns: A list of Subject Alternative Names.
     :rtype: list
 
     """
     return _get_sans_from_cert_or_req(
-        cert, OpenSSL.crypto.load_certificate, typ)
+        cert, crypto.load_certificate, typ)
 
 
 def _get_names_from_cert_or_req(cert_or_req, load_func, typ):
@@ -343,24 +365,24 @@ def _get_names_from_loaded_cert_or_req(loaded_cert_or_req):
     return acme_crypto_util._pyopenssl_cert_or_req_all_names(loaded_cert_or_req)
 
 
-def get_names_from_cert(csr, typ=OpenSSL.crypto.FILETYPE_PEM):
+def get_names_from_cert(csr, typ=crypto.FILETYPE_PEM):
     """Get a list of domains from a cert, including the CN if it is set.
 
     :param str cert: Certificate (encoded).
-    :param typ: `OpenSSL.crypto.FILETYPE_PEM` or `OpenSSL.crypto.FILETYPE_ASN1`
+    :param typ: `crypto.FILETYPE_PEM` or `crypto.FILETYPE_ASN1`
 
     :returns: A list of domain names.
     :rtype: list
 
     """
     return _get_names_from_cert_or_req(
-        csr, OpenSSL.crypto.load_certificate, typ)
+        csr, crypto.load_certificate, typ)
 
 
-def dump_pyopenssl_chain(chain, filetype=OpenSSL.crypto.FILETYPE_PEM):
+def dump_pyopenssl_chain(chain, filetype=crypto.FILETYPE_PEM):
     """Dump certificate chain into a bundle.
 
-    :param list chain: List of `OpenSSL.crypto.X509` (or wrapped in
+    :param list chain: List of `crypto.X509` (or wrapped in
         :class:`josepy.util.ComparableX509`).
 
     """
@@ -378,7 +400,7 @@ def notBefore(cert_path):
     :rtype: :class:`datetime.datetime`
 
     """
-    return _notAfterBefore(cert_path, OpenSSL.crypto.X509.get_notBefore)
+    return _notAfterBefore(cert_path, crypto.X509.get_notBefore)
 
 
 def notAfter(cert_path):
@@ -390,15 +412,15 @@ def notAfter(cert_path):
     :rtype: :class:`datetime.datetime`
 
     """
-    return _notAfterBefore(cert_path, OpenSSL.crypto.X509.get_notAfter)
+    return _notAfterBefore(cert_path, crypto.X509.get_notAfter)
 
 
 def _notAfterBefore(cert_path, method):
     """Internal helper function for finding notbefore/notafter.
 
     :param str cert_path: path to a cert in PEM format
-    :param function method: one of ``OpenSSL.crypto.X509.get_notBefore``
-        or ``OpenSSL.crypto.X509.get_notAfter``
+    :param function method: one of ``crypto.X509.get_notBefore``
+        or ``crypto.X509.get_notAfter``
 
     :returns: the notBefore or notAfter value from the cert at cert_path
     :rtype: :class:`datetime.datetime`
@@ -406,7 +428,7 @@ def _notAfterBefore(cert_path, method):
     """
     # pylint: disable=redefined-outer-name
     with open(cert_path) as f:
-        x509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM,
+        x509 = crypto.load_certificate(crypto.FILETYPE_PEM,
                                                f.read())
     # pyopenssl always returns bytes
     timestamp = method(x509)
@@ -443,7 +465,7 @@ def cert_and_chain_from_fullchain(fullchain_pem):
     :rtype: tuple
 
     """
-    cert = OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM,
-        OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, fullchain_pem)).decode()
+    cert = crypto.dump_certificate(crypto.FILETYPE_PEM,
+        crypto.load_certificate(crypto.FILETYPE_PEM, fullchain_pem)).decode()
     chain = fullchain_pem[len(cert):].lstrip()
     return (cert, chain)

certbot/display/ops.py

@@ -86,13 +86,31 @@ def choose_account(accounts):
     else:
         return None
 
+def choose_values(values, question=None):
+    """Display screen to let user pick one or multiple values from the provided
+    list.
 
-def choose_names(installer):
+    :param list values: Values to select from
+
+    :returns: List of selected values
+    :rtype: list
+    """
+    code, items = z_util(interfaces.IDisplay).checklist(
+        question, tags=values, force_interactive=True)
+    if code == display_util.OK and items:
+        return items
+    else:
+        return []
+
+def choose_names(installer, question=None):
     """Display screen to select domains to validate.
 
     :param installer: An installer object
     :type installer: :class:`certbot.interfaces.IInstaller`
 
+    :param `str` question: Overriding dialog question to ask the user if asked
+        to choose from domain names.
+
     :returns: List of selected names
     :rtype: `list` of `str`
 
@@ -108,7 +126,7 @@ def choose_names(installer):
         return _choose_names_manually(
             "No names were found in your configuration files. ")
 
-    code, names = _filter_names(names)
+    code, names = _filter_names(names, question)
     if code == display_util.OK and names:
         return names
     else:
@@ -142,7 +160,7 @@ def _sort_names(FQDNs):
     return sorted(FQDNs, key=lambda fqdn: fqdn.split('.')[::-1][1:])
 
 
-def _filter_names(names):
+def _filter_names(names, override_question=None):
     """Determine which names the user would like to select from a list.
 
     :param list names: domain names
@@ -155,10 +173,12 @@ def _filter_names(names):
     """
     #Sort by domain first, and then by subdomain
     sorted_names = _sort_names(names)
-
+    if override_question:
+        question = override_question
+    else:
+        question = "Which names would you like to activate HTTPS for?"
     code, names = z_util(interfaces.IDisplay).checklist(
-        "Which names would you like to activate HTTPS for?",
-        tags=sorted_names, cli_flag="--domains", force_interactive=True)
+        question, tags=sorted_names, cli_flag="--domains", force_interactive=True)
     return code, [str(s) for s in names]
 
 

certbot/eff.py

@@ -41,8 +41,8 @@ def _want_subscription():
         'Would you be willing to share your email address with the '
         "Electronic Frontier Foundation, a founding partner of the Let's "
         'Encrypt project and the non-profit organization that develops '
-        "Certbot? We'd like to send you email about EFF and our work to "
-        'encrypt the web, protect its users and defend digital rights.')
+        "Certbot? We'd like to send you email about our work encrypting "
+        "the web, EFF news, campaigns, and ways to support digital freedom. ")
     display = zope.component.getUtility(interfaces.IDisplay)
     return display.yesno(prompt, default=False)
 
@@ -71,11 +71,14 @@ def _check_response(response):
 
     """
     logger.debug('Received response:\n%s', response.content)
-    if response.ok:
-        if not response.json()['status']:
+    try:
+        response.raise_for_status()
+        if response.json()['status'] == False:
             _report_failure('your e-mail address appears to be invalid')
-    else:
+    except requests.exceptions.HTTPError:
         _report_failure()
+    except (ValueError, KeyError):
+        _report_failure('there was a problem with the server response')
 
 
 def _report_failure(reason=None):

certbot/error_handler.py

@@ -5,6 +5,10 @@ import os
 import signal
 import traceback
 
+# pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import Any, Callable, Dict, List, Union
+# pylint: enable=unused-import, no-name-in-module
+
 from certbot import errors
 
 logger = logging.getLogger(__name__)
@@ -56,9 +60,9 @@ class ErrorHandler(object):
     def __init__(self, func=None, *args, **kwargs):
         self.call_on_regular_exit = False
         self.body_executed = False
-        self.funcs = []
-        self.prev_handlers = {}
-        self.received_signals = []
+        self.funcs = []  # type: List[Callable[[], Any]]
+        self.prev_handlers = {}  # type: Dict[int, Union[int, None, Callable]]
+        self.received_signals = []  # type: List[int]
         if func is not None:
             self.register(func, *args, **kwargs)
 
@@ -88,6 +92,7 @@ class ErrorHandler(object):
         return retval
 
     def register(self, func, *args, **kwargs):
+        # type: (Callable, *Any, **Any) -> None
         """Sets func to be run with the given arguments during cleanup.
 
         :param function func: function to be called in case of an error
@@ -101,9 +106,8 @@ class ErrorHandler(object):
         while self.funcs:
             try:
                 self.funcs[-1]()
-            except Exception as error:  # pylint: disable=broad-except
-                logger.error("Encountered exception during recovery")
-                logger.exception(error)
+            except Exception:  # pylint: disable=broad-except
+                logger.error("Encountered exception during recovery: ", exc_info=True)
             self.funcs.pop()
 
     def _set_signal_handlers(self):

certbot/errors.py

@@ -87,6 +87,10 @@ class NotSupportedError(PluginError):
     """Certbot Plugin function not supported error."""
 
 
+class PluginStorageError(PluginError):
+    """Certbot Plugin Storage error."""
+
+
 class StandaloneBindError(Error):
     """Standalone plugin bind error."""
 

certbot/hooks.py

@@ -6,6 +6,7 @@ import os
 
 from subprocess import Popen, PIPE
 
+from acme.magic_typing import Set, List # pylint: disable=unused-import, no-name-in-module
 from certbot import errors
 from certbot import util
 
@@ -76,7 +77,8 @@ def pre_hook(config):
     if cmd:
         _run_pre_hook_if_necessary(cmd)
 
-pre_hook.already = set()  # type: ignore
+
+executed_pre_hooks = set()  # type: Set[str]
 
 
 def _run_pre_hook_if_necessary(command):
@@ -88,12 +90,12 @@ def _run_pre_hook_if_necessary(command):
     :param str command: pre-hook to be run
 
     """
-    if command in pre_hook.already:
+    if command in executed_pre_hooks:
         logger.info("Pre-hook command already run, skipping: %s", command)
     else:
         logger.info("Running pre-hook command: %s", command)
         _run_hook(command)
-        pre_hook.already.add(command)
+        executed_pre_hooks.add(command)
 
 
 def post_hook(config):
@@ -127,7 +129,8 @@ def post_hook(config):
         logger.info("Running post-hook command: %s", cmd)
         _run_hook(cmd)
 
-post_hook.eventually = []  # type: ignore
+
+post_hooks = []  # type: List[str]
 
 
 def _run_eventually(command):
@@ -139,13 +142,13 @@ def _run_eventually(command):
     :param str command: post-hook to register to be run
 
     """
-    if command not in post_hook.eventually:
-        post_hook.eventually.append(command)
+    if command not in post_hooks:
+        post_hooks.append(command)
 
 
 def run_saved_post_hooks():
     """Run any post hooks that were saved up in the course of the 'renew' verb"""
-    for cmd in post_hook.eventually:
+    for cmd in post_hooks:
         logger.info("Running post-hook command: %s", cmd)
         _run_hook(cmd)
 

certbot/interfaces.py

@@ -1,16 +1,16 @@
 """Certbot client interfaces."""
 import abc
+import six
 import zope.interface
 
 # pylint: disable=no-self-argument,no-method-argument,no-init,inherit-non-class
 # pylint: disable=too-few-public-methods
 
 
+@six.add_metaclass(abc.ABCMeta)
 class AccountStorage(object):
     """Accounts storage interface."""
 
-    __metaclass__ = abc.ABCMeta
-
     @abc.abstractmethod
     def find_all(self):  # pragma: no cover
         """Find all accounts.
@@ -201,7 +201,9 @@ class IConfig(zope.interface.Interface):
     """
     server = zope.interface.Attribute("ACME Directory Resource URI.")
     email = zope.interface.Attribute(
-        "Email used for registration and recovery contact. (default: Ask)")
+        "Email used for registration and recovery contact. Use comma to "
+        "register multiple emails, ex: u1@example.com,u2@example.com. "
+        "(default: Ask).")
     rsa_key_size = zope.interface.Attribute("Size of the RSA key.")
     must_staple = zope.interface.Attribute(
         "Adds the OCSP Must Staple extension to the certificate. "
@@ -256,6 +258,10 @@ class IConfig(zope.interface.Interface):
         "user; only needed if your config is somewhere unsafe like /tmp/."
         "This is a boolean")
 
+    disable_renew_updates = zope.interface.Attribute(
+        "If updates provided by installer enhancements when Certbot is being run"
+        " with \"renew\" verb should be disabled.")
+
 class IInstaller(IPlugin):
     """Generic Certbot Installer Interface.
 
@@ -591,3 +597,72 @@ class IReporter(zope.interface.Interface):
 
     def print_messages(self):
         """Prints messages to the user and clears the message queue."""
+
+
+# Updater interfaces
+#
+# When "certbot renew" is run, Certbot will iterate over each lineage and check
+# if the selected installer for that lineage is a subclass of each updater
+# class. If it is and the update of that type is configured to be run for that
+# lineage, the relevant update function will be called for it. These functions
+# are never called for other subcommands, so if an installer wants to perform
+# an update during the run or install subcommand, it should do so when
+# :func:`IInstaller.deploy_cert` is called.
+
+@six.add_metaclass(abc.ABCMeta)
+class GenericUpdater(object):
+    """Interface for update types not currently specified by Certbot.
+
+    This class allows plugins to perform types of updates that Certbot hasn't
+    defined (yet).
+
+    To make use of this interface, the installer should implement the interface
+    methods, and interfaces.GenericUpdater.register(InstallerClass) should
+    be called from the installer code.
+
+    """
+
+    @abc.abstractmethod
+    def generic_updates(self, lineage, *args, **kwargs):
+        """Perform any update types defined by the installer.
+
+        If an installer is a subclass of the class containing this method, this
+        function will always be called when "certbot renew" is run. If the
+        update defined by the installer should be run conditionally, the
+        installer needs to handle checking the conditions itself.
+
+        This method is called once for each lineage.
+
+        :param lineage: Certificate lineage object
+        :type lineage: storage.RenewableCert
+
+        """
+
+
+@six.add_metaclass(abc.ABCMeta)
+class RenewDeployer(object):
+    """Interface for update types run when a lineage is renewed
+
+    This class allows plugins to perform types of updates that need to run at
+    lineage renewal that Certbot hasn't defined (yet).
+
+    To make use of this interface, the installer should implement the interface
+    methods, and interfaces.RenewDeployer.register(InstallerClass) should
+    be called from the installer code.
+    """
+
+    @abc.abstractmethod
+    def renew_deploy(self, lineage, *args, **kwargs):
+        """Perform updates defined by installer when a certificate has been renewed
+
+        If an installer is a subclass of the class containing this method, this
+        function will always be called when a certficate has been renewed by
+        running "certbot renew". For example if a plugin needs to copy a
+        certificate over, or change configuration based on the new certificate.
+
+        This method is called once for each lineage renewed
+
+        :param lineage: Certificate lineage object
+        :type lineage: storage.RenewableCert
+
+        """

certbot/log.py

@@ -191,9 +191,8 @@ class MemoryHandler(logging.handlers.MemoryHandler):
     only happens when flush(force=True) is called.
 
     """
-    def __init__(self, target=None):
+    def __init__(self, target=None, capacity=10000):
         # capacity doesn't matter because should_flush() is overridden
-        capacity = float('inf')
         super(MemoryHandler, self).__init__(capacity, target=target)
 
     def close(self):

certbot/main.py

@@ -11,6 +11,7 @@ import josepy as jose
 import zope.component
 
 from acme import errors as acme_errors
+from acme.magic_typing import Union  # pylint: disable=unused-import, no-name-in-module
 
 import certbot
 
@@ -29,6 +30,7 @@ from certbot import log
 from certbot import renewal
 from certbot import reporter
 from certbot import storage
+from certbot import updater
 from certbot import util
 
 from certbot.display import util as display_util, ops as display_ops
@@ -323,7 +325,7 @@ def _find_lineage_for_domains_and_certname(config, domains, certname):
                 return "newcert", None
             else:
                 raise errors.ConfigurationError("No certificate with name {0} found. "
-                    "Use -d to specify domains, or run certbot --certificates to see "
+                    "Use -d to specify domains, or run certbot certificates to see "
                     "possible certificate names.".format(certname))
 
 def _get_added_removed(after, before):
@@ -339,7 +341,10 @@ def _get_added_removed(after, before):
 def _format_list(character, strings):
     """Format list with given character
     """
-    formatted = "{br}{ch} " + "{br}{ch} ".join(strings)
+    if len(strings) == 0:
+        formatted = "{br}(None)"
+    else:
+        formatted = "{br}{ch} " + "{br}{ch} ".join(strings)
     return formatted.format(
         ch=character,
         br=os.linesep
@@ -382,7 +387,7 @@ def _ask_user_to_confirm_new_names(config, new_domains, certname, old_domains):
     if not obj.yesno(msg, "Update cert", "Cancel", default=True):
         raise errors.ConfigurationError("Specified mismatched cert name and domains.")
 
-def _find_domains_or_certname(config, installer):
+def _find_domains_or_certname(config, installer, question=None):
     """Retrieve domains and certname from config or user input.
 
     :param config: Configuration object
@@ -391,6 +396,8 @@ def _find_domains_or_certname(config, installer):
     :param installer: Installer object
     :type installer: interfaces.IInstaller
 
+    :param `str` question: Overriding dialog question to ask the user if asked
+        to choose from domain names.
 
     :returns: Two-part tuple of domains and certname
     :rtype: `tuple` of list of `str` and `str`
@@ -411,7 +418,7 @@ def _find_domains_or_certname(config, installer):
     # that certname might not have existed, or there was a problem.
     # try to get domains from the user.
     if not domains:
-        domains = display_ops.choose_names(installer)
+        domains = display_ops.choose_names(installer, question)
 
     if not domains and not certname:
         raise errors.Error("Please specify --domains, or --installer that "
@@ -480,6 +487,21 @@ def _determine_account(config):
     :raises errors.Error: If unable to register an account with ACME server
 
     """
+    def _tos_cb(terms_of_service):
+        if config.tos:
+            return True
+        msg = ("Please read the Terms of Service at {0}. You "
+               "must agree in order to register with the ACME "
+               "server at {1}".format(
+                   terms_of_service, config.server))
+        obj = zope.component.getUtility(interfaces.IDisplay)
+        result = obj.yesno(msg, "Agree", "Cancel",
+                         cli_flag="--agree-tos", force_interactive=True)
+        if not result:
+            raise errors.Error(
+                "Registration cannot proceed without accepting "
+                "Terms of Service.")
+
     account_storage = account.AccountFileStorage(config)
     acme = None
 
@@ -494,28 +516,13 @@ def _determine_account(config):
         else:  # no account registered yet
             if config.email is None and not config.register_unsafely_without_email:
                 config.email = display_ops.get_email()
-
-            def _tos_cb(terms_of_service):
-                if config.tos:
-                    return True
-                msg = ("Please read the Terms of Service at {0}. You "
-                       "must agree in order to register with the ACME "
-                       "server at {1}".format(
-                           terms_of_service, config.server))
-                obj = zope.component.getUtility(interfaces.IDisplay)
-                result = obj.yesno(msg, "Agree", "Cancel",
-                                 cli_flag="--agree-tos", force_interactive=True)
-                if not result:
-                    raise errors.Error(
-                        "Registration cannot proceed without accepting "
-                        "Terms of Service.")
             try:
                 acc, acme = client.register(
                     config, account_storage, tos_cb=_tos_cb)
             except errors.MissingCommandlineFlag:
                 raise
-            except errors.Error as error:
-                logger.debug(error, exc_info=True)
+            except errors.Error:
+                logger.debug("", exc_info=True)
                 raise errors.Error(
                     "Unable to register an account with ACME server")
 
@@ -728,8 +735,9 @@ def register(config, unused_plugins):
     acc, acme = _determine_account(config)
     cb_client = client.Client(config, acc, None, None, acme=acme)
     # We rely on an exception to interrupt this process if it didn't work.
+    acc_contacts = ['mailto:' + email for email in config.email.split(',')]
     acc.regr = cb_client.acme.update_registration(acc.regr.update(
-        body=acc.regr.body.update(contact=('mailto:' + config.email,))))
+        body=acc.regr.body.update(contact=acc_contacts)))
     account_storage.save_regr(acc, cb_client.acme)
     eff.handle_subscription(config)
     add_msg("Your e-mail address was updated to {0}.".format(config.email))
@@ -859,6 +867,53 @@ def plugins_cmd(config, plugins):
     logger.debug("Prepared plugins: %s", available)
     notify(str(available))
 
+def enhance(config, plugins):
+    """Add security enhancements to existing configuration
+
+    :param config: Configuration object
+    :type config: interfaces.IConfig
+
+    :param plugins: List of plugins
+    :type plugins: `list` of `str`
+
+    :returns: `None`
+    :rtype: None
+
+    """
+    supported_enhancements = ["hsts", "redirect", "uir", "staple"]
+    # Check that at least one enhancement was requested on command line
+    if not any([getattr(config, enh) for enh in supported_enhancements]):
+        msg = ("Please specify one or more enhancement types to configure. To list "
+               "the available enhancement types, run:\n\n%s --help enhance\n")
+        logger.warning(msg, sys.argv[0])
+        raise errors.MisconfigurationError("No enhancements requested, exiting.")
+
+    try:
+        installer, _ = plug_sel.choose_configurator_plugins(config, plugins, "enhance")
+    except errors.PluginSelectionError as e:
+        return str(e)
+
+    certname_question = ("Which certificate would you like to use to enhance "
+                         "your configuration?")
+    config.certname = cert_manager.get_certnames(
+        config, "enhance", allow_multiple=False,
+        custom_prompt=certname_question)[0]
+    cert_domains = cert_manager.domains_for_certname(config, config.certname)
+    if config.noninteractive_mode:
+        domains = cert_domains
+    else:
+        domain_question = ("Which domain names would you like to enable the "
+                           "selected enhancements for?")
+        domains = display_ops.choose_values(cert_domains, domain_question)
+        if not domains:
+            raise errors.Error("User cancelled the domain selection. No domains "
+                               "defined, exiting.")
+    if not config.chain_path:
+        lineage = cert_manager.lineage_for_certname(config, config.certname)
+        config.chain_path = lineage.chain_path
+    le_client = _init_le_client(config, authenticator=None, installer=installer)
+    le_client.enhance_config(domains, config.chain_path, ask_redirect=False)
+
 
 def rollback(config, plugins):
     """Rollback server configuration changes made during install.
@@ -1096,10 +1151,9 @@ def renew_cert(config, plugins, lineage):
     except errors.PluginSelectionError as e:
         logger.info("Could not choose appropriate plugin: %s", e)
         raise
-
     le_client = _init_le_client(config, auth, installer)
 
-    _get_and_save_cert(le_client, config, lineage=lineage)
+    renewed_lineage = _get_and_save_cert(le_client, config, lineage=lineage)
 
     notify = zope.component.getUtility(interfaces.IDisplay).notification
     if installer is None:
@@ -1109,9 +1163,11 @@ def renew_cert(config, plugins, lineage):
         # In case of a renewal, reload server to pick up new certificate.
         # In principle we could have a configuration option to inhibit this
         # from happening.
+        updater.run_renewal_deployer(config, renewed_lineage, installer)
         installer.restart()
         notify("new certificate deployed with reload of {0} server; fullchain is {1}".format(
                config.installer, lineage.fullchain), pause=False)
+        # Run deployer
 
 def certonly(config, plugins):
     """Authenticate & obtain cert, but do not install it.
@@ -1217,7 +1273,8 @@ def set_displayer(config):
     """
     if config.quiet:
         config.noninteractive_mode = True
-        displayer = display_util.NoninteractiveDisplay(open(os.devnull, "w"))
+        displayer = display_util.NoninteractiveDisplay(open(os.devnull, "w")) \
+        # type: Union[None, display_util.NoninteractiveDisplay, display_util.FileDisplay]
     elif config.noninteractive_mode:
         displayer = display_util.NoninteractiveDisplay(sys.stdout)
     else:

certbot/plugins/common.py

@@ -11,6 +11,8 @@ import zope.interface
 
 from josepy import util as jose_util
 
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
+from certbot import achallenges  # pylint: disable=unused-import
 from certbot import constants
 from certbot import crypto_util
 from certbot import errors
@@ -18,6 +20,8 @@ from certbot import interfaces
 from certbot import reverter
 from certbot import util
 
+from certbot.plugins.storage import PluginStorage
+
 logger = logging.getLogger(__name__)
 
 
@@ -99,7 +103,6 @@ class Plugin(object):
     def conf(self, var):
         """Find a configuration value for variable ``var``."""
         return getattr(self.config, self.dest(var))
-# other
 
 
 class Installer(Plugin):
@@ -110,6 +113,7 @@ class Installer(Plugin):
     """
     def __init__(self, *args, **kwargs):
         super(Installer, self).__init__(*args, **kwargs)
+        self.storage = PluginStorage(self.config, self.name)
         self.reverter = reverter.Reverter(self.config)
 
     def add_to_checkpoint(self, save_files, save_notes, temporary=False):
@@ -329,8 +333,8 @@ class ChallengePerformer(object):
 
     def __init__(self, configurator):
         self.configurator = configurator
-        self.achalls = []
-        self.indices = []
+        self.achalls = []  # type: List[achallenges.KeyAuthorizationAnnotatedChallenge]
+        self.indices = []  # type: List[int]
 
     def add_chall(self, achall, idx=None):
         """Store challenge to be performed when perform() is called.

certbot/plugins/disco.py

@@ -10,6 +10,7 @@ from collections import OrderedDict
 import zope.interface
 import zope.interface.verify
 
+from acme.magic_typing import Dict  # pylint: disable=unused-import, no-name-in-module
 from certbot import constants
 from certbot import errors
 from certbot import interfaces
@@ -189,7 +190,7 @@ class PluginsRegistry(collections.Mapping):
     @classmethod
     def find_all(cls):
         """Find plugins using setuptools entry points."""
-        plugins = {}
+        plugins = {}  # type: Dict[str, PluginEntryPoint]
         # pylint: disable=not-callable
         entry_points = itertools.chain(
             pkg_resources.iter_entry_points(

certbot/plugins/disco_test.py

@@ -8,6 +8,7 @@ import pkg_resources
 import six
 import zope.interface
 
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 from certbot import errors
 from certbot import interfaces
 
@@ -250,7 +251,7 @@ class PluginsRegistryTest(unittest.TestCase):
         self.plugin_ep.prepare.assert_called_once_with()
 
     def test_prepare_order(self):
-        order = []
+        order = []  # type: List[str]
         plugins = dict(
             (c, mock.MagicMock(prepare=functools.partial(order.append, c)))
             for c in string.ascii_letters)

certbot/plugins/manual.py

@@ -5,7 +5,9 @@ import zope.component
 import zope.interface
 
 from acme import challenges
+from acme.magic_typing import Dict  # pylint: disable=unused-import, no-name-in-module
 
+from certbot import achallenges  # pylint: disable=unused-import
 from certbot import interfaces
 from certbot import errors
 from certbot import hooks
@@ -98,7 +100,8 @@ when it receives a TLS ClientHello with the SNI extension set to
         super(Authenticator, self).__init__(*args, **kwargs)
         self.reverter = reverter.Reverter(self.config)
         self.reverter.recovery_routine()
-        self.env = dict()
+        self.env = dict() \
+        # type: Dict[achallenges.KeyAuthorizationAnnotatedChallenge, Dict[str, str]]
         self.tls_sni_01 = None
 
     @classmethod

certbot/plugins/selection.py

@@ -147,6 +147,7 @@ def record_chosen_plugins(config, plugins, auth, inst):
 
 
 def choose_configurator_plugins(config, plugins, verb):
+    # pylint: disable=too-many-branches
     """
     Figure out which configurator we're going to use, modifies
     config.authenticator and config.installer strings to reflect that choice if
@@ -159,6 +160,11 @@ def choose_configurator_plugins(config, plugins, verb):
     """
 
     req_auth, req_inst = cli_plugin_requests(config)
+    installer_question = None
+
+    if verb == "enhance":
+        installer_question = ("Which installer would you like to use to "
+                              "configure the selected enhancements?")
 
     # Which plugins do we need?
     if verb == "run":
@@ -176,11 +182,11 @@ def choose_configurator_plugins(config, plugins, verb):
         need_inst = need_auth = False
     if verb == "certonly":
         need_auth = True
-    if verb == "install":
+    if verb == "install" or verb == "enhance":
         need_inst = True
         if config.authenticator:
-            logger.warning("Specifying an authenticator doesn't make sense in install mode")
-
+            logger.warning("Specifying an authenticator doesn't make sense when "
+                           "running Certbot with verb \"%s\"", verb)
     # Try to meet the user's request and/or ask them to pick plugins
     authenticator = installer = None
     if verb == "run" and req_auth == req_inst:
@@ -189,7 +195,7 @@ def choose_configurator_plugins(config, plugins, verb):
         authenticator = installer = pick_configurator(config, req_inst, plugins)
     else:
         if need_inst or req_inst:
-            installer = pick_installer(config, req_inst, plugins)
+            installer = pick_installer(config, req_inst, plugins, installer_question)
         if need_auth:
             authenticator = pick_authenticator(config, req_auth, plugins)
     logger.debug("Selected authenticator %s and installer %s", authenticator, installer)

certbot/plugins/selection_test.py

@@ -6,6 +6,7 @@ import unittest
 import mock
 import zope.component
 
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 from certbot.display import util as display_util
 from certbot.tests import util as test_util
 from certbot import interfaces
@@ -47,7 +48,7 @@ class PickPluginTest(unittest.TestCase):
         self.default = None
         self.reg = mock.MagicMock()
         self.question = "Question?"
-        self.ifaces = []
+        self.ifaces = []  # type: List[interfaces.IPlugin]
 
     def _call(self):
         from certbot.plugins.selection import pick_plugin

certbot/plugins/standalone.py

@@ -3,6 +3,8 @@ import argparse
 import collections
 import logging
 import socket
+# https://github.com/python/typeshed/blob/master/stdlib/2and3/socket.pyi
+from socket import errno as socket_errors  # type: ignore
 
 import OpenSSL
 import six
@@ -10,7 +12,10 @@ import zope.interface
 
 from acme import challenges
 from acme import standalone as acme_standalone
+# pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import DefaultDict, Dict, Set, Tuple, List, Type, TYPE_CHECKING
 
+from certbot import achallenges  # pylint: disable=unused-import
 from certbot import errors
 from certbot import interfaces
 
@@ -18,6 +23,11 @@ from certbot.plugins import common
 
 logger = logging.getLogger(__name__)
 
+if TYPE_CHECKING:
+    ServedType = DefaultDict[
+        acme_standalone.BaseDualNetworkedServers,
+        Set[achallenges.KeyAuthorizationAnnotatedChallenge]
+    ]
 
 class ServerManager(object):
     """Standalone servers manager.
@@ -33,7 +43,7 @@ class ServerManager(object):
 
     """
     def __init__(self, certs, http_01_resources):
-        self._instances = {}
+        self._instances = {}  # type: Dict[int, acme_standalone.BaseDualNetworkedServers]
         self.certs = certs
         self.http_01_resources = http_01_resources
 
@@ -59,7 +69,8 @@ class ServerManager(object):
         address = (listenaddr, port)
         try:
             if challenge_type is challenges.TLSSNI01:
-                servers = acme_standalone.TLSSNI01DualNetworkedServers(address, self.certs)
+                servers = acme_standalone.TLSSNI01DualNetworkedServers(
+                    address, self.certs)  # type: acme_standalone.BaseDualNetworkedServers
             else:  # challenges.HTTP01
                 servers = acme_standalone.HTTP01DualNetworkedServers(
                     address, self.http_01_resources)
@@ -103,7 +114,8 @@ class ServerManager(object):
         return self._instances.copy()
 
 
-SUPPORTED_CHALLENGES = [challenges.TLSSNI01, challenges.HTTP01]
+SUPPORTED_CHALLENGES = [challenges.TLSSNI01, challenges.HTTP01] \
+# type: List[Type[challenges.KeyAuthorizationChallenge]]
 
 
 class SupportedChallengesAction(argparse.Action):
@@ -179,14 +191,15 @@ class Authenticator(common.Plugin):
         self.key = OpenSSL.crypto.PKey()
         self.key.generate_key(OpenSSL.crypto.TYPE_RSA, 2048)
 
-        self.served = collections.defaultdict(set)
+        self.served = collections.defaultdict(set)  # type: ServedType
 
         # Stuff below is shared across threads (i.e. servers read
         # values, main thread writes). Due to the nature of CPython's
         # GIL, the operations are safe, c.f.
         # https://docs.python.org/2/faq/library.html#what-kinds-of-global-value-mutation-are-thread-safe
-        self.certs = {}
-        self.http_01_resources = set()
+        self.certs = {}  # type: Dict[bytes, Tuple[OpenSSL.crypto.PKey, OpenSSL.crypto.X509]]
+        self.http_01_resources = set() \
+        # type: Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource]
 
         self.servers = ServerManager(self.certs, self.http_01_resources)
 
@@ -265,13 +278,13 @@ class Authenticator(common.Plugin):
 
 
 def _handle_perform_error(error):
-    if error.socket_error.errno == socket.errno.EACCES:
+    if error.socket_error.errno == socket_errors.EACCES:
         raise errors.PluginError(
             "Could not bind TCP port {0} because you don't have "
             "the appropriate permissions (for example, you "
             "aren't running this program as "
             "root).".format(error.port))
-    elif error.socket_error.errno == socket.errno.EADDRINUSE:
+    elif error.socket_error.errno == socket_errors.EADDRINUSE:
         display = zope.component.getUtility(interfaces.IDisplay)
         msg = (
             "Could not bind TCP port {0} because it is already in "

certbot/plugins/standalone_test.py

@@ -2,12 +2,18 @@
 import argparse
 import socket
 import unittest
+# https://github.com/python/typeshed/blob/master/stdlib/2and3/socket.pyi
+from socket import errno as socket_errors  # type: ignore
 
 import josepy as jose
 import mock
 import six
 
+import OpenSSL.crypto  # pylint: disable=unused-import
+
 from acme import challenges
+from acme import standalone as acme_standalone  # pylint: disable=unused-import
+from acme.magic_typing import Dict, Tuple, Set  # pylint: disable=unused-import, no-name-in-module
 
 from certbot import achallenges
 from certbot import errors
@@ -21,8 +27,9 @@ class ServerManagerTest(unittest.TestCase):
 
     def setUp(self):
         from certbot.plugins.standalone import ServerManager
-        self.certs = {}
-        self.http_01_resources = {}
+        self.certs = {}  # type: Dict[bytes, Tuple[OpenSSL.crypto.PKey, OpenSSL.crypto.X509]]
+        self.http_01_resources = {} \
+        # type: Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource]
         self.mgr = ServerManager(self.certs, self.http_01_resources)
 
     def test_init(self):
@@ -159,7 +166,7 @@ class AuthenticatorTest(unittest.TestCase):
     @test_util.patch_get_utility()
     def test_perform_eaddrinuse_retry(self, mock_get_utility):
         mock_utility = mock_get_utility()
-        errno = socket.errno.EADDRINUSE
+        errno = socket_errors.EADDRINUSE
         error = errors.StandaloneBindError(mock.MagicMock(errno=errno), -1)
         self.auth.servers.run.side_effect = [error] + 2 * [mock.MagicMock()]
         mock_yesno = mock_utility.yesno
@@ -174,7 +181,7 @@ class AuthenticatorTest(unittest.TestCase):
         mock_yesno = mock_utility.yesno
         mock_yesno.return_value = False
 
-        errno = socket.errno.EADDRINUSE
+        errno = socket_errors.EADDRINUSE
         self.assertRaises(errors.PluginError, self._fail_perform, errno)
         self._assert_correct_yesno_call(mock_yesno)
 
@@ -184,11 +191,11 @@ class AuthenticatorTest(unittest.TestCase):
         self.assertFalse(yesno_kwargs.get("default", True))
 
     def test_perform_eacces(self):
-        errno = socket.errno.EACCES
+        errno = socket_errors.EACCES
         self.assertRaises(errors.PluginError, self._fail_perform, errno)
 
     def test_perform_unexpected_socket_error(self):
-        errno = socket.errno.ENOTCONN
+        errno = socket_errors.ENOTCONN
         self.assertRaises(
             errors.StandaloneBindError, self._fail_perform, errno)
 

certbot/plugins/storage.py

@@ -0,0 +1,119 @@
+"""Plugin storage class."""
+import json
+import logging
+import os
+
+from acme.magic_typing import Any, Dict  # pylint: disable=unused-import, no-name-in-module
+from certbot import errors
+
+logger = logging.getLogger(__name__)
+
+class PluginStorage(object):
+    """Class implementing storage functionality for plugins"""
+
+    def __init__(self, config, classkey):
+        """Initializes PluginStorage object storing required configuration
+        options.
+
+        :param .configuration.NamespaceConfig config: Configuration object
+        :param str classkey: class name to use as root key in storage file
+
+        """
+
+        self._config = config
+        self._classkey = classkey
+        self._initialized = False
+        self._data = None
+        self._storagepath = None
+
+    def _initialize_storage(self):
+        """Initializes PluginStorage data and reads current state from the disk
+        if the storage json exists."""
+
+        self._storagepath = os.path.join(self._config.config_dir, ".pluginstorage.json")
+        self._load()
+        self._initialized = True
+
+    def _load(self):
+        """Reads PluginStorage content from the disk to a dict structure
+
+        :raises .errors.PluginStorageError: when unable to open or read the file
+        """
+        data = dict()  # type: Dict[str, Any]
+        filedata = ""
+        try:
+            with open(self._storagepath, 'r') as fh:
+                filedata = fh.read()
+        except IOError as e:
+            errmsg = "Could not read PluginStorage data file: {0} : {1}".format(
+                self._storagepath, str(e))
+            if os.path.isfile(self._storagepath):
+                # Only error out if file exists, but cannot be read
+                logger.error(errmsg)
+                raise errors.PluginStorageError(errmsg)
+        try:
+            data = json.loads(filedata)
+        except ValueError:
+            if not filedata:
+                logger.debug("Plugin storage file %s was empty, no values loaded",
+                             self._storagepath)
+            else:
+                errmsg = "PluginStorage file {0} is corrupted.".format(
+                    self._storagepath)
+                logger.error(errmsg)
+                raise errors.PluginStorageError(errmsg)
+        self._data = data
+
+    def save(self):
+        """Saves PluginStorage content to disk
+
+        :raises .errors.PluginStorageError: when unable to serialize the data
+            or write it to the filesystem
+        """
+        if not self._initialized:
+            errmsg = "Unable to save, no values have been added to PluginStorage."
+            logger.error(errmsg)
+            raise errors.PluginStorageError(errmsg)
+
+        try:
+            serialized = json.dumps(self._data)
+        except TypeError as e:
+            errmsg = "Could not serialize PluginStorage data: {0}".format(
+                str(e))
+            logger.error(errmsg)
+            raise errors.PluginStorageError(errmsg)
+        try:
+            with os.fdopen(os.open(self._storagepath,
+                                   os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
+                                   0o600), 'w') as fh:
+                fh.write(serialized)
+        except IOError as e:
+            errmsg = "Could not write PluginStorage data to file {0} : {1}".format(
+                self._storagepath, str(e))
+            logger.error(errmsg)
+            raise errors.PluginStorageError(errmsg)
+
+    def put(self, key, value):
+        """Put configuration value to PluginStorage
+
+        :param str key: Key to store the value to
+        :param value: Data to store
+        """
+        if not self._initialized:
+            self._initialize_storage()
+
+        if not self._classkey in self._data.keys():
+            self._data[self._classkey] = dict()
+        self._data[self._classkey][key] = value
+
+    def fetch(self, key):
+        """Get configuration value from PluginStorage
+
+        :param str key: Key to get value from the storage
+
+        :raises KeyError: If the key doesn't exist in the storage
+        """
+        if not self._initialized:
+            self._initialize_storage()
+
+        return self._data[self._classkey][key]

certbot/plugins/storage_test.py

@@ -0,0 +1,117 @@
+"""Tests for certbot.plugins.storage.PluginStorage"""
+import json
+import mock
+import os
+import unittest
+
+from certbot import errors
+
+from certbot.plugins import common
+from certbot.tests import util as test_util
+
+class PluginStorageTest(test_util.ConfigTestCase):
+    """Test for certbot.plugins.storage.PluginStorage"""
+
+    def setUp(self):
+        super(PluginStorageTest, self).setUp()
+        self.plugin_cls = common.Installer
+        os.mkdir(self.config.config_dir)
+        with mock.patch("certbot.reverter.util"):
+            self.plugin = self.plugin_cls(config=self.config, name="mockplugin")
+
+    def test_load_errors_cant_read(self):
+        with open(os.path.join(self.config.config_dir,
+                               ".pluginstorage.json"), "w") as fh:
+            fh.write("dummy")
+        # When unable to read file that exists
+        mock_open = mock.mock_open()
+        mock_open.side_effect = IOError
+        self.plugin.storage.storagepath = os.path.join(self.config.config_dir,
+                                                       ".pluginstorage.json")
+        with mock.patch("six.moves.builtins.open", mock_open):
+            with mock.patch('os.path.isfile', return_value=True):
+                with mock.patch("certbot.reverter.util"):
+                    self.assertRaises(errors.PluginStorageError,
+                                      self.plugin.storage._load)  # pylint: disable=protected-access
+
+    def test_load_errors_empty(self):
+        with open(os.path.join(self.config.config_dir, ".pluginstorage.json"), "w") as fh:
+            fh.write('')
+        with mock.patch("certbot.plugins.storage.logger.debug") as mock_log:
+            # Should not error out but write a debug log line instead
+            with mock.patch("certbot.reverter.util"):
+                nocontent = self.plugin_cls(self.config, "mockplugin")
+            self.assertRaises(KeyError,
+                              nocontent.storage.fetch, "value")
+            self.assertTrue(mock_log.called)
+            self.assertTrue("no values loaded" in mock_log.call_args[0][0])
+
+    def test_load_errors_corrupted(self):
+        with open(os.path.join(self.config.config_dir,
+                               ".pluginstorage.json"), "w") as fh:
+            fh.write('invalid json')
+        with mock.patch("certbot.plugins.storage.logger.error") as mock_log:
+            with mock.patch("certbot.reverter.util"):
+                corrupted = self.plugin_cls(self.config, "mockplugin")
+            self.assertRaises(errors.PluginError,
+                              corrupted.storage.fetch,
+                              "value")
+            self.assertTrue("is corrupted" in mock_log.call_args[0][0])
+
+    def test_save_errors_cant_serialize(self):
+        with mock.patch("certbot.plugins.storage.logger.error") as mock_log:
+            # Set data as something that can't be serialized
+            self.plugin.storage._initialized = True  # pylint: disable=protected-access
+            self.plugin.storage.storagepath = "/tmp/whatever"
+            self.plugin.storage._data = self.plugin_cls  # pylint: disable=protected-access
+            self.assertRaises(errors.PluginStorageError,
+                              self.plugin.storage.save)
+            self.assertTrue("Could not serialize" in mock_log.call_args[0][0])
+
+    def test_save_errors_unable_to_write_file(self):
+        mock_open = mock.mock_open()
+        mock_open.side_effect = IOError
+        with mock.patch("os.open", mock_open):
+            with mock.patch("certbot.plugins.storage.logger.error") as mock_log:
+                self.plugin.storage._data = {"valid": "data"}  # pylint: disable=protected-access
+                self.plugin.storage._initialized = True  # pylint: disable=protected-access
+                self.assertRaises(errors.PluginStorageError,
+                                  self.plugin.storage.save)
+                self.assertTrue("Could not write" in mock_log.call_args[0][0])
+
+    def test_save_uninitialized(self):
+        with mock.patch("certbot.reverter.util"):
+            self.assertRaises(errors.PluginStorageError,
+                              self.plugin_cls(self.config, "x").storage.save)
+
+    def test_namespace_isolation(self):
+        with mock.patch("certbot.reverter.util"):
+            plugin1 = self.plugin_cls(self.config, "first")
+            plugin2 = self.plugin_cls(self.config, "second")
+        plugin1.storage.put("first_key", "first_value")
+        self.assertRaises(KeyError,
+                          plugin2.storage.fetch, "first_key")
+        self.assertRaises(KeyError,
+                          plugin2.storage.fetch, "first")
+        self.assertEqual(plugin1.storage.fetch("first_key"), "first_value")
+
+
+    def test_saved_state(self):
+        self.plugin.storage.put("testkey", "testvalue")
+        # Write to disk
+        self.plugin.storage.save()
+        with mock.patch("certbot.reverter.util"):
+            another = self.plugin_cls(self.config, "mockplugin")
+        self.assertEqual(another.storage.fetch("testkey"), "testvalue")
+
+        with open(os.path.join(self.config.config_dir,
+                               ".pluginstorage.json"), 'r') as fh:
+            psdata = fh.read()
+        psjson = json.loads(psdata)
+        self.assertTrue("mockplugin" in psjson.keys())
+        self.assertEqual(len(psjson), 1)
+        self.assertEqual(psjson["mockplugin"]["testkey"], "testvalue")
+
+
+if __name__ == "__main__":
+    unittest.main()  # pragma: no cover

certbot/plugins/webroot.py

@@ -10,8 +10,12 @@ import six
 import zope.component
 import zope.interface
 
-from acme import challenges
+from acme import challenges  # pylint: disable=unused-import
+# pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import Dict, Set, DefaultDict, List
+# pylint: enable=unused-import, no-name-in-module
 
+from certbot import achallenges  # pylint: disable=unused-import
 from certbot import cli
 from certbot import errors
 from certbot import interfaces
@@ -64,10 +68,11 @@ to serve all files under specified web root ({0})."""
 
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
-        self.full_roots = {}
-        self.performed = collections.defaultdict(set)
+        self.full_roots = {}  # type: Dict[str, str]
+        self.performed = collections.defaultdict(set) \
+        # type: DefaultDict[str, Set[achallenges.KeyAuthorizationAnnotatedChallenge]]
         # stack of dirs successfully created by this authenticator
-        self._created_dirs = []
+        self._created_dirs = []  # type: List[str]
 
     def prepare(self):  # pylint: disable=missing-docstring
         pass
@@ -156,7 +161,6 @@ to serve all files under specified web root ({0})."""
                 " --help webroot for examples.")
         for name, path in path_map.items():
             self.full_roots[name] = os.path.join(path, challenges.HTTP01.URI_ROOT_PATH)
-
             logger.debug("Creating root challenges validation dir at %s",
                          self.full_roots[name])
 
@@ -207,7 +211,6 @@ to serve all files under specified web root ({0})."""
             os.umask(old_umask)
 
         self.performed[root_path].add(achall)
-
         return response
 
     def cleanup(self, achalls):  # pylint: disable=missing-docstring
@@ -219,7 +222,7 @@ to serve all files under specified web root ({0})."""
                 os.remove(validation_path)
                 self.performed[root_path].remove(achall)
 
-        not_removed = []
+        not_removed = []  # type: List[str]
         while len(self._created_dirs) > 0:
             path = self._created_dirs.pop()
             try:

certbot/renewal.py

@@ -11,14 +11,17 @@ import zope.component
 
 import OpenSSL
 
-from certbot import cli
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 
+from certbot import cli
 from certbot import crypto_util
 from certbot import errors
 from certbot import interfaces
 from certbot import util
 from certbot import hooks
 from certbot import storage
+from certbot import updater
+
 from certbot.plugins import disco as plugins_disco
 
 logger = logging.getLogger(__name__)
@@ -33,7 +36,7 @@ STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
                     "pre_hook", "post_hook", "tls_sni_01_address",
                     "http01_address"]
 INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
-BOOL_CONFIG_ITEMS = ["must_staple", "allow_subset_of_names"]
+BOOL_CONFIG_ITEMS = ["must_staple", "allow_subset_of_names", "reuse_key"]
 
 CONFIG_ITEMS = set(itertools.chain(
     BOOL_CONFIG_ITEMS, INT_CONFIG_ITEMS, STR_CONFIG_ITEMS, ('pref_challs',)))
@@ -58,8 +61,8 @@ def _reconstitute(config, full_path):
     """
     try:
         renewal_candidate = storage.RenewableCert(full_path, config)
-    except (errors.CertStorageError, IOError) as exc:
-        logger.warning(exc)
+    except (errors.CertStorageError, IOError):
+        logger.warning("", exc_info=True)
         logger.warning("Renewal configuration file %s is broken. Skipping.", full_path)
         logger.debug("Traceback was:\n%s", traceback.format_exc())
         return None
@@ -132,14 +135,15 @@ def _restore_plugin_configs(config, renewalparams):
     #      longer defined, stored copies of that parameter will be
     #      deserialized as strings by this logic even if they were
     #      originally meant to be some other type.
+    plugin_prefixes = []  # type: List[str]
     if renewalparams["authenticator"] == "webroot":
         _restore_webroot_config(config, renewalparams)
-        plugin_prefixes = []
     else:
-        plugin_prefixes = [renewalparams["authenticator"]]
+        plugin_prefixes.append(renewalparams["authenticator"])
 
-    if renewalparams.get("installer", None) is not None:
+    if renewalparams.get("installer") is not None:
         plugin_prefixes.append(renewalparams["installer"])
+
     for plugin_prefix in set(plugin_prefixes):
         plugin_prefix = plugin_prefix.replace('-', '_')
         for config_item, config_value in six.iteritems(renewalparams):
@@ -294,7 +298,10 @@ def renew_cert(config, domains, le_client, lineage):
     _avoid_invalidating_lineage(config, lineage, original_server)
     if not domains:
         domains = lineage.names()
-    new_cert, new_chain, new_key, _ = le_client.obtain_certificate(domains)
+    # The private key is the existing lineage private key if reuse_key is set.
+    # Otherwise, generate a fresh private key by passing None.
+    new_key = lineage.privkey if config.reuse_key else None
+    new_cert, new_chain, new_key, _ = le_client.obtain_certificate(domains, new_key)
     if config.dry_run:
         logger.debug("Dry run: skipping updating lineage at %s",
                     os.path.dirname(lineage.cert))
@@ -315,13 +322,13 @@ def report(msgs, category):
 def _renew_describe_results(config, renew_successes, renew_failures,
                             renew_skipped, parse_failures):
 
-    out = []
+    out = []  # type: List[str]
     notify = out.append
     disp = zope.component.getUtility(interfaces.IDisplay)
 
     def notify_error(err):
         """Notify and log errors."""
-        notify(err)
+        notify(str(err))
         logger.error(err)
 
     if config.dry_run:
@@ -411,9 +418,9 @@ def handle_renewal_request(config):
                 # XXX: ensure that each call here replaces the previous one
                 zope.component.provideUtility(lineage_config)
                 renewal_candidate.ensure_deployed()
+                from certbot import main
+                plugins = plugins_disco.PluginsRegistry.find_all()
                 if should_renew(lineage_config, renewal_candidate):
-                    plugins = plugins_disco.PluginsRegistry.find_all()
-                    from certbot import main
                     # domains have been restored into lineage_config by reconstitute
                     # but they're unnecessary anyway because renew_cert here
                     # will just grab them from the certificate
@@ -426,6 +433,10 @@ def handle_renewal_request(config):
                         "cert", renewal_candidate.latest_common_version()))
                     renew_skipped.append("%s expires on %s" % (renewal_candidate.fullchain,
                                          expiry.strftime("%Y-%m-%d")))
+                # Run updater interface methods
+                updater.run_generic_updaters(lineage_config, renewal_candidate,
+                                             plugins)
+
         except Exception as e:  # pylint: disable=broad-except
             # obtain_cert (presumably) encountered an unanticipated problem.
             logger.warning("Attempting to renew cert (%s) from %s produced an "

certbot/reverter.py

@@ -82,8 +82,10 @@ class Reverter(object):
                 self._recover_checkpoint(self.config.temp_checkpoint_dir)
             except errors.ReverterError:
                 # We have a partial or incomplete recovery
-                logger.fatal("Incomplete or failed recovery for %s",
-                             self.config.temp_checkpoint_dir)
+                logger.critical(
+                    "Incomplete or failed recovery for %s",
+                    self.config.temp_checkpoint_dir,
+                )
                 raise errors.ReverterError("Unable to revert temporary config")
 
     def rollback_checkpoints(self, rollback=1):
@@ -123,7 +125,7 @@ class Reverter(object):
             try:
                 self._recover_checkpoint(cp_dir)
             except errors.ReverterError:
-                logger.fatal("Failed to load checkpoint during rollback")
+                logger.critical("Failed to load checkpoint during rollback")
                 raise errors.ReverterError(
                     "Unable to load checkpoint during rollback")
             rollback -= 1
@@ -181,7 +183,7 @@ class Reverter(object):
         if for_logging:
             return os.linesep.join(output)
         zope.component.getUtility(interfaces.IDisplay).notification(
-            os.linesep.join(output), force_interactive=True)
+            os.linesep.join(output), force_interactive=True, pause=False)
 
     def add_to_temp_checkpoint(self, save_files, save_notes):
         """Add files to temporary checkpoint.
@@ -457,7 +459,7 @@ class Reverter(object):
                 self._recover_checkpoint(self.config.in_progress_dir)
             except errors.ReverterError:
                 # We have a partial or incomplete recovery
-                logger.fatal("Incomplete or failed recovery for IN_PROGRESS "
+                logger.critical("Incomplete or failed recovery for IN_PROGRESS "
                              "checkpoint - %s",
                              self.config.in_progress_dir)
                 raise errors.ReverterError(
@@ -494,7 +496,7 @@ class Reverter(object):
                             "Certbot probably shut down unexpectedly",
                             os.linesep, path)
         except (IOError, OSError):
-            logger.fatal(
+            logger.critical(
                 "Unable to remove filepaths contained within %s", file_list)
             raise errors.ReverterError(
                 "Unable to remove filepaths contained within "

certbot/storage.py

@@ -1053,6 +1053,9 @@ class RenewableCert(object):
                     "`cert.pem`     : will break many server configurations, and "
                                         "should not be used\n"
                     "                 without reading further documentation (see link below).\n\n"
+                    "WARNING: DO NOT MOVE THESE FILES!\n"
+                    "         Certbot expects these files to remain in this location in order\n"
+                    "         to function properly!\n\n"
                     "We recommend not moving these files. For more information, see the Certbot\n"
                     "User Guide at https://certbot.eff.org/docs/using.html#where-are-my-"
                                         "certificates.\n")

certbot/tests/account_test.py

@@ -95,6 +95,7 @@ class AccountMemoryStorageTest(unittest.TestCase):
 
 class AccountFileStorageTest(test_util.ConfigTestCase):
     """Tests for certbot.account.AccountFileStorage."""
+    #pylint: disable=too-many-public-methods
 
     def setUp(self):
         super(AccountFileStorageTest, self).setUp()
@@ -159,7 +160,8 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         self.assertEqual([], self.storage.find_all())
 
     def test_find_all_load_skips(self):
-        self.storage.load = mock.MagicMock(
+        # pylint: disable=protected-access
+        self.storage._load_for_server_path = mock.MagicMock(
             side_effect=["x", errors.AccountStorageError, "z"])
         with mock.patch("certbot.account.os.listdir") as mock_listdir:
             mock_listdir.return_value = ["x", "y", "z"]
@@ -175,6 +177,64 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         self.assertRaises(errors.AccountStorageError, self.storage.load,
                           "x" + self.acc.id)
 
+    def _set_server(self, server):
+        self.config.server = server
+        from certbot.account import AccountFileStorage
+        self.storage = AccountFileStorage(self.config)
+
+    def test_find_all_neither_exists(self):
+        self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
+        self.assertEqual([], self.storage.find_all())
+        self.assertEqual([], self.storage.find_all())
+        self.assertFalse(os.path.islink(self.config.accounts_dir))
+
+    def test_find_all_find_before_save(self):
+        self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
+        self.assertEqual([], self.storage.find_all())
+        self.storage.save(self.acc, self.mock_client)
+        self.assertEqual([self.acc], self.storage.find_all())
+        self.assertEqual([self.acc], self.storage.find_all())
+        self.assertFalse(os.path.islink(self.config.accounts_dir))
+        # we shouldn't have created a v1 account
+        prev_server_path = 'https://acme-staging.api.letsencrypt.org/directory'
+        self.assertFalse(os.path.isdir(self.config.accounts_dir_for_server_path(prev_server_path)))
+
+    def test_find_all_save_before_find(self):
+        self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
+        self.storage.save(self.acc, self.mock_client)
+        self.assertEqual([self.acc], self.storage.find_all())
+        self.assertEqual([self.acc], self.storage.find_all())
+        self.assertFalse(os.path.islink(self.config.accounts_dir))
+        self.assertTrue(os.path.isdir(self.config.accounts_dir))
+        prev_server_path = 'https://acme-staging.api.letsencrypt.org/directory'
+        self.assertFalse(os.path.isdir(self.config.accounts_dir_for_server_path(prev_server_path)))
+
+    def test_find_all_server_downgrade(self):
+        # don't use v2 accounts with a v1 url
+        self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
+        self.assertEqual([], self.storage.find_all())
+        self.storage.save(self.acc, self.mock_client)
+        self.assertEqual([self.acc], self.storage.find_all())
+        self._set_server('https://acme-staging.api.letsencrypt.org/directory')
+        self.assertEqual([], self.storage.find_all())
+
+    def test_upgrade_version(self):
+        self._set_server('https://acme-staging.api.letsencrypt.org/directory')
+        self.storage.save(self.acc, self.mock_client)
+        self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
+        self.assertEqual([self.acc], self.storage.find_all())
+
+    @mock.patch('os.rmdir')
+    def test_corrupted_account(self, mock_rmdir):
+        # pylint: disable=protected-access
+        self._set_server('https://acme-staging.api.letsencrypt.org/directory')
+        self.storage.save(self.acc, self.mock_client)
+        mock_rmdir.side_effect = OSError
+        self.storage._load_for_server_path = mock.MagicMock(
+            side_effect=errors.AccountStorageError)
+        self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
+        self.assertEqual([], self.storage.find_all())
+
     def test_load_ioerror(self):
         self.storage.save(self.acc, self.mock_client)
         mock_open = mock.mock_open()

certbot/tests/auth_handler_test.py

@@ -10,6 +10,7 @@ import zope.component
 from acme import challenges
 from acme import client as acme_client
 from acme import messages
+from acme.magic_typing import Dict  # pylint: disable=unused-import, no-name-in-module
 
 from certbot import achallenges
 from certbot import errors
@@ -354,12 +355,13 @@ class PollChallengesTest(unittest.TestCase):
                 acme_util.CHALLENGES, [messages.STATUS_PENDING] * 3, False), [])
         ]
 
-        self.chall_update = {}
+        self.chall_update = {}  # type: Dict[int, achallenges.KeyAuthorizationAnnotatedChallenge]
         for i, aauthzr in enumerate(self.aauthzrs):
             self.chall_update[i] = [
                 challb_to_achall(challb, mock.Mock(key="dummy_key"), self.doms[i])
                 for challb in aauthzr.authzr.body.challenges]
 
+
     @mock.patch("certbot.auth_handler.time")
     def test_poll_challenges(self, unused_mock_time):
         self.mock_net.poll.side_effect = self._mock_poll_solve_one_valid