From 339d034d6a5a57d296607795a4706203f81d7059 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Wed, 27 Feb 2019 18:21:47 +0100 Subject: [PATCH 1/5] Remove keyAuthorization field from the challenge response JWS token (#6758) Fixes #6755. POSTing the `keyAuthorization` in a JWS token when answering an ACME challenge, has been deprecated for some time now. Indeed, this is superfluous as the request is already authentified by the JWS signature. Boulder still accepts to see this field in the JWS token, and ignore it. Pebble in non strict mode also. But Pebble in strict mode refuses the request, to prepare complete removal of this field in ACME v2. Certbot still sends the `keyAuthorization` field. This PR removes it, and makes Certbot compliant with current ACME v2 protocol, and so Pebble in strict mode. See also [letsencrypt/pebble#192](https://github.com/letsencrypt/pebble/issues/192) for implementation details server side. * New implementation, with a fallback. * Add deprecation on changelog * Update acme/acme/client.py Co-Authored-By: adferrand * Fix an instance parameter * Update changelog, extend coverage * Update comment * Add unit tests on keyAuthorization dump * Update acme/acme/client.py Co-Authored-By: adferrand * Restrict the magic of setting a variable in immutable object in one place. Make a soon to be removed method private. --- CHANGELOG.md | 7 +++++++ acme/acme/challenges.py | 20 ++++++++++++++++++++ acme/acme/challenges_test.py | 12 ++++++++++++ acme/acme/client.py | 26 ++++++++++++++++++++------ acme/acme/client_test.py | 28 ++++++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce7eac2e9..53c158f8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,13 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * 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. +* The `acme` module avoids sending the `keyAuthorization` field in the JWS + payload when responding to a challenge as the field is not included in the + current ACME protocol. To ease the migration path for ACME CA servers, + Certbot and its `acme` module will first try the request without the + `keyAuthorization` field but will temporarily retry the request with the + field included if a `malformed` error is received. This fallback will be + removed in version 0.34.0. ### Fixed diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 501f74881..6f2b3757b 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -108,6 +108,10 @@ class KeyAuthorizationChallengeResponse(ChallengeResponse): key_authorization = jose.Field("keyAuthorization") thumbprint_hash_function = hashes.SHA256 + def __init__(self, *args, **kwargs): + super(KeyAuthorizationChallengeResponse, self).__init__(*args, **kwargs) + self._dump_authorization_key(False) + def verify(self, chall, account_public_key): """Verify the key authorization. @@ -140,6 +144,22 @@ class KeyAuthorizationChallengeResponse(ChallengeResponse): return True + def _dump_authorization_key(self, dump): + # type: (bool) -> None + """ + Set if keyAuthorization is dumped in the JSON representation of this ChallengeResponse. + NB: This method is declared as private because it will eventually be removed. + :param bool dump: True to dump the keyAuthorization, False otherwise + """ + object.__setattr__(self, '_dump_auth_key', dump) + + def to_partial_json(self): + jobj = super(KeyAuthorizationChallengeResponse, self).to_partial_json() + if not self._dump_auth_key: # pylint: disable=no-member + jobj.pop('keyAuthorization', None) + + return jobj + @six.add_metaclass(abc.ABCMeta) class KeyAuthorizationChallenge(_TokenChallenge): diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 81d39058e..4b905c1e5 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -94,6 +94,9 @@ class DNS01ResponseTest(unittest.TestCase): self.response = self.chall.response(KEY) def test_to_partial_json(self): + self.assertEqual({k: v for k, v in self.jmsg.items() if k != 'keyAuthorization'}, + self.msg.to_partial_json()) + self.msg._dump_authorization_key(True) # pylint: disable=protected-access self.assertEqual(self.jmsg, self.msg.to_partial_json()) def test_from_json(self): @@ -165,6 +168,9 @@ class HTTP01ResponseTest(unittest.TestCase): self.response = self.chall.response(KEY) def test_to_partial_json(self): + self.assertEqual({k: v for k, v in self.jmsg.items() if k != 'keyAuthorization'}, + self.msg.to_partial_json()) + self.msg._dump_authorization_key(True) # pylint: disable=protected-access self.assertEqual(self.jmsg, self.msg.to_partial_json()) def test_from_json(self): @@ -285,6 +291,9 @@ class TLSSNI01ResponseTest(unittest.TestCase): self.assertEqual(self.z_domain, self.response.z_domain) def test_to_partial_json(self): + self.assertEqual({k: v for k, v in self.jmsg.items() if k != 'keyAuthorization'}, + self.response.to_partial_json()) + self.response._dump_authorization_key(True) # pylint: disable=protected-access self.assertEqual(self.jmsg, self.response.to_partial_json()) def test_from_json(self): @@ -419,6 +428,9 @@ class TLSALPN01ResponseTest(unittest.TestCase): self.response = self.chall.response(KEY) def test_to_partial_json(self): + self.assertEqual({k: v for k, v in self.jmsg.items() if k != 'keyAuthorization'}, + self.msg.to_partial_json()) + self.msg._dump_authorization_key(True) # pylint: disable=protected-access self.assertEqual(self.jmsg, self.msg.to_partial_json()) def test_from_json(self): diff --git a/acme/acme/client.py b/acme/acme/client.py index 76b6a7788..d74713a7e 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -17,6 +17,7 @@ import requests from requests.adapters import HTTPAdapter import sys +from acme import challenges from acme import crypto_util from acme import errors from acme import jws @@ -155,7 +156,23 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes :raises .UnexpectedUpdate: """ - response = self._post(challb.uri, response) + # Because sending keyAuthorization in a response challenge has been removed from the ACME + # spec, it is not included in the KeyAuthorizationResponseChallenge JSON by default. + # However as a migration path, we temporarily expect a malformed error from the server, + # and fallback by resending the challenge response with the keyAuthorization field. + # TODO: Remove this fallback for Certbot 0.34.0 + try: + response = self._post(challb.uri, response) + except messages.Error as error: + if (error.code == 'malformed' + and isinstance(response, challenges.KeyAuthorizationChallengeResponse)): + logger.debug('Error while responding to a challenge without keyAuthorization ' + 'in the JWS, your ACME CA server may not support it:\n%s', error) + logger.debug('Retrying request with keyAuthorization set.') + response._dump_authorization_key(True) # pylint: disable=protected-access + response = self._post(challb.uri, response) + else: + raise try: authzr_uri = response.links['up']['url'] except KeyError: @@ -781,7 +798,7 @@ class ClientV2(ClientBase): except messages.Error as error: if error.code == 'malformed': logger.debug('Error during a POST-as-GET request, ' - 'your ACME CA may not support it:\n%s', error) + 'your ACME CA server may not support it:\n%s', error) logger.debug('Retrying request with GET.') else: # pragma: no cover raise @@ -1191,10 +1208,7 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes def _post_once(self, url, obj, content_type=JOSE_CONTENT_TYPE, acme_version=1, **kwargs): - try: - new_nonce_url = kwargs.pop('new_nonce_url') - except KeyError: - new_nonce_url = None + new_nonce_url = kwargs.pop('new_nonce_url', None) data = self._wrap_in_jws(obj, self._get_nonce(url, new_nonce_url), url, acme_version) kwargs.setdefault('headers', {'Content-Type': content_type}) response = self._send_request('POST', url, data=data, **kwargs) diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index b3d0f1921..dc8cc0c93 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -463,6 +463,34 @@ class ClientTest(ClientTestBase): errors.ClientError, self.client.answer_challenge, self.challr.body, challenges.DNSResponse(validation=None)) + def test_answer_challenge_key_authorization_fallback(self): + self.response.links['up'] = {'url': self.challr.authzr_uri} + self.response.json.return_value = self.challr.body.to_json() + + def _wrapper_post(url, obj, *args, **kwargs): # pylint: disable=unused-argument + """ + Simulate an old ACME CA server, that would respond a 'malformed' + error if keyAuthorization is missing. + """ + jobj = obj.to_partial_json() + if 'keyAuthorization' not in jobj: + raise messages.Error.with_code('malformed') + return self.response + self.net.post.side_effect = _wrapper_post + + # This challenge response is of type KeyAuthorizationChallengeResponse, so the fallback + # should be triggered, and avoid an exception. + http_chall_response = challenges.HTTP01Response(key_authorization='test', + resource=mock.MagicMock()) + self.client.answer_challenge(self.challr.body, http_chall_response) + + # This challenge response is not of type KeyAuthorizationChallengeResponse, so the fallback + # should not be triggered, leading to an exception. + dns_chall_response = challenges.DNSResponse(validation=None) + self.assertRaises( + errors.Error, self.client.answer_challenge, + self.challr.body, dns_chall_response) + def test_retry_after_date(self): self.response.headers['Retry-After'] = 'Fri, 31 Dec 1999 23:59:59 GMT' self.assertEqual( From 9c405a3cd12588ba27cdd8f79c8a101831bd6ad3 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 28 Feb 2019 00:16:52 +0100 Subject: [PATCH 2/5] Fix cryptography OCSP support (#6751) * Reenabling OCSP cryptography support * Refactor the validation logic of OCSP response to match the OpenSSL one * Prepare runtime for OCSP response test * Move unrelated test to another relevant place * Reimplement OCSP status checks in integration tests * Clean script * Protect OCSP check against connection errors * Update tests/certbot-boulder-integration.sh Co-Authored-By: adferrand * Cleaning * Add a specific script for letsencrypt-auto install+help * Remove inconsistent assertion * Add executable permissions * Remove unused variable * Move testdata * Corrected cleanup code * Empty commit --- CHANGELOG.md | 3 + certbot/crypto_util.py | 53 +++-- certbot/ocsp.py | 211 ++++++++++++++---- certbot/tests/ocsp_test.py | 172 ++++++++++++-- certbot/tests/testdata/google_certificate.pem | 41 ++++ .../testdata/google_issuer_certificate.pem | 26 +++ certbot/util.py | 2 +- letsencrypt-auto-source/letsencrypt-auto | 40 ++-- .../pieces/dependency-requirements.txt | 40 ++-- tests/certbot-boulder-integration.sh | 55 +++++ .../meta.json | 0 .../private_key.json | 0 .../regr.json | 0 .../a.encryption-example.com/cert1.pem | 0 .../a.encryption-example.com/chain1.pem | 0 .../a.encryption-example.com/fullchain1.pem | 0 .../a.encryption-example.com/privkey1.pem | 0 .../b.encryption-example.com/cert1.pem | 0 .../b.encryption-example.com/chain1.pem | 0 .../b.encryption-example.com/fullchain1.pem | 0 .../b.encryption-example.com/privkey1.pem | 0 .../sample-config/csr/0000_csr-certbot.pem | 0 .../sample-config/csr/0001_csr-certbot.pem | 0 .../sample-config/csr/0002_csr-certbot.pem | 0 .../sample-config/csr/0003_csr-certbot.pem | 0 .../sample-config/keys/0000_key-certbot.pem | 0 .../sample-config/keys/0001_key-certbot.pem | 0 .../sample-config/keys/0002_key-certbot.pem | 0 .../sample-config/keys/0003_key-certbot.pem | 0 .../live/a.encryption-example.com/README | 0 .../live/a.encryption-example.com/cert.pem | 0 .../live/a.encryption-example.com/chain.pem | 0 .../a.encryption-example.com/fullchain.pem | 0 .../live/a.encryption-example.com/privkey.pem | 0 .../live/b.encryption-example.com/README | 0 .../live/b.encryption-example.com/cert.pem | 0 .../live/b.encryption-example.com/chain.pem | 0 .../b.encryption-example.com/fullchain.pem | 0 .../live/b.encryption-example.com/privkey.pem | 0 .../sample-config/options-ssl-apache.conf | 0 .../renewal/a.encryption-example.com.conf | 0 .../renewal/b.encryption-example.com.conf | 0 ...st_letsencrypt_auto_certonly_standalone.sh | 21 -- 43 files changed, 519 insertions(+), 145 deletions(-) create mode 100644 certbot/tests/testdata/google_certificate.pem create mode 100644 certbot/tests/testdata/google_issuer_certificate.pem rename tests/{letstest/testdata => integration}/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/meta.json (100%) rename tests/{letstest/testdata => integration}/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/private_key.json (100%) rename tests/{letstest/testdata => integration}/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/regr.json (100%) rename tests/{letstest/testdata => integration}/sample-config/archive/a.encryption-example.com/cert1.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/archive/a.encryption-example.com/chain1.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/archive/a.encryption-example.com/fullchain1.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/archive/a.encryption-example.com/privkey1.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/archive/b.encryption-example.com/cert1.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/archive/b.encryption-example.com/chain1.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/archive/b.encryption-example.com/fullchain1.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/archive/b.encryption-example.com/privkey1.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/csr/0000_csr-certbot.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/csr/0001_csr-certbot.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/csr/0002_csr-certbot.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/csr/0003_csr-certbot.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/keys/0000_key-certbot.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/keys/0001_key-certbot.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/keys/0002_key-certbot.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/keys/0003_key-certbot.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/live/a.encryption-example.com/README (100%) rename tests/{letstest/testdata => integration}/sample-config/live/a.encryption-example.com/cert.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/live/a.encryption-example.com/chain.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/live/a.encryption-example.com/fullchain.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/live/a.encryption-example.com/privkey.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/live/b.encryption-example.com/README (100%) rename tests/{letstest/testdata => integration}/sample-config/live/b.encryption-example.com/cert.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/live/b.encryption-example.com/chain.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/live/b.encryption-example.com/fullchain.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/live/b.encryption-example.com/privkey.pem (100%) rename tests/{letstest/testdata => integration}/sample-config/options-ssl-apache.conf (100%) rename tests/{letstest/testdata => integration}/sample-config/renewal/a.encryption-example.com.conf (100%) rename tests/{letstest/testdata => integration}/sample-config/renewal/b.encryption-example.com.conf (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53c158f8f..3eea1923d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,9 @@ More details about these changes can be found on our GitHub repo. * Avoid reprocessing challenges that are already validated when a certificate is issued. +* If possible, Certbot uses built-in support for OCSP from recent cryptography + versions instead of the OpenSSL binary: as a consequence Certbot does not need + the OpenSSL binary to be installed anymore if cryptography>=2.5 is installed. * Support for initiating (but not solving end-to-end) TLS-ALPN-01 challenges with the `acme` module. diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index c4a389cd5..66c68eb38 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -19,7 +19,7 @@ from cryptography.hazmat.primitives.asymmetric.ec import EllipticCurvePublicKey from cryptography.hazmat.primitives.asymmetric.padding import PKCS1v15 from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey # https://github.com/python/typeshed/tree/master/third_party/2/cryptography -from cryptography import x509 # type: ignore +from cryptography import x509 # type: ignore from OpenSSL import crypto from OpenSSL import SSL # type: ignore @@ -226,7 +226,7 @@ def verify_renewable_cert(renewable_cert): def verify_renewable_cert_sig(renewable_cert): - """ Verifies the signature of a `.storage.RenewableCert` object. + """Verifies the signature of a `.storage.RenewableCert` object. :param `.storage.RenewableCert` renewable_cert: cert to verify @@ -239,22 +239,8 @@ def verify_renewable_cert_sig(renewable_cert): cert = x509.load_pem_x509_certificate(cert_file.read(), default_backend()) pk = chain.public_key() with warnings.catch_warnings(): - warnings.simplefilter("ignore") - if isinstance(pk, RSAPublicKey): - # https://github.com/python/typeshed/blob/master/third_party/2/cryptography/hazmat/primitives/asymmetric/rsa.pyi - verifier = pk.verifier( # type: ignore - cert.signature, PKCS1v15(), cert.signature_hash_algorithm - ) - verifier.update(cert.tbs_certificate_bytes) - verifier.verify() - elif isinstance(pk, EllipticCurvePublicKey): - verifier = pk.verifier( - cert.signature, ECDSA(cert.signature_hash_algorithm) - ) - verifier.update(cert.tbs_certificate_bytes) - verifier.verify() - else: - raise errors.Error("Unsupported public key type") + verify_signed_payload(pk, cert.signature, cert.tbs_certificate_bytes, + cert.signature_hash_algorithm) except (IOError, ValueError, InvalidSignature) as e: error_str = "verifying the signature of the cert located at {0} has failed. \ Details: {1}".format(renewable_cert.cert, e) @@ -262,6 +248,37 @@ def verify_renewable_cert_sig(renewable_cert): raise errors.Error(error_str) +def verify_signed_payload(public_key, signature, payload, signature_hash_algorithm): + """Check the signature of a payload. + + :param RSAPublicKey/EllipticCurvePublicKey public_key: the public_key to check signature + :param bytes signature: the signature bytes + :param bytes payload: the payload bytes + :param cryptography.hazmat.primitives.hashes.HashAlgorithm + signature_hash_algorithm: algorithm used to hash the payload + + :raises InvalidSignature: If signature verification fails. + :raises errors.Error: If public key type is not supported + """ + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + if isinstance(public_key, RSAPublicKey): + # https://github.com/python/typeshed/blob/master/third_party/2/cryptography/hazmat/primitives/asymmetric/rsa.pyi + verifier = public_key.verifier( # type: ignore + signature, PKCS1v15(), signature_hash_algorithm + ) + verifier.update(payload) + verifier.verify() + elif isinstance(public_key, EllipticCurvePublicKey): + verifier = public_key.verifier( + signature, ECDSA(signature_hash_algorithm) + ) + verifier.update(payload) + verifier.verify() + else: + raise errors.Error("Unsupported public key type") + + def verify_cert_matches_priv_key(cert_path, key_path): """ Verifies that the private key and cert match. diff --git a/certbot/ocsp.py b/certbot/ocsp.py index 049e14827..6f4c0b0fb 100644 --- a/certbot/ocsp.py +++ b/certbot/ocsp.py @@ -1,53 +1,79 @@ """Tools for checking certificate revocation.""" import logging import re - +from datetime import datetime, timedelta from subprocess import Popen, PIPE +try: + # Only cryptography>=2.5 has ocsp module + # and signature_hash_algorithm attribute in OCSPResponse class + from cryptography.x509 import ocsp # pylint: disable=import-error + getattr(ocsp.OCSPResponse, 'signature_hash_algorithm') +except (ImportError, AttributeError): # pragma: no cover + ocsp = None # type: ignore +from cryptography import x509 +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives import hashes # type: ignore +from cryptography.exceptions import UnsupportedAlgorithm, InvalidSignature +import requests + +from acme.magic_typing import Optional, Tuple # pylint: disable=unused-import, no-name-in-module +from certbot import crypto_util from certbot import errors from certbot import util logger = logging.getLogger(__name__) + class RevocationChecker(object): - "This class figures out OCSP checking on this system, and performs it." + """This class figures out OCSP checking on this system, and performs it.""" - def __init__(self): + def __init__(self, enforce_openssl_binary_usage=False): self.broken = False + self.use_openssl_binary = enforce_openssl_binary_usage or not ocsp - if not util.exe_exists("openssl"): - logger.info("openssl not installed, can't check revocation") - self.broken = True - return - - # New versions of openssl want -header var=val, old ones want -header var val - test_host_format = Popen(["openssl", "ocsp", "-header", "var", "val"], - stdout=PIPE, stderr=PIPE, universal_newlines=True) - _out, err = test_host_format.communicate() - if "Missing =" in err: - self.host_args = lambda host: ["Host=" + host] - else: - self.host_args = lambda host: ["Host", host] + if self.use_openssl_binary: + if not util.exe_exists("openssl"): + logger.info("openssl not installed, can't check revocation") + self.broken = True + return + # New versions of openssl want -header var=val, old ones want -header var val + test_host_format = Popen(["openssl", "ocsp", "-header", "var", "val"], + stdout=PIPE, stderr=PIPE, universal_newlines=True) + _out, err = test_host_format.communicate() + if "Missing =" in err: + self.host_args = lambda host: ["Host=" + host] + else: + self.host_args = lambda host: ["Host", host] def ocsp_revoked(self, cert_path, chain_path): + # type: (str, str) -> bool """Get revoked status for a particular cert version. .. todo:: Make this a non-blocking call :param str cert_path: Path to certificate :param str chain_path: Path to intermediate cert - :rtype bool or None: :returns: True if revoked; False if valid or the check failed + :rtype: bool """ if self.broken: return False - - url, host = self.determine_ocsp_server(cert_path) - if not host: + url, host = _determine_ocsp_server(cert_path) + if not host or not url: return False + + if self.use_openssl_binary: + return self._check_ocsp_openssl_bin(cert_path, chain_path, host, url) + else: + return _check_ocsp_cryptography(cert_path, chain_path, url) + + def _check_ocsp_openssl_bin(self, cert_path, chain_path, host, url): + # type: (str, str, str, str) -> bool # jdkasten thanks "Bulletproof SSL and TLS - Ivan Ristic" for documenting this! cmd = ["openssl", "ocsp", "-no_nonce", @@ -65,33 +91,131 @@ class RevocationChecker(object): except errors.SubprocessError: logger.info("OCSP check failed for %s (are we offline?)", cert_path) return False - return _translate_ocsp_query(cert_path, output, err) - def determine_ocsp_server(self, cert_path): - """Extract the OCSP server host from a certificate. +def _determine_ocsp_server(cert_path): + # type: (str) -> Tuple[Optional[str], Optional[str]] + """Extract the OCSP server host from a certificate. - :param str cert_path: Path to the cert we're checking OCSP for - :rtype tuple: - :returns: (OCSP server URL or None, OCSP server host or None) + :param str cert_path: Path to the cert we're checking OCSP for + :rtype tuple: + :returns: (OCSP server URL or None, OCSP server host or None) - """ - try: - url, _err = util.run_script( - ["openssl", "x509", "-in", cert_path, "-noout", "-ocsp_uri"], - log=logger.debug) - except errors.SubprocessError: - logger.info("Cannot extract OCSP URI from %s", cert_path) - return None, None + """ + with open(cert_path, 'rb') as file_handler: + cert = x509.load_pem_x509_certificate(file_handler.read(), default_backend()) + try: + extension = cert.extensions.get_extension_for_class(x509.AuthorityInformationAccess) + ocsp_oid = x509.AuthorityInformationAccessOID.OCSP + descriptions = [description for description in extension.value + if description.access_method == ocsp_oid] + + url = descriptions[0].access_location.value + except (x509.ExtensionNotFound, IndexError): + logger.info("Cannot extract OCSP URI from %s", cert_path) + return None, None + + url = url.rstrip() + host = url.partition("://")[2].rstrip("/") + + if host: + return url, host + else: + logger.info("Cannot process OCSP host from URL (%s) in cert at %s", url, cert_path) + return None, None + + +def _check_ocsp_cryptography(cert_path, chain_path, url): + # type: (str, str, str) -> bool + # Retrieve OCSP response + with open(chain_path, 'rb') as file_handler: + issuer = x509.load_pem_x509_certificate(file_handler.read(), default_backend()) + with open(cert_path, 'rb') as file_handler: + cert = x509.load_pem_x509_certificate(file_handler.read(), default_backend()) + builder = ocsp.OCSPRequestBuilder() + builder = builder.add_certificate(cert, issuer, hashes.SHA1()) + request = builder.build() + request_binary = request.public_bytes(serialization.Encoding.DER) + try: + response = requests.post(url, data=request_binary, + headers={'Content-Type': 'application/ocsp-request'}) + except requests.exceptions.RequestException: + logger.info("OCSP check failed for %s (are we offline?)", cert_path, exc_info=True) + return False + if response.status_code != 200: + logger.info("OCSP check failed for %s (HTTP status: %d)", cert_path, response.status_code) + return False + + response_ocsp = ocsp.load_der_ocsp_response(response.content) + + # Check OCSP response validity + if response_ocsp.response_status != ocsp.OCSPResponseStatus.SUCCESSFUL: + logger.error("Invalid OCSP response status for %s: %s", + cert_path, response_ocsp.response_status) + return False + + # Check OCSP signature + try: + _check_ocsp_response(response_ocsp, request, issuer) + except UnsupportedAlgorithm as e: + logger.error(str(e)) + except errors.Error as e: + logger.error(str(e)) + except InvalidSignature: + logger.error('Invalid signature on OCSP response for %s', cert_path) + except AssertionError as error: + logger.error('Invalid OCSP response for %s: %s.', cert_path, str(error)) + else: + # Check OCSP certificate status + logger.debug("OCSP certificate status for %s is: %s", + cert_path, response_ocsp.certificate_status) + return response_ocsp.certificate_status == ocsp.OCSPCertStatus.REVOKED + + return False + + +def _check_ocsp_response(response_ocsp, request_ocsp, issuer_cert): + """Verify that the OCSP is valid for serveral criterias""" + # Assert OCSP response corresponds to the certificate we are talking about + if response_ocsp.serial_number != request_ocsp.serial_number: + raise AssertionError('the certificate in response does not correspond ' + 'to the certificate in request') + + # Assert signature is valid + _check_ocsp_response_signature(response_ocsp, issuer_cert) + + # Assert issuer in response is the expected one + if (not isinstance(response_ocsp.hash_algorithm, type(request_ocsp.hash_algorithm)) + or response_ocsp.issuer_key_hash != request_ocsp.issuer_key_hash + or response_ocsp.issuer_name_hash != request_ocsp.issuer_name_hash): + raise AssertionError('the issuer does not correspond to issuer of the certificate.') + + # In following checks, two situations can occur: + # * nextUpdate is set, and requirement is thisUpdate < now < nextUpdate + # * nextUpdate is not set, and requirement is thisUpdate < now + # NB1: We add a validity period tolerance to handle clock time inconsistencies, + # value is 5 min like for OpenSSL. + # NB2: Another check is to verify that thisUpdate is not too old, it is optional + # for OpenSSL, so we do not do it here. + # See OpenSSL implementation as a reference: + # https://github.com/openssl/openssl/blob/ef45aa14c5af024fcb8bef1c9007f3d1c115bd85/crypto/ocsp/ocsp_cl.c#L338-L391 + now = datetime.now() + if not response_ocsp.this_update: + raise AssertionError('param thisUpdate is not set.') + if response_ocsp.this_update > now + timedelta(minutes=5): + raise AssertionError('param thisUpdate is in the future.') + if response_ocsp.next_update and response_ocsp.next_update < now - timedelta(minutes=5): + raise AssertionError('param nextUpdate is in the past.') + + +def _check_ocsp_response_signature(response_ocsp, issuer_cert): + """Verify an OCSP response signature against certificate issuer""" + # Following line may raise UnsupportedAlgorithm + chosen_hash = response_ocsp.signature_hash_algorithm + crypto_util.verify_signed_payload(issuer_cert.public_key(), response_ocsp.signature, + response_ocsp.tbs_response_bytes, chosen_hash) - url = url.rstrip() - host = url.partition("://")[2].rstrip("/") - if host: - return url, host - else: - logger.info("Cannot process OCSP host from URL (%s) in cert at %s", url, cert_path) - return None, None def _translate_ocsp_query(cert_path, ocsp_output, ocsp_errors): """Parse openssl's weird output to work out what it means.""" @@ -102,7 +226,7 @@ def _translate_ocsp_query(cert_path, ocsp_output, ocsp_errors): warning = good.group(1) if good else None - if (not "Response verify OK" in ocsp_errors) or (good and warning) or unknown: + if ("Response verify OK" not in ocsp_errors) or (good and warning) or unknown: logger.info("Revocation status for %s is unknown", cert_path) logger.debug("Uncertain output:\n%s\nstderr:\n%s", ocsp_output, ocsp_errors) return False @@ -115,6 +239,5 @@ def _translate_ocsp_query(cert_path, ocsp_output, ocsp_errors): return True else: logger.warning("Unable to properly parse OCSP output: %s\nstderr:%s", - ocsp_output, ocsp_errors) + ocsp_output, ocsp_errors) return False - diff --git a/certbot/tests/ocsp_test.py b/certbot/tests/ocsp_test.py index 55cd24adb..ad3467e5a 100644 --- a/certbot/tests/ocsp_test.py +++ b/certbot/tests/ocsp_test.py @@ -1,18 +1,33 @@ """Tests for ocsp.py""" # pylint: disable=protected-access - import unittest +from datetime import datetime, timedelta +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import hashes # type: ignore +from cryptography.exceptions import UnsupportedAlgorithm, InvalidSignature +from cryptography import x509 +try: + # Only cryptography>=2.5 has ocsp module + # and signature_hash_algorithm attribute in OCSPResponse class + from cryptography.x509 import ocsp as ocsp_lib # pylint: disable=import-error + getattr(ocsp_lib.OCSPResponse, 'signature_hash_algorithm') +except (ImportError, AttributeError): # pragma: no cover + ocsp_lib = None # type: ignore import mock from certbot import errors +from certbot.tests import util as test_util out = """Missing = in header key=value ocsp: Use -help for summary. """ -class OCSPTest(unittest.TestCase): +class OCSPTestOpenSSL(unittest.TestCase): + """ + OCSP revokation tests using OpenSSL binary. + """ def setUp(self): from certbot import ocsp @@ -22,7 +37,7 @@ class OCSPTest(unittest.TestCase): mock_communicate.communicate.return_value = (None, out) mock_popen.return_value = mock_communicate mock_exists.return_value = True - self.checker = ocsp.RevocationChecker() + self.checker = ocsp.RevocationChecker(enforce_openssl_binary_usage=True) def tearDown(self): pass @@ -37,23 +52,23 @@ class OCSPTest(unittest.TestCase): mock_exists.return_value = True from certbot import ocsp - checker = ocsp.RevocationChecker() + checker = ocsp.RevocationChecker(enforce_openssl_binary_usage=True) self.assertEqual(mock_popen.call_count, 1) self.assertEqual(checker.host_args("x"), ["Host=x"]) mock_communicate.communicate.return_value = (None, out.partition("\n")[2]) - checker = ocsp.RevocationChecker() + checker = ocsp.RevocationChecker(enforce_openssl_binary_usage=True) self.assertEqual(checker.host_args("x"), ["Host", "x"]) self.assertEqual(checker.broken, False) mock_exists.return_value = False mock_popen.call_count = 0 - checker = ocsp.RevocationChecker() + checker = ocsp.RevocationChecker(enforce_openssl_binary_usage=True) self.assertEqual(mock_popen.call_count, 0) self.assertEqual(mock_log.call_count, 1) self.assertEqual(checker.broken, True) - @mock.patch('certbot.ocsp.RevocationChecker.determine_ocsp_server') + @mock.patch('certbot.ocsp._determine_ocsp_server') @mock.patch('certbot.util.run_script') def test_ocsp_revoked(self, mock_run, mock_determine): self.checker.broken = True @@ -71,21 +86,12 @@ class OCSPTest(unittest.TestCase): self.assertEqual(self.checker.ocsp_revoked("x", "y"), False) self.assertEqual(mock_run.call_count, 2) + def test_determine_ocsp_server(self): + cert_path = test_util.vector_path('google_certificate.pem') - @mock.patch('certbot.ocsp.logger.info') - @mock.patch('certbot.util.run_script') - def test_determine_ocsp_server(self, mock_run, mock_info): - uri = "http://ocsp.stg-int-x1.letsencrypt.org/" - host = "ocsp.stg-int-x1.letsencrypt.org" - mock_run.return_value = uri, "" - self.assertEqual(self.checker.determine_ocsp_server("beep"), (uri, host)) - mock_run.return_value = "ftp:/" + host + "/", "" - self.assertEqual(self.checker.determine_ocsp_server("beep"), (None, None)) - self.assertEqual(mock_info.call_count, 1) - - c = "confusion" - mock_run.side_effect = errors.SubprocessError(c) - self.assertEqual(self.checker.determine_ocsp_server("beep"), (None, None)) + from certbot import ocsp + result = ocsp._determine_ocsp_server(cert_path) + self.assertEqual(('http://ocsp.digicert.com', 'ocsp.digicert.com'), result) @mock.patch('certbot.ocsp.logger') @mock.patch('certbot.util.run_script') @@ -112,6 +118,129 @@ class OCSPTest(unittest.TestCase): self.assertEqual(mock_log.info.call_count, 1) +@unittest.skipIf(not ocsp_lib, + reason='This class tests functionalities available only on cryptography>=2.5.0') +class OSCPTestCryptography(unittest.TestCase): + """ + OCSP revokation tests using Cryptography >= 2.4.0 + """ + + def setUp(self): + from certbot import ocsp + self.checker = ocsp.RevocationChecker() + self.cert_path = test_util.vector_path('google_certificate.pem') + self.chain_path = test_util.vector_path('google_issuer_certificate.pem') + + @mock.patch('certbot.ocsp._determine_ocsp_server') + @mock.patch('certbot.ocsp._check_ocsp_cryptography') + def test_ensure_cryptography_toggled(self, mock_revoke, mock_determine): + mock_determine.return_value = ('http://example.com', 'example.com') + self.checker.ocsp_revoked(self.cert_path, self.chain_path) + + mock_revoke.assert_called_once_with(self.cert_path, self.chain_path, 'http://example.com') + + @mock.patch('certbot.ocsp.requests.post') + @mock.patch('certbot.ocsp.ocsp.load_der_ocsp_response') + def test_revoke(self, mock_ocsp_response, mock_post): + with mock.patch('certbot.ocsp.crypto_util.verify_signed_payload'): + mock_ocsp_response.return_value = _construct_mock_ocsp_response( + ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL) + mock_post.return_value = mock.Mock(status_code=200) + revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path) + + self.assertTrue(revoked) + + @mock.patch('certbot.ocsp.crypto_util.verify_signed_payload') + @mock.patch('certbot.ocsp.requests.post') + @mock.patch('certbot.ocsp.ocsp.load_der_ocsp_response') + def test_revoke_resiliency(self, mock_ocsp_response, mock_post, mock_check): + # Server return an invalid HTTP response + mock_ocsp_response.return_value = _construct_mock_ocsp_response( + ocsp_lib.OCSPCertStatus.UNKNOWN, ocsp_lib.OCSPResponseStatus.SUCCESSFUL) + mock_post.return_value = mock.Mock(status_code=400) + revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path) + + self.assertFalse(revoked) + + # OCSP response in invalid + mock_ocsp_response.return_value = _construct_mock_ocsp_response( + ocsp_lib.OCSPCertStatus.UNKNOWN, ocsp_lib.OCSPResponseStatus.UNAUTHORIZED) + mock_post.return_value = mock.Mock(status_code=200) + revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path) + + self.assertFalse(revoked) + + # OCSP response is valid, but certificate status is unknown + mock_ocsp_response.return_value = _construct_mock_ocsp_response( + ocsp_lib.OCSPCertStatus.UNKNOWN, ocsp_lib.OCSPResponseStatus.SUCCESSFUL) + mock_post.return_value = mock.Mock(status_code=200) + revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path) + + self.assertFalse(revoked) + + # The OCSP response says that the certificate is revoked, but certificate + # does not contain the OCSP extension. + mock_ocsp_response.return_value = _construct_mock_ocsp_response( + ocsp_lib.OCSPCertStatus.UNKNOWN, ocsp_lib.OCSPResponseStatus.SUCCESSFUL) + mock_post.return_value = mock.Mock(status_code=200) + with mock.patch('cryptography.x509.Extensions.get_extension_for_class', + side_effect=x509.ExtensionNotFound( + 'Not found', x509.AuthorityInformationAccessOID.OCSP)): + revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path) + + self.assertFalse(revoked) + + # Valid response, OCSP extension is present, + # but OCSP response uses an unsupported signature. + mock_ocsp_response.return_value = _construct_mock_ocsp_response( + ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL) + mock_post.return_value = mock.Mock(status_code=200) + mock_check.side_effect = UnsupportedAlgorithm('foo') + revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path) + + self.assertFalse(revoked) + + # And now, the signature itself is invalid. + mock_ocsp_response.return_value = _construct_mock_ocsp_response( + ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL) + mock_post.return_value = mock.Mock(status_code=200) + mock_check.side_effect = InvalidSignature('foo') + revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path) + + self.assertFalse(revoked) + + # Finally, assertion error on OCSP response validity + mock_ocsp_response.return_value = _construct_mock_ocsp_response( + ocsp_lib.OCSPCertStatus.REVOKED, ocsp_lib.OCSPResponseStatus.SUCCESSFUL) + mock_post.return_value = mock.Mock(status_code=200) + mock_check.side_effect = AssertionError('foo') + revoked = self.checker.ocsp_revoked(self.cert_path, self.chain_path) + + self.assertFalse(revoked) + + +def _construct_mock_ocsp_response(certificate_status, response_status): + cert = x509.load_pem_x509_certificate( + test_util.load_vector('google_certificate.pem'), default_backend()) + issuer = x509.load_pem_x509_certificate( + test_util.load_vector('google_issuer_certificate.pem'), default_backend()) + builder = ocsp_lib.OCSPRequestBuilder() + builder = builder.add_certificate(cert, issuer, hashes.SHA1()) + request = builder.build() + + return mock.Mock( + response_status=response_status, + certificate_status=certificate_status, + serial_number=request.serial_number, + issuer_key_hash=request.issuer_key_hash, + issuer_name_hash=request.issuer_name_hash, + hash_algorithm=hashes.SHA1(), + next_update=datetime.now() + timedelta(days=1), + this_update=datetime.now() - timedelta(days=1), + signature_algorithm_oid=x509.oid.SignatureAlgorithmOID.RSA_WITH_SHA1, + ) + + # pylint: disable=line-too-long openssl_confused = ("", """ /etc/letsencrypt/live/example.org/cert.pem: good @@ -165,5 +294,6 @@ revoked """, """Response verify OK""") + if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/certbot/tests/testdata/google_certificate.pem b/certbot/tests/testdata/google_certificate.pem new file mode 100644 index 000000000..c26fea0b1 --- /dev/null +++ b/certbot/tests/testdata/google_certificate.pem @@ -0,0 +1,41 @@ +-----BEGIN CERTIFICATE----- +MIIHQjCCBiqgAwIBAgIQCgYwQn9bvO1pVzllk7ZFHzANBgkqhkiG9w0BAQsFADB1 +MQswCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMRkwFwYDVQQLExB3 +d3cuZGlnaWNlcnQuY29tMTQwMgYDVQQDEytEaWdpQ2VydCBTSEEyIEV4dGVuZGVk +IFZhbGlkYXRpb24gU2VydmVyIENBMB4XDTE4MDUwODAwMDAwMFoXDTIwMDYwMzEy +MDAwMFowgccxHTAbBgNVBA8MFFByaXZhdGUgT3JnYW5pemF0aW9uMRMwEQYLKwYB +BAGCNzwCAQMTAlVTMRkwFwYLKwYBBAGCNzwCAQITCERlbGF3YXJlMRAwDgYDVQQF +Ewc1MTU3NTUwMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQG +A1UEBxMNU2FuIEZyYW5jaXNjbzEVMBMGA1UEChMMR2l0SHViLCBJbmMuMRMwEQYD +VQQDEwpnaXRodWIuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA +xjyq8jyXDDrBTyitcnB90865tWBzpHSbindG/XqYQkzFMBlXmqkzC+FdTRBYyneZ +w5Pz+XWQvL+74JW6LsWNc2EF0xCEqLOJuC9zjPAqbr7uroNLghGxYf13YdqbG5oj +/4x+ogEG3dF/U5YIwVr658DKyESMV6eoYV9mDVfTuJastkqcwero+5ZAKfYVMLUE +sMwFtoTDJFmVf6JlkOWwsxp1WcQ/MRQK1cyqOoUFUgYylgdh3yeCDPeF22Ax8AlQ +xbcaI+GwfQL1FB7Jy+h+KjME9lE/UpgV6Qt2R1xNSmvFCBWu+NFX6epwFP/JRbkM +fLz0beYFUvmMgLtwVpEPSwIDAQABo4IDeTCCA3UwHwYDVR0jBBgwFoAUPdNQpdag +re7zSmAKZdMh1Pj41g8wHQYDVR0OBBYEFMnCU2FmnV+rJfQmzQ84mqhJ6kipMCUG +A1UdEQQeMByCCmdpdGh1Yi5jb22CDnd3dy5naXRodWIuY29tMA4GA1UdDwEB/wQE +AwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwdQYDVR0fBG4wbDA0 +oDKgMIYuaHR0cDovL2NybDMuZGlnaWNlcnQuY29tL3NoYTItZXYtc2VydmVyLWcy +LmNybDA0oDKgMIYuaHR0cDovL2NybDQuZGlnaWNlcnQuY29tL3NoYTItZXYtc2Vy +dmVyLWcyLmNybDBLBgNVHSAERDBCMDcGCWCGSAGG/WwCATAqMCgGCCsGAQUFBwIB +FhxodHRwczovL3d3dy5kaWdpY2VydC5jb20vQ1BTMAcGBWeBDAEBMIGIBggrBgEF +BQcBAQR8MHowJAYIKwYBBQUHMAGGGGh0dHA6Ly9vY3NwLmRpZ2ljZXJ0LmNvbTBS +BggrBgEFBQcwAoZGaHR0cDovL2NhY2VydHMuZGlnaWNlcnQuY29tL0RpZ2lDZXJ0 +U0hBMkV4dGVuZGVkVmFsaWRhdGlvblNlcnZlckNBLmNydDAMBgNVHRMBAf8EAjAA +MIIBfgYKKwYBBAHWeQIEAgSCAW4EggFqAWgAdgCkuQmQtBhYFIe7E6LMZ3AKPDWY +BPkb37jjd80OyA3cEAAAAWNBYm0KAAAEAwBHMEUCIQDRZp38cTWsWH2GdBpe/uPT +Wnsu/m4BEC2+dIcvSykZYgIgCP5gGv6yzaazxBK2NwGdmmyuEFNSg2pARbMJlUFg +U5UAdgBWFAaaL9fC7NP14b1Esj7HRna5vJkRXMDvlJhV1onQ3QAAAWNBYm0tAAAE +AwBHMEUCIQCi7omUvYLm0b2LobtEeRAYnlIo7n6JxbYdrtYdmPUWJQIgVgw1AZ51 +vK9ENinBg22FPxb82TvNDO05T17hxXRC2IYAdgC72d+8H4pxtZOUI5eqkntHOFeV +CqtS6BqQlmQ2jh7RhQAAAWNBYm3fAAAEAwBHMEUCIQChzdTKUU2N+XcqcK0OJYrN +8EYynloVxho4yPk6Dq3EPgIgdNH5u8rC3UcslQV4B9o0a0w204omDREGKTVuEpxG +eOQwDQYJKoZIhvcNAQELBQADggEBAHAPWpanWOW/ip2oJ5grAH8mqQfaunuCVE+v +ac+88lkDK/LVdFgl2B6kIHZiYClzKtfczG93hWvKbST4NRNHP9LiaQqdNC17e5vN +HnXVUGw+yxyjMLGqkgepOnZ2Rb14kcTOGp4i5AuJuuaMwXmCo7jUwPwfLe1NUlVB +Kqg6LK0Hcq4K0sZnxE8HFxiZ92WpV2AVWjRMEc/2z2shNoDvxvFUYyY1Oe67xINk +myQKc+ygSBZzyLnXSFVWmHr3u5dcaaQGGAR42v6Ydr4iL38Hd4dOiBma+FXsXBIq +WUjbST4VXmdaol7uzFMojA4zkxQDZAvF5XgJlAFadfySna/teik= +-----END CERTIFICATE----- diff --git a/certbot/tests/testdata/google_issuer_certificate.pem b/certbot/tests/testdata/google_issuer_certificate.pem new file mode 100644 index 000000000..50db47bc4 --- /dev/null +++ b/certbot/tests/testdata/google_issuer_certificate.pem @@ -0,0 +1,26 @@ +-----BEGIN CERTIFICATE----- +MIIEXDCCA0SgAwIBAgINAeOpMBz8cgY4P5pTHTANBgkqhkiG9w0BAQsFADBMMSAw +HgYDVQQLExdHbG9iYWxTaWduIFJvb3QgQ0EgLSBSMjETMBEGA1UEChMKR2xvYmFs +U2lnbjETMBEGA1UEAxMKR2xvYmFsU2lnbjAeFw0xNzA2MTUwMDAwNDJaFw0yMTEy +MTUwMDAwNDJaMFQxCzAJBgNVBAYTAlVTMR4wHAYDVQQKExVHb29nbGUgVHJ1c3Qg +U2VydmljZXMxJTAjBgNVBAMTHEdvb2dsZSBJbnRlcm5ldCBBdXRob3JpdHkgRzMw +ggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDKUkvqHv/OJGuo2nIYaNVW +XQ5IWi01CXZaz6TIHLGp/lOJ+600/4hbn7vn6AAB3DVzdQOts7G5pH0rJnnOFUAK +71G4nzKMfHCGUksW/mona+Y2emJQ2N+aicwJKetPKRSIgAuPOB6Aahh8Hb2XO3h9 +RUk2T0HNouB2VzxoMXlkyW7XUR5mw6JkLHnA52XDVoRTWkNty5oCINLvGmnRsJ1z +ouAqYGVQMc/7sy+/EYhALrVJEA8KbtyX+r8snwU5C1hUrwaW6MWOARa8qBpNQcWT +kaIeoYvy/sGIJEmjR0vFEwHdp1cSaWIr6/4g72n7OqXwfinu7ZYW97EfoOSQJeAz +AgMBAAGjggEzMIIBLzAOBgNVHQ8BAf8EBAMCAYYwHQYDVR0lBBYwFAYIKwYBBQUH +AwEGCCsGAQUFBwMCMBIGA1UdEwEB/wQIMAYBAf8CAQAwHQYDVR0OBBYEFHfCuFCa +Z3Z2sS3ChtCDoH6mfrpLMB8GA1UdIwQYMBaAFJviB1dnHB7AagbeWbSaLd/cGYYu +MDUGCCsGAQUFBwEBBCkwJzAlBggrBgEFBQcwAYYZaHR0cDovL29jc3AucGtpLmdv +b2cvZ3NyMjAyBgNVHR8EKzApMCegJaAjhiFodHRwOi8vY3JsLnBraS5nb29nL2dz +cjIvZ3NyMi5jcmwwPwYDVR0gBDgwNjA0BgZngQwBAgIwKjAoBggrBgEFBQcCARYc +aHR0cHM6Ly9wa2kuZ29vZy9yZXBvc2l0b3J5LzANBgkqhkiG9w0BAQsFAAOCAQEA +HLeJluRT7bvs26gyAZ8so81trUISd7O45skDUmAge1cnxhG1P2cNmSxbWsoiCt2e +ux9LSD+PAj2LIYRFHW31/6xoic1k4tbWXkDCjir37xTTNqRAMPUyFRWSdvt+nlPq +wnb8Oa2I/maSJukcxDjNSfpDh/Bd1lZNgdd/8cLdsE3+wypufJ9uXO1iQpnh9zbu +FIwsIONGl1p3A8CgxkqI/UAih3JaGOqcpcdaCIzkBaR9uYQ1X4k2Vg5APRLouzVy +7a8IVk6wuy6pm+T7HT4LY8ibS5FEZlfAFLSW8NwsVz9SBK2Vqn1N0PIMn5xA6NZV +c7o835DLAFshEWfC7TIe3g== +-----END CERTIFICATE----- diff --git a/certbot/util.py b/certbot/util.py index 416075ce8..097593e9d 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -62,7 +62,7 @@ def run_script(params, log=logger.error): """Run the script with the given params. :param list params: List of parameters to pass to Popen - :param logging.Logger log: Logger to use for errors + :param callable log: Logger method to use for errors """ try: diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 02656521f..19aaa4eeb 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -1034,26 +1034,26 @@ ConfigArgParse==0.12.0 \ configobj==5.0.6 \ --hash=sha256:a2f5650770e1c87fb335af19a9b7eb73fc05ccf22144eb68db7d00cd2bcb0902 \ --no-binary configobj -cryptography==2.2.2 \ - --hash=sha256:3f3b65d5a16e6b52fba63dc860b62ca9832f51f1a2ae5083c78b6840275f12dd \ - --hash=sha256:5251e7de0de66810833606439ca65c9b9e45da62196b0c88bfadf27740aac09f \ - --hash=sha256:551a3abfe0c8c6833df4192a63371aa2ff43afd8f570ed345d31f251d78e7e04 \ - --hash=sha256:5cb990056b7cadcca26813311187ad751ea644712022a3976443691168781b6f \ - --hash=sha256:60bda7f12ecb828358be53095fc9c6edda7de8f1ef571f96c00b2363643fa3cd \ - --hash=sha256:64b5c67acc9a7c83fbb4b69166f3105a0ab722d27934fac2cb26456718eec2ba \ - --hash=sha256:6fef51ec447fe9f8351894024e94736862900d3a9aa2961528e602eb65c92bdb \ - --hash=sha256:77d0ad229d47a6e0272d00f6bf8ac06ce14715a9fd02c9a97f5a2869aab3ccb2 \ - --hash=sha256:808fe471b1a6b777f026f7dc7bd9a4959da4bfab64972f2bbe91e22527c1c037 \ - --hash=sha256:9b62fb4d18529c84b961efd9187fecbb48e89aa1a0f9f4161c61b7fc42a101bd \ - --hash=sha256:9e5bed45ec6b4f828866ac6a6bedf08388ffcfa68abe9e94b34bb40977aba531 \ - --hash=sha256:9fc295bf69130a342e7a19a39d7bbeb15c0bcaabc7382ec33ef3b2b7d18d2f63 \ - --hash=sha256:abd070b5849ed64e6d349199bef955ee0ad99aefbad792f0c587f8effa681a5e \ - --hash=sha256:ba6a774749b6e510cffc2fb98535f717e0e5fd91c7c99a61d223293df79ab351 \ - --hash=sha256:c332118647f084c983c6a3e1dba0f3bcb051f69d12baccac68db8d62d177eb8a \ - --hash=sha256:d6f46e862ee36df81e6342c2177ba84e70f722d9dc9c6c394f9f1f434c4a5563 \ - --hash=sha256:db6013746f73bf8edd9c3d1d3f94db635b9422f503db3fc5ef105233d4c011ab \ - --hash=sha256:f57008eaff597c69cf692c3518f6d4800f0309253bb138b526a37fe9ef0c7471 \ - --hash=sha256:f6c821ac253c19f2ad4c8691633ae1d1a17f120d5b01ea1d256d7b602bc59887 +cryptography==2.5 \ + --hash=sha256:9e29af877c29338f0cab5f049ccc8bd3ead289a557f144376c4fbc7d1b98914f \ + --hash=sha256:b13c80b877e73bcb6f012813c6f4a9334fcf4b0e96681c5a15dac578f2eedfa0 \ + --hash=sha256:8504661ffe324837f5c4607347eeee4cf0fcad689163c6e9c8d3b18cf1f4a4ad \ + --hash=sha256:e091bd424567efa4b9d94287a952597c05d22155a13716bf5f9f746b9dc906d3 \ + --hash=sha256:42fad67d7072216a49e34f923d8cbda9edacbf6633b19a79655e88a1b4857063 \ + --hash=sha256:9a30384cc402eac099210ab9b8801b2ae21e591831253883decdb4513b77a3cd \ + --hash=sha256:08b753df3672b7066e74376f42ce8fc4683e4fd1358d34c80f502e939ee944d2 \ + --hash=sha256:6f841c7272645dd7c65b07b7108adfa8af0aaea57f27b7f59e01d41f75444c85 \ + --hash=sha256:bfe66b577a7118e05b04141f0f1ed0959552d45672aa7ecb3d91e319d846001e \ + --hash=sha256:522fdb2809603ee97a4d0ef2f8d617bc791eb483313ba307cb9c0a773e5e5695 \ + --hash=sha256:05b3ded5e88747d28ee3ef493f2b92cbb947c1e45cf98cfef22e6d38bb67d4af \ + --hash=sha256:fa2b38c8519c5a3aa6e2b4e1cf1a549b54acda6adb25397ff542068e73d1ed00 \ + --hash=sha256:ab50da871bc109b2d9389259aac269dd1b7c7413ee02d06fe4e486ed26882159 \ + --hash=sha256:9260b201ce584d7825d900c88700aa0bd6b40d4ebac7b213857bd2babee9dbca \ + --hash=sha256:06826e7f72d1770e186e9c90e76b4f84d90cdb917b47ff88d8dc59a7b10e2b1e \ + --hash=sha256:2cd29bd1911782baaee890544c653bb03ec7d95ebeb144d714b0f5c33deb55c7 \ + --hash=sha256:7d335e35306af5b9bc0560ca39f740dfc8def72749645e193dd35be11fb323b3 \ + --hash=sha256:31e5637e9036d966824edaa91bf0aa39dc6f525a1c599f39fd5c50340264e079 \ + --hash=sha256:4946b67235b9d2ea7d31307be9d5ad5959d6c4a8f98f900157b47abddf698401 enum34==1.1.2 ; python_version < '3.4' \ --hash=sha256:2475d7fcddf5951e92ff546972758802de5260bf409319a9f1934e6bbc8b1dc7 \ --hash=sha256:35907defb0f992b75ab7788f65fedc1cf20ffa22688e0e6f6f12afc06b3ea501 diff --git a/letsencrypt-auto-source/pieces/dependency-requirements.txt b/letsencrypt-auto-source/pieces/dependency-requirements.txt index 1fac78836..dff57dfd5 100644 --- a/letsencrypt-auto-source/pieces/dependency-requirements.txt +++ b/letsencrypt-auto-source/pieces/dependency-requirements.txt @@ -60,26 +60,26 @@ ConfigArgParse==0.12.0 \ configobj==5.0.6 \ --hash=sha256:a2f5650770e1c87fb335af19a9b7eb73fc05ccf22144eb68db7d00cd2bcb0902 \ --no-binary configobj -cryptography==2.2.2 \ - --hash=sha256:3f3b65d5a16e6b52fba63dc860b62ca9832f51f1a2ae5083c78b6840275f12dd \ - --hash=sha256:5251e7de0de66810833606439ca65c9b9e45da62196b0c88bfadf27740aac09f \ - --hash=sha256:551a3abfe0c8c6833df4192a63371aa2ff43afd8f570ed345d31f251d78e7e04 \ - --hash=sha256:5cb990056b7cadcca26813311187ad751ea644712022a3976443691168781b6f \ - --hash=sha256:60bda7f12ecb828358be53095fc9c6edda7de8f1ef571f96c00b2363643fa3cd \ - --hash=sha256:64b5c67acc9a7c83fbb4b69166f3105a0ab722d27934fac2cb26456718eec2ba \ - --hash=sha256:6fef51ec447fe9f8351894024e94736862900d3a9aa2961528e602eb65c92bdb \ - --hash=sha256:77d0ad229d47a6e0272d00f6bf8ac06ce14715a9fd02c9a97f5a2869aab3ccb2 \ - --hash=sha256:808fe471b1a6b777f026f7dc7bd9a4959da4bfab64972f2bbe91e22527c1c037 \ - --hash=sha256:9b62fb4d18529c84b961efd9187fecbb48e89aa1a0f9f4161c61b7fc42a101bd \ - --hash=sha256:9e5bed45ec6b4f828866ac6a6bedf08388ffcfa68abe9e94b34bb40977aba531 \ - --hash=sha256:9fc295bf69130a342e7a19a39d7bbeb15c0bcaabc7382ec33ef3b2b7d18d2f63 \ - --hash=sha256:abd070b5849ed64e6d349199bef955ee0ad99aefbad792f0c587f8effa681a5e \ - --hash=sha256:ba6a774749b6e510cffc2fb98535f717e0e5fd91c7c99a61d223293df79ab351 \ - --hash=sha256:c332118647f084c983c6a3e1dba0f3bcb051f69d12baccac68db8d62d177eb8a \ - --hash=sha256:d6f46e862ee36df81e6342c2177ba84e70f722d9dc9c6c394f9f1f434c4a5563 \ - --hash=sha256:db6013746f73bf8edd9c3d1d3f94db635b9422f503db3fc5ef105233d4c011ab \ - --hash=sha256:f57008eaff597c69cf692c3518f6d4800f0309253bb138b526a37fe9ef0c7471 \ - --hash=sha256:f6c821ac253c19f2ad4c8691633ae1d1a17f120d5b01ea1d256d7b602bc59887 +cryptography==2.5 \ + --hash=sha256:9e29af877c29338f0cab5f049ccc8bd3ead289a557f144376c4fbc7d1b98914f \ + --hash=sha256:b13c80b877e73bcb6f012813c6f4a9334fcf4b0e96681c5a15dac578f2eedfa0 \ + --hash=sha256:8504661ffe324837f5c4607347eeee4cf0fcad689163c6e9c8d3b18cf1f4a4ad \ + --hash=sha256:e091bd424567efa4b9d94287a952597c05d22155a13716bf5f9f746b9dc906d3 \ + --hash=sha256:42fad67d7072216a49e34f923d8cbda9edacbf6633b19a79655e88a1b4857063 \ + --hash=sha256:9a30384cc402eac099210ab9b8801b2ae21e591831253883decdb4513b77a3cd \ + --hash=sha256:08b753df3672b7066e74376f42ce8fc4683e4fd1358d34c80f502e939ee944d2 \ + --hash=sha256:6f841c7272645dd7c65b07b7108adfa8af0aaea57f27b7f59e01d41f75444c85 \ + --hash=sha256:bfe66b577a7118e05b04141f0f1ed0959552d45672aa7ecb3d91e319d846001e \ + --hash=sha256:522fdb2809603ee97a4d0ef2f8d617bc791eb483313ba307cb9c0a773e5e5695 \ + --hash=sha256:05b3ded5e88747d28ee3ef493f2b92cbb947c1e45cf98cfef22e6d38bb67d4af \ + --hash=sha256:fa2b38c8519c5a3aa6e2b4e1cf1a549b54acda6adb25397ff542068e73d1ed00 \ + --hash=sha256:ab50da871bc109b2d9389259aac269dd1b7c7413ee02d06fe4e486ed26882159 \ + --hash=sha256:9260b201ce584d7825d900c88700aa0bd6b40d4ebac7b213857bd2babee9dbca \ + --hash=sha256:06826e7f72d1770e186e9c90e76b4f84d90cdb917b47ff88d8dc59a7b10e2b1e \ + --hash=sha256:2cd29bd1911782baaee890544c653bb03ec7d95ebeb144d714b0f5c33deb55c7 \ + --hash=sha256:7d335e35306af5b9bc0560ca39f740dfc8def72749645e193dd35be11fb323b3 \ + --hash=sha256:31e5637e9036d966824edaa91bf0aa39dc6f525a1c599f39fd5c50340264e079 \ + --hash=sha256:4946b67235b9d2ea7d31307be9d5ad5959d6c4a8f98f900157b47abddf698401 enum34==1.1.2 ; python_version < '3.4' \ --hash=sha256:2475d7fcddf5951e92ff546972758802de5260bf409319a9f1934e6bbc8b1dc7 \ --hash=sha256:35907defb0f992b75ab7788f65fedc1cf20ffa22688e0e6f6f12afc06b3ea501 diff --git a/tests/certbot-boulder-integration.sh b/tests/certbot-boulder-integration.sh index 630571148..9d1ae9ec2 100755 --- a/tests/certbot-boulder-integration.sh +++ b/tests/certbot-boulder-integration.sh @@ -15,9 +15,11 @@ command -v python > /dev/null || (echo "Error, python executable is not in the P . ./tests/integration/_common.sh export PATH="$PATH:/usr/sbin" # /usr/sbin/nginx +CURRENT_DIR="$(pwd)" cleanup_and_exit() { EXIT_STATUS=$? + cd $CURRENT_DIR if SERVER_STILL_RUNNING=`ps -p $python_server_pid -o pid=` then echo Kill server subprocess, left running by abnormal exit @@ -527,3 +529,56 @@ if [ "${BOULDER_INTEGRATION:-v1}" = "v2" ]; then fi coverage report --fail-under 64 --include 'certbot/*' --show-missing + +# Test OCSP status + +## OCSP 1: Check stale OCSP status +pushd ./tests/integration + +OUT=`common certificates --config-dir sample-config` +TEST_CERTS=`echo "$OUT" | grep TEST_CERT | wc -l` +EXPIRED=`echo "$OUT" | grep EXPIRED | wc -l` + +if [ "$TEST_CERTS" != 2 ] ; then + echo "Did not find two test certs as expected ($TEST_CERTS)" + exit 1 +fi + +if [ "$EXPIRED" != 2 ] ; then + echo "Did not find two test certs as expected ($EXPIRED)" + exit 1 +fi + +popd + +## OSCP 2: Check live certificate OCSP status (VALID) +common --domains le-ocsp-check.wtf +OUT=`common certificates` +VALID=`echo $OUT | grep 'Domains: le-ocsp-check.wtf' -A 1 | grep VALID | wc -l` +EXPIRED=`echo $OUT | grep 'Domains: le-ocsp-check.wtf' -A 1 | grep EXPIRED | wc -l` + +if [ "$VALID" != 1 ] ; then + echo "Expected le-ocsp-check.wtf to be VALID" + exit 1 +fi + +if [ "$EXPIRED" != 0 ] ; then + echo "Did not expect le-ocsp-check.wtf to be EXPIRED" + exit 1 +fi + +## OSCP 3: Check live certificate OCSP status (REVOKED) +common revoke --cert-name le-ocsp-check.wtf --no-delete-after-revoke +OUT=`common certificates` +INVALID=`echo $OUT | grep 'Domains: le-ocsp-check.wtf' -A 1 | grep INVALID | wc -l` +REVOKED=`echo $OUT | grep 'Domains: le-ocsp-check.wtf' -A 1 | grep REVOKED | wc -l` + +if [ "$INVALID" != 1 ] ; then + echo "Expected le-ocsp-check.wtf to be INVALID" + exit 1 +fi + +if [ "$REVOKED" != 1 ] ; then + echo "Expected le-ocsp-check.wtf to be REVOKED" + exit 1 +fi diff --git a/tests/letstest/testdata/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/meta.json b/tests/integration/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/meta.json similarity index 100% rename from tests/letstest/testdata/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/meta.json rename to tests/integration/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/meta.json diff --git a/tests/letstest/testdata/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/private_key.json b/tests/integration/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/private_key.json similarity index 100% rename from tests/letstest/testdata/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/private_key.json rename to tests/integration/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/private_key.json diff --git a/tests/letstest/testdata/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/regr.json b/tests/integration/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/regr.json similarity index 100% rename from tests/letstest/testdata/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/regr.json rename to tests/integration/sample-config/accounts/acme-staging.api.letsencrypt.org/directory/48d6b9e8d767eccf7e4d877d6ffa81e3/regr.json diff --git a/tests/letstest/testdata/sample-config/archive/a.encryption-example.com/cert1.pem b/tests/integration/sample-config/archive/a.encryption-example.com/cert1.pem similarity index 100% rename from tests/letstest/testdata/sample-config/archive/a.encryption-example.com/cert1.pem rename to tests/integration/sample-config/archive/a.encryption-example.com/cert1.pem diff --git a/tests/letstest/testdata/sample-config/archive/a.encryption-example.com/chain1.pem b/tests/integration/sample-config/archive/a.encryption-example.com/chain1.pem similarity index 100% rename from tests/letstest/testdata/sample-config/archive/a.encryption-example.com/chain1.pem rename to tests/integration/sample-config/archive/a.encryption-example.com/chain1.pem diff --git a/tests/letstest/testdata/sample-config/archive/a.encryption-example.com/fullchain1.pem b/tests/integration/sample-config/archive/a.encryption-example.com/fullchain1.pem similarity index 100% rename from tests/letstest/testdata/sample-config/archive/a.encryption-example.com/fullchain1.pem rename to tests/integration/sample-config/archive/a.encryption-example.com/fullchain1.pem diff --git a/tests/letstest/testdata/sample-config/archive/a.encryption-example.com/privkey1.pem b/tests/integration/sample-config/archive/a.encryption-example.com/privkey1.pem similarity index 100% rename from tests/letstest/testdata/sample-config/archive/a.encryption-example.com/privkey1.pem rename to tests/integration/sample-config/archive/a.encryption-example.com/privkey1.pem diff --git a/tests/letstest/testdata/sample-config/archive/b.encryption-example.com/cert1.pem b/tests/integration/sample-config/archive/b.encryption-example.com/cert1.pem similarity index 100% rename from tests/letstest/testdata/sample-config/archive/b.encryption-example.com/cert1.pem rename to tests/integration/sample-config/archive/b.encryption-example.com/cert1.pem diff --git a/tests/letstest/testdata/sample-config/archive/b.encryption-example.com/chain1.pem b/tests/integration/sample-config/archive/b.encryption-example.com/chain1.pem similarity index 100% rename from tests/letstest/testdata/sample-config/archive/b.encryption-example.com/chain1.pem rename to tests/integration/sample-config/archive/b.encryption-example.com/chain1.pem diff --git a/tests/letstest/testdata/sample-config/archive/b.encryption-example.com/fullchain1.pem b/tests/integration/sample-config/archive/b.encryption-example.com/fullchain1.pem similarity index 100% rename from tests/letstest/testdata/sample-config/archive/b.encryption-example.com/fullchain1.pem rename to tests/integration/sample-config/archive/b.encryption-example.com/fullchain1.pem diff --git a/tests/letstest/testdata/sample-config/archive/b.encryption-example.com/privkey1.pem b/tests/integration/sample-config/archive/b.encryption-example.com/privkey1.pem similarity index 100% rename from tests/letstest/testdata/sample-config/archive/b.encryption-example.com/privkey1.pem rename to tests/integration/sample-config/archive/b.encryption-example.com/privkey1.pem diff --git a/tests/letstest/testdata/sample-config/csr/0000_csr-certbot.pem b/tests/integration/sample-config/csr/0000_csr-certbot.pem similarity index 100% rename from tests/letstest/testdata/sample-config/csr/0000_csr-certbot.pem rename to tests/integration/sample-config/csr/0000_csr-certbot.pem diff --git a/tests/letstest/testdata/sample-config/csr/0001_csr-certbot.pem b/tests/integration/sample-config/csr/0001_csr-certbot.pem similarity index 100% rename from tests/letstest/testdata/sample-config/csr/0001_csr-certbot.pem rename to tests/integration/sample-config/csr/0001_csr-certbot.pem diff --git a/tests/letstest/testdata/sample-config/csr/0002_csr-certbot.pem b/tests/integration/sample-config/csr/0002_csr-certbot.pem similarity index 100% rename from tests/letstest/testdata/sample-config/csr/0002_csr-certbot.pem rename to tests/integration/sample-config/csr/0002_csr-certbot.pem diff --git a/tests/letstest/testdata/sample-config/csr/0003_csr-certbot.pem b/tests/integration/sample-config/csr/0003_csr-certbot.pem similarity index 100% rename from tests/letstest/testdata/sample-config/csr/0003_csr-certbot.pem rename to tests/integration/sample-config/csr/0003_csr-certbot.pem diff --git a/tests/letstest/testdata/sample-config/keys/0000_key-certbot.pem b/tests/integration/sample-config/keys/0000_key-certbot.pem similarity index 100% rename from tests/letstest/testdata/sample-config/keys/0000_key-certbot.pem rename to tests/integration/sample-config/keys/0000_key-certbot.pem diff --git a/tests/letstest/testdata/sample-config/keys/0001_key-certbot.pem b/tests/integration/sample-config/keys/0001_key-certbot.pem similarity index 100% rename from tests/letstest/testdata/sample-config/keys/0001_key-certbot.pem rename to tests/integration/sample-config/keys/0001_key-certbot.pem diff --git a/tests/letstest/testdata/sample-config/keys/0002_key-certbot.pem b/tests/integration/sample-config/keys/0002_key-certbot.pem similarity index 100% rename from tests/letstest/testdata/sample-config/keys/0002_key-certbot.pem rename to tests/integration/sample-config/keys/0002_key-certbot.pem diff --git a/tests/letstest/testdata/sample-config/keys/0003_key-certbot.pem b/tests/integration/sample-config/keys/0003_key-certbot.pem similarity index 100% rename from tests/letstest/testdata/sample-config/keys/0003_key-certbot.pem rename to tests/integration/sample-config/keys/0003_key-certbot.pem diff --git a/tests/letstest/testdata/sample-config/live/a.encryption-example.com/README b/tests/integration/sample-config/live/a.encryption-example.com/README similarity index 100% rename from tests/letstest/testdata/sample-config/live/a.encryption-example.com/README rename to tests/integration/sample-config/live/a.encryption-example.com/README diff --git a/tests/letstest/testdata/sample-config/live/a.encryption-example.com/cert.pem b/tests/integration/sample-config/live/a.encryption-example.com/cert.pem similarity index 100% rename from tests/letstest/testdata/sample-config/live/a.encryption-example.com/cert.pem rename to tests/integration/sample-config/live/a.encryption-example.com/cert.pem diff --git a/tests/letstest/testdata/sample-config/live/a.encryption-example.com/chain.pem b/tests/integration/sample-config/live/a.encryption-example.com/chain.pem similarity index 100% rename from tests/letstest/testdata/sample-config/live/a.encryption-example.com/chain.pem rename to tests/integration/sample-config/live/a.encryption-example.com/chain.pem diff --git a/tests/letstest/testdata/sample-config/live/a.encryption-example.com/fullchain.pem b/tests/integration/sample-config/live/a.encryption-example.com/fullchain.pem similarity index 100% rename from tests/letstest/testdata/sample-config/live/a.encryption-example.com/fullchain.pem rename to tests/integration/sample-config/live/a.encryption-example.com/fullchain.pem diff --git a/tests/letstest/testdata/sample-config/live/a.encryption-example.com/privkey.pem b/tests/integration/sample-config/live/a.encryption-example.com/privkey.pem similarity index 100% rename from tests/letstest/testdata/sample-config/live/a.encryption-example.com/privkey.pem rename to tests/integration/sample-config/live/a.encryption-example.com/privkey.pem diff --git a/tests/letstest/testdata/sample-config/live/b.encryption-example.com/README b/tests/integration/sample-config/live/b.encryption-example.com/README similarity index 100% rename from tests/letstest/testdata/sample-config/live/b.encryption-example.com/README rename to tests/integration/sample-config/live/b.encryption-example.com/README diff --git a/tests/letstest/testdata/sample-config/live/b.encryption-example.com/cert.pem b/tests/integration/sample-config/live/b.encryption-example.com/cert.pem similarity index 100% rename from tests/letstest/testdata/sample-config/live/b.encryption-example.com/cert.pem rename to tests/integration/sample-config/live/b.encryption-example.com/cert.pem diff --git a/tests/letstest/testdata/sample-config/live/b.encryption-example.com/chain.pem b/tests/integration/sample-config/live/b.encryption-example.com/chain.pem similarity index 100% rename from tests/letstest/testdata/sample-config/live/b.encryption-example.com/chain.pem rename to tests/integration/sample-config/live/b.encryption-example.com/chain.pem diff --git a/tests/letstest/testdata/sample-config/live/b.encryption-example.com/fullchain.pem b/tests/integration/sample-config/live/b.encryption-example.com/fullchain.pem similarity index 100% rename from tests/letstest/testdata/sample-config/live/b.encryption-example.com/fullchain.pem rename to tests/integration/sample-config/live/b.encryption-example.com/fullchain.pem diff --git a/tests/letstest/testdata/sample-config/live/b.encryption-example.com/privkey.pem b/tests/integration/sample-config/live/b.encryption-example.com/privkey.pem similarity index 100% rename from tests/letstest/testdata/sample-config/live/b.encryption-example.com/privkey.pem rename to tests/integration/sample-config/live/b.encryption-example.com/privkey.pem diff --git a/tests/letstest/testdata/sample-config/options-ssl-apache.conf b/tests/integration/sample-config/options-ssl-apache.conf similarity index 100% rename from tests/letstest/testdata/sample-config/options-ssl-apache.conf rename to tests/integration/sample-config/options-ssl-apache.conf diff --git a/tests/letstest/testdata/sample-config/renewal/a.encryption-example.com.conf b/tests/integration/sample-config/renewal/a.encryption-example.com.conf similarity index 100% rename from tests/letstest/testdata/sample-config/renewal/a.encryption-example.com.conf rename to tests/integration/sample-config/renewal/a.encryption-example.com.conf diff --git a/tests/letstest/testdata/sample-config/renewal/b.encryption-example.com.conf b/tests/integration/sample-config/renewal/b.encryption-example.com.conf similarity index 100% rename from tests/letstest/testdata/sample-config/renewal/b.encryption-example.com.conf rename to tests/integration/sample-config/renewal/b.encryption-example.com.conf diff --git a/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh b/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh index 2cbe66a83..081ff3829 100755 --- a/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh +++ b/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh @@ -17,27 +17,6 @@ letsencrypt-auto certonly --no-self-upgrade -v --standalone --debug \ --register-unsafely-without-email \ --domain $PUBLIC_HOSTNAME --server $BOULDER_URL -# we have to jump through some hoops to cope with relative paths in renewal -# conf files ... -# 1. be in the right directory -cd tests/letstest/testdata/ - -# 2. refer to the config with the same level of relativity that it itself -# contains :/ -OUT=`letsencrypt-auto certificates --config-dir sample-config -v --no-self-upgrade` -TEST_CERTS=`echo "$OUT" | grep TEST_CERT | wc -l` -REVOKED=`echo "$OUT" | grep REVOKED | wc -l` - -if [ "$TEST_CERTS" != 2 ] ; then - echo "Did not find two test certs as expected ($TEST_CERTS)" - exit 1 -fi - -if [ "$REVOKED" != 1 ] ; then - echo "Did not find one revoked cert as expected ($REVOKED)" - exit 1 -fi - if ! letsencrypt-auto --help --no-self-upgrade | grep -F "letsencrypt-auto [SUBCOMMAND]"; then echo "letsencrypt-auto not included in help output!" exit 1 From a809c3697d433515517ba07b85c5ba42b34ff7c1 Mon Sep 17 00:00:00 2001 From: schoen Date: Wed, 27 Feb 2019 16:32:57 -0800 Subject: [PATCH 3/5] Warn sysadmins about privilege escalation risk (#6795) --- docs/install.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/install.rst b/docs/install.rst index 35b262482..eae40c1f0 100644 --- a/docs/install.rst +++ b/docs/install.rst @@ -11,6 +11,8 @@ About Certbot *Certbot is meant to be run directly on a web server*, normally by a system administrator. In most cases, running Certbot on your personal computer is not a useful option. The instructions below relate to installing and running Certbot on a server. +System administrators can use Certbot directly to request certificates; they should *not* allow unprivileged users to run arbitrary Certbot commands as ``root``, because Certbot allows its user to specify arbitrary file locations and run arbitrary scripts. + Certbot is packaged for many common operating systems and web servers. Check whether ``certbot`` (or ``letsencrypt``) is packaged for your web server's OS by visiting certbot.eff.org_, where you will also find the correct installation instructions for From 5e849e03f6e8dd2aca31888f9482da63359bb961 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 28 Feb 2019 20:35:58 +0100 Subject: [PATCH 4/5] [Windows] Fix pipstrap (#6775) * Fix pipstrap on windows * Pipstrap pin setuptools version, so explicit install it is not needed anymore. * Rebuild letsencrypt-auto source * Use sys.executable in pipstrap to allow straightforward execution in a venv by choosing the python interpreter from this venv. * Update letsencrypt-auto * Simulate test-everything * Revert "Simulate test-everything" This reverts commit b62c4d719a6e741cb11126c7490097a79c68cf4d. * Clean pipstrap code --- letsencrypt-auto-source/letsencrypt-auto | 22 +++++++++--------- letsencrypt-auto-source/pieces/pipstrap.py | 22 +++++++++--------- tools/_venv_common.py | 27 ++++++++-------------- 3 files changed, 32 insertions(+), 39 deletions(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 19aaa4eeb..c390f28ec 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -1221,7 +1221,6 @@ from distutils.version import StrictVersion from hashlib import sha256 from os import environ from os.path import join -from pipes import quote from shutil import rmtree try: from subprocess import check_output @@ -1241,7 +1240,7 @@ except ImportError: cmd = popenargs[0] raise CalledProcessError(retcode, cmd) return output -from sys import exit, version_info +import sys from tempfile import mkdtemp try: from urllib2 import build_opener, HTTPHandler, HTTPSHandler @@ -1263,7 +1262,7 @@ maybe_argparse = ( [('18/dd/e617cfc3f6210ae183374cd9f6a26b20514bbb5a792af97949c5aacddf0f/' 'argparse-1.4.0.tar.gz', '62b089a55be1d8949cd2bc7e0df0bddb9e028faefc8c32038cc84862aefdd6e4')] - if version_info < (2, 7, 0) else []) + if sys.version_info < (2, 7, 0) else []) PACKAGES = maybe_argparse + [ @@ -1344,7 +1343,8 @@ def get_index_base(): def main(): - pip_version = StrictVersion(check_output(['pip', '--version']) + python = sys.executable or 'python' + pip_version = StrictVersion(check_output([python, '-m', 'pip', '--version']) .decode('utf-8').split()[1]) has_pip_cache = pip_version >= StrictVersion('6.0') index_base = get_index_base() @@ -1354,12 +1354,12 @@ def main(): temp, digest) for path, digest in PACKAGES] - check_output('pip install --no-index --no-deps -U ' + - # Disable cache since we're not using it and it otherwise - # sometimes throws permission warnings: - ('--no-cache-dir ' if has_pip_cache else '') + - ' '.join(quote(d) for d in downloads), - shell=True) + # On Windows, pip self-upgrade is not possible, it must be done through python interpreter. + command = [python, '-m', 'pip', 'install', '--no-index', '--no-deps', '-U'] + # Disable cache since it is not used and it otherwise sometimes throws permission warnings: + command.extend(['--no-cache-dir'] if has_pip_cache else []) + command.extend(downloads) + check_output(command) except HashError as exc: print(exc) except Exception: @@ -1372,7 +1372,7 @@ def main(): if __name__ == '__main__': - exit(main()) + sys.exit(main()) UNLIKELY_EOF # ------------------------------------------------------------------------- diff --git a/letsencrypt-auto-source/pieces/pipstrap.py b/letsencrypt-auto-source/pieces/pipstrap.py index 6a00dd9cb..f04ebe6fe 100755 --- a/letsencrypt-auto-source/pieces/pipstrap.py +++ b/letsencrypt-auto-source/pieces/pipstrap.py @@ -23,7 +23,6 @@ from distutils.version import StrictVersion from hashlib import sha256 from os import environ from os.path import join -from pipes import quote from shutil import rmtree try: from subprocess import check_output @@ -43,7 +42,7 @@ except ImportError: cmd = popenargs[0] raise CalledProcessError(retcode, cmd) return output -from sys import exit, version_info +import sys from tempfile import mkdtemp try: from urllib2 import build_opener, HTTPHandler, HTTPSHandler @@ -65,7 +64,7 @@ maybe_argparse = ( [('18/dd/e617cfc3f6210ae183374cd9f6a26b20514bbb5a792af97949c5aacddf0f/' 'argparse-1.4.0.tar.gz', '62b089a55be1d8949cd2bc7e0df0bddb9e028faefc8c32038cc84862aefdd6e4')] - if version_info < (2, 7, 0) else []) + if sys.version_info < (2, 7, 0) else []) PACKAGES = maybe_argparse + [ @@ -146,7 +145,8 @@ def get_index_base(): def main(): - pip_version = StrictVersion(check_output(['pip', '--version']) + python = sys.executable or 'python' + pip_version = StrictVersion(check_output([python, '-m', 'pip', '--version']) .decode('utf-8').split()[1]) has_pip_cache = pip_version >= StrictVersion('6.0') index_base = get_index_base() @@ -156,12 +156,12 @@ def main(): temp, digest) for path, digest in PACKAGES] - check_output('pip install --no-index --no-deps -U ' + - # Disable cache since we're not using it and it otherwise - # sometimes throws permission warnings: - ('--no-cache-dir ' if has_pip_cache else '') + - ' '.join(quote(d) for d in downloads), - shell=True) + # On Windows, pip self-upgrade is not possible, it must be done through python interpreter. + command = [python, '-m', 'pip', 'install', '--no-index', '--no-deps', '-U'] + # Disable cache since it is not used and it otherwise sometimes throws permission warnings: + command.extend(['--no-cache-dir'] if has_pip_cache else []) + command.extend(downloads) + check_output(command) except HashError as exc: print(exc) except Exception: @@ -174,4 +174,4 @@ def main(): if __name__ == '__main__': - exit(main()) + sys.exit(main()) diff --git a/tools/_venv_common.py b/tools/_venv_common.py index 540842773..09383b4c0 100755 --- a/tools/_venv_common.py +++ b/tools/_venv_common.py @@ -107,13 +107,13 @@ def subprocess_with_print(cmd, env=os.environ, shell=False): subprocess.check_call(cmd, env=env, shell=shell) -def get_venv_bin_path(venv_path): +def get_venv_python_path(venv_path): python_linux = os.path.join(venv_path, 'bin/python') if os.path.isfile(python_linux): - return os.path.abspath(os.path.dirname(python_linux)) + return os.path.abspath(python_linux) python_windows = os.path.join(venv_path, 'Scripts\\python.exe') if os.path.isfile(python_windows): - return os.path.abspath(os.path.dirname(python_windows)) + return os.path.abspath(python_windows) raise ValueError(( 'Error, could not find python executable in venv path {0}: is it a valid venv ?' @@ -149,17 +149,12 @@ def main(venv_name, venv_args, args): command.extend(shlex.split(venv_args)) subprocess_with_print(command) - # We execute the following commands in the context of the virtual environment, to install - # the packages in it. To do so, we append the venv binary to the PATH that will be used for - # these commands. With this trick, correct python executable will be selected. - new_environ = os.environ.copy() - new_environ['PATH'] = os.pathsep.join([get_venv_bin_path(venv_name), new_environ['PATH']]) - subprocess_with_print('python {0}'.format('./letsencrypt-auto-source/pieces/pipstrap.py'), - env=new_environ, shell=True) - subprocess_with_print('python -m pip install --upgrade "setuptools>=30.3"', - env=new_environ, shell=True) - subprocess_with_print('python {0} {1}'.format('./tools/pip_install.py', ' '.join(args)), - env=new_environ, shell=True) + # Using the python executable from venv, we ensure to execute following commands in this venv. + py_venv = get_venv_python_path(venv_name) + subprocess_with_print([py_venv, os.path.abspath('letsencrypt-auto-source/pieces/pipstrap.py')]) + command = [py_venv, os.path.abspath('tools/pip_install.py')] + command.extend(args) + subprocess_with_print(command) if os.path.isdir(os.path.join(venv_name, 'bin')): # Linux/OSX specific @@ -179,6 +174,4 @@ def main(venv_name, venv_args, args): if __name__ == '__main__': - main('venv', - '', - sys.argv[1:]) + main('venv', '', sys.argv[1:]) From efc8d49806b14a31d88cfc0f1b6daca1dd373d8d Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 28 Feb 2019 21:49:36 +0100 Subject: [PATCH 5/5] Disable rerun feature of Travis (#6800) The rerun capability in a test campaign can be a nice feature. It can also a bad design. It is right that with high level tests, like performance or end-to-end tests, a given test runtime can depend on an external component, outside the scope of the developers, that would spuriously fail. In theses situations, having the capacity to rerun several time a test can be a great benefit. Indeed, as these tests are inherently flaky, the rerun greatly reduces their failure because of external reasons, reducing also the Pierre et le Loup effect (from a well-known french book for children): because tests constantly fail for no reason, you stop to listen to them and to not see when they fail for a real reason. However this not apply to unit tests, or operations about code quality. For theses executions, the flakiness should not exist: a unit test is supposed to have no external dependency. A rerun approach would just hide a situation that is not desirable for this kind of tests Also effectively I see that our tests are usually not flaky: so the only effect of the rerun is to give the failure state about a test three times slower. If a test becomes flaky, this should be fixed, and so be visible immediately in our CI. For these reasons, I remove the travis_retry in the script section of .travis-ci.yml, to call directly tox and let the pipeline fails on the first error. * Disable rerun feature of Travis * Update .travis.yml * Remove completely the retry logic --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0d59ec9e1..4adab1e4e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -197,10 +197,10 @@ addons: - nginx-light - openssl -install: "travis_retry $(command -v pip || command -v pip3) install codecov tox" +install: "$(command -v pip || command -v pip3) install codecov tox" script: - - travis_retry tox - - '[ -z "${BOULDER_INTEGRATION+x}" ] || (travis_retry tests/boulder-fetch.sh && tests/tox-boulder-integration.sh)' + - tox + - '[ -z "${BOULDER_INTEGRATION+x}" ] || (tests/boulder-fetch.sh && tests/tox-boulder-integration.sh)' after_success: '[ "$TOXENV" == "py27-cover" ] && codecov'