Merge branch 'master' into no-keyauthorization

Remove display.py.

.travis.yml

@@ -8,6 +8,16 @@ before_script:
   - 'if [ $TRAVIS_OS_NAME = osx ] ; then ulimit -n 1024 ; fi'
   - export TOX_TESTENV_PASSENV=TRAVIS
 
+# Only build pushes to the master branch, PRs, and branches beginning with
+# `test-` or of the form `digit(s).digit(s).x`. This reduces the number of
+# simultaneous Travis runs, which speeds turnaround time on review since there
+# is a cap of on the number of simultaneous runs.
+branches:
+  only:
+    - master
+    - /^\d+\.\d+\.x$/
+    - /^test-.*$/
+
 matrix:
   include:
     # These environments are always executed
@@ -62,85 +72,87 @@ matrix:
     - python: "2.7"
       env: TOXENV=nginxroundtrip
 
-    # These environments are executed on cron events only
+    # These environments are executed on cron events and commits to tested
+    # branches other than master. Which branches are tested is controlled by
+    # the "branches" section earlier in this file.
     - python: "3.7"
       dist: xenial
       env: TOXENV=py37 CERTBOT_NO_PIN=1
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "2.7"
       env: BOULDER_INTEGRATION=v1 INTEGRATION_TEST=certbot TOXENV=py27-certbot-oldest
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "2.7"
       env: BOULDER_INTEGRATION=v2 INTEGRATION_TEST=certbot TOXENV=py27-certbot-oldest
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "2.7"
       env: BOULDER_INTEGRATION=v1 INTEGRATION_TEST=nginx TOXENV=py27-nginx-oldest
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "2.7"
       env: BOULDER_INTEGRATION=v2 INTEGRATION_TEST=nginx TOXENV=py27-nginx-oldest
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "3.4"
       env: TOXENV=py34 BOULDER_INTEGRATION=v1
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "3.4"
       env: TOXENV=py34 BOULDER_INTEGRATION=v2
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "3.5"
       env: TOXENV=py35 BOULDER_INTEGRATION=v1
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "3.5"
       env: TOXENV=py35 BOULDER_INTEGRATION=v2
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "3.6"
       env: TOXENV=py36 BOULDER_INTEGRATION=v1
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "3.6"
       env: TOXENV=py36 BOULDER_INTEGRATION=v2
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "3.7"
       dist: xenial
       env: TOXENV=py37 BOULDER_INTEGRATION=v1
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - python: "3.7"
       dist: xenial
       env: TOXENV=py37 BOULDER_INTEGRATION=v2
       sudo: required
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - sudo: required
       env: TOXENV=le_auto_xenial
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - sudo: required
       env: TOXENV=le_auto_jessie
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - sudo: required
       env: TOXENV=le_auto_centos6
       services: docker
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - sudo: required
       env: TOXENV=docker_dev
       services: docker
@@ -148,7 +160,7 @@ matrix:
         apt:
           packages:  # don't install nginx and apache
             - libaugeas0
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - language: generic
       env: TOXENV=py27
       os: osx
@@ -157,7 +169,7 @@ matrix:
           packages:
             - augeas
             - python2
-      if: type = cron
+      if: type = cron OR (type = push AND branch != master)
     - language: generic
       env: TOXENV=py3
       os: osx
@@ -166,19 +178,7 @@ matrix:
           packages:
             - augeas
             - python3
-      if: type = cron
-
-
-
-# Only build pushes to the master branch, PRs, and branches beginning with
-# `test-` or of the form `digit(s).digit(s).x`. This reduces the number of
-# simultaneous Travis runs, which speeds turnaround time on review since there
-# is a cap of on the number of simultaneous runs.
-branches:
-  only:
-    - master
-    - /^\d+\.\d+\.x$/
-    - /^test-.*$/
+      if: type = cron OR (type = push AND branch != master)
 
 # container-based infrastructure
 sudo: false

CHANGELOG.md

@@ -2,17 +2,54 @@
 
 Certbot adheres to [Semantic Versioning](https://semver.org/).
 
-## 0.31.0 - master
+## 0.32.0 - master
 
 ### Added
 
-* Avoid to process again challenges that are already validated
-  when a certificate is issued.
+*
 
 ### Changed
 
+* Certbot and its acme module now depend on josepy>=1.1.0 to avoid printing the
+  warnings described at https://github.com/certbot/josepy/issues/13.
+* Apache plugin now respects CERTBOT_DOCS environment variable when adding
+  command line defaults.
+* The running of manual plugin hooks is now always included in Certbot's log
+  output.
+* Tests execution for certbot, certbot-apache and certbot-nginx packages now relies on pytest.
+
+### Fixed
+
+*
+
+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
+package with changes other than its version number was:
+
+* acme
+* certbot
+* certbot-apache
+* certbot-nginx
+
+More details about these changes can be found on our GitHub repo.
+
+## 0.31.0 - 2019-02-07
+
+### Added
+
+* Avoid reprocessing challenges that are already validated
+  when a certificate is issued.
+* Support for initiating (but not solving end-to-end) TLS-ALPN-01 challenges
+  with the `acme` module.
+
+### Changed
+
+* Certbot's official Docker images are now based on Alpine Linux 3.9 rather
+  than 3.7. The new version comes with OpenSSL 1.1.1.
 * Lexicon-based DNS plugins are now fully compatible with Lexicon 3.x (support
   on 2.x branch is maintained).
+* Apache plugin now attempts to configure all VirtualHosts matching requested
+  domain name instead of only a single one when answering the HTTP-01 challenge.
 
 ### Fixed
 
@@ -26,6 +63,7 @@ package with changes other than its version number was:
 
 * acme
 * certbot
+* certbot-apache
 * certbot-dns-cloudxns
 * certbot-dns-dnsimple
 * certbot-dns-dnsmadeeasy

Dockerfile

@@ -1,4 +1,4 @@
-FROM python:2-alpine3.7
+FROM python:2-alpine3.9
 
 ENTRYPOINT [ "certbot" ]
 EXPOSE 80 443
@@ -12,7 +12,7 @@ COPY certbot src/certbot
 
 RUN apk add --no-cache --virtual .certbot-deps \
         libffi \
-        libssl1.0 \
+        libssl1.1 \
         openssl \
         ca-certificates \
         binutils

acme/acme/challenges.py

@@ -105,7 +105,6 @@ class KeyAuthorizationChallengeResponse(ChallengeResponse):
     :param unicode key_authorization:
 
     """
-    key_authorization = jose.Field("keyAuthorization")
     thumbprint_hash_function = hashes.SHA256
 
     def verify(self, chall, account_public_key):
@@ -115,29 +114,10 @@ class KeyAuthorizationChallengeResponse(ChallengeResponse):
             this response.
         :param JWK account_public_key:
 
-        :return: ``True`` iff verification of the key authorization was
-            successful.
+        :return: ``True``
         :rtype: bool
 
         """
-        parts = self.key_authorization.split('.')  # pylint: disable=no-member
-        if len(parts) != 2:
-            logger.debug("Key authorization (%r) is not well formed",
-                         self.key_authorization)
-            return False
-
-        if parts[0] != chall.encode("token"):
-            logger.debug("Mismatching token in key authorization: "
-                         "%r instead of %r", parts[0], chall.encode("token"))
-            return False
-
-        thumbprint = jose.b64encode(account_public_key.thumbprint(
-            hash_function=self.thumbprint_hash_function)).decode()
-        if parts[1] != thumbprint:
-            logger.debug("Mismatching thumbprint in key authorization: "
-                         "%r instead of %r", parts[0], thumbprint)
-            return False
-
         return True
 
 
@@ -175,8 +155,7 @@ class KeyAuthorizationChallenge(_TokenChallenge):
         :rtype: KeyAuthorizationChallengeResponse
 
         """
-        return self.response_cls(
-            key_authorization=self.key_authorization(account_key))
+        return self.response_cls()
 
     @abc.abstractmethod
     def validation(self, account_key, **kwargs):
@@ -513,6 +492,17 @@ 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.
+
+    This class only allows initiating a TLS-ALPN-01 challenge returned from the
+    CA. Full support for responding to TLS-ALPN-01 challenges by generating and
+    serving the expected response certificate is not currently provided.
+    """
+    typ = "tls-alpn-01"
+
+
 @Challenge.register  # pylint: disable=too-many-ancestors
 class TLSALPN01(KeyAuthorizationChallenge):
     """ACME tls-alpn-01 challenge.
@@ -522,6 +512,7 @@ class TLSALPN01(KeyAuthorizationChallenge):
 
     """
     typ = "tls-alpn-01"
+    response_cls = TLSALPN01Response
 
     def validation(self, account_key, **kwargs):
         """Generate validation for the challenge."""

acme/acme/challenges_test.py

@@ -402,6 +402,33 @@ class TLSSNI01Test(unittest.TestCase):
             KEY, cert_key=mock.sentinel.cert_key))
         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 TLSALPN01Response
+        self.msg = TLSALPN01Response(key_authorization=u'foo')
+        self.jmsg = {
+            'resource': 'challenge',
+            'type': 'tls-alpn-01',
+            'keyAuthorization': u'foo',
+        }
+
+        from acme.challenges import TLSALPN01
+        self.chall = TLSALPN01(token=(b'x' * 16))
+        self.response = self.chall.response(KEY)
+
+    def test_to_partial_json(self):
+        self.assertEqual(self.jmsg, self.msg.to_partial_json())
+
+    def test_from_json(self):
+        from acme.challenges import TLSALPN01Response
+        self.assertEqual(self.msg, TLSALPN01Response.from_json(self.jmsg))
+
+    def test_from_json_hashable(self):
+        from acme.challenges import TLSALPN01Response
+        hash(TLSALPN01Response.from_json(self.jmsg))
+
 
 class TLSALPN01Test(unittest.TestCase):
 

acme/acme/client.py

@@ -739,8 +739,7 @@ class ClientV2(ClientBase):
             if body.error is not None:
                 raise errors.IssuanceError(body.error)
             if body.certificate is not None:
-                certificate_response = self._post_as_get(body.certificate,
-                                                    content_type=DER_CONTENT_TYPE).text
+                certificate_response = self._post_as_get(body.certificate).text
                 return orderr.update(body=body, fullchain_pem=certificate_response)
         raise errors.TimeoutError()
 

acme/examples/http01_example.py

@@ -0,0 +1,240 @@
+"""Example ACME-V2 API for HTTP-01 challenge.
+
+Brief:
+
+This a complete usage example of the python-acme API.
+
+Limitations of this example:
+    - Works for only one Domain name
+    - Performs only HTTP-01 challenge
+    - Uses ACME-v2
+
+Workflow:
+    (Account creation)
+    - Create account key
+    - Register account and accept TOS
+    (Certificate actions)
+    - Select HTTP-01 within offered challenges by the CA server
+    - Set up http challenge resource
+    - Set up standalone web server
+    - Create domain private key and CSR
+    - Issue certificate
+    - Renew certificate
+    - Revoke certificate
+    (Account update actions)
+    - Change contact information
+    - Deactivate Account
+"""
+from contextlib import contextmanager
+from cryptography.hazmat.backends import default_backend
+from cryptography.hazmat.primitives.asymmetric import rsa
+import OpenSSL
+
+from acme import challenges
+from acme import client
+from acme import crypto_util
+from acme import errors
+from acme import messages
+from acme import standalone
+import josepy as jose
+
+# Constants:
+
+# This is the staging point for ACME-V2 within Let's Encrypt.
+DIRECTORY_URL = 'https://acme-staging-v02.api.letsencrypt.org/directory'
+
+USER_AGENT = 'python-acme-example'
+
+# Account key size
+ACC_KEY_BITS = 2048
+
+# Certificate private key size
+CERT_PKEY_BITS = 2048
+
+# Domain name for the certificate.
+DOMAIN = 'client.example.com'
+
+# If you are running Boulder locally, it is possible to configure any port
+# number to execute the challenge, but real CA servers will always use port
+# 80, as described in the ACME specification.
+PORT = 80
+
+
+# Useful methods and classes:
+
+
+def new_csr_comp(domain_name, pkey_pem=None):
+    """Create certificate signing request."""
+    if pkey_pem is None:
+        # Create private key.
+        pkey = OpenSSL.crypto.PKey()
+        pkey.generate_key(OpenSSL.crypto.TYPE_RSA, CERT_PKEY_BITS)
+        pkey_pem = OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM,
+                                                  pkey)
+    csr_pem = crypto_util.make_csr(pkey_pem, [domain_name])
+    return pkey_pem, csr_pem
+
+
+def select_http01_chall(orderr):
+    """Extract authorization resource from within order resource."""
+    # Authorization Resource: authz.
+    # This object holds the offered challenges by the server and their status.
+    authz_list = orderr.authorizations
+
+    for authz in authz_list:
+        # Choosing challenge.
+        # authz.body.challenges is a set of ChallengeBody objects.
+        for i in authz.body.challenges:
+            # Find the supported challenge.
+            if isinstance(i.chall, challenges.HTTP01):
+                return i
+
+    raise Exception('HTTP-01 challenge was not offered by the CA server.')
+
+
+@contextmanager
+def challenge_server(http_01_resources):
+    """Manage standalone server set up and shutdown."""
+
+    # Setting up a fake server that binds at PORT and any address.
+    address = ('', PORT)
+    try:
+        servers = standalone.HTTP01DualNetworkedServers(address,
+                                                        http_01_resources)
+        # Start client standalone web server.
+        servers.serve_forever()
+        yield servers
+    finally:
+        # Shutdown client web server and unbind from PORT
+        servers.shutdown_and_server_close()
+
+
+def perform_http01(client_acme, challb, orderr):
+    """Set up standalone webserver and perform HTTP-01 challenge."""
+
+    response, validation = challb.response_and_validation(client_acme.net.key)
+
+    resource = standalone.HTTP01RequestHandler.HTTP01Resource(
+        chall=challb.chall, response=response, validation=validation)
+
+    with challenge_server({resource}):
+        # Let the CA server know that we are ready for the challenge.
+        client_acme.answer_challenge(challb, response)
+
+        # Wait for challenge status and then issue a certificate.
+        # It is possible to set a deadline time.
+        finalized_orderr = client_acme.poll_and_finalize(orderr)
+
+    return finalized_orderr.fullchain_pem
+
+
+# Main examples:
+
+
+def example_http():
+    """This example executes the whole process of fulfilling a HTTP-01
+    challenge for one specific domain.
+
+    The workflow consists of:
+    (Account creation)
+    - Create account key
+    - Register account and accept TOS
+    (Certificate actions)
+    - Select HTTP-01 within offered challenges by the CA server
+    - Set up http challenge resource
+    - Set up standalone web server
+    - Create domain private key and CSR
+    - Issue certificate
+    - Renew certificate
+    - Revoke certificate
+    (Account update actions)
+    - Change contact information
+    - Deactivate Account
+
+    """
+    # Create account key
+
+    acc_key = jose.JWKRSA(
+        key=rsa.generate_private_key(public_exponent=65537,
+                                     key_size=ACC_KEY_BITS,
+                                     backend=default_backend()))
+
+    # Register account and accept TOS
+
+    net = client.ClientNetwork(acc_key, user_agent=USER_AGENT)
+    directory = messages.Directory.from_json(net.get(DIRECTORY_URL).json())
+    client_acme = client.ClientV2(directory, net=net)
+
+    # Terms of Service URL is in client_acme.directory.meta.terms_of_service
+    # Registration Resource: regr
+    # Creates account with contact information.
+    email = ('fake@example.com')
+    regr = client_acme.new_account(
+        messages.NewRegistration.from_data(
+            email=email, terms_of_service_agreed=True))
+
+    # Create domain private key and CSR
+    pkey_pem, csr_pem = new_csr_comp(DOMAIN)
+
+    # Issue certificate
+
+    orderr = client_acme.new_order(csr_pem)
+
+    # Select HTTP-01 within offered challenges by the CA server
+    challb = select_http01_chall(orderr)
+
+    # The certificate is ready to be used in the variable "fullchain_pem".
+    fullchain_pem = perform_http01(client_acme, challb, orderr)
+
+    # Renew certificate
+
+    _, csr_pem = new_csr_comp(DOMAIN, pkey_pem)
+
+    orderr = client_acme.new_order(csr_pem)
+
+    challb = select_http01_chall(orderr)
+
+    # Performing challenge
+    fullchain_pem = perform_http01(client_acme, challb, orderr)
+
+    # Revoke certificate
+
+    fullchain_com = jose.ComparableX509(
+        OpenSSL.crypto.load_certificate(
+            OpenSSL.crypto.FILETYPE_PEM, fullchain_pem))
+
+    try:
+        client_acme.revoke(fullchain_com, 0)  # revocation reason = 0
+    except errors.ConflictError:
+        # Certificate already revoked.
+        pass
+
+    # Query registration status.
+    client_acme.net.account = regr
+    try:
+        regr = client_acme.query_registration(regr)
+    except errors.Error as err:
+        if err.typ == messages.OLD_ERROR_PREFIX + 'unauthorized' \
+                or err.typ == messages.ERROR_PREFIX + 'unauthorized':
+            # Status is deactivated.
+            pass
+        raise
+
+    # Change contact information
+
+    email = 'newfake@example.com'
+    regr = client_acme.update_registration(
+        regr.update(
+            body=regr.body.update(
+                contact=('mailto:' + email,)
+            )
+        )
+    )
+
+    # Deactivate account/registration
+
+    regr = client_acme.deactivate_registration(regr)
+
+
+if __name__ == "__main__":
+    example_http()

acme/setup.py

@@ -3,7 +3,7 @@ from setuptools import find_packages
 from setuptools.command.test import test as TestCommand
 import sys
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
@@ -11,7 +11,9 @@ install_requires = [
     # rsa_recover_prime_factors (>=0.8)
     'cryptography>=1.2.3',
     # formerly known as acme.jose:
-    'josepy>=1.0.0',
+    # 1.1.0+ is required to avoid the warnings described at
+    # https://github.com/certbot/josepy/issues/13.
+    'josepy>=1.1.0',
     # Connection.set_tlsext_host_name (>=0.13)
     'mock',
     'PyOpenSSL>=0.13.1',
@@ -34,6 +36,7 @@ docs_extras = [
     'sphinx_rtd_theme',
 ]
 
+
 class PyTest(TestCommand):
     user_options = []
 
@@ -48,6 +51,7 @@ class PyTest(TestCommand):
         errno = pytest.main(shlex.split(self.pytest_args))
         sys.exit(errno)
 
+
 setup(
     name='acme',
     version=version,
@@ -80,7 +84,7 @@ setup(
         'dev': dev_extras,
         'docs': docs_extras,
     },
-    tests_require=["pytest"],
     test_suite='acme',
+    tests_require=["pytest"],
     cmdclass={"test": PyTest},
 )

certbot-apache/certbot_apache/configurator.py

@@ -92,6 +92,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
     """
 
     description = "Apache Web Server plugin"
+    if os.environ.get("CERTBOT_DOCS") == "1":
+        description += (  # pragma: no cover
+            " (Please note that the default values of the Apache plugin options"
+            " change depending on the operating system Certbot is run on.)"
+        )
 
     OS_DEFAULTS = dict(
         server_root="/etc/apache2",
@@ -141,28 +146,36 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # When adding, modifying or deleting command line arguments, be sure to
         # include the changes in the list used in method _prepare_options() to
         # ensure consistent behavior.
-        add("enmod", default=cls.OS_DEFAULTS["enmod"],
+
+        # Respect CERTBOT_DOCS environment variable and use default values from
+        # base class regardless of the underlying distribution (overrides).
+        if os.environ.get("CERTBOT_DOCS") == "1":
+            DEFAULTS = ApacheConfigurator.OS_DEFAULTS
+        else:
+            # cls.OS_DEFAULTS can be distribution specific, see override classes
+            DEFAULTS = cls.OS_DEFAULTS
+        add("enmod", default=DEFAULTS["enmod"],
             help="Path to the Apache 'a2enmod' binary")
-        add("dismod", default=cls.OS_DEFAULTS["dismod"],
+        add("dismod", default=DEFAULTS["dismod"],
             help="Path to the Apache 'a2dismod' binary")
-        add("le-vhost-ext", default=cls.OS_DEFAULTS["le_vhost_ext"],
+        add("le-vhost-ext", default=DEFAULTS["le_vhost_ext"],
             help="SSL vhost configuration extension")
-        add("server-root", default=cls.OS_DEFAULTS["server_root"],
+        add("server-root", default=DEFAULTS["server_root"],
             help="Apache server root directory")
         add("vhost-root", default=None,
             help="Apache server VirtualHost configuration root")
-        add("logs-root", default=cls.OS_DEFAULTS["logs_root"],
+        add("logs-root", default=DEFAULTS["logs_root"],
             help="Apache server logs directory")
         add("challenge-location",
-            default=cls.OS_DEFAULTS["challenge_location"],
+            default=DEFAULTS["challenge_location"],
             help="Directory path for challenge configuration")
-        add("handle-modules", default=cls.OS_DEFAULTS["handle_modules"],
+        add("handle-modules", default=DEFAULTS["handle_modules"],
             help="Let installer handle enabling required modules for you " +
                  "(Only Ubuntu/Debian currently)")
-        add("handle-sites", default=cls.OS_DEFAULTS["handle_sites"],
+        add("handle-sites", default=DEFAULTS["handle_sites"],
             help="Let installer handle enabling sites for you " +
                  "(Only Ubuntu/Debian currently)")
-        add("ctl", default=cls.OS_DEFAULTS["ctl"],
+        add("ctl", default=DEFAULTS["ctl"],
             help="Full path to Apache control script")
         util.add_deprecated_argument(
             add, argument_name="init-script", nargs=1)
@@ -577,8 +590,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         self.assoc[target_name] = vhost
         return vhost
 
-    def included_in_wildcard(self, names, target_name):
-        """Is target_name covered by a wildcard?
+    def domain_in_names(self, names, target_name):
+        """Checks if target domain is covered by one or more of the provided
+        names. The target name is matched by wildcard as well as exact match.
 
         :param names: server aliases
         :type names: `collections.Iterable` of `str`
@@ -649,7 +663,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             names = vhost.get_names()
             if target_name in names:
                 points = 3
-            elif self.included_in_wildcard(names, target_name):
+            elif self.domain_in_names(names, target_name):
                 points = 2
             elif any(addr.get_addr() == target_name for addr in vhost.addrs):
                 points = 1
@@ -1463,7 +1477,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         matches = self.parser.find_dir(
             "ServerAlias", start=vh_path, exclude=False)
         aliases = (self.aug.get(match) for match in matches)
-        return self.included_in_wildcard(aliases, target_name)
+        return self.domain_in_names(aliases, target_name)
 
     def _add_name_vhost_if_necessary(self, vhost):
         """Add NameVirtualHost Directives if necessary for new vhost.

certbot-apache/certbot_apache/http_01.py

@@ -2,7 +2,7 @@
 import logging
 import os
 
-from acme.magic_typing import Set  # pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import List, 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
@@ -89,15 +89,27 @@ class ApacheHttp01(common.TLSSNI01):
                     self.configurator.enable_mod(mod, temp=True)
 
     def _mod_config(self):
+        selected_vhosts = []  # type: List[VirtualHost]
+        http_port = str(self.configurator.config.http01_port)
         for chall in self.achalls:
-            vh = self.configurator.find_best_http_vhost(
-                chall.domain, filter_defaults=False,
-                port=str(self.configurator.config.http01_port))
-            if vh:
-                self._set_up_include_directives(vh)
-            else:
-                for vh in self._relevant_vhosts():
-                    self._set_up_include_directives(vh)
+            # Search for matching VirtualHosts
+            for vh in self._matching_vhosts(chall.domain):
+                selected_vhosts.append(vh)
+
+        # Ensure that we have one or more VirtualHosts that we can continue
+        # with. (one that listens to port configured with --http-01-port)
+        found = False
+        for vhost in selected_vhosts:
+            if any(a.is_wildcard() or a.get_port() == http_port for a in vhost.addrs):
+                found = True
+
+        if not found:
+            for vh in self._relevant_vhosts():
+                selected_vhosts.append(vh)
+
+        # Add the challenge configuration
+        for vh in selected_vhosts:
+            self._set_up_include_directives(vh)
 
         self.configurator.reverter.register_file_creation(
             True, self.challenge_conf_pre)
@@ -121,6 +133,20 @@ class ApacheHttp01(common.TLSSNI01):
         with open(self.challenge_conf_post, "w") as new_conf:
             new_conf.write(config_text_post)
 
+    def _matching_vhosts(self, domain):
+        """Return all VirtualHost objects that have the requested domain name or
+        a wildcard name that would match the domain in ServerName or ServerAlias
+        directive.
+        """
+        matching_vhosts = []
+        for vhost in self.configurator.vhosts:
+            if self.configurator.domain_in_names(vhost.get_names(), domain):
+                # domain_in_names also matches the exact names, so no need
+                # to check "domain in vhost.get_names()" explicitly here
+                matching_vhosts.append(vhost)
+
+        return matching_vhosts
+
     def _relevant_vhosts(self):
         http01_port = str(self.configurator.config.http01_port)
         relevant_vhosts = []

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -115,6 +115,37 @@ class MultipleVhostsTest(util.ApacheTest):
         # Weak test..
         ApacheConfigurator.add_parser_arguments(mock.MagicMock())
 
+    def test_docs_parser_arguments(self):
+        os.environ["CERTBOT_DOCS"] = "1"
+        from certbot_apache.configurator import ApacheConfigurator
+        mock_add = mock.MagicMock()
+        ApacheConfigurator.add_parser_arguments(mock_add)
+        parserargs = ["server_root", "enmod", "dismod", "le_vhost_ext",
+                      "vhost_root", "logs_root", "challenge_location",
+                      "handle_modules", "handle_sites", "ctl"]
+        exp = dict()
+
+        for k in ApacheConfigurator.OS_DEFAULTS:
+            if k in parserargs:
+                exp[k.replace("_", "-")] = ApacheConfigurator.OS_DEFAULTS[k]
+        # Special cases
+        exp["vhost-root"] = None
+        exp["init-script"] = None
+
+        found = set()
+        for call in mock_add.call_args_list:
+            # init-script is a special case: deprecated argument
+            if call[0][0] != "init-script":
+                self.assertEqual(exp[call[0][0]], call[1]['default'])
+            found.add(call[0][0])
+
+        # Make sure that all (and only) the expected values exist
+        self.assertEqual(len(mock_add.call_args_list), len(found))
+        for e in exp:
+            self.assertTrue(e in found)
+
+        del os.environ["CERTBOT_DOCS"]
+
     def test_add_parser_arguments_all_configurators(self):  # pylint: disable=no-self-use
         from certbot_apache.entrypoint import OVERRIDE_CLASSES
         for cls in OVERRIDE_CLASSES.values():
@@ -139,7 +170,8 @@ class MultipleVhostsTest(util.ApacheTest):
         names = self.config.get_all_names()
         self.assertEqual(names, set(
             ["certbot.demo", "ocspvhost.com", "encryption-example.demo",
-             "nonsym.link", "vhost.in.rootconf", "www.certbot.demo"]
+             "nonsym.link", "vhost.in.rootconf", "www.certbot.demo",
+             "duplicate.example.com"]
         ))
 
     @certbot_util.patch_get_utility()
@@ -158,8 +190,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.config.vhosts.append(vhost)
 
         names = self.config.get_all_names()
-        # Names get filtered, only 5 are returned
-        self.assertEqual(len(names), 8)
+        self.assertEqual(len(names), 9)
         self.assertTrue("zombo.com" in names)
         self.assertTrue("google.com" in names)
         self.assertTrue("certbot.demo" in names)
@@ -200,7 +231,7 @@ class MultipleVhostsTest(util.ApacheTest):
     def test_get_virtual_hosts(self):
         """Make sure all vhosts are being properly found."""
         vhs = self.config.get_virtual_hosts()
-        self.assertEqual(len(vhs), 10)
+        self.assertEqual(len(vhs), 12)
         found = 0
 
         for vhost in vhs:
@@ -211,7 +242,7 @@ class MultipleVhostsTest(util.ApacheTest):
             else:
                 raise Exception("Missed: %s" % vhost)  # pragma: no cover
 
-        self.assertEqual(found, 10)
+        self.assertEqual(found, 12)
 
         # Handle case of non-debian layout get_virtual_hosts
         with mock.patch(
@@ -219,7 +250,7 @@ class MultipleVhostsTest(util.ApacheTest):
         ) as mock_conf:
             mock_conf.return_value = False
             vhs = self.config.get_virtual_hosts()
-            self.assertEqual(len(vhs), 10)
+            self.assertEqual(len(vhs), 12)
 
     @mock.patch("certbot_apache.display_ops.select_vhost")
     def test_choose_vhost_none_avail(self, mock_select):
@@ -322,7 +353,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.config.vhosts = [
             vh for vh in self.config.vhosts
             if vh.name not in ["certbot.demo", "nonsym.link",
-                "encryption-example.demo",
+                "encryption-example.demo", "duplicate.example.com",
                 "ocspvhost.com", "vhost.in.rootconf"]
             and "*.blue.purple.com" not in vh.aliases
         ]
@@ -333,7 +364,7 @@ class MultipleVhostsTest(util.ApacheTest):
     def test_non_default_vhosts(self):
         # pylint: disable=protected-access
         vhosts = self.config._non_default_vhosts(self.config.vhosts)
-        self.assertEqual(len(vhosts), 8)
+        self.assertEqual(len(vhosts), 10)
 
     def test_deploy_cert_enable_new_vhost(self):
         # Create
@@ -688,7 +719,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertEqual(self.config.is_name_vhost(self.vh_truth[0]),
                          self.config.is_name_vhost(ssl_vhost))
 
-        self.assertEqual(len(self.config.vhosts), 11)
+        self.assertEqual(len(self.config.vhosts), 13)
 
     def test_clean_vhost_ssl(self):
         # pylint: disable=protected-access
@@ -1269,7 +1300,7 @@ class MultipleVhostsTest(util.ApacheTest):
 
         # pylint: disable=protected-access
         self.config._enable_redirect(self.vh_truth[1], "")
-        self.assertEqual(len(self.config.vhosts), 11)
+        self.assertEqual(len(self.config.vhosts), 13)
 
     def test_create_own_redirect_for_old_apache_version(self):
         self.config.parser.modules.add("rewrite_module")
@@ -1280,7 +1311,7 @@ class MultipleVhostsTest(util.ApacheTest):
 
         # pylint: disable=protected-access
         self.config._enable_redirect(self.vh_truth[1], "")
-        self.assertEqual(len(self.config.vhosts), 11)
+        self.assertEqual(len(self.config.vhosts), 13)
 
     def test_sift_rewrite_rule(self):
         # pylint: disable=protected-access

certbot-apache/certbot_apache/tests/http_01_test.py

@@ -27,8 +27,8 @@ class ApacheHttp01Test(util.ApacheTest):
         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
-        # vhost.in.rootconf
+        # Takes the vhosts for encryption-example.demo, certbot.demo
+        # and vhost.in.rootconf
         self.vhosts = [vh_truth[0], vh_truth[3], vh_truth[10]]
 
         for i in range(NUM_ACHALLS):
@@ -39,7 +39,7 @@ class ApacheHttp01Test(util.ApacheTest):
                         "pending"),
                     domain=self.vhosts[i].name, account_key=self.account_key))
 
-        modules = ["rewrite", "authz_core", "authz_host"]
+        modules = ["ssl", "rewrite", "authz_core", "authz_host"]
         for mod in modules:
             self.config.parser.modules.add("mod_{0}.c".format(mod))
             self.config.parser.modules.add(mod + "_module")
@@ -111,6 +111,17 @@ class ApacheHttp01Test(util.ApacheTest):
                 domain="something.nonexistent", account_key=self.account_key)]
         self.common_perform_test(achalls, vhosts)
 
+    def test_configure_multiple_vhosts(self):
+        vhosts = [v for v in self.config.vhosts if "duplicate.example.com" in v.get_names()]
+        self.assertEqual(len(vhosts), 2)
+        achalls = [
+            achallenges.KeyAuthorizationAnnotatedChallenge(
+                challb=acme_util.chall_to_challb(
+                    challenges.HTTP01(token=((b'a' * 16))),
+                    "pending"),
+                domain="duplicate.example.com", account_key=self.account_key)]
+        self.common_perform_test(achalls, vhosts)
+
     def test_no_vhost(self):
         for achall in self.achalls:
             self.http.add_chall(achall)
@@ -176,15 +187,14 @@ class ApacheHttp01Test(util.ApacheTest):
             self._test_challenge_file(achall)
 
         for vhost in vhosts:
-            if not vhost.ssl:
-                matches = self.config.parser.find_dir("Include",
-                                                      self.http.challenge_conf_pre,
-                                                      vhost.path)
-                self.assertEqual(len(matches), 1)
-                matches = self.config.parser.find_dir("Include",
-                                                      self.http.challenge_conf_post,
-                                                      vhost.path)
-                self.assertEqual(len(matches), 1)
+            matches = self.config.parser.find_dir("Include",
+                                                self.http.challenge_conf_pre,
+                                                vhost.path)
+            self.assertEqual(len(matches), 1)
+            matches = self.config.parser.find_dir("Include",
+                                                self.http.challenge_conf_post,
+                                                vhost.path)
+            self.assertEqual(len(matches), 1)
 
         self.assertTrue(os.path.exists(challenge_dir))
 

certbot-apache/certbot_apache/tests/parser_test.py

@@ -52,7 +52,7 @@ class BasicParserTest(util.ParserTest):
         test2 = self.parser.find_dir("documentroot")
 
         self.assertEqual(len(test), 1)
-        self.assertEqual(len(test2), 7)
+        self.assertEqual(len(test2), 8)
 
     def test_add_dir(self):
         aug_default = "/files" + self.parser.loc["default"]

certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/duplicatehttp.conf

@@ -0,0 +1,9 @@
+<VirtualHost 10.2.3.4:80>
+	ServerName duplicate.example.com
+
+	ServerAdmin webmaster@certbot.demo
+	DocumentRoot /var/www/html
+
+	ErrorLog ${APACHE_LOG_DIR}/error.log
+	CustomLog ${APACHE_LOG_DIR}/access.log combined
+</VirtualHost>

certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/duplicatehttps.conf

@@ -0,0 +1,14 @@
+<IfModule mod_ssl.c>
+<VirtualHost 10.2.3.4:443>
+	ServerName duplicate.example.com
+
+	ServerAdmin webmaster@certbot.demo
+	DocumentRoot /var/www/html
+
+	ErrorLog ${APACHE_LOG_DIR}/error.log
+	CustomLog ${APACHE_LOG_DIR}/access.log combined
+
+SSLCertificateFile	/etc/apache2/certs/certbot-cert_5.pem
+SSLCertificateKeyFile /etc/apache2/ssl/key-certbot_15.pem
+</VirtualHost>
+</IfModule>

certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/duplicatehttp.conf

@@ -0,0 +1 @@
+../sites-available/duplicatehttp.conf

certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/duplicatehttps.conf

@@ -0,0 +1 @@
+../sites-available/duplicatehttps.conf

certbot-apache/certbot_apache/tests/util.py

@@ -196,7 +196,17 @@ def get_vh_truth(temp_dir, config_name):
                 "/files" + os.path.join(temp_dir, config_name,
                                         "apache2/apache2.conf/VirtualHost"),
                 set([obj.Addr.fromstring("*:80")]), False, True,
-                "vhost.in.rootconf")]
+                "vhost.in.rootconf"),
+            obj.VirtualHost(
+                os.path.join(prefix, "duplicatehttp.conf"),
+                os.path.join(aug_pre, "duplicatehttp.conf/VirtualHost"),
+                set([obj.Addr.fromstring("10.2.3.4:80")]), False, True,
+                "duplicate.example.com"),
+            obj.VirtualHost(
+                os.path.join(prefix, "duplicatehttps.conf"),
+                os.path.join(aug_pre, "duplicatehttps.conf/IfModule/VirtualHost"),
+                set([obj.Addr.fromstring("10.2.3.4:443")]), True, True,
+                "duplicate.example.com")]
         return vh_truth
     if config_name == "debian_apache_2_4/multi_vhosts":
         prefix = os.path.join(

certbot-apache/setup.py

@@ -1,8 +1,10 @@
 from setuptools import setup
 from setuptools import find_packages
+from setuptools.command.test import test as TestCommand
+import sys
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -21,6 +23,22 @@ docs_extras = [
     'sphinx_rtd_theme',
 ]
 
+
+class PyTest(TestCommand):
+    user_options = []
+
+    def initialize_options(self):
+        TestCommand.initialize_options(self)
+        self.pytest_args = ''
+
+    def run_tests(self):
+        import shlex
+        # import here, cause outside the eggs aren't loaded
+        import pytest
+        errno = pytest.main(shlex.split(self.pytest_args))
+        sys.exit(errno)
+
+
 setup(
     name='certbot-apache',
     version=version,
@@ -64,4 +82,6 @@ setup(
         ],
     },
     test_suite='certbot_apache',
+    tests_require=["pytest"],
+    cmdclass={"test": PyTest},
 )

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.30.2"
+LE_AUTO_VERSION="0.31.0"
 BASENAME=$(basename $0)
 USAGE="Usage: $BASENAME [OPTIONS]
 A self-updating wrapper script for the Certbot ACME client. When run, updates
@@ -333,63 +333,11 @@ BootstrapDebCommon() {
   fi
 
   augeas_pkg="libaugeas0 augeas-lenses"
-  AUGVERSION=`LC_ALL=C apt-cache show --no-all-versions libaugeas0 | grep ^Version: | cut -d" " -f2`
 
   if [ "$ASSUME_YES" = 1 ]; then
     YES_FLAG="-y"
   fi
 
-  AddBackportRepo() {
-    # ARGS:
-    BACKPORT_NAME="$1"
-    BACKPORT_SOURCELINE="$2"
-    say "To use the Apache Certbot plugin, augeas needs to be installed from $BACKPORT_NAME."
-    if ! grep -v -e ' *#' /etc/apt/sources.list | grep -q "$BACKPORT_NAME" ; then
-      # This can theoretically error if sources.list.d is empty, but in that case we don't care.
-      if ! grep -v -e ' *#' /etc/apt/sources.list.d/* 2>/dev/null | grep -q "$BACKPORT_NAME"; then
-        if [ "$ASSUME_YES" = 1 ]; then
-          /bin/echo -n "Installing augeas from $BACKPORT_NAME in 3 seconds..."
-          sleep 1s
-          /bin/echo -ne "\e[0K\rInstalling augeas from $BACKPORT_NAME in 2 seconds..."
-          sleep 1s
-          /bin/echo -e "\e[0K\rInstalling augeas from $BACKPORT_NAME in 1 second ..."
-          sleep 1s
-          add_backports=1
-        else
-          read -p "Would you like to enable the $BACKPORT_NAME repository [Y/n]? " response
-          case $response in
-            [yY][eE][sS]|[yY]|"")
-              add_backports=1;;
-            *)
-              add_backports=0;;
-          esac
-        fi
-        if [ "$add_backports" = 1 ]; then
-          sh -c "echo $BACKPORT_SOURCELINE >> /etc/apt/sources.list.d/$BACKPORT_NAME.list"
-          apt-get $QUIET_FLAG update
-        fi
-      fi
-    fi
-    if [ "$add_backports" != 0 ]; then
-      apt-get install $QUIET_FLAG $YES_FLAG --no-install-recommends -t "$BACKPORT_NAME" $augeas_pkg
-      augeas_pkg=
-    fi
-  }
-
-
-  if dpkg --compare-versions 1.0 gt "$AUGVERSION" ; then
-    if lsb_release -a | grep -q wheezy ; then
-      AddBackportRepo wheezy-backports "deb http://http.debian.net/debian wheezy-backports main"
-    elif lsb_release -a | grep -q precise ; then
-      # XXX add ARM case
-      AddBackportRepo precise-backports "deb http://archive.ubuntu.com/ubuntu precise-backports main restricted universe multiverse"
-    else
-      echo "No libaugeas0 version is available that's new enough to run the"
-      echo "Certbot apache plugin..."
-    fi
-    # XXX add a case for ubuntu PPAs
-  fi
-
   apt-get install $QUIET_FLAG $YES_FLAG --no-install-recommends \
     python \
     python-dev \
@@ -1140,9 +1088,9 @@ parsedatetime==2.1 \
 pbr==1.8.1 \
     --hash=sha256:46c8db75ae75a056bd1cc07fa21734fe2e603d11a07833ecc1eeb74c35c72e0c \
     --hash=sha256:e2127626a91e6c885db89668976db31020f0af2da728924b56480fc7ccf09649
-pyOpenSSL==16.2.0 \
-    --hash=sha256:26ca380ddf272f7556e48064bbcd5bd71f83dfc144f3583501c7ddbd9434ee17 \
-    --hash=sha256:7779a3bbb74e79db234af6a08775568c6769b5821faecf6e2f4143edb227516e
+pyOpenSSL==18.0.0 \
+    --hash=sha256:26ff56a6b5ecaf3a2a59f132681e2a80afcc76b4f902f612f518f92c2a1bf854 \
+    --hash=sha256:6488f1423b00f73b7ad5167885312bb0ce410d3312eb212393795b53c8caa580
 pyparsing==2.1.8 \
     --hash=sha256:2f0f5ceb14eccd5aef809d6382e87df22ca1da583c79f6db01675ce7d7f49c18 \
     --hash=sha256:03a4869b9f3493807ee1f1cb405e6d576a1a2ca4d81a982677c0c1ad6177c56b \
@@ -1232,18 +1180,18 @@ letsencrypt==0.7.0 \
     --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \
     --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9
 
-certbot==0.30.2 \
-    --hash=sha256:e411b72fa86eec1018e6de28e649e8c9c71191a7431dcc77f207b57ca9484c11 \
-    --hash=sha256:534487cb552ced8e47948ba3d2e7ca12c3a439133fc609485012b1a02fc7776e
-acme==0.30.2 \
-    --hash=sha256:68982576492dfa99c7e2be0fce4371adc9344740b05420ce0ab53238d2bb9b3b \
-    --hash=sha256:295a5b7fce9f908e6e5cff8c40be1a3daf3e1ebabd2e139a4c87274e68eeb8f2
-certbot-apache==0.30.2 \
-    --hash=sha256:3b7fa4e59772da7c9975ef2a49ceff157c9d7cb31eb9475928b5986d89701a3a \
-    --hash=sha256:32fa915a8a51810fdfe828ac1361da4425c231d7384891e49e6338e4741464b2
-certbot-nginx==0.30.2 \
-    --hash=sha256:7dc785f6f0c0c57b19cea8d98f9ea8feef53945613967b52c9348c81327010e2 \
-    --hash=sha256:6ba4dd772d0c7cdfb3383ca325b35639e01ac9e142e4baa6445cd85c7fb59552
+certbot==0.31.0 \
+    --hash=sha256:1a1b4b2675daf5266cc2cf2a44ded44de1d83e9541ffa078913c0e4c3231a1c4 \
+    --hash=sha256:0c3196f80a102c0f9d82d566ba859efe3b70e9ed4670520224c844fafd930473
+acme==0.31.0 \
+    --hash=sha256:a0c851f6b7845a0faa3a47a3e871440eed9ec11b4ab949de0dc4a0fb1201cd24 \
+    --hash=sha256:7e5c2d01986e0f34ca08fee58981892704c82c48435dcd3592b424c312d8b2bf
+certbot-apache==0.31.0 \
+    --hash=sha256:740bb55dd71723a21eebabb16e6ee5d8883f8b8f8cf6956dd1d4873e0cccae21 \
+    --hash=sha256:cc4b840b2a439a63e2dce809272c3c3cd4b1aeefc4053cd188935135be137edd
+certbot-nginx==0.31.0 \
+    --hash=sha256:7a1ffda9d93dc7c2aaf89452ce190250de8932e624d31ebba8e4fa7d950025c5 \
+    --hash=sha256:d450d75650384f74baccb7673c89e2f52468afa478ed354eb6d4b99aa33bf865
 
 UNLIKELY_EOF
     # -------------------------------------------------------------------------

certbot-compatibility-test/certbot_compatibility_test/validator.py

@@ -2,20 +2,17 @@
 import logging
 import socket
 import requests
-import zope.interface
 
 import six
 from six.moves import xrange  # pylint: disable=import-error,redefined-builtin
 
 from acme import crypto_util
 from acme import errors as acme_errors
-from certbot import interfaces
 
 
 logger = logging.getLogger(__name__)
 
 
-@zope.interface.implementer(interfaces.IValidator)
 class Validator(object):
     # pylint: disable=no-self-use
     """Collection of functions to test a live webserver's configuration"""

certbot-compatibility-test/setup.py

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

certbot-dns-cloudflare/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

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

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

certbot-dns-cloudxns/setup.py

@@ -2,13 +2,13 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
-    'acme>=0.31.0.dev0',
-    'certbot>=0.31.0.dev0',
+    'acme>=0.31.0',
+    'certbot>=0.31.0',
     'dns-lexicon>=2.2.1',  # Support for >1 TXT record per name
     'mock',
     'setuptools',

certbot-dns-digitalocean/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

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

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

certbot-dns-dnsimple/setup.py

@@ -2,13 +2,13 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
-    'acme>=0.31.0.dev0',
-    'certbot>=0.31.0.dev0',
+    'acme>=0.31.0',
+    'certbot>=0.31.0',
     'dns-lexicon>=2.2.1',  # Support for >1 TXT record per name
     'mock',
     'setuptools',

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

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

certbot-dns-dnsmadeeasy/setup.py

@@ -2,13 +2,13 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
-    'acme>=0.31.0.dev0',
-    'certbot>=0.31.0.dev0',
+    'acme>=0.31.0',
+    'certbot>=0.31.0',
     'dns-lexicon>=2.2.1',  # Support for >1 TXT record per name
     'mock',
     'setuptools',

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

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

certbot-dns-gehirn/setup.py

@@ -2,12 +2,12 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
-    'acme>=0.31.0.dev0',
-    'certbot>=0.31.0.dev0',
+    'acme>=0.31.0',
+    'certbot>=0.31.0',
     'dns-lexicon>=2.1.22',
     'mock',
     'setuptools',

certbot-dns-google/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

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

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

certbot-dns-linode/setup.py

@@ -1,12 +1,12 @@
 from setuptools import setup
 from setuptools import find_packages
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
-    'acme>=0.31.0.dev0',
-    'certbot>=0.31.0.dev0',
+    'acme>=0.31.0',
+    'certbot>=0.31.0',
     'dns-lexicon>=2.2.1',
     'mock',
     'setuptools',

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

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

certbot-dns-luadns/setup.py

@@ -2,13 +2,13 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
-    'acme>=0.31.0.dev0',
-    'certbot>=0.31.0.dev0',
+    'acme>=0.31.0',
+    'certbot>=0.31.0',
     'dns-lexicon>=2.2.1',  # Support for >1 TXT record per name
     'mock',
     'setuptools',

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

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

certbot-dns-nsone/setup.py

@@ -2,13 +2,13 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
-    'acme>=0.31.0.dev0',
-    'certbot>=0.31.0.dev0',
+    'acme>=0.31.0',
+    'certbot>=0.31.0',
     'dns-lexicon>=2.2.1',  # Support for >1 TXT record per name
     'mock',
     'setuptools',

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

@@ -1,3 +1,3 @@
--e acme[dev]
--e .[dev]
+acme[dev]==0.31.0
+certbot[dev]==0.31.0
 dns-lexicon==2.7.14

certbot-dns-ovh/setup.py

@@ -2,13 +2,13 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
-    'acme>=0.31.0.dev0',
-    'certbot>=0.31.0.dev0',
+    'acme>=0.31.0',
+    'certbot>=0.31.0',
     'dns-lexicon>=2.7.14',  # Correct proxy use on OVH provider
     'mock',
     'setuptools',

certbot-dns-rfc2136/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-route53/setup.py

@@ -1,7 +1,7 @@
 from setuptools import setup
 from setuptools import find_packages
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

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

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

certbot-dns-sakuracloud/setup.py

@@ -2,12 +2,12 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
-    'acme>=0.31.0.dev0',
-    'certbot>=0.31.0.dev0',
+    'acme>=0.31.0',
+    'certbot>=0.31.0',
     'dns-lexicon>=2.1.23',
     'mock',
     'setuptools',

certbot-nginx/certbot_nginx/configurator.py

@@ -13,6 +13,7 @@ import zope.interface
 from acme import challenges
 from acme import crypto_util as acme_crypto_util
 
+from certbot import compat
 from certbot import constants as core_constants
 from certbot import crypto_util
 from certbot import errors
@@ -164,9 +165,7 @@ class NginxConfigurator(common.Installer):
             util.lock_dir_until_exit(self.conf('server-root'))
         except (OSError, errors.LockError):
             logger.debug('Encountered error:', exc_info=True)
-            raise errors.PluginError(
-                'Unable to lock %s', self.conf('server-root'))
-
+            raise errors.PluginError('Unable to lock {0}'.format(self.conf('server-root')))
 
     # Entry point in main.py for installing cert
     def deploy_cert(self, domain, cert_path, key_path,
@@ -899,7 +898,7 @@ class NginxConfigurator(common.Installer):
         have permissions of root.
 
         """
-        uid = os.geteuid()
+        uid = compat.os_geteuid()
         util.make_or_verify_dir(
             self.config.work_dir, core_constants.CONFIG_DIRS_MODE, uid)
         util.make_or_verify_dir(

certbot-nginx/certbot_nginx/parser.py

@@ -81,9 +81,9 @@ class NginxParser(object):
 
         """
         if not os.path.isabs(path):
-            return os.path.join(self.root, path)
+            return os.path.normpath(os.path.join(self.root, path))
         else:
-            return path
+            return os.path.normpath(path)
 
     def _build_addr_to_ssl(self):
         """Builds a map from address to whether it listens on ssl in any server block

certbot-nginx/certbot_nginx/tests/configurator_test.py

@@ -1,7 +1,6 @@
 # pylint: disable=too-many-public-methods
 """Test for certbot_nginx.configurator."""
 import os
-import shutil
 import unittest
 
 import mock
@@ -33,12 +32,6 @@ class NginxConfiguratorTest(util.NginxTest):
         self.config = util.get_nginx_configurator(
             self.config_path, self.config_dir, self.work_dir, self.logs_dir)
 
-    def tearDown(self):
-        shutil.rmtree(self.temp_dir)
-        shutil.rmtree(self.config_dir)
-        shutil.rmtree(self.work_dir)
-        shutil.rmtree(self.logs_dir)
-
     @mock.patch("certbot_nginx.configurator.util.exe_exists")
     def test_prepare_no_install(self, mock_exe_exists):
         mock_exe_exists.return_value = False
@@ -69,8 +62,11 @@ class NginxConfiguratorTest(util.NginxTest):
 
     def test_prepare_locked(self):
         server_root = self.config.conf("server-root")
+
+        from certbot import util as certbot_util
+        certbot_util._LOCKS[server_root].release()  # pylint: disable=protected-access
+
         self.config.config_test = mock.Mock()
-        os.remove(os.path.join(server_root, ".certbot.lock"))
         certbot_test_util.lock_and_call(self._test_prepare_locked, server_root)
 
     @mock.patch("certbot_nginx.configurator.util.exe_exists")
@@ -88,11 +84,11 @@ class NginxConfiguratorTest(util.NginxTest):
     def test_get_all_names(self, mock_gethostbyaddr):
         mock_gethostbyaddr.return_value = ('155.225.50.69.nephoscale.net', [], [])
         names = self.config.get_all_names()
-        self.assertEqual(names, set(
-            ["155.225.50.69.nephoscale.net", "www.example.org", "another.alias",
+        self.assertEqual(names, {
+            "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",
-             "headers.com"]))
+             "headers.com"})
 
     def test_supported_enhancements(self):
         self.assertEqual(['redirect', 'ensure-http-header', 'staple-ocsp'],
@@ -171,6 +167,7 @@ class NginxConfiguratorTest(util.NginxTest):
                    'abc.www.foo.com': "etc_nginx/foo.conf",
                    'www.bar.co.uk': "etc_nginx/nginx.conf",
                    'ipv6.com': "etc_nginx/sites-enabled/ipv6.com"}
+        conf_path = {key: os.path.normpath(value) for key, value in conf_path.items()}
 
         vhost = self.config.choose_vhosts(name)[0]
         path = os.path.relpath(vhost.filep, self.temp_dir)

certbot-nginx/certbot_nginx/tests/http_01_test.py

@@ -1,6 +1,5 @@
 """Tests for certbot_nginx.http_01"""
 import unittest
-import shutil
 
 import mock
 import six
@@ -54,11 +53,6 @@ class HttpPerformTest(util.NginxTest):
         from certbot_nginx import http_01
         self.http01 = http_01.NginxHttp01(config)
 
-    def tearDown(self):
-        shutil.rmtree(self.temp_dir)
-        shutil.rmtree(self.config_dir)
-        shutil.rmtree(self.work_dir)
-
     def test_perform0(self):
         responses = self.http01.perform()
         self.assertEqual([], responses)

certbot-nginx/certbot_nginx/tests/parser_test.py

@@ -67,9 +67,15 @@ class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods
 
     def test_abs_path(self):
         nparser = parser.NginxParser(self.config_path)
-        self.assertEqual('/etc/nginx/*', nparser.abs_path('/etc/nginx/*'))
-        self.assertEqual(os.path.join(self.config_path, 'foo/bar/'),
-                         nparser.abs_path('foo/bar/'))
+        if os.name != 'nt':
+            self.assertEqual('/etc/nginx/*', nparser.abs_path('/etc/nginx/*'))
+            self.assertEqual(os.path.join(self.config_path, 'foo/bar'),
+                             nparser.abs_path('foo/bar'))
+        else:
+            self.assertEqual('C:\\etc\\nginx\\*', nparser.abs_path('C:\\etc\\nginx\\*'))
+            self.assertEqual(os.path.join(self.config_path, 'foo\\bar'),
+                             nparser.abs_path('foo\\bar'))
+
 
     def test_filedump(self):
         nparser = parser.NginxParser(self.config_path)

certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py

@@ -1,6 +1,5 @@
 """Tests for certbot_nginx.tls_sni_01"""
 import unittest
-import shutil
 
 import mock
 import six
@@ -55,11 +54,6 @@ class TlsSniPerformTest(util.NginxTest):
         from certbot_nginx import tls_sni_01
         self.sni = tls_sni_01.NginxTlsSni01(config)
 
-    def tearDown(self):
-        shutil.rmtree(self.temp_dir)
-        shutil.rmtree(self.config_dir)
-        shutil.rmtree(self.work_dir)
-
     @mock.patch("certbot_nginx.configurator"
                 ".NginxConfigurator.choose_vhosts")
     def test_perform(self, mock_choose):

certbot-nginx/certbot_nginx/tests/util.py

@@ -4,6 +4,8 @@ import os
 import pkg_resources
 import tempfile
 import unittest
+import shutil
+import warnings
 
 import josepy as jose
 import mock
@@ -33,6 +35,22 @@ class NginxTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
         self.rsa512jwk = jose.JWKRSA.load(test_util.load_vector(
             "rsa512_key.pem"))
 
+    def tearDown(self):
+        # On Windows we have various files which are not correctly closed at the time of tearDown.
+        # For know, we log them until a proper file close handling is written.
+        # Useful for development only, so no warning when we are on a CI process.
+        def onerror_handler(_, path, excinfo):
+            """On error handler"""
+            if not os.environ.get('APPVEYOR'):  # pragma: no cover
+                message = ('Following error occurred when deleting path {0}'
+                           'during tearDown process: {1}'.format(path, str(excinfo)))
+                warnings.warn(message)
+
+        shutil.rmtree(self.temp_dir, onerror=onerror_handler)
+        shutil.rmtree(self.config_dir, onerror=onerror_handler)
+        shutil.rmtree(self.work_dir, onerror=onerror_handler)
+        shutil.rmtree(self.logs_dir, onerror=onerror_handler)
+
 
 def get_data_filename(filename):
     """Gets the filename of a test data file."""

certbot-nginx/local-oldest-requirements.txt

@@ -1,2 +1,2 @@
-acme[dev]==0.26.0
-certbot[dev]==0.22.0
+acme[dev]==0.29.0
+-e .[dev]

certbot-nginx/setup.py

@@ -1,8 +1,10 @@
 from setuptools import setup
 from setuptools import find_packages
+from setuptools.command.test import test as TestCommand
+import sys
 
 
-version = '0.31.0.dev0'
+version = '0.32.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -21,6 +23,22 @@ docs_extras = [
     'sphinx_rtd_theme',
 ]
 
+
+class PyTest(TestCommand):
+    user_options = []
+
+    def initialize_options(self):
+        TestCommand.initialize_options(self)
+        self.pytest_args = ''
+
+    def run_tests(self):
+        import shlex
+        # import here, cause outside the eggs aren't loaded
+        import pytest
+        errno = pytest.main(shlex.split(self.pytest_args))
+        sys.exit(errno)
+
+
 setup(
     name='certbot-nginx',
     version=version,
@@ -64,4 +82,6 @@ setup(
         ],
     },
     test_suite='certbot_nginx',
+    tests_require=["pytest"],
+    cmdclass={"test": PyTest},
 )

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.31.0.dev0'
+__version__ = '0.32.0.dev0'

certbot/account.py

@@ -142,7 +142,7 @@ class AccountFileStorage(interfaces.AccountStorage):
     def __init__(self, config):
         self.config = config
         util.make_or_verify_dir(config.accounts_dir, 0o700, compat.os_geteuid(),
-                                   self.config.strict_permissions)
+                                self.config.strict_permissions)
 
     def _account_dir_path(self, account_id):
         return self._account_dir_path_for_server_path(account_id, self.config.server_path)

certbot/cli.py

@@ -108,7 +108,7 @@ manage your account with Let's Encrypt:
 
 # This is the short help for certbot --help, where we disable argparse
 # altogether
-HELP_USAGE = """
+HELP_AND_VERSION_USAGE = """
 More detailed help:
 
   -h, --help [TOPIC]    print this message, or detailed help on a topic;
@@ -117,6 +117,8 @@ More detailed help:
    all, automation, commands, paths, security, testing, or any of the
    subcommands or plugins (certonly, renew, install, register, nginx,
    apache, standalone, webroot, etc.)
+
+  --version             print the version number
 """
 
 
@@ -566,7 +568,7 @@ class HelpfulArgumentParser(object):
 
         usage = SHORT_USAGE
         if help_arg == True:
-            self.notify(usage + COMMAND_OVERVIEW % (apache_doc, nginx_doc) + HELP_USAGE)
+            self.notify(usage + COMMAND_OVERVIEW % (apache_doc, nginx_doc) + HELP_AND_VERSION_USAGE)
             sys.exit(0)
         elif help_arg in self.COMMANDS_TOPICS:
             self.notify(usage + self._list_subcommands())

certbot/compat.py

@@ -1,12 +1,8 @@
 """
 Compatibility layer to run certbot both on Linux and Windows.
 
-The approach used here is similar to Modernizr for Web browsers.
-We do not check the platform type to determine if a particular logic is supported.
-Instead, we apply a logic, and then fallback to another logic if first logic
-is not supported at runtime.
-
-Then logic chains are abstracted into single functions to be exposed to certbot.
+This module contains all required platform specific code,
+allowing the rest of Certbot codebase to be platform agnostic.
 """
 import os
 import select
@@ -17,16 +13,11 @@ import stat
 
 from certbot import errors
 
-try:
-    # Linux specific
-    import fcntl # pylint: disable=import-error
-except ImportError:
-    # Windows specific
-    import msvcrt # pylint: disable=import-error
-
 UNPRIVILEGED_SUBCOMMANDS_ALLOWED = [
     'certificates', 'enhance', 'revoke', 'delete',
     'register', 'unregister', 'config_changes', 'plugins']
+
+
 def raise_for_non_administrative_windows_rights(subcommand):
     """
     On Windows, raise if current shell does not have the administrative rights.
@@ -50,6 +41,7 @@ def raise_for_non_administrative_windows_rights(subcommand):
                 'Error, "{0}" subcommand must be run on a shell with administrative rights.'
                 .format(subcommand))
 
+
 def os_geteuid():
     """
     Get current user uid
@@ -65,6 +57,7 @@ def os_geteuid():
         # Windows specific
         return 0
 
+
 def os_rename(src, dst):
     """
     Rename a file to a destination path and handles situations where the destination exists.
@@ -117,62 +110,17 @@ def readline_with_timeout(timeout, prompt):
         # So no timeout on Windows for now.
         return sys.stdin.readline()
 
-def lock_file(fd):
-    """
-    Lock the file linked to the specified file descriptor.
-
-    :param int fd: The file descriptor of the file to lock.
-
-    """
-    if 'fcntl' in sys.modules:
-        # Linux specific
-        fcntl.lockf(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
-    else:
-        # Windows specific
-        msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)
-
-def release_locked_file(fd, path):
-    """
-    Remove, close, and release a lock file specified by its file descriptor and its path.
-
-    :param int fd: The file descriptor of the lock file.
-    :param str path: The path of the lock file.
-
-    """
-    # Linux specific
-    #
-    # It is important the lock file is removed before it's released,
-    # otherwise:
-    #
-    # process A: open lock file
-    # process B: release lock file
-    # process A: lock file
-    # process A: check device and inode
-    # process B: delete file
-    # process C: open and lock a different file at the same path
-    try:
-        os.remove(path)
-    except OSError as err:
-        if err.errno == errno.EACCES:
-            # Windows specific
-            # We will not be able to remove a file before closing it.
-            # To avoid race conditions described for Linux, we will not delete the lockfile,
-            # just close it to be reused on the next Certbot call.
-            pass
-        else:
-            raise
-    finally:
-        os.close(fd)
 
 def compare_file_modes(mode1, mode2):
     """Return true if the two modes can be considered as equals for this platform"""
-    if 'fcntl' in sys.modules:
+    if os.name != 'nt':
         # Linux specific: standard compare
         return oct(stat.S_IMODE(mode1)) == oct(stat.S_IMODE(mode2))
     # Windows specific: most of mode bits are ignored on Windows. Only check user R/W rights.
     return (stat.S_IMODE(mode1) & stat.S_IREAD == stat.S_IMODE(mode2) & stat.S_IREAD
             and stat.S_IMODE(mode1) & stat.S_IWRITE == stat.S_IMODE(mode2) & stat.S_IWRITE)
 
+
 WINDOWS_DEFAULT_FOLDERS = {
     'config': 'C:\\Certbot',
     'work': 'C:\\Certbot\\lib',
@@ -184,6 +132,7 @@ LINUX_DEFAULT_FOLDERS = {
     'logs': '/var/log/letsencrypt',
 }
 
+
 def get_default_folder(folder_type):
     """
     Return the relevant default folder for the current OS
@@ -194,8 +143,25 @@ def get_default_folder(folder_type):
     :rtype: str
 
     """
-    if 'fcntl' in sys.modules:
+    if os.name != 'nt':
         # Linux specific
         return LINUX_DEFAULT_FOLDERS[folder_type]
     # Windows specific
     return WINDOWS_DEFAULT_FOLDERS[folder_type]
+
+
+def underscores_for_unsupported_characters_in_path(path):
+    # type: (str) -> str
+    """
+    Replace unsupported characters in path for current OS by underscores.
+    :param str path: the path to normalize
+    :return: the normalized path
+    :rtype: str
+    """
+    if os.name != 'nt':
+        # Linux specific
+        return path
+
+    # Windows specific
+    drive, tail = os.path.splitdrive(path)
+    return drive + tail.replace(':', '_')

certbot/configuration.py

@@ -5,6 +5,7 @@ import os
 from six.moves.urllib import parse  # pylint: disable=import-error
 import zope.interface
 
+from certbot import compat
 from certbot import constants
 from certbot import errors
 from certbot import interfaces
@@ -69,6 +70,7 @@ class NamespaceConfig(object):
 
     def accounts_dir_for_server_path(self, server_path):
         """Path to accounts directory based on server_path"""
+        server_path = compat.underscores_for_unsupported_characters_in_path(server_path)
         return os.path.join(
             self.namespace.config_dir, constants.ACCOUNTS_DIR, server_path)
 

certbot/constants.py

@@ -146,7 +146,7 @@ RENEWER_DEFAULTS = dict(
 """Defaults for renewer script."""
 
 
-ENHANCEMENTS = ["redirect", "ensure-http-header", "ocsp-stapling", "spdy"]
+ENHANCEMENTS = ["redirect", "ensure-http-header", "ocsp-stapling"]
 """List of possible :class:`certbot.interfaces.IInstaller`
 enhancements.
 
@@ -154,7 +154,6 @@ List of expected options parameters:
 - redirect: None
 - ensure-http-header: name of header (i.e. Strict-Transport-Security)
 - ocsp-stapling: certificate chain file path
-- spdy: TODO
 
 """
 

certbot/hooks.py

@@ -93,8 +93,7 @@ def _run_pre_hook_if_necessary(command):
     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)
+        _run_hook("pre-hook", command)
         executed_pre_hooks.add(command)
 
 
@@ -126,8 +125,7 @@ def post_hook(config):
             _run_eventually(cmd)
     # certonly / run
     elif cmd:
-        logger.info("Running post-hook command: %s", cmd)
-        _run_hook(cmd)
+        _run_hook("post-hook", cmd)
 
 
 post_hooks = []  # type: List[str]
@@ -149,8 +147,7 @@ def _run_eventually(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_hooks:
-        logger.info("Running post-hook command: %s", cmd)
-        _run_hook(cmd)
+        _run_hook("post-hook", cmd)
 
 
 def deploy_hook(config, domains, lineage_path):
@@ -220,23 +217,30 @@ def _run_deploy_hook(command, domains, lineage_path, dry_run):
 
     os.environ["RENEWED_DOMAINS"] = " ".join(domains)
     os.environ["RENEWED_LINEAGE"] = lineage_path
-    logger.info("Running deploy-hook command: %s", command)
-    _run_hook(command)
+    _run_hook("deploy-hook", command)
 
 
-def _run_hook(shell_cmd):
+def _run_hook(cmd_name, shell_cmd):
     """Run a hook command.
 
-    :returns: stderr if there was any"""
+    :param str cmd_name: the user facing name of the hook being run
+    :param shell_cmd: shell command to execute
+    :type shell_cmd: `list` of `str` or `str`
 
-    err, _ = execute(shell_cmd)
+    :returns: stderr if there was any"""
+    err, _ = execute(cmd_name, shell_cmd)
     return err
 
 
-def execute(shell_cmd):
+def execute(cmd_name, shell_cmd):
     """Run a command.
 
+    :param str cmd_name: the user facing name of the hook being run
+    :param shell_cmd: shell command to execute
+    :type shell_cmd: `list` of `str` or `str`
+
     :returns: `tuple` (`str` stderr, `str` stdout)"""
+    logger.info("Running %s command: %s", cmd_name, shell_cmd)
 
     # universal_newlines causes Popen.communicate()
     # to return str objects instead of bytes in Python 3
@@ -245,12 +249,12 @@ def execute(shell_cmd):
     out, err = cmd.communicate()
     base_cmd = os.path.basename(shell_cmd.split(None, 1)[0])
     if out:
-        logger.info('Output from %s:\n%s', base_cmd, out)
+        logger.info('Output from %s command %s:\n%s', cmd_name, base_cmd, out)
     if cmd.returncode != 0:
-        logger.error('Hook command "%s" returned error code %d',
-                     shell_cmd, cmd.returncode)
+        logger.error('%s command "%s" returned error code %d',
+                     cmd_name, shell_cmd, cmd.returncode)
     if err:
-        logger.error('Error output from %s:\n%s', base_cmd, err)
+        logger.error('Error output from %s command %s:\n%s', cmd_name, base_cmd, err)
     return (err, out)
 
 

certbot/interfaces.py

@@ -522,56 +522,6 @@ class IDisplay(zope.interface.Interface):
         """
 
 
-class IValidator(zope.interface.Interface):
-    """Configuration validator."""
-
-    def certificate(cert, name, alt_host=None, port=443):
-        """Verifies the certificate presented at name is cert
-
-        :param OpenSSL.crypto.X509 cert: Expected certificate
-        :param str name: Server's domain name
-        :param bytes alt_host: Host to connect to instead of the IP
-            address of host
-        :param int port: Port to connect to
-
-        :returns: True if the certificate was verified successfully
-        :rtype: bool
-
-        """
-
-    def redirect(name, port=80, headers=None):
-        """Verify redirect to HTTPS
-
-        :param str name: Server's domain name
-        :param int port: Port to connect to
-        :param dict headers: HTTP headers to include in request
-
-        :returns: True if redirect is successfully enabled
-        :rtype: bool
-
-        """
-
-    def hsts(name):
-        """Verify HSTS header is enabled
-
-        :param str name: Server's domain name
-
-        :returns: True if HSTS header is successfully enabled
-        :rtype: bool
-
-        """
-
-    def ocsp_stapling(name):
-        """Verify ocsp stapling for domain
-
-        :param str name: Server's domain name
-
-        :returns: True if ocsp stapling is successfully enabled
-        :rtype: bool
-
-        """
-
-
 class IReporter(zope.interface.Interface):
     """Interface to collect and display information to the user."""
 

certbot/lock.py

@@ -1,15 +1,23 @@
-"""Implements file locks for locking files and directories in UNIX."""
+"""Implements file locks compatible with Linux and Windows for locking files and directories."""
 import errno
 import logging
 import os
+try:
+    import fcntl  # pylint: disable=import-error
+except ImportError:
+    import msvcrt  # pylint: disable=import-error
+    POSIX_MODE = False
+else:
+    POSIX_MODE = True
 
-from certbot import compat
 from certbot import errors
+from acme.magic_typing import Optional, Callable  # pylint: disable=unused-import, no-name-in-module
 
 logger = logging.getLogger(__name__)
 
 
 def lock_dir(dir_path):
+    # type: (str) -> LockFile
     """Place a lock file on the directory at dir_path.
 
     The lock file is placed in the root of dir_path with the name
@@ -27,34 +35,99 @@ def lock_dir(dir_path):
 
 
 class LockFile(object):
-    """A UNIX lock file.
-
-    This lock file is released when the locked file is closed or the
-    process exits. It cannot be used to provide synchronization between
-    threads. It is based on the lock_file package by Martin Horcicka.
-
+    """
+    Platform independent file lock system.
+    LockFile accepts a parameter, the path to a file acting as a lock. Once the LockFile,
+    instance is created, the associated file is 'locked from the point of view of the OS,
+    meaning that if another instance of Certbot try at the same time to acquire the same lock,
+    it will raise an Exception. Calling release method will release the lock, and make it
+    available to every other instance.
+    Upon exit, Certbot will also release all the locks.
+    This allows us to protect a file or directory from being concurrently accessed
+    or modified by two Certbot instances.
+    LockFile is platform independent: it will proceed to the appropriate OS lock mechanism
+    depending on Linux or Windows.
     """
     def __init__(self, path):
-        """Initialize and acquire the lock file.
-
-        :param str path: path to the file to lock
-
-        :raises errors.LockError: if unable to acquire the lock
-
+        # type: (str) -> None
+        """
+        Create a LockFile instance on the given file path, and acquire lock.
+        :param str path: the path to the file that will hold a lock
         """
-        super(LockFile, self).__init__()
         self._path = path
-        self._fd = None
+        mechanism = _UnixLockMechanism if POSIX_MODE else _WindowsLockMechanism
+        self._lock_mechanism = mechanism(path)
 
         self.acquire()
 
+    def __repr__(self):
+        # type: () -> str
+        repr_str = '{0}({1}) <'.format(self.__class__.__name__, self._path)
+        if self.is_locked():
+            repr_str += 'acquired>'
+        else:
+            repr_str += 'released>'
+        return repr_str
+
     def acquire(self):
-        """Acquire the lock file.
-
-        :raises errors.LockError: if lock is already held
-        :raises OSError: if unable to open or stat the lock file
-
+        # type: () -> None
         """
+        Acquire the lock on the file, forbidding any other Certbot instance to acquire it.
+        :raises errors.LockError: if unable to acquire the lock
+        """
+        self._lock_mechanism.acquire()
+
+    def release(self):
+        # type: () -> None
+        """
+        Release the lock on the file, allowing any other Certbot instance to acquire it.
+        """
+        self._lock_mechanism.release()
+
+    def is_locked(self):
+        # type: () -> bool
+        """
+        Check if the file is currently locked.
+        :return: True if the file is locked, False otherwise
+        """
+        return self._lock_mechanism.is_locked()
+
+
+class _BaseLockMechanism(object):
+    def __init__(self, path):
+        # type: (str) -> None
+        """
+        Create a lock file mechanism for Unix.
+        :param str path: the path to the lock file
+        """
+        self._path = path
+        self._fd = None  # type: Optional[int]
+
+    def is_locked(self):
+        # type: () -> bool
+        """Check if lock file is currently locked.
+        :return: True if the lock file is locked
+        :rtype: bool
+        """
+        return self._fd is not None
+
+    def acquire(self):  # pylint: disable=missing-docstring
+        pass  # pragma: no cover
+
+    def release(self):  # pylint: disable=missing-docstring
+        pass  # pragma: no cover
+
+
+class _UnixLockMechanism(_BaseLockMechanism):
+    """
+    A UNIX lock file mechanism.
+    This lock file is released when the locked file is closed or the
+    process exits. It cannot be used to provide synchronization between
+    threads. It is based on the lock_file package by Martin Horcicka.
+    """
+    def acquire(self):
+        # type: () -> None
+        """Acquire the lock."""
         while self._fd is None:
             # Open the file
             fd = os.open(self._path, os.O_CREAT | os.O_WRONLY, 0o600)
@@ -68,33 +141,29 @@ class LockFile(object):
                     os.close(fd)
 
     def _try_lock(self, fd):
-        """Try to acquire the lock file without blocking.
-
+        # type: (int) -> None
+        """
+        Try to acquire the lock file without blocking.
         :param int fd: file descriptor of the opened file to lock
-
         """
         try:
-            compat.lock_file(fd)
+            fcntl.lockf(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
         except IOError as err:
             if err.errno in (errno.EACCES, errno.EAGAIN):
-                logger.debug(
-                    "A lock on %s is held by another process.", self._path)
-                raise errors.LockError(
-                    "Another instance of Certbot is already running.")
+                logger.debug('A lock on %s is held by another process.', self._path)
+                raise errors.LockError('Another instance of Certbot is already running.')
             raise
 
     def _lock_success(self, fd):
-        """Did we successfully grab the lock?
-
+        # type: (int) -> bool
+        """
+        Did we successfully grab the lock?
         Because this class deletes the locked file when the lock is
         released, it is possible another process removed and recreated
         the file between us opening the file and acquiring the lock.
-
         :param int fd: file descriptor of the opened file to lock
-
         :returns: True if the lock was successfully acquired
         :rtype: bool
-
         """
         try:
             stat1 = os.stat(self._path)
@@ -108,17 +177,75 @@ class LockFile(object):
         # the same device and inode, they're the same file.
         return stat1.st_dev == stat2.st_dev and stat1.st_ino == stat2.st_ino
 
-    def __repr__(self):
-        repr_str = '{0}({1}) <'.format(self.__class__.__name__, self._path)
-        if self._fd is None:
-            repr_str += 'released>'
-        else:
-            repr_str += 'acquired>'
-        return repr_str
+    def release(self):
+        # type: () -> None
+        """Remove, close, and release the lock file."""
+        # It is important the lock file is removed before it's released,
+        # otherwise:
+        #
+        # process A: open lock file
+        # process B: release lock file
+        # process A: lock file
+        # process A: check device and inode
+        # process B: delete file
+        # process C: open and lock a different file at the same path
+        try:
+            os.remove(self._path)
+        finally:
+            # Following check is done to make mypy happy: it ensure that self._fd, marked
+            # as Optional[int] is effectively int to make it compatible with os.close signature.
+            if self._fd is None:  # pragma: no cover
+                raise TypeError('Error, self._fd is None.')
+            try:
+                os.close(self._fd)
+            finally:
+                self._fd = None
+
+
+class _WindowsLockMechanism(_BaseLockMechanism):
+    """
+    A Windows lock file mechanism.
+    By default on Windows, acquiring a file handler gives exclusive access to the process
+    and results in an effective lock. However, it is possible to explicitly acquire the
+    file handler in shared access in terms of read and write, and this is done by os.open
+    and io.open in Python. So an explicit lock needs to be done through the call of
+    msvcrt.locking, that will lock the first byte of the file. In theory, it is also
+    possible to access a file in shared delete access, allowing other processes to delete an
+    opened file. But this needs also to be done explicitly by all processes using the Windows
+    low level APIs, and Python does not do it. As of Python 3.7 and below, Python developers
+    state that deleting a file opened by a process from another process is not possible with
+    os.open and io.open.
+    Consequently, mscvrt.locking is sufficient to obtain an effective lock, and the race
+    condition encountered on Linux is not possible on Windows, leading to a simpler workflow.
+    """
+    def acquire(self):
+        """Acquire the lock"""
+        open_mode = os.O_RDWR | os.O_CREAT | os.O_TRUNC
+
+        fd = os.open(self._path, open_mode, 0o600)
+        try:
+            msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)
+        except (IOError, OSError) as err:
+            os.close(fd)
+            # Anything except EACCES is unexpected. Raise directly the error in that case.
+            if err.errno != errno.EACCES:
+                raise
+            logger.debug('A lock on %s is held by another process.', self._path)
+            raise errors.LockError('Another instance of Certbot is already running.')
+
+        self._fd = fd
 
     def release(self):
-        """Remove, close, and release the lock file."""
+        """Release the lock."""
         try:
-            compat.release_locked_file(self._fd, self._path)
+            msvcrt.locking(self._fd, msvcrt.LK_UNLCK, 1)
+            os.close(self._fd)
+
+            try:
+                os.remove(self._path)
+            except OSError as e:
+                # If the lock file cannot be removed, it is not a big deal.
+                # Likely another instance is acquiring the lock we just released.
+                logger.debug(str(e))
         finally:
             self._fd = None

certbot/plugins/manual.py

@@ -202,7 +202,7 @@ permitted by DNS standards.)
             os.environ.pop('CERTBOT_KEY_PATH', None)
             os.environ.pop('CERTBOT_SNI_DOMAIN', None)
         os.environ.update(env)
-        _, out = hooks.execute(self.conf('auth-hook'))
+        _, out = self._execute_hook('auth-hook')
         env['CERTBOT_AUTH_OUTPUT'] = out.strip()
         self.env[achall] = env
 
@@ -243,5 +243,8 @@ permitted by DNS standards.)
                 if 'CERTBOT_TOKEN' not in env:
                     os.environ.pop('CERTBOT_TOKEN', None)
                 os.environ.update(env)
-                hooks.execute(self.conf('cleanup-hook'))
+                self._execute_hook('cleanup-hook')
         self.reverter.recovery_routine()
+
+    def _execute_hook(self, hook_name):
+        return hooks.execute(self.option_name(hook_name), self.conf(hook_name))

certbot/tests/account_test.py

@@ -12,6 +12,7 @@ import pytz
 
 from acme import messages
 
+from certbot import compat
 from certbot import errors
 
 import certbot.tests.util as test_util
@@ -114,7 +115,8 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         self.mock_client.directory.new_authz = new_authzr_uri
 
     def test_init_creates_dir(self):
-        self.assertTrue(os.path.isdir(self.config.accounts_dir))
+        self.assertTrue(os.path.isdir(
+            compat.underscores_for_unsupported_characters_in_path(self.config.accounts_dir)))
 
     @test_util.broken_on_windows
     def test_save_and_restore(self):

certbot/tests/configuration_test.py

@@ -4,6 +4,7 @@ import unittest
 
 import mock
 
+from certbot import compat
 from certbot import constants
 from certbot import errors
 
@@ -47,9 +48,11 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
         mock_constants.KEY_DIR = 'keys'
         mock_constants.TEMP_CHECKPOINT_DIR = 't'
 
+        ref_path = compat.underscores_for_unsupported_characters_in_path(
+            'acc/acme-server.org:443/new')
         self.assertEqual(
             os.path.normpath(self.config.accounts_dir),
-            os.path.normpath(os.path.join(self.config.config_dir, 'acc/acme-server.org:443/new')))
+            os.path.normpath(os.path.join(self.config.config_dir, ref_path)))
         self.assertEqual(
             os.path.normpath(self.config.backup_dir),
             os.path.normpath(os.path.join(self.config.work_dir, 'backups')))

certbot/tests/hook_test.py

@@ -121,7 +121,7 @@ class PreHookTest(HookTest):
 
     def _test_nonrenew_common(self):
         mock_execute = self._call_with_mock_execute(self.config)
-        mock_execute.assert_called_once_with(self.config.pre_hook)
+        mock_execute.assert_called_once_with("pre-hook", self.config.pre_hook)
         self._test_no_executions_common()
 
     def test_no_hooks(self):
@@ -137,21 +137,21 @@ class PreHookTest(HookTest):
     def test_renew_disabled_dir_hooks(self):
         self.config.directory_hooks = False
         mock_execute = self._call_with_mock_execute(self.config)
-        mock_execute.assert_called_once_with(self.config.pre_hook)
+        mock_execute.assert_called_once_with("pre-hook", self.config.pre_hook)
         self._test_no_executions_common()
 
     def test_renew_no_overlap(self):
         self.config.verb = "renew"
         mock_execute = self._call_with_mock_execute(self.config)
-        mock_execute.assert_any_call(self.dir_hook)
-        mock_execute.assert_called_with(self.config.pre_hook)
+        mock_execute.assert_any_call("pre-hook", self.dir_hook)
+        mock_execute.assert_called_with("pre-hook", self.config.pre_hook)
         self._test_no_executions_common()
 
     def test_renew_with_overlap(self):
         self.config.pre_hook = self.dir_hook
         self.config.verb = "renew"
         mock_execute = self._call_with_mock_execute(self.config)
-        mock_execute.assert_called_once_with(self.dir_hook)
+        mock_execute.assert_called_once_with("pre-hook", self.dir_hook)
         self._test_no_executions_common()
 
     def _test_no_executions_common(self):
@@ -193,7 +193,7 @@ class PostHookTest(HookTest):
         for verb in ("certonly", "run",):
             self.config.verb = verb
             mock_execute = self._call_with_mock_execute(self.config)
-            mock_execute.assert_called_once_with(self.config.post_hook)
+            mock_execute.assert_called_once_with("post-hook", self.config.post_hook)
             self.assertFalse(self._get_eventually())
 
     def test_cert_only_and_run_without_hook(self):
@@ -277,12 +277,12 @@ class RunSavedPostHooksTest(HookTest):
 
         calls = mock_execute.call_args_list
         for actual_call, expected_arg in zip(calls, self.eventually):
-            self.assertEqual(actual_call[0][0], expected_arg)
+            self.assertEqual(actual_call[0][1], expected_arg)
 
     def test_single(self):
         self.eventually = ["foo"]
         mock_execute = self._call_with_mock_execute_and_eventually()
-        mock_execute.assert_called_once_with(self.eventually[0])
+        mock_execute.assert_called_once_with("post-hook", self.eventually[0])
 
 
 class RenewalHookTest(HookTest):
@@ -360,7 +360,7 @@ class DeployHookTest(RenewalHookTest):
         self.config.deploy_hook = "foo"
         mock_execute = self._call_with_mock_execute(
             self.config, domains, lineage)
-        mock_execute.assert_called_once_with(self.config.deploy_hook)
+        mock_execute.assert_called_once_with("deploy-hook", self.config.deploy_hook)
 
 
 class RenewHookTest(RenewalHookTest):
@@ -384,7 +384,7 @@ class RenewHookTest(RenewalHookTest):
         self.config.directory_hooks = False
         mock_execute = self._call_with_mock_execute(
             self.config, ["example.org"], "/foo/bar")
-        mock_execute.assert_called_once_with(self.config.renew_hook)
+        mock_execute.assert_called_once_with("deploy-hook", self.config.renew_hook)
 
     @mock.patch("certbot.hooks.logger")
     def test_dry_run(self, mock_logger):
@@ -408,13 +408,13 @@ class RenewHookTest(RenewalHookTest):
         self.config.renew_hook = self.dir_hook
         mock_execute = self._call_with_mock_execute(
             self.config, ["example.net", "example.org"], "/foo/bar")
-        mock_execute.assert_called_once_with(self.dir_hook)
+        mock_execute.assert_called_once_with("deploy-hook", self.dir_hook)
 
     def test_no_overlap(self):
         mock_execute = self._call_with_mock_execute(
             self.config, ["example.org"], "/foo/bar")
-        mock_execute.assert_any_call(self.dir_hook)
-        mock_execute.assert_called_with(self.config.renew_hook)
+        mock_execute.assert_any_call("deploy-hook", self.dir_hook)
+        mock_execute.assert_called_with("deploy-hook", self.config.renew_hook)
 
 
 class ExecuteTest(unittest.TestCase):
@@ -433,18 +433,22 @@ class ExecuteTest(unittest.TestCase):
 
     def _test_common(self, returncode, stdout, stderr):
         given_command = "foo"
+        given_name = "foo-hook"
         with mock.patch("certbot.hooks.Popen") as mock_popen:
             mock_popen.return_value.communicate.return_value = (stdout, stderr)
             mock_popen.return_value.returncode = returncode
             with mock.patch("certbot.hooks.logger") as mock_logger:
-                self.assertEqual(self._call(given_command), (stderr, stdout))
+                self.assertEqual(self._call(given_name, given_command), (stderr, stdout))
 
         executed_command = mock_popen.call_args[1].get(
             "args", mock_popen.call_args[0][0])
         self.assertEqual(executed_command, given_command)
 
+        mock_logger.info.assert_any_call("Running %s command: %s",
+                                         given_name, given_command)
         if stdout:
-            self.assertTrue(mock_logger.info.called)
+            mock_logger.info.assert_any_call(mock.ANY, mock.ANY,
+                                             mock.ANY, stdout)
         if stderr or returncode:
             self.assertTrue(mock_logger.error.called)
 

certbot/tests/lock_test.py

@@ -3,6 +3,12 @@ import functools
 import multiprocessing
 import os
 import unittest
+try:
+    import fcntl  # pylint: disable=import-error,unused-import
+except ImportError:
+    POSIX_MODE = False
+else:
+    POSIX_MODE = True
 
 import mock
 
@@ -71,7 +77,8 @@ class LockFileTest(test_util.TempDirTestCase):
         self.assertTrue(lock_file.__class__.__name__ in lock_repr)
         self.assertTrue(self.lock_path in lock_repr)
 
-    @test_util.broken_on_windows
+    @test_util.skip_on_windows(
+        'Race conditions on lock are specific to the non-blocking file access approach on Linux.')
     def test_race(self):
         should_delete = [True, False]
         stat = os.stat
@@ -93,28 +100,36 @@ class LockFileTest(test_util.TempDirTestCase):
         lock_file.release()
         self.assertFalse(os.path.exists(self.lock_path))
 
-    @test_util.broken_on_windows
-    @mock.patch('certbot.compat.fcntl.lockf')
-    def test_unexpected_lockf_err(self, mock_lockf):
+    def test_unexpected_lockf_or_locking_err(self):
+        if POSIX_MODE:
+            mocked_function = 'certbot.lock.fcntl.lockf'
+        else:
+            mocked_function = 'certbot.lock.msvcrt.locking'
         msg = 'hi there'
-        mock_lockf.side_effect = IOError(msg)
-        try:
-            self._call(self.lock_path)
-        except IOError as err:
-            self.assertTrue(msg in str(err))
-        else:  # pragma: no cover
-            self.fail('IOError not raised')
+        with mock.patch(mocked_function) as mock_lock:
+            mock_lock.side_effect = IOError(msg)
+            try:
+                self._call(self.lock_path)
+            except IOError as err:
+                self.assertTrue(msg in str(err))
+            else:  # pragma: no cover
+                self.fail('IOError not raised')
 
-    @mock.patch('certbot.lock.os.stat')
-    def test_unexpected_stat_err(self, mock_stat):
+    def test_unexpected_os_err(self):
+        if POSIX_MODE:
+            mock_function = 'certbot.lock.os.stat'
+        else:
+            mock_function = 'certbot.lock.msvcrt.locking'
+        # The only expected errno are ENOENT and EACCES in lock module.
         msg = 'hi there'
-        mock_stat.side_effect = OSError(msg)
-        try:
-            self._call(self.lock_path)
-        except OSError as err:
-            self.assertTrue(msg in str(err))
-        else:  # pragma: no cover
-            self.fail('OSError not raised')
+        with mock.patch(mock_function) as mock_os:
+            mock_os.side_effect = OSError(msg)
+            try:
+                self._call(self.lock_path)
+            except OSError as err:
+                self.assertTrue(msg in str(err))
+            else:  # pragma: no cover
+                self.fail('OSError not raised')
 
 
 if __name__ == "__main__":

certbot/tests/util.py

@@ -4,7 +4,6 @@
 
 """
 import logging
-import multiprocessing
 import os
 import pkg_resources
 import shutil
@@ -12,6 +11,7 @@ import stat
 import tempfile
 import unittest
 import sys
+from multiprocessing import Process, Event
 
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import serialization
@@ -24,8 +24,9 @@ from six.moves import reload_module  # pylint: disable=import-error
 from certbot import constants
 from certbot import interfaces
 from certbot import storage
-from certbot import util
 from certbot import configuration
+from certbot import lock
+from certbot import util
 
 from certbot.display import util as display_util
 
@@ -212,7 +213,7 @@ class FreezableMock(object):
 
     """
     def __init__(self, frozen=False, func=None, return_value=mock.sentinel.DEFAULT):
-        self._frozen_set = set() if frozen else set(('freeze',))
+        self._frozen_set = set() if frozen else {'freeze', }
         self._func = func
         self._mock = mock.MagicMock()
         if return_value != mock.sentinel.DEFAULT:
@@ -334,6 +335,9 @@ class TempDirTestCase(unittest.TestCase):
         # called and instead will run them right before the entire test process exits.
         # It is a problem on Windows, that does not accept to clean resources before closing them.
         logging.shutdown()
+        # Remove logging handlers that have been closed so they won't be
+        # accidentally used in future tests.
+        logging.getLogger().handlers = []
         util._release_locks()  # pylint: disable=protected-access
 
         def handle_rw_files(_, path, __):
@@ -359,47 +363,51 @@ class ConfigTestCase(TempDirTestCase):
         self.config.chain_path = constants.CLI_DEFAULTS['auth_chain_path']
         self.config.server = "https://example.com"
 
-def lock_and_call(func, lock_path):
-    """Grab a lock for lock_path and call func.
-
-    :param callable func: object to call after acquiring the lock
-    :param str lock_path: path to file or directory to lock
 
+def _handle_lock(event_in, event_out, path):
     """
-    # Reload module to reset internal _LOCKS dictionary
+    Acquire a file lock on given path, then wait to release it. This worker is coordinated
+    using events to signal when the lock should be acquired and released.
+    :param multiprocessing.Event event_in: event object to signal when to release the lock
+    :param multiprocessing.Event event_out: event object to signal when the lock is acquired
+    :param path: the path to lock
+    """
+    if os.path.isdir(path):
+        my_lock = lock.lock_dir(path)
+    else:
+        my_lock = lock.LockFile(path)
+    try:
+        event_out.set()
+        assert event_in.wait(timeout=20), 'Timeout while waiting to release the lock.'
+    finally:
+        my_lock.release()
+
+
+def lock_and_call(callback, path_to_lock):
+    """
+    Grab a lock on path_to_lock from a foreign process then execute the callback.
+    :param callable callback: object to call after acquiring the lock
+    :param str path_to_lock: path to file or directory to lock
+    """
+    # Reload certbot.util module to reset internal _LOCKS dictionary.
     reload_module(util)
 
-    # start child and wait for it to grab the lock
-    cv = multiprocessing.Condition()
-    cv.acquire()
-    child_args = (cv, lock_path,)
-    child = multiprocessing.Process(target=hold_lock, args=child_args)
-    child.start()
-    cv.wait()
+    emit_event = Event()
+    receive_event = Event()
+    process = Process(target=_handle_lock, args=(emit_event, receive_event, path_to_lock))
+    process.start()
 
-    # call func and terminate the child
-    func()
-    cv.notify()
-    cv.release()
-    child.join()
-    assert child.exitcode == 0
+    # Wait confirmation that lock is acquired
+    assert receive_event.wait(timeout=10), 'Timeout while waiting to acquire the lock.'
+    # Execute the callback
+    callback()
+    # Trigger unlock from foreign process
+    emit_event.set()
 
-def hold_lock(cv, lock_path):  # pragma: no cover
-    """Acquire a file lock at lock_path and wait to release it.
+    # Wait for process termination
+    process.join(timeout=10)
+    assert process.exitcode == 0
 
-    :param multiprocessing.Condition cv: condition for synchronization
-    :param str lock_path: path to the file lock
-
-    """
-    from certbot import lock
-    if os.path.isdir(lock_path):
-        my_lock = lock.lock_dir(lock_path)
-    else:
-        my_lock = lock.LockFile(lock_path)
-    cv.acquire()
-    cv.notify()
-    cv.wait()
-    my_lock.release()
 
 def skip_on_windows(reason):
     """Decorator to skip permanently a test on Windows. A reason is required."""
@@ -408,6 +416,7 @@ def skip_on_windows(reason):
         return unittest.skipIf(sys.platform == 'win32', reason)(function)
     return wrapper
 
+
 def broken_on_windows(function):
     """Decorator to skip temporarily a broken test on Windows."""
     reason = 'Test is broken and ignored on windows but should be fixed.'
@@ -416,9 +425,10 @@ def broken_on_windows(function):
         and os.environ.get('SKIP_BROKEN_TESTS_ON_WINDOWS', 'true') == 'true',
         reason)(function)
 
+
 def temp_join(path):
     """
     Return the given path joined to the tempdir path for the current platform
     Eg.: 'cert' => /tmp/cert (Linux) or 'C:\\Users\\currentuser\\AppData\\Temp\\cert' (Windows)
     """
-    return  os.path.join(tempfile.gettempdir(), path)
+    return os.path.join(tempfile.gettempdir(), path)

certbot/tests/util_test.py

@@ -2,7 +2,6 @@
 import argparse
 import errno
 import os
-import shutil
 import unittest
 
 import mock
@@ -88,7 +87,6 @@ class LockDirUntilExit(test_util.TempDirTestCase):
         import certbot.util
         reload_module(certbot.util)
 
-    @test_util.broken_on_windows
     @mock.patch('certbot.util.logger')
     @mock.patch('certbot.util.atexit_register')
     def test_it(self, mock_register, mock_logger):
@@ -100,11 +98,15 @@ class LockDirUntilExit(test_util.TempDirTestCase):
 
         self.assertEqual(mock_register.call_count, 1)
         registered_func = mock_register.call_args[0][0]
-        shutil.rmtree(subdir)
-        registered_func()  # exception not raised
-        # logger.debug is only called once because the second call
-        # to lock subdir was ignored because it was already locked
-        self.assertEqual(mock_logger.debug.call_count, 1)
+
+        from certbot import util
+        # Despite lock_dir_until_exit has been called twice to subdir, its lock should have been
+        # added only once. So we expect to have two lock references: for self.tempdir and subdir
+        self.assertTrue(len(util._LOCKS) == 2)  # pylint: disable=protected-access
+        registered_func()  # Exception should not be raised
+        # Logically, logger.debug, that would be invoked in case of unlock failure,
+        # should never been called.
+        self.assertEqual(mock_logger.debug.call_count, 0)
 
 
 class SetUpCoreDirTest(test_util.TempDirTestCase):

docs/cli-help.txt

@@ -113,7 +113,7 @@ optional arguments:
                         case, and to know when to deprecate support for past
                         Python versions and flags. If you wish to hide this
                         information from the Let's Encrypt server, set this to
-                        "". (default: CertbotACMEClient/0.30.2
+                        "". (default: CertbotACMEClient/0.31.0
                         (certbot(-auto); OS_NAME OS_VERSION) Authenticator/XXX
                         Installer/YYY (SUBCOMMAND; flags: FLAGS)
                         Py/major.minor.patchlevel). The flags encoded in the
@@ -351,8 +351,9 @@ revoke:
                         Specify reason for revoking certificate. (default:
                         unspecified)
   --delete-after-revoke
-                        Delete certificates after revoking them. (default:
-                        None)
+                        Delete certificates after revoking them, along with
+                        all previous and later versions of those certificates.
+                        (default: None)
   --no-delete-after-revoke
                         Do not delete certificates after revoking them. This
                         option should be used with caution because the 'renew'
@@ -479,10 +480,9 @@ apache:
   Apache Web Server plugin
 
   --apache-enmod APACHE_ENMOD
-                        Path to the Apache 'a2enmod' binary (default: a2enmod)
+                        Path to the Apache 'a2enmod' binary (default: None)
   --apache-dismod APACHE_DISMOD
-                        Path to the Apache 'a2dismod' binary (default:
-                        a2dismod)
+                        Path to the Apache 'a2dismod' binary (default: None)
   --apache-le-vhost-ext APACHE_LE_VHOST_EXT
                         SSL vhost configuration extension (default: -le-
                         ssl.conf)
@@ -496,16 +496,16 @@ apache:
                         /var/log/apache2)
   --apache-challenge-location APACHE_CHALLENGE_LOCATION
                         Directory path for challenge configuration (default:
-                        /etc/apache2)
+                        /etc/apache2/other)
   --apache-handle-modules APACHE_HANDLE_MODULES
                         Let installer handle enabling required modules for you
-                        (Only Ubuntu/Debian currently) (default: True)
+                        (Only Ubuntu/Debian currently) (default: False)
   --apache-handle-sites APACHE_HANDLE_SITES
                         Let installer handle enabling sites for you (Only
-                        Ubuntu/Debian currently) (default: True)
+                        Ubuntu/Debian currently) (default: False)
   --apache-ctl APACHE_CTL
                         Full path to Apache control script (default:
-                        apache2ctl)
+                        apachectl)
 
 dns-cloudflare:
   Obtain certificates using a DNS TXT record (if you are using Cloudflare

docs/using.rst

@@ -486,8 +486,9 @@ non-zero exit code. Hooks will only be run if a certificate is due for
 renewal, so you can run the above command frequently without
 unnecessarily stopping your webserver.
 
-``--pre-hook`` and ``--post-hook`` hooks run before and after every renewal
-attempt. If you want your hook to run only after a successful renewal, use
+When Certbot detects that a certificate is due for renewal, ``--pre-hook``
+and ``--post-hook`` hooks run before and after each attempt to renew it.
+If you want your hook to run only after a successful renewal, use
 ``--deploy-hook`` in a command like this.
 
 ``certbot renew --deploy-hook /path/to/deploy-hook-script``
@@ -903,7 +904,7 @@ that by default two instances of Certbot will not be able to run in parallel.
 Since the directories used by Certbot are configurable, Certbot
 will write a lock file for all of the directories it uses. This include Certbot's
 ``--work-dir``, ``--logs-dir``, and ``--config-dir``. By default these are
-``/var/lib/letsencrypt``, ``/var/logs/letsencrypt``, and ``/etc/letsencrypt``
+``/var/lib/letsencrypt``, ``/var/log/letsencrypt``, and ``/etc/letsencrypt``
 respectively. Additionally if you are using Certbot with Apache or nginx it will
 lock the configuration folder for that program, which are typically also in the
 ``/etc`` directory.

letsencrypt-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.30.2"
+LE_AUTO_VERSION="0.31.0"
 BASENAME=$(basename $0)
 USAGE="Usage: $BASENAME [OPTIONS]
 A self-updating wrapper script for the Certbot ACME client. When run, updates
@@ -333,63 +333,11 @@ BootstrapDebCommon() {
   fi
 
   augeas_pkg="libaugeas0 augeas-lenses"
-  AUGVERSION=`LC_ALL=C apt-cache show --no-all-versions libaugeas0 | grep ^Version: | cut -d" " -f2`
 
   if [ "$ASSUME_YES" = 1 ]; then
     YES_FLAG="-y"
   fi
 
-  AddBackportRepo() {
-    # ARGS:
-    BACKPORT_NAME="$1"
-    BACKPORT_SOURCELINE="$2"
-    say "To use the Apache Certbot plugin, augeas needs to be installed from $BACKPORT_NAME."
-    if ! grep -v -e ' *#' /etc/apt/sources.list | grep -q "$BACKPORT_NAME" ; then
-      # This can theoretically error if sources.list.d is empty, but in that case we don't care.
-      if ! grep -v -e ' *#' /etc/apt/sources.list.d/* 2>/dev/null | grep -q "$BACKPORT_NAME"; then
-        if [ "$ASSUME_YES" = 1 ]; then
-          /bin/echo -n "Installing augeas from $BACKPORT_NAME in 3 seconds..."
-          sleep 1s
-          /bin/echo -ne "\e[0K\rInstalling augeas from $BACKPORT_NAME in 2 seconds..."
-          sleep 1s
-          /bin/echo -e "\e[0K\rInstalling augeas from $BACKPORT_NAME in 1 second ..."
-          sleep 1s
-          add_backports=1
-        else
-          read -p "Would you like to enable the $BACKPORT_NAME repository [Y/n]? " response
-          case $response in
-            [yY][eE][sS]|[yY]|"")
-              add_backports=1;;
-            *)
-              add_backports=0;;
-          esac
-        fi
-        if [ "$add_backports" = 1 ]; then
-          sh -c "echo $BACKPORT_SOURCELINE >> /etc/apt/sources.list.d/$BACKPORT_NAME.list"
-          apt-get $QUIET_FLAG update
-        fi
-      fi
-    fi
-    if [ "$add_backports" != 0 ]; then
-      apt-get install $QUIET_FLAG $YES_FLAG --no-install-recommends -t "$BACKPORT_NAME" $augeas_pkg
-      augeas_pkg=
-    fi
-  }
-
-
-  if dpkg --compare-versions 1.0 gt "$AUGVERSION" ; then
-    if lsb_release -a | grep -q wheezy ; then
-      AddBackportRepo wheezy-backports "deb http://http.debian.net/debian wheezy-backports main"
-    elif lsb_release -a | grep -q precise ; then
-      # XXX add ARM case
-      AddBackportRepo precise-backports "deb http://archive.ubuntu.com/ubuntu precise-backports main restricted universe multiverse"
-    else
-      echo "No libaugeas0 version is available that's new enough to run the"
-      echo "Certbot apache plugin..."
-    fi
-    # XXX add a case for ubuntu PPAs
-  fi
-
   apt-get install $QUIET_FLAG $YES_FLAG --no-install-recommends \
     python \
     python-dev \
@@ -1140,9 +1088,9 @@ parsedatetime==2.1 \
 pbr==1.8.1 \
     --hash=sha256:46c8db75ae75a056bd1cc07fa21734fe2e603d11a07833ecc1eeb74c35c72e0c \
     --hash=sha256:e2127626a91e6c885db89668976db31020f0af2da728924b56480fc7ccf09649
-pyOpenSSL==16.2.0 \
-    --hash=sha256:26ca380ddf272f7556e48064bbcd5bd71f83dfc144f3583501c7ddbd9434ee17 \
-    --hash=sha256:7779a3bbb74e79db234af6a08775568c6769b5821faecf6e2f4143edb227516e
+pyOpenSSL==18.0.0 \
+    --hash=sha256:26ff56a6b5ecaf3a2a59f132681e2a80afcc76b4f902f612f518f92c2a1bf854 \
+    --hash=sha256:6488f1423b00f73b7ad5167885312bb0ce410d3312eb212393795b53c8caa580
 pyparsing==2.1.8 \
     --hash=sha256:2f0f5ceb14eccd5aef809d6382e87df22ca1da583c79f6db01675ce7d7f49c18 \
     --hash=sha256:03a4869b9f3493807ee1f1cb405e6d576a1a2ca4d81a982677c0c1ad6177c56b \
@@ -1232,18 +1180,18 @@ letsencrypt==0.7.0 \
     --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \
     --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9
 
-certbot==0.30.2 \
-    --hash=sha256:e411b72fa86eec1018e6de28e649e8c9c71191a7431dcc77f207b57ca9484c11 \
-    --hash=sha256:534487cb552ced8e47948ba3d2e7ca12c3a439133fc609485012b1a02fc7776e
-acme==0.30.2 \
-    --hash=sha256:68982576492dfa99c7e2be0fce4371adc9344740b05420ce0ab53238d2bb9b3b \
-    --hash=sha256:295a5b7fce9f908e6e5cff8c40be1a3daf3e1ebabd2e139a4c87274e68eeb8f2
-certbot-apache==0.30.2 \
-    --hash=sha256:3b7fa4e59772da7c9975ef2a49ceff157c9d7cb31eb9475928b5986d89701a3a \
-    --hash=sha256:32fa915a8a51810fdfe828ac1361da4425c231d7384891e49e6338e4741464b2
-certbot-nginx==0.30.2 \
-    --hash=sha256:7dc785f6f0c0c57b19cea8d98f9ea8feef53945613967b52c9348c81327010e2 \
-    --hash=sha256:6ba4dd772d0c7cdfb3383ca325b35639e01ac9e142e4baa6445cd85c7fb59552
+certbot==0.31.0 \
+    --hash=sha256:1a1b4b2675daf5266cc2cf2a44ded44de1d83e9541ffa078913c0e4c3231a1c4 \
+    --hash=sha256:0c3196f80a102c0f9d82d566ba859efe3b70e9ed4670520224c844fafd930473
+acme==0.31.0 \
+    --hash=sha256:a0c851f6b7845a0faa3a47a3e871440eed9ec11b4ab949de0dc4a0fb1201cd24 \
+    --hash=sha256:7e5c2d01986e0f34ca08fee58981892704c82c48435dcd3592b424c312d8b2bf
+certbot-apache==0.31.0 \
+    --hash=sha256:740bb55dd71723a21eebabb16e6ee5d8883f8b8f8cf6956dd1d4873e0cccae21 \
+    --hash=sha256:cc4b840b2a439a63e2dce809272c3c3cd4b1aeefc4053cd188935135be137edd
+certbot-nginx==0.31.0 \
+    --hash=sha256:7a1ffda9d93dc7c2aaf89452ce190250de8932e624d31ebba8e4fa7d950025c5 \
+    --hash=sha256:d450d75650384f74baccb7673c89e2f52468afa478ed354eb6d4b99aa33bf865
 
 UNLIKELY_EOF
     # -------------------------------------------------------------------------

letsencrypt-auto-source/certbot-auto.asc

@@ -1,11 +1,11 @@
 -----BEGIN PGP SIGNATURE-----
 
-iQEzBAABCAAdFiEEos+1H6J1pyhiNOeyTRfJlc2XdfIFAlxLcw8ACgkQTRfJlc2X
-dfK35gf+PoxtrJJIjvybNqd3lb8HOg2ntIVmXcYJGuuUo6m09fzai+XI6cOm5Dpu
-l2D5OrbLqmez8tYkCkEWHV0OfwyVWw+m8T3sXlcrv14eA1RfgMnZ+cmmlpDskzHU
-EOtaXo1/IkLDwBRrsl8IUbwD2XxbjuLsA2Sevoa59NlfTXJUApfAzohl3epRiJjB
-gugdqcsfjRRAqQqOz+iJCKBCWSTIrr/g6Y9aZu9V93t/WDSLRFjehxO1GQrLnCnX
-17JGlr0/AXd67jOKS1OWmORPPAFfLIXezUMtgrz5hE7T5UviaUu9ySV8UCxq1N79
-cfSBb/HIUxZ0wf1CkTUMRFQpA7cGtw==
-=cNcT
+iQEzBAABCAAdFiEEos+1H6J1pyhiNOeyTRfJlc2XdfIFAlxcop8ACgkQTRfJlc2X
+dfIbZwf/faKu7IjLi0qFQ+kw8zaAnV47JDgfWqbR5GSdwWPqld+QyHlcRfPgwYma
+fKj9+g/FvPNPSfjHRRCoFrYvpZ4lZ+f4HPN9+OjydfM77rdDhVDwzs8dbKIk02yU
+0IEJhXj5Q9hF3TSDZcyXAJdBU1lz51ohtVIXelMBPmzhYPCZF47iE9/k9pApQi86
+RTji7hxPcF/n7mzXrbyTvk+kDxSdDlE0Eg9syK7XaFDBTa2lqgG8wTnMPVqhc/hm
+WM/uwkzbYarjy05ffV1kM683nP0rECnHlYT38pYcT2puw2kn/QthwR5j/jB/DWSc
+94Kw7BeMH651V8EaNwYIiouylnVH3A==
+=U+Qh
 -----END PGP SIGNATURE-----

letsencrypt-auto-source/letsencrypt-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.31.0.dev0"
+LE_AUTO_VERSION="0.32.0.dev0"
 BASENAME=$(basename $0)
 USAGE="Usage: $BASENAME [OPTIONS]
 A self-updating wrapper script for the Certbot ACME client. When run, updates
@@ -1180,18 +1180,18 @@ letsencrypt==0.7.0 \
     --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \
     --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9
 
-certbot==0.30.2 \
-    --hash=sha256:e411b72fa86eec1018e6de28e649e8c9c71191a7431dcc77f207b57ca9484c11 \
-    --hash=sha256:534487cb552ced8e47948ba3d2e7ca12c3a439133fc609485012b1a02fc7776e
-acme==0.30.2 \
-    --hash=sha256:68982576492dfa99c7e2be0fce4371adc9344740b05420ce0ab53238d2bb9b3b \
-    --hash=sha256:295a5b7fce9f908e6e5cff8c40be1a3daf3e1ebabd2e139a4c87274e68eeb8f2
-certbot-apache==0.30.2 \
-    --hash=sha256:3b7fa4e59772da7c9975ef2a49ceff157c9d7cb31eb9475928b5986d89701a3a \
-    --hash=sha256:32fa915a8a51810fdfe828ac1361da4425c231d7384891e49e6338e4741464b2
-certbot-nginx==0.30.2 \
-    --hash=sha256:7dc785f6f0c0c57b19cea8d98f9ea8feef53945613967b52c9348c81327010e2 \
-    --hash=sha256:6ba4dd772d0c7cdfb3383ca325b35639e01ac9e142e4baa6445cd85c7fb59552
+certbot==0.31.0 \
+    --hash=sha256:1a1b4b2675daf5266cc2cf2a44ded44de1d83e9541ffa078913c0e4c3231a1c4 \
+    --hash=sha256:0c3196f80a102c0f9d82d566ba859efe3b70e9ed4670520224c844fafd930473
+acme==0.31.0 \
+    --hash=sha256:a0c851f6b7845a0faa3a47a3e871440eed9ec11b4ab949de0dc4a0fb1201cd24 \
+    --hash=sha256:7e5c2d01986e0f34ca08fee58981892704c82c48435dcd3592b424c312d8b2bf
+certbot-apache==0.31.0 \
+    --hash=sha256:740bb55dd71723a21eebabb16e6ee5d8883f8b8f8cf6956dd1d4873e0cccae21 \
+    --hash=sha256:cc4b840b2a439a63e2dce809272c3c3cd4b1aeefc4053cd188935135be137edd
+certbot-nginx==0.31.0 \
+    --hash=sha256:7a1ffda9d93dc7c2aaf89452ce190250de8932e624d31ebba8e4fa7d950025c5 \
+    --hash=sha256:d450d75650384f74baccb7673c89e2f52468afa478ed354eb6d4b99aa33bf865
 
 UNLIKELY_EOF
     # -------------------------------------------------------------------------

letsencrypt-auto-source/pieces/certbot-requirements.txt

@@ -1,12 +1,12 @@
-certbot==0.30.2 \
-    --hash=sha256:e411b72fa86eec1018e6de28e649e8c9c71191a7431dcc77f207b57ca9484c11 \
-    --hash=sha256:534487cb552ced8e47948ba3d2e7ca12c3a439133fc609485012b1a02fc7776e
-acme==0.30.2 \
-    --hash=sha256:68982576492dfa99c7e2be0fce4371adc9344740b05420ce0ab53238d2bb9b3b \
-    --hash=sha256:295a5b7fce9f908e6e5cff8c40be1a3daf3e1ebabd2e139a4c87274e68eeb8f2
-certbot-apache==0.30.2 \
-    --hash=sha256:3b7fa4e59772da7c9975ef2a49ceff157c9d7cb31eb9475928b5986d89701a3a \
-    --hash=sha256:32fa915a8a51810fdfe828ac1361da4425c231d7384891e49e6338e4741464b2
-certbot-nginx==0.30.2 \
-    --hash=sha256:7dc785f6f0c0c57b19cea8d98f9ea8feef53945613967b52c9348c81327010e2 \
-    --hash=sha256:6ba4dd772d0c7cdfb3383ca325b35639e01ac9e142e4baa6445cd85c7fb59552
+certbot==0.31.0 \
+    --hash=sha256:1a1b4b2675daf5266cc2cf2a44ded44de1d83e9541ffa078913c0e4c3231a1c4 \
+    --hash=sha256:0c3196f80a102c0f9d82d566ba859efe3b70e9ed4670520224c844fafd930473
+acme==0.31.0 \
+    --hash=sha256:a0c851f6b7845a0faa3a47a3e871440eed9ec11b4ab949de0dc4a0fb1201cd24 \
+    --hash=sha256:7e5c2d01986e0f34ca08fee58981892704c82c48435dcd3592b424c312d8b2bf
+certbot-apache==0.31.0 \
+    --hash=sha256:740bb55dd71723a21eebabb16e6ee5d8883f8b8f8cf6956dd1d4873e0cccae21 \
+    --hash=sha256:cc4b840b2a439a63e2dce809272c3c3cd4b1aeefc4053cd188935135be137edd
+certbot-nginx==0.31.0 \
+    --hash=sha256:7a1ffda9d93dc7c2aaf89452ce190250de8932e624d31ebba8e4fa7d950025c5 \
+    --hash=sha256:d450d75650384f74baccb7673c89e2f52468afa478ed354eb6d4b99aa33bf865

setup.py

@@ -1,8 +1,10 @@
 import codecs
 import os
 import re
+import sys
 
 from setuptools import find_packages, setup
+from setuptools.command.test import test as TestCommand
 
 # Workaround for http://bugs.python.org/issue8876, see
 # http://bugs.python.org/issue8876#msg208792
@@ -38,7 +40,9 @@ install_requires = [
     'ConfigArgParse>=0.9.3',
     'configobj',
     'cryptography>=1.2.3',  # load_pem_x509_certificate
-    'josepy',
+    # 1.1.0+ is required to avoid the warnings described at
+    # https://github.com/certbot/josepy/issues/13.
+    'josepy>=1.1.0',
     'mock',
     'parsedatetime>=1.3',  # Calendar.parseDT
     'pyrfc3339',
@@ -75,6 +79,22 @@ docs_extras = [
     'sphinx_rtd_theme',
 ]
 
+
+class PyTest(TestCommand):
+    user_options = []
+
+    def initialize_options(self):
+        TestCommand.initialize_options(self)
+        self.pytest_args = ''
+
+    def run_tests(self):
+        import shlex
+        # import here, cause outside the eggs aren't loaded
+        import pytest
+        errno = pytest.main(shlex.split(self.pytest_args))
+        sys.exit(errno)
+
+
 setup(
     name='certbot',
     version=version,
@@ -121,6 +141,8 @@ setup(
     # to test all packages run "python setup.py test -s
     # {acme,certbot_apache,certbot_nginx}"
     test_suite='certbot',
+    tests_require=["pytest"],
+    cmdclass={"test": PyTest},
 
     entry_points={
         'console_scripts': [

tests/display.py

@@ -1,22 +0,0 @@
-"""Manual test of display functions."""
-import sys
-
-from certbot.display import util
-from certbot.tests.display import util_test
-
-
-def test_visual(displayer, choices):
-    """Visually test all of the display functions."""
-    displayer.notification("Random notification!")
-    displayer.menu("Question?", choices,
-                   ok_label="O", cancel_label="Can", help_label="??")
-    displayer.menu("Question?", [choice[1] for choice in choices],
-                   ok_label="O", cancel_label="Can", help_label="??")
-    displayer.input("Input Message")
-    displayer.yesno("YesNo Message", yes_label="Yessir", no_label="Nosir")
-    displayer.checklist("Checklist Message", [choice[0] for choice in choices])
-
-
-if __name__ == "__main__":
-    displayer = util.FileDisplay(sys.stdout, False)
-    test_visual(displayer, util_test.CHOICES)

tests/letstest/multitester.py

@@ -564,6 +564,11 @@ try:
         ii, target, status = outq
         print('%d %s %s'%(ii, target['name'], status))
         results_file.write('%d %s %s\n'%(ii, target['name'], status))
+    if len(outputs) != num_processes:
+        failure_message = 'FAILURE: Some target machines failed to run and were not tested. ' +\
+            'Tests should be rerun.'
+        print(failure_message)
+        results_file.write(failure_message + '\n')
     results_file.close()
 
 finally:

tests/letstest/scripts/test_leauto_upgrades.sh

@@ -23,7 +23,7 @@ fi
 # started failing on newer distros with newer versions of OpenSSL.
 INITIAL_VERSION="0.17.0"
 git checkout -f "v$INITIAL_VERSION" letsencrypt-auto
-if ! ./letsencrypt-auto -v --debug --version --no-self-upgrade 2>&1 | grep "$INITIAL_VERSION" ; then
+if ! ./letsencrypt-auto -v --debug --version --no-self-upgrade 2>&1 | tail -n1 | grep "^certbot $INITIAL_VERSION$" ; then
     echo initial installation appeared to fail
     exit 1
 fi
@@ -85,7 +85,7 @@ if [ $(python -V 2>&1 | cut -d" " -f 2 | cut -d. -f1,2 | sed 's/\.//') -eq 26 ];
     fi
     # Create a 2nd venv at the new path to ensure we properly handle this case
     export VENV_PATH="/opt/eff.org/certbot/venv"
-    if ! sudo -E ./letsencrypt-auto -v --debug --version --no-self-upgrade 2>&1 | grep "$INITIAL_VERSION" ; then
+    if ! sudo -E ./letsencrypt-auto -v --debug --version --no-self-upgrade 2>&1 | tail -n1 | grep "^certbot $INITIAL_VERSION$" ; then
         echo second installation appeared to fail
         exit 1
     fi
@@ -98,7 +98,7 @@ if ./letsencrypt-auto -v --debug --version | grep "WARNING: couldn't find Python
 fi
 
 EXPECTED_VERSION=$(grep -m1 LE_AUTO_VERSION certbot-auto | cut -d\" -f2)
-if ! /opt/eff.org/certbot/venv/bin/letsencrypt --version 2>&1 | grep "$EXPECTED_VERSION" ; then
+if ! /opt/eff.org/certbot/venv/bin/letsencrypt --version 2>&1 | tail -n1 | grep "^certbot $EXPECTED_VERSION$" ; then
     echo upgrade appeared to fail
     exit 1
 fi

tests/letstest/scripts/test_sdists.sh

@@ -12,6 +12,8 @@ export VENV_ARGS="-p $PYTHON"
 # setup venv
 tools/_venv_common.py --requirement letsencrypt-auto-source/pieces/dependency-requirements.txt
 . ./venv/bin/activate
+# pytest is needed to run tests on some of our packages so we install a pinned version here.
+tools/pip_install.py pytest
 
 # build sdists
 for pkg_dir in acme . $PLUGINS; do

tests/letstest/scripts/test_tests.sh

@@ -1,18 +1,20 @@
 #!/bin/sh -xe
 
-LE_AUTO="letsencrypt/letsencrypt-auto-source/letsencrypt-auto"
+REPO_ROOT="letsencrypt"
+LE_AUTO="$REPO_ROOT/letsencrypt-auto-source/letsencrypt-auto"
 LE_AUTO="$LE_AUTO --debug --no-self-upgrade --non-interactive"
 MODULES="acme certbot certbot_apache certbot_nginx"
+PIP_INSTALL="$REPO_ROOT/tools/pip_install.py"
 VENV_NAME=venv
 
 # *-auto respects VENV_PATH
 $LE_AUTO --os-packages-only
 LE_AUTO_SUDO="" VENV_PATH="$VENV_NAME" $LE_AUTO --no-bootstrap --version
 . $VENV_NAME/bin/activate
+"$PIP_INSTALL" pytest
 
 # change to an empty directory to ensure CWD doesn't affect tests
 cd $(mktemp -d)
-pip install pytest==3.2.5
 
 for module in $MODULES ; do
     echo testing $module

tests/pebble-fetch.sh

@@ -2,7 +2,7 @@
 # Download and run Pebble instance for integration testing
 set -xe
 
-PEBBLE_VERSION=2018-11-02
+PEBBLE_VERSION=v1.0.1
 
 # We reuse the same GOPATH-style directory than for Boulder.
 # Pebble does not need it, but it will make the installation consistent with Boulder's one.
@@ -13,15 +13,32 @@ mkdir -p ${PEBBLEPATH}
 
 cat << UNLIKELY_EOF > "$PEBBLEPATH/docker-compose.yml"
 version: '3'
-
 services:
- pebble:
-  image: letsencrypt/pebble:${PEBBLE_VERSION}
-  command: pebble -strict ${PEBBLE_STRICT:-false} -dnsserver 10.77.77.1
-  ports:
-    - 14000:14000
-  environment:
+  pebble:
+    image: letsencrypt/pebble:${PEBBLE_VERSION}
+    command: pebble -dnsserver 10.30.50.3:8053
+    environment:
     - PEBBLE_VA_NOSLEEP=1
+    ports:
+      - 14000:14000
+    networks:
+      acmenet:
+        ipv4_address: 10.30.50.2
+  challtestsrv:
+    image: letsencrypt/pebble-challtestsrv:${PEBBLE_VERSION}
+    command: pebble-challtestsrv -defaultIPv6 "" -defaultIPv4 10.30.50.1
+    ports:
+      - 8055:8055
+    networks:
+      acmenet:
+        ipv4_address: 10.30.50.3
+networks:
+  acmenet:
+    driver: bridge
+    ipam:
+      driver: default
+      config:
+        - subnet: 10.30.50.0/24
 UNLIKELY_EOF
 
 docker-compose -f "$PEBBLEPATH/docker-compose.yml" up -d pebble

tools/install_and_test.py

@@ -15,7 +15,7 @@ import subprocess
 import re
 
 SKIP_PROJECTS_ON_WINDOWS = [
-    'certbot-apache', 'certbot-nginx', 'certbot-postfix', 'letshelp-certbot']
+    'certbot-apache', 'certbot-postfix', 'letshelp-certbot']
 
 
 def call_with_print(command, cwd=None):
@@ -49,7 +49,7 @@ def main(args):
         shutil.copy2("pytest.ini", temp_cwd)
         try:
             call_with_print(' '.join([
-                sys.executable, '-m', 'pytest', pkg.replace('-', '_')]), cwd=temp_cwd)
+                sys.executable, '-m', 'pytest', '--pyargs', pkg.replace('-', '_')]), cwd=temp_cwd)
         finally:
             shutil.rmtree(temp_cwd)
 

tox.cover.py

@@ -35,7 +35,8 @@ COVER_THRESHOLDS = {
 }
 
 SKIP_PROJECTS_ON_WINDOWS = [
-    'certbot-apache', 'certbot-nginx', 'certbot-postfix', 'letshelp-certbot']
+    'certbot-apache', 'certbot-postfix', 'letshelp-certbot']
+
 
 def cover(package):
     threshold = COVER_THRESHOLDS.get(package)['windows' if os.name == 'nt' else 'linux']
@@ -48,12 +49,13 @@ def cover(package):
             .format(pkg_dir)))
         return
 
-    subprocess.check_call([
-        sys.executable, '-m', 'pytest', '--cov', pkg_dir, '--cov-append', '--cov-report=', package])
+    subprocess.check_call([sys.executable, '-m', 'pytest', '--pyargs',
+                           '--cov', pkg_dir, '--cov-append', '--cov-report=', package])
     subprocess.check_call([
         sys.executable, '-m', 'coverage', 'report', '--fail-under', str(threshold), '--include',
         '{0}/*'.format(pkg_dir), '--show-missing'])
 
+
 def main():
     description = """
 This script is used by tox.ini (and thus by Travis CI and AppVeyor) in order
@@ -77,5 +79,6 @@ Option -e makes sure we fail fast and don't submit to codecov."""
     for package in packages:
         cover(package)
 
+
 if __name__ == '__main__':
     main()

tox.ini

@@ -70,7 +70,7 @@ commands =
     {[base]install_and_test} {[base]all_packages}
     python tests/lock_test.py
 setenv =
-    PYTEST_ADDOPTS = {env:PYTEST_ADDOPTS:--numprocesses auto --pyargs}
+    PYTEST_ADDOPTS = {env:PYTEST_ADDOPTS:--numprocesses auto}
     PYTHONHASHSEED = 0
 
 [testenv:py27-oldest]
@@ -166,7 +166,6 @@ passenv =
     HOME
     GOPATH
     PEBBLEPATH
-    PEBBLE_STRICT
 setenv =
     SERVER=https://localhost:14000/dir