From 15502bb64e50b75a4a5e46a29179492c1b5c9fda Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 17 Mar 2016 16:14:29 -0700 Subject: [PATCH 01/71] renew implies noninteractive should be a property of config Not a property of the config we change later --- letsencrypt/cli.py | 3 +++ letsencrypt/main.py | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 017e0a62e..d3ffd3441 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -409,6 +409,9 @@ class HelpfulArgumentParser(object): # Do any post-parsing homework here + if self.verb == "renew": + self.noninteractive_mode = True + # we get domains from -d, but also from the webroot map... if parsed_args.webroot_map: for domain in parsed_args.webroot_map.keys(): diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 8d59993df..0148bdeca 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -685,9 +685,6 @@ def main(cli_args=sys.argv[1:]): displayer = display_util.NoninteractiveDisplay(sys.stdout) elif config.text_mode: displayer = display_util.FileDisplay(sys.stdout) - elif config.verb == "renew": - config.noninteractive_mode = True - displayer = display_util.NoninteractiveDisplay(sys.stdout) else: displayer = display_util.NcursesDisplay() zope.component.provideUtility(displayer) From b8ea2c19a38d870556496b980de2472b4e369d32 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 6 Apr 2016 16:57:52 -0700 Subject: [PATCH 02/71] Lintian bug fix --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f2ad50d45..adcc32a5e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -272,7 +272,7 @@ class HelpfulArgumentParser(object): # Do any post-parsing homework here if self.verb == "renew": - self.noninteractive_mode = True + parsed_args.noninteractive_mode = True # we get domains from -d, but also from the webroot map... if parsed_args.webroot_map: From 0bcc80756d16de6aed3365b0fa3d27c2e2e98855 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 6 Apr 2016 17:10:30 -0700 Subject: [PATCH 03/71] Refactor config.server complexity out of parse_args --- letsencrypt/cli.py | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 8c2cd839a..75c7e116f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -318,24 +318,7 @@ class HelpfulArgumentParser(object): parsed_args.noninteractive_mode = True if parsed_args.staging or parsed_args.dry_run: - if parsed_args.server not in (flag_default("server"), constants.STAGING_URI): - conflicts = ["--staging"] if parsed_args.staging else [] - conflicts += ["--dry-run"] if parsed_args.dry_run else [] - raise errors.Error("--server value conflicts with {0}".format( - " and ".join(conflicts))) - - parsed_args.server = constants.STAGING_URI - - if parsed_args.dry_run: - if self.verb not in ["certonly", "renew"]: - raise errors.Error("--dry-run currently only works with the " - "'certonly' or 'renew' subcommands (%r)" % self.verb) - parsed_args.break_my_certs = parsed_args.staging = True - if glob.glob(os.path.join(parsed_args.config_dir, constants.ACCOUNTS_DIR, "*")): - # The user has a prod account, but might not have a staging - # one; we don't want to start trying to perform interactive registration - parsed_args.tos = True - parsed_args.register_unsafely_without_email = True + self.set_test_server(parsed_args) if parsed_args.csr: if parsed_args.allow_subset_of_names: @@ -347,6 +330,30 @@ class HelpfulArgumentParser(object): return parsed_args + + def set_test_server(self, parsed_args): + "We have --staging/--dry-run; perform sanity check and set config.server" + + if parsed_args.server not in (flag_default("server"), constants.STAGING_URI): + conflicts = ["--staging"] if parsed_args.staging else [] + conflicts += ["--dry-run"] if parsed_args.dry_run else [] + raise errors.Error("--server value conflicts with {0}".format( + " and ".join(conflicts))) + + parsed_args.server = constants.STAGING_URI + + if parsed_args.dry_run: + if self.verb not in ["certonly", "renew"]: + raise errors.Error("--dry-run currently only works with the " + "'certonly' or 'renew' subcommands (%r)" % self.verb) + parsed_args.break_my_certs = parsed_args.staging = True + if glob.glob(os.path.join(parsed_args.config_dir, constants.ACCOUNTS_DIR, "*")): + # The user has a prod account, but might not have a staging + # one; we don't want to start trying to perform interactive registration + parsed_args.tos = True + parsed_args.register_unsafely_without_email = True + + def handle_csr(self, parsed_args): """Process a --csr flag.""" if parsed_args.verb != "certonly": From 5e3fc3a957eaaff225e21916b1241cd5a37ec522 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 6 Apr 2016 17:14:29 -0700 Subject: [PATCH 04/71] Keep all --csr checks in HelpfulArgumentParser.handle_csr --- letsencrypt/cli.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 75c7e116f..74c51f5e3 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -321,9 +321,6 @@ class HelpfulArgumentParser(object): self.set_test_server(parsed_args) if parsed_args.csr: - if parsed_args.allow_subset_of_names: - raise errors.Error("--allow-subset-of-names " - "cannot be used with --csr") self.handle_csr(parsed_args) hooks.validate_hooks(parsed_args) @@ -361,6 +358,8 @@ class HelpfulArgumentParser(object): "when obtaining a new or replacement " "via the certonly command. Please try the " "certonly command instead.") + if parsed_args.allow_subset_of_names: + raise errors.Error("--allow-subset-of-names cannot be used with --csr") try: csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="der") From 526bc5cf84aeaca0669d87ada3ced32615efe8e9 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 6 Apr 2016 17:35:29 -0700 Subject: [PATCH 05/71] Refactor CSR importing from cli -> crypto_util More specifically: HelpfulArgumentParser.handle_csr -> crypto_util.import_csr_file --- letsencrypt/cli.py | 18 ++---------------- letsencrypt/crypto_util.py | 30 ++++++++++++++++++++++++++++++ letsencrypt/tests/client_test.py | 6 ++++-- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 74c51f5e3..61fc57777 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -6,10 +6,8 @@ import logging import logging.handlers import os import sys -import traceback import configargparse -import OpenSSL import six import letsencrypt @@ -361,20 +359,8 @@ class HelpfulArgumentParser(object): if parsed_args.allow_subset_of_names: raise errors.Error("--allow-subset-of-names cannot be used with --csr") - try: - csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="der") - typ = OpenSSL.crypto.FILETYPE_ASN1 - domains = crypto_util.get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) - except OpenSSL.crypto.Error: - try: - e1 = traceback.format_exc() - typ = OpenSSL.crypto.FILETYPE_PEM - csr = le_util.CSR(file=parsed_args.csr[0], data=parsed_args.csr[1], form="pem") - domains = crypto_util.get_sans_from_csr(csr.data, typ) - except OpenSSL.crypto.Error: - logger.debug("DER CSR parse error %s", e1) - logger.debug("PEM CSR parse error %s", traceback.format_exc()) - raise errors.Error("Failed to parse CSR file: {0}".format(parsed_args.csr[0])) + csrfile, contents = parsed_args.csr[0:2] + typ, csr, domains = crypto_util.import_csr_file(csrfile, contents) # This is not necessary for webroot to work, however, # obtain_certificate_from_csr requires parsed_args.domains to be set diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index 5fdcba843..2ca43f76f 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -6,6 +6,7 @@ """ import logging import os +import traceback import OpenSSL import pyrfc3339 @@ -171,6 +172,35 @@ def csr_matches_pubkey(csr, privkey): return False +def import_csr_file(csrfile, contents): + """Import a CSR file, which can be either PEM or DER. + + :param str csrfile: CSR filename + :param str contents: contens of the CSR file + + :rtype: tuple + + :returns: (le_util.CSR object representing the CSR, + OpenSSL FILETYPE_ representing DER or PEM, + list of domains requested in the CSR) + """ + try: + csr = le_util.CSR(file=csrfile, data=contents, form="der") + typ = OpenSSL.crypto.FILETYPE_ASN1 + domains = get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) + except OpenSSL.crypto.Error: + try: + e1 = traceback.format_exc() + typ = OpenSSL.crypto.FILETYPE_PEM + csr = le_util.CSR(file=csrfile, data=contents, form="pem") + domains = get_sans_from_csr(csr.data, typ) + except OpenSSL.crypto.Error: + logger.debug("DER CSR parse error %s", e1) + logger.debug("PEM CSR parse error %s", traceback.format_exc()) + raise errors.Error("Failed to parse CSR file: {0}".format(csrfile)) + return typ, csr, domains + + def make_key(bits): """Generate PEM encoded RSA key. diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index cd6b11158..c3d5b322f 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -134,7 +134,7 @@ class ClientTest(unittest.TestCase): self.acme.fetch_chain.assert_called_once_with(mock.sentinel.certr) - # FIXME move parts of this to test_cli.py... + # FIXME move parts of this to crypto_util tests... @mock.patch("letsencrypt.client.logger") def test_obtain_certificate_from_csr(self, mock_logger): self._mock_obtain_certificate() @@ -144,9 +144,11 @@ class ClientTest(unittest.TestCase): # The CLI should believe that this is a certonly request, because # a CSR would not be allowed with other kinds of requests! mock_parsed_args.verb = "certonly" - with mock.patch("letsencrypt.client.le_util.CSR") as mock_CSR: + with mock.patch("letsencrypt.cli.crypto_util.le_util.CSR") as mock_CSR: mock_CSR.return_value = test_csr mock_parsed_args.domains = self.eg_domains[:] + mock_parsed_args.allow_subset_of_names = False + mock_parsed_args.csr = (mock.MagicMock(), mock.MagicMock()) mock_parser = mock.MagicMock(cli.HelpfulArgumentParser) cli.HelpfulArgumentParser.handle_csr(mock_parser, mock_parsed_args) From 639efaeb7ba590ba056c60fdcccb7c0a5b77e69a Mon Sep 17 00:00:00 2001 From: chrismarget Date: Wed, 11 May 2016 12:01:53 -0400 Subject: [PATCH 06/71] Randomize serial numbers of DVSNI challenge certificates. --- acme/acme/crypto_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 73f7f8f62..e121b1ac3 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -203,7 +203,7 @@ def gen_ss_cert(key, domains, not_before=None, """ assert domains, "Must provide one or more hostnames for the cert." cert = OpenSSL.crypto.X509() - cert.set_serial_number(1337) + cert.set_serial_number(int(OpenSSL.rand.bytes(16).encode("hex"), 16)) cert.set_version(2) extensions = [ From a7ef4940b6fabde1730c2a4cf7aef2d689895dff Mon Sep 17 00:00:00 2001 From: chrismarget Date: Wed, 11 May 2016 13:57:18 -0400 Subject: [PATCH 07/71] Randomize DVSNI challenge certificate serial number, now for python 3.3. --- acme/acme/crypto_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index e121b1ac3..6cb5ad6fc 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -203,7 +203,7 @@ def gen_ss_cert(key, domains, not_before=None, """ assert domains, "Must provide one or more hostnames for the cert." cert = OpenSSL.crypto.X509() - cert.set_serial_number(int(OpenSSL.rand.bytes(16).encode("hex"), 16)) + cert.set_serial_number(int.from_bytes(OpenSSL.rand.bytes(16),'big')) cert.set_version(2) extensions = [ From 7f70c09c5374ab225a341c73984ed12b3351abd7 Mon Sep 17 00:00:00 2001 From: chrismarget Date: Wed, 11 May 2016 15:19:39 -0400 Subject: [PATCH 08/71] Randomize serial numbers of DVSNI challenge certs. Should now work on python 2.7 and 3.3+ --- acme/acme/crypto_util.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 6cb5ad6fc..6465533a1 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -203,7 +203,12 @@ def gen_ss_cert(key, domains, not_before=None, """ assert domains, "Must provide one or more hostnames for the cert." cert = OpenSSL.crypto.X509() - cert.set_serial_number(int.from_bytes(OpenSSL.rand.bytes(16),'big')) + + try: + cert.set_serial_number(int(OpenSSL.rand.bytes(16).encode("hex"), 16)) + except AttributeError: + cert.set_serial_number(int.from_bytes(OpenSSL.rand.bytes(16),'big')) + cert.set_version(2) extensions = [ From 6fbd5fa81110e5e0c6aca139e4f827d2bab1da4e Mon Sep 17 00:00:00 2001 From: chrismarget Date: Wed, 11 May 2016 16:04:08 -0400 Subject: [PATCH 09/71] Added missing whitespace. --- acme/acme/crypto_util.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 6465533a1..66590bb26 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -203,12 +203,10 @@ def gen_ss_cert(key, domains, not_before=None, """ assert domains, "Must provide one or more hostnames for the cert." cert = OpenSSL.crypto.X509() - try: cert.set_serial_number(int(OpenSSL.rand.bytes(16).encode("hex"), 16)) except AttributeError: - cert.set_serial_number(int.from_bytes(OpenSSL.rand.bytes(16),'big')) - + cert.set_serial_number(int.from_bytes(OpenSSL.rand.bytes(16), 'big')) cert.set_version(2) extensions = [ From 4759bc9034a3e2dffbab561345a1716354e89b55 Mon Sep 17 00:00:00 2001 From: chrismarget Date: Wed, 11 May 2016 16:41:19 -0400 Subject: [PATCH 10/71] Trying to make pylint happy. --- acme/acme/crypto_util.py | 1 + 1 file changed, 1 insertion(+) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 66590bb26..40004c4d0 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -206,6 +206,7 @@ def gen_ss_cert(key, domains, not_before=None, try: cert.set_serial_number(int(OpenSSL.rand.bytes(16).encode("hex"), 16)) except AttributeError: + # pylint: disable=E1101 cert.set_serial_number(int.from_bytes(OpenSSL.rand.bytes(16), 'big')) cert.set_version(2) From f7b10bb83e01f21bf9aac06b6b7b78d65a1d279d Mon Sep 17 00:00:00 2001 From: chrismarget Date: Wed, 11 May 2016 17:06:29 -0400 Subject: [PATCH 11/71] Serial number randomization with improved portability. No exception handling required this time. --- acme/acme/crypto_util.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 40004c4d0..2b2133475 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -1,4 +1,5 @@ """Crypto utilities.""" +import binascii import contextlib import logging import re @@ -203,11 +204,7 @@ def gen_ss_cert(key, domains, not_before=None, """ assert domains, "Must provide one or more hostnames for the cert." cert = OpenSSL.crypto.X509() - try: - cert.set_serial_number(int(OpenSSL.rand.bytes(16).encode("hex"), 16)) - except AttributeError: - # pylint: disable=E1101 - cert.set_serial_number(int.from_bytes(OpenSSL.rand.bytes(16), 'big')) + cert.set_serial_number(int(binascii.hexlify(OpenSSL.rand.bytes(16)), 16)) cert.set_version(2) extensions = [ From a7d0b1a7d38111eeea96bf3aaa0eedee46ecc234 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 12 May 2016 17:49:44 -0700 Subject: [PATCH 12/71] Address review comments --- certbot/cli.py | 2 +- certbot/crypto_util.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 796925c1d..430384a5a 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -349,7 +349,7 @@ class HelpfulArgumentParser(object): def set_test_server(self, parsed_args): - "We have --staging/--dry-run; perform sanity check and set config.server" + """We have --staging/--dry-run; perform sanity check and set config.server""" if parsed_args.server not in (flag_default("server"), constants.STAGING_URI): conflicts = ["--staging"] if parsed_args.staging else [] diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index ceec6db71..07e7f9fd2 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -178,11 +178,11 @@ def import_csr_file(csrfile, contents): :param str csrfile: CSR filename :param str contents: contens of the CSR file - :rtype: tuple - :returns: (le_util.CSR object representing the CSR, - OpenSSL FILETYPE_ representing DER or PEM, + `OpenSSL.crypto.FILETYPE_PEM` or `OpenSSL.crypto.FILETYPE_ASN1`, list of domains requested in the CSR) + + :rtype: tuple """ try: csr = le_util.CSR(file=csrfile, data=contents, form="der") From f092669347291463047707551a736b21c8200de9 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 16 May 2016 21:19:07 +0000 Subject: [PATCH 13/71] If cert_path provided - do not randomize it --- certbot/client.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/certbot/client.py b/certbot/client.py index 6f41a3a0b..fda7707f6 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -317,7 +317,13 @@ class Client(object): cert_pem = OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped) - cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) + + if cert_path != constants.CLI_DEFAULTS['auth_cert_path']: + cert_file = le_util.safe_open(cert_path, chmod=0o644) + act_cert_path = cert_path + else: + cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) + try: cert_file.write(cert_pem) finally: From c0228ef1aa505a84a87529f76177f7dc6aa51214 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 16 May 2016 22:11:15 +0000 Subject: [PATCH 14/71] Boulder integration scripts provides a cert_path --- tests/boulder-integration.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 201343525..a1245e1c9 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -43,7 +43,7 @@ export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \ common auth --csr "$CSR_PATH" \ --cert-path "${root}/csr/cert.pem" \ --chain-path "${root}/csr/chain.pem" -openssl x509 -in "${root}/csr/0000_cert.pem" -text +openssl x509 -in "${root}/csr/cert.pem" -text openssl x509 -in "${root}/csr/0000_chain.pem" -text common --domains le3.wtf install \ From 85e962455559e25933cdf4ef7ecf8d37c8a03bde Mon Sep 17 00:00:00 2001 From: chrismarget Date: Tue, 17 May 2016 19:50:57 +0000 Subject: [PATCH 15/71] Added test for random certificate serial numbers from gen_ss_cert. --- acme/acme/crypto_util_test.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index 147cd5a2a..b41243b8f 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -8,6 +8,8 @@ import unittest import six from six.moves import socketserver # pylint: disable=import-error +import OpenSSL + from acme import errors from acme import jose from acme import test_util @@ -126,5 +128,23 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): self._get_idn_names()) +class RandomSnTest(unittest.TestCase): + """Test for random certificate serial numbers.""" + + def setUp(self): + self.certCount = 5 + self.serialNum = [] + self.key = OpenSSL.crypto.PKey() + self.key.generate_key(OpenSSL.crypto.TYPE_RSA, 2048) + + def test_sn_collisions(self): + from acme.crypto_util import gen_ss_cert + + for _ in range(self.certCount): + cert = gen_ss_cert(self.key, ['dummy'], force_san=True) + self.serialNum.append(cert.get_serial_number()) + self.assertTrue(len(set(self.serialNum)) > 1) + + if __name__ == '__main__': unittest.main() # pragma: no cover From 6dd99913719df02d5fbee28952774cdbf16a596e Mon Sep 17 00:00:00 2001 From: chrismarget Date: Tue, 17 May 2016 20:10:20 +0000 Subject: [PATCH 16/71] Fix invalid attribute for pylint --- acme/acme/crypto_util_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index b41243b8f..75a908d4f 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -132,18 +132,18 @@ class RandomSnTest(unittest.TestCase): """Test for random certificate serial numbers.""" def setUp(self): - self.certCount = 5 - self.serialNum = [] + self.cert_count = 5 + self.serial_num = [] self.key = OpenSSL.crypto.PKey() self.key.generate_key(OpenSSL.crypto.TYPE_RSA, 2048) def test_sn_collisions(self): from acme.crypto_util import gen_ss_cert - for _ in range(self.certCount): + for _ in range(self.cert_count): cert = gen_ss_cert(self.key, ['dummy'], force_san=True) - self.serialNum.append(cert.get_serial_number()) - self.assertTrue(len(set(self.serialNum)) > 1) + self.serial_num.append(cert.get_serial_number()) + self.assertTrue(len(set(self.serial_num)) > 1) if __name__ == '__main__': From 7e3c9399e54f2fb4960296e6211595707ff2dcf5 Mon Sep 17 00:00:00 2001 From: sagi Date: Tue, 17 May 2016 22:12:11 +0000 Subject: [PATCH 17/71] Use cli.set_by_cli to detect if the user explicitly set cert_path --- certbot/client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/certbot/client.py b/certbot/client.py index fda7707f6..dd38e47b0 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -24,6 +24,7 @@ from certbot import interfaces from certbot import le_util from certbot import reverter from certbot import storage +from certbot import cli from certbot.display import ops as display_ops from certbot.display import enhancements @@ -318,7 +319,7 @@ class Client(object): cert_pem = OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped) - if cert_path != constants.CLI_DEFAULTS['auth_cert_path']: + if cli.set_by_cli('cert_path'): cert_file = le_util.safe_open(cert_path, chmod=0o644) act_cert_path = cert_path else: From 50421e99beefd319cc864fa90cd7ba13e9f41786 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 18 May 2016 09:56:34 -0700 Subject: [PATCH 18/71] Factor loading cert/req into its own function --- certbot/crypto_util.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index 3f2267af2..41e675471 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -228,15 +228,20 @@ def pyopenssl_load_certificate(data): str(error) for error in openssl_errors))) -def _get_sans_from_cert_or_req(cert_or_req_str, load_func, - typ=OpenSSL.crypto.FILETYPE_PEM): +def _load_cert_or_req(cert_or_req_str, load_func, + typ=OpenSSL.crypto.FILETYPE_PEM): try: - cert_or_req = load_func(typ, cert_or_req_str) + return load_func(typ, cert_or_req_str) except OpenSSL.crypto.Error as error: logger.exception(error) raise + + +def _get_sans_from_cert_or_req(cert_or_req_str, load_func, + typ=OpenSSL.crypto.FILETYPE_PEM): # pylint: disable=protected-access - return acme_crypto_util._pyopenssl_cert_or_req_san(cert_or_req) + return acme_crypto_util._pyopenssl_cert_or_req_san(_load_cert_or_req( + cert_or_req_str, load_func, typ)) def get_sans_from_cert(cert, typ=OpenSSL.crypto.FILETYPE_PEM): From 8e17d7498de484857daead51b56f50b186341233 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 18 May 2016 10:14:15 -0700 Subject: [PATCH 19/71] Add get_names_from_csr --- certbot/crypto_util.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index 41e675471..b273cf59f 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -272,6 +272,27 @@ def get_sans_from_csr(csr, typ=OpenSSL.crypto.FILETYPE_PEM): csr, OpenSSL.crypto.load_certificate_request, typ) +def get_names_from_csr(csr, typ=OpenSSL.crypto.FILETYPE_PEM): + """Get a list of domains from a CSR, including the CN if it is set. + + :param str csr: CSR (encoded). + :param typ: `OpenSSL.crypto.FILETYPE_PEM` or `OpenSSL.crypto.FILETYPE_ASN1` + + :returns: A list of domain names. + :rtype: list + + """ + loaded_csr = _load_cert_or_req( + csr, OpenSSL.crypto.load_certificate_request, typ) + common_name = loaded_csr.get_subject().CN + + # Use a set to avoid duplication with CN and Subject Alt Names + domains = set() if common_name is None else set((common_name,)) + # pylint: disable=protected-access + domains.update(acme_crypto_util._pyopenssl_cert_or_req_san(loaded_csr)) + return list(domains) + + def dump_pyopenssl_chain(chain, filetype=OpenSSL.crypto.FILETYPE_PEM): """Dump certificate chain into a bundle. From 77e4be933cbfffaadda97d45e2dfb17672de56b5 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 18 May 2016 13:59:17 -0700 Subject: [PATCH 20/71] Simplify get_names_from_csr --- certbot/crypto_util.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index b273cf59f..1f87dc816 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -284,10 +284,8 @@ def get_names_from_csr(csr, typ=OpenSSL.crypto.FILETYPE_PEM): """ loaded_csr = _load_cert_or_req( csr, OpenSSL.crypto.load_certificate_request, typ) - common_name = loaded_csr.get_subject().CN - # Use a set to avoid duplication with CN and Subject Alt Names - domains = set() if common_name is None else set((common_name,)) + domains = set(d for d in (loaded_csr.get_subject().CN,) if d is not None) # pylint: disable=protected-access domains.update(acme_crypto_util._pyopenssl_cert_or_req_san(loaded_csr)) return list(domains) From 94549219c593dfc687b01f65ee89dc2287dae06e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 18 May 2016 14:06:32 -0700 Subject: [PATCH 21/71] Add get_names_from_csr tests --- certbot/tests/crypto_util_test.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index ff8d8142e..eade4861f 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -234,6 +234,36 @@ class GetSANsFromCSRTest(unittest.TestCase): [], self._call(test_util.load_vector('csr-nosans.pem'))) +class GetNamesFromCSRTest(unittest.TestCase): + """Tests for certbot.crypto_util.get_names_from_csr.""" + @classmethod + def _call(cls, *args, **kwargs): + from certbot.crypto_util import get_names_from_csr + return get_names_from_csr(*args, **kwargs) + + def test_extract_one_san(self): + self.assertEqual(['example.com'], self._call( + test_util.load_vector('csr.pem'))) + + def test_extract_two_sans(self): + self.assertEqual(set(('example.com', 'www.example.com',)), set( + self._call(test_util.load_vector('csr-san.pem')))) + + def test_extract_six_sans(self): + self.assertEqual( + set(self._call(test_util.load_vector('csr-6sans.pem'))), + set(("example.com", "example.org", "example.net", + "example.info", "subdomain.example.com", + "other.subdomain.example.com",))) + + def test_parse_non_csr(self): + self.assertRaises(OpenSSL.crypto.Error, self._call, "hello there") + + def test_parse_no_sans(self): + self.assertEqual(["example.org"], + self._call(test_util.load_vector('csr-nosans.pem'))) + + class CertLoaderTest(unittest.TestCase): """Tests for certbot.crypto_util.pyopenssl_load_certificate""" From 01ebab26bfa0eee521e8767411419007efa50814 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 18 May 2016 14:21:57 -0700 Subject: [PATCH 22/71] update pypi for auto --- letsencrypt-auto-source/letsencrypt-auto | 11 +++++++++-- letsencrypt-auto-source/pieces/fetch.py | 2 +- letsencrypt-auto-source/tests/auto_test.py | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index bbb2cda54..dd7dc06ec 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -452,6 +452,11 @@ BootstrapMac() { fi } +BootstrapSmartOS() { + pkgin update + pkgin -y install 'gcc49' 'py27-augeas' 'py27-virtualenv' +} + # Install required OS packages: Bootstrap() { @@ -484,8 +489,10 @@ Bootstrap() { ExperimentalBootstrap "FreeBSD" BootstrapFreeBsd elif uname | grep -iq Darwin ; then ExperimentalBootstrap "Mac OS X" BootstrapMac - elif grep -iq "Amazon Linux" /etc/issue ; then + elif [ -f /etc/issue ] && grep -iq "Amazon Linux" /etc/issue ; then ExperimentalBootstrap "Amazon Linux" BootstrapRpmCommon + elif [ -f /etc/product ] && grep -q "Joyent Instance" /etc/product ; then + ExperimentalBootstrap "Joyent SmartOS Zone" BootstrapSmartOS else echo "Sorry, I don't know how to bootstrap Certbot on your operating system!" echo @@ -998,7 +1005,7 @@ def latest_stable_version(get): """Return the latest stable release of letsencrypt.""" metadata = loads(get( environ.get('LE_AUTO_JSON_URL', - 'https://pypi.python.org/pypi/letsencrypt/json'))) + 'https://pypi.python.org/pypi/certbot/json'))) # metadata['info']['version'] actually returns the latest of any kind of # release release, contrary to https://wiki.python.org/moin/PyPIJSON. # The regex is a sufficient regex for picking out prereleases for most diff --git a/letsencrypt-auto-source/pieces/fetch.py b/letsencrypt-auto-source/pieces/fetch.py index 38f4aa255..4a2287fff 100644 --- a/letsencrypt-auto-source/pieces/fetch.py +++ b/letsencrypt-auto-source/pieces/fetch.py @@ -68,7 +68,7 @@ def latest_stable_version(get): """Return the latest stable release of letsencrypt.""" metadata = loads(get( environ.get('LE_AUTO_JSON_URL', - 'https://pypi.python.org/pypi/letsencrypt/json'))) + 'https://pypi.python.org/pypi/certbot/json'))) # metadata['info']['version'] actually returns the latest of any kind of # release release, contrary to https://wiki.python.org/moin/PyPIJSON. # The regex is a sufficient regex for picking out prereleases for most diff --git a/letsencrypt-auto-source/tests/auto_test.py b/letsencrypt-auto-source/tests/auto_test.py index 3b7e8731b..2c733f858 100644 --- a/letsencrypt-auto-source/tests/auto_test.py +++ b/letsencrypt-auto-source/tests/auto_test.py @@ -183,7 +183,7 @@ def run_le_auto(venv_dir, base_url, **kwargs): d = dict(XDG_DATA_HOME=venv_dir, # URL to PyPI-style JSON that tell us the latest released version # of LE: - LE_AUTO_JSON_URL=base_url + 'letsencrypt/json', + LE_AUTO_JSON_URL=base_url + 'certbot/json', # URL to dir containing letsencrypt-auto and letsencrypt-auto.sig: LE_AUTO_DIR_TEMPLATE=base_url + '%s/', # The public key corresponding to signing.key: From 55755d818af57809cd23ec6ccff855ae8a30c50a Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 18 May 2016 15:42:55 -0700 Subject: [PATCH 23/71] update secret pypi? --- letsencrypt-auto-source/tests/auto_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-auto-source/tests/auto_test.py b/letsencrypt-auto-source/tests/auto_test.py index 2c733f858..8018bab0f 100644 --- a/letsencrypt-auto-source/tests/auto_test.py +++ b/letsencrypt-auto-source/tests/auto_test.py @@ -301,8 +301,8 @@ class AutoTests(TestCase): with ephemeral_dir() as venv_dir: # Serve an unrelated hash signed with the good key (easier than # making a bad key, and a mismatch is a mismatch): - resources = {'': 'letsencrypt/', - 'letsencrypt/json': dumps({'releases': {'99.9.9': None}}), + resources = {'': 'certbot/', + 'certbot/json': dumps({'releases': {'99.9.9': None}}), 'v99.9.9/letsencrypt-auto': build_le_auto(version='99.9.9'), 'v99.9.9/letsencrypt-auto.sig': signed('something else')} with serving(resources) as base_url: From e8e009cc854246e9b059783f77506328c417191c Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 18 May 2016 17:00:42 -0700 Subject: [PATCH 24/71] Revert "update secret pypi?" This reverts commit 55755d818af57809cd23ec6ccff855ae8a30c50a. --- letsencrypt-auto-source/tests/auto_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-auto-source/tests/auto_test.py b/letsencrypt-auto-source/tests/auto_test.py index 8018bab0f..2c733f858 100644 --- a/letsencrypt-auto-source/tests/auto_test.py +++ b/letsencrypt-auto-source/tests/auto_test.py @@ -301,8 +301,8 @@ class AutoTests(TestCase): with ephemeral_dir() as venv_dir: # Serve an unrelated hash signed with the good key (easier than # making a bad key, and a mismatch is a mismatch): - resources = {'': 'certbot/', - 'certbot/json': dumps({'releases': {'99.9.9': None}}), + resources = {'': 'letsencrypt/', + 'letsencrypt/json': dumps({'releases': {'99.9.9': None}}), 'v99.9.9/letsencrypt-auto': build_le_auto(version='99.9.9'), 'v99.9.9/letsencrypt-auto.sig': signed('something else')} with serving(resources) as base_url: From e7374811294be55b45d8de30a36883f836da6590 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 19 May 2016 18:20:27 +0000 Subject: [PATCH 25/71] WIP --- certbot/client.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index dd38e47b0..6dd0420eb 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -292,6 +292,7 @@ class Client(object): key.pem, crypto_util.dump_pyopenssl_chain(chain), configuration.RenewerConfiguration(self.config.namespace)) + def save_certificate(self, certr, chain_cert, cert_path, chain_path, fullchain_path): """Saves the certificate received from the ACME server. @@ -318,12 +319,15 @@ class Client(object): cert_pem = OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped) - + """ if cli.set_by_cli('cert_path'): cert_file = le_util.safe_open(cert_path, chmod=0o644) act_cert_path = cert_path else: cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) + """ + cert_file, act_cert_path = _open_pem_file('cert_path', cert_path) + #import ipdb; ipdb.set_trace try: cert_file.write(cert_pem) @@ -331,7 +335,14 @@ class Client(object): cert_file.close() logger.info("Server issued certificate; certificate written to %s", act_cert_path) - + + if cli.set_by_cli('chain_path'): + #import ipdb; ipdb.set_trace() + pass + if cli.set_by_cli('fullchain_path'): + #import ipdb; ipdb.set_trace() + pass + cert_chain_abspath = None fullchain_abspath = None if chain_cert: @@ -569,6 +580,11 @@ def view_config_changes(config, num=None): rev.recovery_routine() rev.view_config_changes(num) +def _open_pem_file(cli_arg_path, pem_path): + if cli.set_by_cli(cli_arg_path): + return le_util.safe_open(pem_path, chmod=0o644), pem_path + else: + return le_util.unique_file(pem_path, 0o644) def _save_chain(chain_pem, chain_path): """Saves chain_pem at a unique path based on chain_path. From 409640fb87a89487e46be0427c072a05074958a0 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Thu, 19 May 2016 12:05:42 -0700 Subject: [PATCH 26/71] le to cb for test package --- letsencrypt-auto-source/tests/auto_test.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/letsencrypt-auto-source/tests/auto_test.py b/letsencrypt-auto-source/tests/auto_test.py index 2c733f858..357e8302c 100644 --- a/letsencrypt-auto-source/tests/auto_test.py +++ b/letsencrypt-auto-source/tests/auto_test.py @@ -258,9 +258,9 @@ class AutoTests(TestCase): with ephemeral_dir() as venv_dir: # This serves a PyPI page with a higher version, a GitHub-alike # with a corresponding le-auto script, and a matching signature. - resources = {'letsencrypt/json': dumps({'releases': {'99.9.9': None}}), - 'v99.9.9/letsencrypt-auto': NEW_LE_AUTO, - 'v99.9.9/letsencrypt-auto.sig': NEW_LE_AUTO_SIG} + resources = {'certbot/json': dumps({'releases': {'99.9.9': None}}), + 'v99.9.9/certbot-auto': NEW_LE_AUTO, + 'v99.9.9/certbot-auto.sig': NEW_LE_AUTO_SIG} with serving(resources) as base_url: run_letsencrypt_auto = partial( run_le_auto, @@ -301,10 +301,10 @@ class AutoTests(TestCase): with ephemeral_dir() as venv_dir: # Serve an unrelated hash signed with the good key (easier than # making a bad key, and a mismatch is a mismatch): - resources = {'': 'letsencrypt/', - 'letsencrypt/json': dumps({'releases': {'99.9.9': None}}), - 'v99.9.9/letsencrypt-auto': build_le_auto(version='99.9.9'), - 'v99.9.9/letsencrypt-auto.sig': signed('something else')} + resources = {'': 'certbot/', + 'certbot/json': dumps({'releases': {'99.9.9': None}}), + 'v99.9.9/certbot-auto': build_le_auto(version='99.9.9'), + 'v99.9.9/certbot-auto.sig': signed('something else')} with serving(resources) as base_url: copy(LE_AUTO_PATH, venv_dir) try: @@ -320,8 +320,8 @@ class AutoTests(TestCase): def test_pip_failure(self): """Make sure pip stops us if there is a hash mismatch.""" with ephemeral_dir() as venv_dir: - resources = {'': 'letsencrypt/', - 'letsencrypt/json': dumps({'releases': {'99.9.9': None}})} + resources = {'': 'certbot/', + 'certbot/json': dumps({'releases': {'99.9.9': None}})} with serving(resources) as base_url: # Build a le-auto script embedding a bad requirements file: install_le_auto( From fde151848d4b1a29ada511db77308979ba247989 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 19 May 2016 19:11:25 +0000 Subject: [PATCH 27/71] Use set_by_cli for fullchain_path and chain_path --- certbot/client.py | 40 ++++++++++++++++------------------------ certbot/le_util.py | 3 ++- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 6dd0420eb..3475312f0 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -319,15 +319,8 @@ class Client(object): cert_pem = OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped) - """ - if cli.set_by_cli('cert_path'): - cert_file = le_util.safe_open(cert_path, chmod=0o644) - act_cert_path = cert_path - else: - cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) - """ + cert_file, act_cert_path = _open_pem_file('cert_path', cert_path) - #import ipdb; ipdb.set_trace try: cert_file.write(cert_pem) @@ -335,21 +328,20 @@ class Client(object): cert_file.close() logger.info("Server issued certificate; certificate written to %s", act_cert_path) - - if cli.set_by_cli('chain_path'): - #import ipdb; ipdb.set_trace() - pass - if cli.set_by_cli('fullchain_path'): - #import ipdb; ipdb.set_trace() - pass - + cert_chain_abspath = None fullchain_abspath = None if chain_cert: chain_pem = crypto_util.dump_pyopenssl_chain(chain_cert) - cert_chain_abspath = _save_chain(chain_pem, chain_path) + + chain_file, act_chain_path =\ + _open_pem_file('chain_path', chain_path) + fullchain_file, act_fullchain_path =\ + _open_pem_file('fullchain_path', fullchain_path) + + cert_chain_abspath = _save_chain(chain_pem, chain_file) fullchain_abspath = _save_chain(cert_pem + chain_pem, - fullchain_path) + fullchain_file) return os.path.abspath(act_cert_path), cert_chain_abspath, fullchain_abspath @@ -582,27 +574,27 @@ def view_config_changes(config, num=None): def _open_pem_file(cli_arg_path, pem_path): if cli.set_by_cli(cli_arg_path): - return le_util.safe_open(pem_path, chmod=0o644), pem_path + return le_util.safe_open(pem_path, chmod=0o644),\ + os.path.abspath(pem_path) else: return le_util.unique_file(pem_path, 0o644) -def _save_chain(chain_pem, chain_path): +def _save_chain(chain_pem, chain_file): """Saves chain_pem at a unique path based on chain_path. :param str chain_pem: certificate chain in PEM format - :param str chain_path: candidate path for the cert chain + :param str chain_file: chain file object :returns: absolute path to saved cert chain :rtype: str """ - chain_file, act_chain_path = le_util.unique_file(chain_path, 0o644) try: chain_file.write(chain_pem) finally: chain_file.close() - logger.info("Cert chain written to %s", act_chain_path) + logger.info("Cert chain written to %s", chain_file.name) # This expects a valid chain file - return os.path.abspath(act_chain_path) + return os.path.abspath(chain_file.name) diff --git a/certbot/le_util.py b/certbot/le_util.py index f5148b949..fe2577a4c 100644 --- a/certbot/le_util.py +++ b/certbot/le_util.py @@ -151,7 +151,8 @@ def _unique_file(path, filename_pat, count, mode): while True: current_path = os.path.join(path, filename_pat(count)) try: - return safe_open(current_path, chmod=mode), current_path + return safe_open(current_path, chmod=mode),\ + os.path.abspath(current_path) except OSError as err: # "File exists," is okay, try a different name. if err.errno != errno.EEXIST: From 0bb8b0bcd5231c896ee9449bac22d30204381dac Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Thu, 19 May 2016 12:27:17 -0700 Subject: [PATCH 28/71] change invocation --- letsencrypt-auto-source/tests/auto_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-auto-source/tests/auto_test.py b/letsencrypt-auto-source/tests/auto_test.py index 357e8302c..6fccdb56e 100644 --- a/letsencrypt-auto-source/tests/auto_test.py +++ b/letsencrypt-auto-source/tests/auto_test.py @@ -199,7 +199,7 @@ iQIDAQAB **kwargs) env.update(d) return out_and_err( - join(venv_dir, 'letsencrypt-auto') + ' --version', + join(venv_dir, 'certbot-auto') + ' --version', shell=True, env=env) From e1eb3eff164610b7359582478d02c3fbf4b8c6b9 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 19 May 2016 19:27:18 +0000 Subject: [PATCH 29/71] Improve code reuse --- certbot/client.py | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 3475312f0..ee1ab8bb8 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -320,30 +320,27 @@ class Client(object): cert_pem = OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped) - cert_file, act_cert_path = _open_pem_file('cert_path', cert_path) + cert_file, abs_cert_path = _open_pem_file('cert_path', cert_path) try: cert_file.write(cert_pem) finally: cert_file.close() logger.info("Server issued certificate; certificate written to %s", - act_cert_path) + abs_cert_path) - cert_chain_abspath = None - fullchain_abspath = None if chain_cert: chain_pem = crypto_util.dump_pyopenssl_chain(chain_cert) - chain_file, act_chain_path =\ + chain_file, abs_chain_path =\ _open_pem_file('chain_path', chain_path) - fullchain_file, act_fullchain_path =\ + fullchain_file, abs_fullchain_path =\ _open_pem_file('fullchain_path', fullchain_path) - cert_chain_abspath = _save_chain(chain_pem, chain_file) - fullchain_abspath = _save_chain(cert_pem + chain_pem, - fullchain_file) + _save_chain(chain_pem, chain_file) + _save_chain(cert_pem + chain_pem, fullchain_file - return os.path.abspath(act_cert_path), cert_chain_abspath, fullchain_abspath + return abs_cert_path, abs_chain_path, abs_fullchain_path def deploy_certificate(self, domains, privkey_path, cert_path, chain_path, fullchain_path): @@ -577,7 +574,8 @@ def _open_pem_file(cli_arg_path, pem_path): return le_util.safe_open(pem_path, chmod=0o644),\ os.path.abspath(pem_path) else: - return le_util.unique_file(pem_path, 0o644) + uniq = le_util.unique_file(pem_path, 0o644) + return uniq[0], os.path.abspath(uniq) def _save_chain(chain_pem, chain_file): """Saves chain_pem at a unique path based on chain_path. @@ -585,9 +583,6 @@ def _save_chain(chain_pem, chain_file): :param str chain_pem: certificate chain in PEM format :param str chain_file: chain file object - :returns: absolute path to saved cert chain - :rtype: str - """ try: chain_file.write(chain_pem) @@ -595,6 +590,3 @@ def _save_chain(chain_pem, chain_file): chain_file.close() logger.info("Cert chain written to %s", chain_file.name) - - # This expects a valid chain file - return os.path.abspath(chain_file.name) From 501c19ef2a254e44b768eb4cd57e5832f5afb792 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 19 May 2016 19:33:04 +0000 Subject: [PATCH 30/71] Syntax --- certbot/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/client.py b/certbot/client.py index ee1ab8bb8..8964686e6 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -338,7 +338,7 @@ class Client(object): _open_pem_file('fullchain_path', fullchain_path) _save_chain(chain_pem, chain_file) - _save_chain(cert_pem + chain_pem, fullchain_file + _save_chain(cert_pem + chain_pem, fullchain_file) return abs_cert_path, abs_chain_path, abs_fullchain_path From 3589b25dc3a7d4c0fb2477b21d6039a327bd1b52 Mon Sep 17 00:00:00 2001 From: sagi Date: Thu, 19 May 2016 19:35:38 +0000 Subject: [PATCH 31/71] Make lint happy --- certbot/client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/certbot/client.py b/certbot/client.py index 8964686e6..4fc662948 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -292,7 +292,6 @@ class Client(object): key.pem, crypto_util.dump_pyopenssl_chain(chain), configuration.RenewerConfiguration(self.config.namespace)) - def save_certificate(self, certr, chain_cert, cert_path, chain_path, fullchain_path): """Saves the certificate received from the ACME server. From 22badb2380bc405032d47e9f90212a0f1a507fad Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Thu, 19 May 2016 17:29:39 -0700 Subject: [PATCH 32/71] tests pass? --- letsencrypt-auto-source/tests/auto_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt-auto-source/tests/auto_test.py b/letsencrypt-auto-source/tests/auto_test.py index 6fccdb56e..7e131f4cf 100644 --- a/letsencrypt-auto-source/tests/auto_test.py +++ b/letsencrypt-auto-source/tests/auto_test.py @@ -199,7 +199,7 @@ iQIDAQAB **kwargs) env.update(d) return out_and_err( - join(venv_dir, 'certbot-auto') + ' --version', + join(venv_dir, 'letsencrypt-auto') + ' --version', shell=True, env=env) @@ -259,8 +259,8 @@ class AutoTests(TestCase): # This serves a PyPI page with a higher version, a GitHub-alike # with a corresponding le-auto script, and a matching signature. resources = {'certbot/json': dumps({'releases': {'99.9.9': None}}), - 'v99.9.9/certbot-auto': NEW_LE_AUTO, - 'v99.9.9/certbot-auto.sig': NEW_LE_AUTO_SIG} + 'v99.9.9/letsencrypt-auto': NEW_LE_AUTO, + 'v99.9.9/letsencrypt-auto.sig': NEW_LE_AUTO_SIG} with serving(resources) as base_url: run_letsencrypt_auto = partial( run_le_auto, @@ -303,8 +303,8 @@ class AutoTests(TestCase): # making a bad key, and a mismatch is a mismatch): resources = {'': 'certbot/', 'certbot/json': dumps({'releases': {'99.9.9': None}}), - 'v99.9.9/certbot-auto': build_le_auto(version='99.9.9'), - 'v99.9.9/certbot-auto.sig': signed('something else')} + 'v99.9.9/letsencrypt-auto': build_le_auto(version='99.9.9'), + 'v99.9.9/letsencrypt-auto.sig': signed('something else')} with serving(resources) as base_url: copy(LE_AUTO_PATH, venv_dir) try: From 7689de2ad8da074015df2150b455115f3b5816b5 Mon Sep 17 00:00:00 2001 From: sagi Date: Fri, 20 May 2016 01:18:50 +0000 Subject: [PATCH 33/71] Fix tests --- certbot/client.py | 12 +++++++++++- certbot/tests/client_test.py | 8 +++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 4fc662948..818701f08 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -569,12 +569,22 @@ def view_config_changes(config, num=None): rev.view_config_changes(num) def _open_pem_file(cli_arg_path, pem_path): + """Open a pem file. + + If cli_arg_path was set by the client, open that. + Otherwise, uniquify the file path. + + :param str cli_arg_path: the cli arg name, e.g. cert_path + :param str pem_path: the pem file path to open + + :returns a file object + """ if cli.set_by_cli(cli_arg_path): return le_util.safe_open(pem_path, chmod=0o644),\ os.path.abspath(pem_path) else: uniq = le_util.unique_file(pem_path, 0o644) - return uniq[0], os.path.abspath(uniq) + return uniq[0], os.path.abspath(uniq[1]) def _save_chain(chain_pem, chain_file): """Saves chain_pem at a unique path based on chain_path. diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 8ceefe8ae..49596fdee 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -221,7 +221,9 @@ class ClientTest(unittest.TestCase): mock.sentinel.key, domains, self.config.csr_dir) self._check_obtain_certificate() - def test_save_certificate(self): + @mock.patch("certbot.cli.helpful_parser") + def test_save_certificate(self, mock_parser): + # pylint: disable=too-many-locals certs = ["matching_cert.pem", "cert.pem", "cert-san.pem"] tmp_path = tempfile.mkdtemp() os.chmod(tmp_path, 0o755) # TODO: really?? @@ -232,6 +234,10 @@ class ClientTest(unittest.TestCase): candidate_cert_path = os.path.join(tmp_path, "certs", "cert.pem") candidate_chain_path = os.path.join(tmp_path, "chains", "chain.pem") candidate_fullchain_path = os.path.join(tmp_path, "chains", "fullchain.pem") + mock_parser.verb = "certonly" + mock_parser.args = ["--cert-path", candidate_cert_path, + "--chain-path", candidate_chain_path, + "--fullchain-path", candidate_fullchain_path] cert_path, chain_path, fullchain_path = self.client.save_certificate( certr, chain_cert, candidate_cert_path, candidate_chain_path, From 46be2df1999e65e49e6194484f7bee5fea894fcb Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Fri, 20 May 2016 13:25:34 -0700 Subject: [PATCH 34/71] fix syntax error --- certbot/reverter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/reverter.py b/certbot/reverter.py index fe6d9f24f..b023d18a7 100644 --- a/certbot/reverter.py +++ b/certbot/reverter.py @@ -489,7 +489,7 @@ class Reverter(object): if not os.path.exists(changes_since_path): logger.info("Rollback checkpoint is empty (no changes made?)") - with open(self.config.changes_since_path) as f: + with open(changes_since_path, 'w') as f: f.write("No changes\n") # Add title to self.config.in_progress_dir CHANGES_SINCE From 32d32dbc1279603a8cdc0cef7e0a0c42563f71ba Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 May 2016 15:20:28 -0700 Subject: [PATCH 35/71] cli.py PEP8 fixes --- certbot/cli.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index af4cd3013..3bd565496 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -347,7 +347,6 @@ class HelpfulArgumentParser(object): return parsed_args - def set_test_server(self, parsed_args): """We have --staging/--dry-run; perform sanity check and set config.server""" @@ -370,7 +369,6 @@ class HelpfulArgumentParser(object): parsed_args.tos = True parsed_args.register_unsafely_without_email = True - def handle_csr(self, parsed_args): """Process a --csr flag.""" if parsed_args.verb != "certonly": From 73c4e8f7a40b34e8dfed12d0e784e1aef0d25523 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 May 2016 16:24:49 -0700 Subject: [PATCH 36/71] Cleanup test_obtain_certificate_from_csr --- certbot/tests/client_test.py | 74 +++++++++++++----------------------- 1 file changed, 27 insertions(+), 47 deletions(-) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 148857be7..b7195a777 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -134,59 +134,39 @@ class ClientTest(unittest.TestCase): self.acme.fetch_chain.assert_called_once_with(mock.sentinel.certr) - # FIXME move parts of this to crypto_util tests... @mock.patch("certbot.client.logger") def test_obtain_certificate_from_csr(self, mock_logger): self._mock_obtain_certificate() - from certbot import cli test_csr = le_util.CSR(form="der", file=None, data=CSR_SAN) - mock_parsed_args = mock.MagicMock() - # The CLI should believe that this is a certonly request, because - # a CSR would not be allowed with other kinds of requests! - mock_parsed_args.verb = "certonly" - with mock.patch("certbot.cli.crypto_util.le_util.CSR") as mock_CSR: - mock_CSR.return_value = test_csr - mock_parsed_args.domains = self.eg_domains[:] - mock_parsed_args.allow_subset_of_names = False - mock_parsed_args.csr = (mock.MagicMock(), mock.MagicMock()) - mock_parser = mock.MagicMock(cli.HelpfulArgumentParser) - cli.HelpfulArgumentParser.handle_csr(mock_parser, mock_parsed_args) + auth_handler = self.client.auth_handler - # Now provoke an inconsistent domains error... - mock_parsed_args.domains.append("hippopotamus.io") - self.assertRaises(errors.ConfigurationError, - cli.HelpfulArgumentParser.handle_csr, mock_parser, mock_parsed_args) - - authzr = self.client.auth_handler.get_authorizations(self.eg_domains, False) - - self.assertEqual( - (mock.sentinel.certr, mock.sentinel.chain), - self.client.obtain_certificate_from_csr( - self.eg_domains, - test_csr, - authzr=authzr)) - # and that the cert was obtained correctly - self._check_obtain_certificate() - - # Test for authzr=None - self.assertEqual( - (mock.sentinel.certr, mock.sentinel.chain), - self.client.obtain_certificate_from_csr( - self.eg_domains, - test_csr, - authzr=None)) - - self.client.auth_handler.get_authorizations.assert_called_with( - self.eg_domains) - - # Test for no auth_handler - self.client.auth_handler = None - self.assertRaises( - errors.Error, - self.client.obtain_certificate_from_csr, + authzr = auth_handler.get_authorizations(self.eg_domains, False) + self.assertEqual( + (mock.sentinel.certr, mock.sentinel.chain), + self.client.obtain_certificate_from_csr( self.eg_domains, - test_csr) - mock_logger.warning.assert_called_once_with(mock.ANY) + test_csr, + authzr=authzr)) + # and that the cert was obtained correctly + self._check_obtain_certificate() + + # Test for authzr=None + self.assertEqual( + (mock.sentinel.certr, mock.sentinel.chain), + self.client.obtain_certificate_from_csr( + self.eg_domains, + test_csr, + authzr=None)) + auth_handler.get_authorizations.assert_called_with(self.eg_domains) + + # Test for no auth_handler + self.client.auth_handler = None + self.assertRaises( + errors.Error, + self.client.obtain_certificate_from_csr, + self.eg_domains, + test_csr) + mock_logger.warning.assert_called_once_with(mock.ANY) @mock.patch("certbot.client.crypto_util") def test_obtain_certificate(self, mock_crypto_util): From 53286863fefa47b7464340b8b46bbd93b5358932 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 May 2016 16:26:09 -0700 Subject: [PATCH 37/71] Simplify import_csr_file --- certbot/crypto_util.py | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index ba368b15b..68e07e059 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -180,33 +180,28 @@ def csr_matches_pubkey(csr, privkey): return False -def import_csr_file(csrfile, contents): +def import_csr_file(csrfile, data): """Import a CSR file, which can be either PEM or DER. :param str csrfile: CSR filename - :param str contents: contens of the CSR file + :param str data: contents of the CSR file - :returns: (le_util.CSR object representing the CSR, - `OpenSSL.crypto.FILETYPE_PEM` or `OpenSSL.crypto.FILETYPE_ASN1`, + :returns: (`OpenSSL.crypto.FILETYPE_PEM` or `OpenSSL.crypto.FILETYPE_ASN1`, + le_util.CSR object representing the CSR, list of domains requested in the CSR) - :rtype: tuple + """ - try: - csr = le_util.CSR(file=csrfile, data=contents, form="der") - typ = OpenSSL.crypto.FILETYPE_ASN1 - domains = get_sans_from_csr(csr.data, OpenSSL.crypto.FILETYPE_ASN1) - except OpenSSL.crypto.Error: + for form, typ in (("der", OpenSSL.crypto.FILETYPE_ASN1,), + ("pem", OpenSSL.crypto.FILETYPE_PEM,),): try: - e1 = traceback.format_exc() - typ = OpenSSL.crypto.FILETYPE_PEM - csr = le_util.CSR(file=csrfile, data=contents, form="pem") - domains = get_sans_from_csr(csr.data, typ) + domains = get_names_from_csr(data, typ) except OpenSSL.crypto.Error: - logger.debug("DER CSR parse error %s", e1) - logger.debug("PEM CSR parse error %s", traceback.format_exc()) - raise errors.Error("Failed to parse CSR file: {0}".format(csrfile)) - return typ, csr, domains + logger.debug("CSR parse error (form=%s, typ=%s):", form, typ) + logger.debug(traceback.format_exc()) + continue + return typ, le_util.CSR(file=csrfile, data=data, form=form), domains + raise errors.Error("Failed to parse CSR file: {0}".format(csrfile)) def make_key(bits): From 271316a41343593e598f721f3aee3e1621b987e9 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Fri, 20 May 2016 16:32:39 -0700 Subject: [PATCH 38/71] cover test --- certbot/reverter.py | 1 + certbot/tests/reverter_test.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/certbot/reverter.py b/certbot/reverter.py index b023d18a7..16ee5d8a4 100644 --- a/certbot/reverter.py +++ b/certbot/reverter.py @@ -7,6 +7,7 @@ import shutil import time import traceback + import zope.component from certbot import constants diff --git a/certbot/tests/reverter_test.py b/certbot/tests/reverter_test.py index eda5ffb36..c79c72a48 100644 --- a/certbot/tests/reverter_test.py +++ b/certbot/tests/reverter_test.py @@ -34,6 +34,20 @@ class ReverterCheckpointLocalTest(unittest.TestCase): logging.disable(logging.NOTSET) + @mock.patch("certbot.reverter.Reverter._read_and_append") + def test_no_change(self, mock_read): + mock_read.side_effect = OSError("cannot even") + try: + self.reverter.add_to_checkpoint(self.sets[0], "save1") + except: + pass + self.reverter.finalize_checkpoint("blah") + path = os.listdir(self.reverter.config.backup_dir)[0] + no_change = os.path.join(self.reverter.config.backup_dir, path, "CHANGES_SINCE") + with open(no_change, "r") as f: + x = f.read() + self.assertTrue("No changes" in x) + def test_basic_add_to_temp_checkpoint(self): # These shouldn't conflict even though they are both named config.txt self.reverter.add_to_temp_checkpoint(self.sets[0], "save1") From a48afd498c9c7a022eff72d8f903879f824cbe14 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 May 2016 16:52:35 -0700 Subject: [PATCH 39/71] Start import_csr_file tests --- certbot/tests/crypto_util_test.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index eade4861f..af8308fd8 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -10,6 +10,7 @@ import zope.component from certbot import errors from certbot import interfaces +from certbot import le_util from certbot.tests import test_util @@ -159,6 +160,27 @@ class CSRMatchesPubkeyTest(unittest.TestCase): test_util.load_vector('csr.pem'), RSA256_KEY)) +class ImportCSRFileTest(unittest.TestCase): + """Tests for certbot.certbot_util.import_csr_file.""" + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.crypto_util import import_csr_file + return import_csr_file(*args, **kwargs) + + def test_der_csr(self): + csrfile = test_util.vector_path('csr.der') + data = test_util.load_vector('csr.der') + + self.assertEqual( + (OpenSSL.crypto.FILETYPE_ASN1, + le_util.CSR(file=csrfile, + data=data, + form="der"), + ["example.com"],), + self._call(csrfile, data)) + + class MakeKeyTest(unittest.TestCase): # pylint: disable=too-few-public-methods """Tests for certbot.crypto_util.make_key.""" From 953d4957b8e32a74d894c2d3eeab56e0105ebf94 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 May 2016 16:57:13 -0700 Subject: [PATCH 40/71] Add csr test --- certbot/tests/crypto_util_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index af8308fd8..d5a016320 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -180,6 +180,18 @@ class ImportCSRFileTest(unittest.TestCase): ["example.com"],), self._call(csrfile, data)) + def test_pem_csr(self): + csrfile = test_util.vector_path('csr.pem') + data = test_util.load_vector('csr.pem') + + self.assertEqual( + (OpenSSL.crypto.FILETYPE_PEM, + le_util.CSR(file=csrfile, + data=data, + form="pem"), + ["example.com"],), + self._call(csrfile, data)) + class MakeKeyTest(unittest.TestCase): # pylint: disable=too-few-public-methods """Tests for certbot.crypto_util.make_key.""" From 2c4c8c081c80d00be04149afd7da2396f52a0a93 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 May 2016 17:00:06 -0700 Subject: [PATCH 41/71] Test bad csr --- certbot/tests/crypto_util_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index d5a016320..eeea0f4ab 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -192,6 +192,11 @@ class ImportCSRFileTest(unittest.TestCase): ["example.com"],), self._call(csrfile, data)) + def test_bad_csr(self): + self.assertRaises(errors.Error, self._call, + test_util.vector_path('cert.pem'), + test_util.load_vector('cert.pem')) + class MakeKeyTest(unittest.TestCase): # pylint: disable=too-few-public-methods """Tests for certbot.crypto_util.make_key.""" From 3c11733006b03da942d0ee43a8d16c1c6bd40d77 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 May 2016 17:32:37 -0700 Subject: [PATCH 42/71] fix --csr and --allow-subset-of-names test --- certbot/tests/cli_test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index d7965a24e..62ec36d2a 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -349,8 +349,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods ['-d', '204.11.231.35']) def test_csr_with_besteffort(self): - args = ["--csr", CSR, "--allow-subset-of-names"] - self.assertRaises(errors.Error, self._call, args) + self.assertRaises( + errors.Error, self._call, + 'certonly --csr {0} --allow-subset-of-names'.format(CSR).split()) def test_run_with_csr(self): # This is an error because you can only use --csr with certonly From 0b85a8f1c8d26accccd148701f2e7ddd6065a6e2 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 May 2016 17:37:48 -0700 Subject: [PATCH 43/71] Add a test for a CSR with no domains --- certbot/tests/cli_test.py | 6 ++++++ certbot/tests/testdata/csr-nonames.pem | 8 ++++++++ 2 files changed, 14 insertions(+) create mode 100644 certbot/tests/testdata/csr-nonames.pem diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 62ec36d2a..c24da4989 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -362,6 +362,12 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods return assert False, "Expected supplying --csr to fail with default verb" + def test_csr_with_no_domains(self): + self.assertRaises( + errors.Error, self._call, + 'certonly --csr {0}'.format( + test_util.vector_path('csr-nonames.pem')).split()) + def _get_argument_parser(self): plugins = disco.PluginsRegistry.find_all() return functools.partial(cli.prepare_and_parse_args, plugins) diff --git a/certbot/tests/testdata/csr-nonames.pem b/certbot/tests/testdata/csr-nonames.pem new file mode 100644 index 000000000..abe1029ca --- /dev/null +++ b/certbot/tests/testdata/csr-nonames.pem @@ -0,0 +1,8 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIH/MIGqAgEAMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw +HwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwXDANBgkqhkiG9w0BAQEF +AANLADBIAkEArHVztFHtH92ucFJD/N/HW9AsdRsUuHUBBBDlHwNlRd3fp580rv2+ +6QWE30cWgdmJS86ObRz6lUTor4R0T+3C5QIDAQABoAAwDQYJKoZIhvcNAQELBQAD +QQBt9XLSZ9DGfWcGGaBUTCiSY7lWBegpNlCeo8pK3ydWmKpjcza+j7lF5paph2LH +lKWVQ8+xwYMscGWK0NApHGco +-----END CERTIFICATE REQUEST----- From 5cb0e0f264ef94187b774561329d2ab432e2634e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 20 May 2016 17:41:58 -0700 Subject: [PATCH 44/71] Add test for csr with inconsistent domains --- certbot/tests/cli_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index c24da4989..4ae69e69d 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -368,6 +368,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods 'certonly --csr {0}'.format( test_util.vector_path('csr-nonames.pem')).split()) + def test_csr_with_inconsistent_domains(self): + self.assertRaises( + errors.Error, self._call, + 'certonly -d example.org --csr {0}'.format(CSR).split()) + def _get_argument_parser(self): plugins = disco.PluginsRegistry.find_all() return functools.partial(cli.prepare_and_parse_args, plugins) From fd899d21252b8bebd2729fbbebec216496c57902 Mon Sep 17 00:00:00 2001 From: Nick Le Mouton Date: Mon, 23 May 2016 09:44:06 +1200 Subject: [PATCH 45/71] Fixing package names for Debian Jessie --- docs/using.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/using.rst b/docs/using.rst index b10532259..f9af07613 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -460,7 +460,7 @@ repo, if you have not already done so. Then run: .. code-block:: shell - sudo apt-get install certbot python-certbot-apache -t jessie-backports + sudo apt-get install letsencrypt python-letsencrypt-apache -t jessie-backports **Fedora** From d4de026372780f079af3e6b811da991bad824ffe Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:37:47 -0700 Subject: [PATCH 46/71] Add get_strict_version --- certbot/le_util.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/certbot/le_util.py b/certbot/le_util.py index f5148b949..1e5997d92 100644 --- a/certbot/le_util.py +++ b/certbot/le_util.py @@ -1,6 +1,9 @@ """Utilities for all Certbot.""" import argparse import collections +# distutils.version under virtualenv confuses pylint +# For more info, see: https://github.com/PyCQA/pylint/issues/73 +import distutils.version # pylint: disable=import-error,no-name-in-module import errno import logging import os @@ -342,3 +345,17 @@ def enforce_domain_sanity(domain): if not fqdn.match(domain): raise errors.ConfigurationError("Requested domain {0} is not a FQDN".format(domain)) return domain + + +def get_strict_version(normalized): + """Converts a normalized version to a strict version. + + :param str normalized: normalized version string + + :returns: An equivalent strict version + :rtype: distutils.version.StrictVersion + + """ + # strict version ending with "a" and a number designates a pre-release + # pylint: disable=no-member + return distutils.version.StrictVersion(normalized.replace(".dev", "a")) From 40a0354b360214485fbd19d90e24c9c9d040856c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:50:14 -0700 Subject: [PATCH 47/71] Start get_strict_version tests --- certbot/tests/le_util_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/certbot/tests/le_util_test.py b/certbot/tests/le_util_test.py index b6da4525f..db76615ee 100644 --- a/certbot/tests/le_util_test.py +++ b/certbot/tests/le_util_test.py @@ -339,5 +339,18 @@ class EnforceDomainSanityTest(unittest.TestCase): u"eichh\u00f6rnchen.example.com") +class GetStrictVersionTest(unittest.TestCase): + """Tests for certbot.le_util.get_strict_version.""" + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.le_util import get_strict_version + return get_strict_version(*args, **kwargs) + + def test_two_dev_versions(self): + self.assertTrue( + self._call("0.0.0.dev20151006") < self._call("0.0.0.dev20151008")) + + if __name__ == "__main__": unittest.main() # pragma: no cover From 4caba1fc75749eb6c4f45dd61e314f62e26b962b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:53:09 -0700 Subject: [PATCH 48/71] Test mixed versions --- certbot/tests/le_util_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/certbot/tests/le_util_test.py b/certbot/tests/le_util_test.py index db76615ee..2dcecae2c 100644 --- a/certbot/tests/le_util_test.py +++ b/certbot/tests/le_util_test.py @@ -351,6 +351,10 @@ class GetStrictVersionTest(unittest.TestCase): self.assertTrue( self._call("0.0.0.dev20151006") < self._call("0.0.0.dev20151008")) + def test_one_dev_one_release_version(self): + self.assertTrue(self._call("1.0.0.dev0") < self._call("1.0.0")) + self.assertTrue(self._call("1.0.0") < self._call("1.0.1.dev0")) + if __name__ == "__main__": unittest.main() # pragma: no cover From ac37b9de6fdd5f935df4a5f83e0840f88df24d29 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:55:17 -0700 Subject: [PATCH 49/71] test release version comparison --- certbot/tests/le_util_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/certbot/tests/le_util_test.py b/certbot/tests/le_util_test.py index 2dcecae2c..5bf93a406 100644 --- a/certbot/tests/le_util_test.py +++ b/certbot/tests/le_util_test.py @@ -355,6 +355,11 @@ class GetStrictVersionTest(unittest.TestCase): self.assertTrue(self._call("1.0.0.dev0") < self._call("1.0.0")) self.assertTrue(self._call("1.0.0") < self._call("1.0.1.dev0")) + def test_two_release_versions(self): + self.assertTrue(self._call("0.0.0") < self._call("0.0.1")) + self.assertTrue(self._call("0.0.0") < self._call("0.1.0")) + self.assertTrue(self._call("0.0.0") < self._call("1.0.0")) + if __name__ == "__main__": unittest.main() # pragma: no cover From 536234c5595347a472060beccdd9fd2e83791483 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:59:16 -0700 Subject: [PATCH 50/71] try and catch problems if we do something silly with our version in the future --- certbot/tests/le_util_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/certbot/tests/le_util_test.py b/certbot/tests/le_util_test.py index 5bf93a406..6e4eef0f1 100644 --- a/certbot/tests/le_util_test.py +++ b/certbot/tests/le_util_test.py @@ -10,6 +10,7 @@ import unittest import mock import six +import certbot from certbot import errors @@ -360,6 +361,11 @@ class GetStrictVersionTest(unittest.TestCase): self.assertTrue(self._call("0.0.0") < self._call("0.1.0")) self.assertTrue(self._call("0.0.0") < self._call("1.0.0")) + def test_current_version(self): + current_version = self._call(certbot.__version__) + self.assertTrue(self._call("0.6.0") < current_version) + self.assertTrue(current_version < self._call("99.99.99")) + if __name__ == "__main__": unittest.main() # pragma: no cover From 0e9aec20a76d06895173ef815962447d75bb87d8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 14:04:13 -0700 Subject: [PATCH 51/71] Add CURRENT_VERSION constant --- certbot/storage.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot/storage.py b/certbot/storage.py index c4bfb3e28..e2b26d322 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -8,6 +8,7 @@ import configobj import parsedatetime import pytz +import certbot from certbot import constants from certbot import crypto_util from certbot import errors @@ -17,6 +18,7 @@ from certbot import le_util logger = logging.getLogger(__name__) ALL_FOUR = ("cert", "privkey", "chain", "fullchain") +CURRENT_VERSION = le_util.get_strict_version(certbot.__version__) def config_with_defaults(config=None): From c3c9441a599226efe261d140d547977eb6ac8d71 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 14:09:31 -0700 Subject: [PATCH 52/71] Save version in renewal config file --- certbot/storage.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot/storage.py b/certbot/storage.py index e2b26d322..d4a469ca1 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -65,6 +65,7 @@ def write_renewal_config(o_filename, n_filename, target, relevant_data): """ config = configobj.ConfigObj(o_filename) + config["version"] = certbot.__version__ for kind in ALL_FOUR: config[kind] = target[kind] From 3576d372a67882a584c207b7bf720c10fc8d69c6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 14:32:43 -0700 Subject: [PATCH 53/71] Add warning about parsing old configuration file --- certbot/storage.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/certbot/storage.py b/certbot/storage.py index d4a469ca1..6c13eb844 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -262,6 +262,14 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes "renewal config file {0} is missing a required " "file reference".format(self.configfile)) + conf_version = self.configuration.get("version") + if (conf_version is not None and + le_util.get_strict_version(conf_version) > CURRENT_VERSION): + logger.warning( + "Attempting to parse the version %s renewal configuration " + "file found at %s with version %s of Certbot. This might not " + "work.", conf_version, config_filename, certbot.__version__) + self.cert = self.configuration["cert"] self.privkey = self.configuration["privkey"] self.chain = self.configuration["chain"] From ea53a14b57910bf9749d9304a3e93e02bea36d02 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 14:58:14 -0700 Subject: [PATCH 54/71] test version is stored --- certbot/tests/storage_test.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index be626edc5..aeba5c3ae 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -10,6 +10,7 @@ import configobj import mock import pytz +import certbot from certbot import configuration from certbot import errors from certbot.storage import ALL_FOUR @@ -760,11 +761,14 @@ class RenewableCertTests(BaseRenewableCertTest): with open(temp2, "r") as f: content = f.read() # useful value was updated - assert "useful = new_value" in content + self.assertTrue("useful = new_value" in content) # associated comment was preserved - assert "A useful value" in content + self.assertTrue("A useful value" in content) # useless value was deleted - assert "useless" not in content + self.assertTrue("useless" not in content) + # check version was stored + self.assertTrue("version = {0}".format(certbot.__version__) in content) + if __name__ == "__main__": unittest.main() # pragma: no cover From 15ddf2ea32a4f020b655c7ac4ca6d62cb0cafd50 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 15:16:43 -0700 Subject: [PATCH 55/71] Test init with newer conf file --- certbot/tests/storage_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index aeba5c3ae..b1444f311 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -138,6 +138,18 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertRaises(errors.CertStorageError, storage.RenewableCert, config.filename, self.cli_config) + def test_renewal_newer_version(self): + from certbot import storage + + self._write_out_ex_kinds() + self.config["version"] = "99.99.99" + self.config.write() + + with mock.patch("certbot.storage.logger") as mock_logger: + storage.RenewableCert(self.config.filename, self.cli_config) + self.assertTrue(mock_logger.warning.called) + self.assertTrue("version" in mock_logger.warning.call_args[0][0]) + def test_consistent(self): # pylint: disable=too-many-statements,protected-access oldcert = self.test_rc.cert From 4919814dd17bdb8a7b0100ac89a5196cb4766310 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 15:39:00 -0700 Subject: [PATCH 56/71] Pin old pkginfo version --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 4ee56576b..59da23de4 100644 --- a/setup.py +++ b/setup.py @@ -71,6 +71,7 @@ dev_extras = [ 'nose', 'nosexcover', 'pep8', + 'pkginfo<=1.2.1', 'pylint==1.4.2', # upstream #248 'tox', 'twine', From 2cfcfd6988dc84cf118004f860d926eb024cf1d6 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 23 May 2016 13:18:02 -0700 Subject: [PATCH 57/71] Run Boulder via docker-compose in tests. This removes a lot of setup code we used to need in order to get Boulder to run, and should reduce brittleness of tests based on Boulder changes. This also unblocks Boulder from upgrading to MariaDB 10.1 in integration tests, since changing to 10.1 syntax for user creation would break the current certbot integration tests (which run 10.0). --- .travis.yml | 25 ++++---------- tests/boulder-fetch.sh | 40 +++-------------------- tests/letstest/scripts/boulder_install.sh | 29 +++------------- tests/travis-integration.sh | 15 +++------ 4 files changed, 22 insertions(+), 87 deletions(-) diff --git a/.travis.yml b/.travis.yml index 16b5700e5..172f4c659 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,26 +4,18 @@ cache: directories: - $HOME/.cache/pip -services: - - rabbitmq - - mariadb - # apacheconftest - #- apache2 +# This makes sure we get a host with docker-compose present. +dist: trusty -# http://docs.travis-ci.com/user/ci-environment/#CI-environment-OS -# gimme has to be kept in sync with Boulder's Go version setting in .travis.yml before_install: - 'dpkg -s libaugeas0' - - '[ "xxx$BOULDER_INTEGRATION" = "xxx" ] || eval "$(gimme 1.5.1)"' # using separate envs with different TOXENVs creates 4x1 Travis build # matrix, which allows us to clearly distinguish which component under # test has failed env: global: - - GOPATH=/tmp/go - - PATH=$GOPATH/bin:$PATH - - GO15VENDOREXPERIMENT=1 # Fixes problems with vendor directories + - BOULDERPATH=$GOPATH/src/github.com/letsencrypt/boulder/ matrix: include: @@ -93,7 +85,6 @@ addons: - boulder - boulder-mysql - boulder-rabbitmq - mariadb: "10.0" apt: sources: - augeas @@ -109,13 +100,11 @@ addons: # For certbot-nginx integration testing - nginx-light - openssl - # For Boulder integration testing - - rsyslog # for apacheconftest - #- apache2 - #- libapache2-mod-wsgi - #- libapache2-mod-macro - #- sudo + - apache2 + - libapache2-mod-wsgi + - libapache2-mod-macro + - sudo install: "travis_retry pip install tox coveralls" script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/travis-integration.sh)' diff --git a/tests/boulder-fetch.sh b/tests/boulder-fetch.sh index 0d8a3de38..11e36835a 100755 --- a/tests/boulder-fetch.sh +++ b/tests/boulder-fetch.sh @@ -1,40 +1,10 @@ #!/bin/bash # Download and run Boulder instance for integration testing - -# ugh, go version output is like: -# go version go1.4.2 linux/amd64 -GOVER=`go version | cut -d" " -f3 | cut -do -f2` - -# version comparison -function verlte { - #OS X doesn't support version sorting; emulate with sed - if [ `uname` == 'Darwin' ]; then - [ "$1" = "`echo -e \"$1\n$2\" | sed 's/\b\([0-9]\)\b/0\1/g' \ - | sort | sed 's/\b0\([0-9]\)/\1/g' | head -n1`" ] - else - [ "$1" = "`echo -e "$1\n$2" | sort -V | head -n1`" ] - fi -} - -if ! verlte 1.5 "$GOVER" ; then - echo "We require go version 1.5 or later; you have... $GOVER" - exit 1 -fi - set -xe -# `/...` avoids `no buildable Go source files` errors, for more info -# see `go help packages` -go get -d github.com/letsencrypt/boulder/... -cd $GOPATH/src/github.com/letsencrypt/boulder -# goose is needed for ./test/create_db.sh -wget https://github.com/jsha/boulder-tools/raw/master/goose.gz && \ - mkdir $GOPATH/bin && \ - zcat goose.gz > $GOPATH/bin/goose && \ - chmod +x $GOPATH/bin/goose -./test/create_db.sh -go run cmd/rabbitmq-setup/main.go -server amqp://localhost -# listenbuddy is needed for ./start.py -go get github.com/jsha/listenbuddy -cd - +# Check out special branch until latest docker changes land in Boulder master. +git clone -b rev-rev https://github.com/letsencrypt/boulder $BOULDERPATH +cd $BOULDERPATH +sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml +docker-compose up -d diff --git a/tests/letstest/scripts/boulder_install.sh b/tests/letstest/scripts/boulder_install.sh index b5ddf2c5b..936a64802 100755 --- a/tests/letstest/scripts/boulder_install.sh +++ b/tests/letstest/scripts/boulder_install.sh @@ -2,27 +2,8 @@ # >>>> only tested on Ubuntu 14.04LTS <<<< -# non-interactive install of mariadb and other dependencies -export DEBIAN_FRONTEND=noninteractive -sudo debconf-set-selections <<< 'mariadb-server mysql-server/root_password password PASS' -sudo debconf-set-selections <<< 'mariadb-server mysql-server/root_password_again password PASS' -apt-get -y --no-upgrade install git make libltdl3-dev mariadb-server rabbitmq-server -sudo mysql -uroot -pPASS -e "SET PASSWORD = PASSWORD(\'\');" - -# install go -wget https://storage.googleapis.com/golang/go1.5.1.linux-amd64.tar.gz -tar xzvf go1.5.1.linux-amd64.tar.gz -mkdir gocode -echo "export GOROOT=/home/ubuntu/go \n\ - export GOPATH=/home/ubuntu/gocode\n\ - export PATH=/home/ubuntu/go/bin:/home/ubuntu/gocode/bin:$PATH" >> .bashrc - -# install boulder and its go dependencies -go get -d github.com/letsencrypt/boulder/... -cd $GOPATH/src/github.com/letsencrypt/boulder -wget https://github.com/jsha/boulder-tools/raw/master/goose.gz -mkdir $GOPATH/bin -zcat goose.gz > $GOPATH/bin/goose -chmod +x $GOPATH/bin/goose -./test/create_db.sh -go get github.com/jsha/listenbuddy +# Check out special branch until latest docker changes land in Boulder master. +git clone -b rev-rev https://github.com/letsencrypt/boulder $BOULDERPATH +cd $BOULDERPATH +sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml +docker-compose up -d diff --git a/tests/travis-integration.sh b/tests/travis-integration.sh index 159a2ef80..b42617400 100755 --- a/tests/travis-integration.sh +++ b/tests/travis-integration.sh @@ -6,14 +6,9 @@ set -o errexit source .tox/$TOXENV/bin/activate -export CERTBOT_PATH=`pwd` +until curl http://boulder:4000/directory 2>/dev/null; do + echo waiting for boulder + sleep 1 +done -cd $GOPATH/src/github.com/letsencrypt/boulder/ - -# boulder's integration-test.py has code that knows to start and wait for the -# boulder processes to start reliably and then will run the certbot -# boulder-interation.sh on its own. The --certbot flag says to run only the -# certbot tests (instead of any other client tests it might run). We're -# going to want to define a more robust interaction point between the boulder -# and certbot tests, but that will be better built off of this. -python test/integration-test.py --certbot +./tests/boulder-integration.sh From ed802a664813ff12d4da4874b1591a90c8391df4 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 23 May 2016 18:47:52 -0700 Subject: [PATCH 58/71] Fix BOULDERPATH --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 172f4c659..6f964dbec 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ before_install: # test has failed env: global: - - BOULDERPATH=$GOPATH/src/github.com/letsencrypt/boulder/ + - BOULDERPATH=$PWD/boulder/ matrix: include: From b54497d8145e0999ac3f163ae38e3bc70a7a5549 Mon Sep 17 00:00:00 2001 From: sagi Date: Tue, 24 May 2016 19:33:13 +0000 Subject: [PATCH 59/71] Fix chain filename --- tests/boulder-integration.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index a1245e1c9..323ea004b 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -44,7 +44,7 @@ common auth --csr "$CSR_PATH" \ --cert-path "${root}/csr/cert.pem" \ --chain-path "${root}/csr/chain.pem" openssl x509 -in "${root}/csr/cert.pem" -text -openssl x509 -in "${root}/csr/0000_chain.pem" -text +openssl x509 -in "${root}/csr/chain.pem" -text common --domains le3.wtf install \ --cert-path "${root}/csr/cert.pem" \ From b1eff0fe3528f0cef2a077b6dcee777d1dbebb8c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 May 2016 13:03:53 -0700 Subject: [PATCH 60/71] Build le-auto to bring it up to date --- letsencrypt-auto-source/letsencrypt-auto | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index eb5561070..ea085454c 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -425,7 +425,8 @@ BootstrapMac() { $pkgcmd augeas $pkgcmd dialog - if [ "$(which python)" = "/System/Library/Frameworks/Python.framework/Versions/2.7/bin/python" ]; then + if [ "$(which python)" = "/System/Library/Frameworks/Python.framework/Versions/2.7/bin/python" \ + -o "$(which python)" = "/usr/bin/python" ]; then # We want to avoid using the system Python because it requires root to use pip. # python.org, MacPorts or HomeBrew Python installations should all be OK. echo "Installing python..." From 70bb7ff68f2a9eb5fac7b6cc494a50dce99ade20 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 May 2016 13:08:10 -0700 Subject: [PATCH 61/71] fixes #3060 --- letsencrypt-auto-source/letsencrypt-auto | 3 +-- letsencrypt-auto-source/letsencrypt-auto.template | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index ea085454c..2de4b053e 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -935,6 +935,7 @@ else if [ "$NO_SELF_UPGRADE" != 1 ]; then TEMP_DIR=$(TempDir) + trap 'rm -rf "$TEMP_DIR"' EXIT # --------------------------------------------------------------------------- cat << "UNLIKELY_EOF" > "$TEMP_DIR/fetch.py" """Do downloading and JSON parsing without additional dependencies. :: @@ -1089,8 +1090,6 @@ UNLIKELY_EOF # filesystems is non-atomic, doing `rm dest, cp src dest, rm src`, but the # cp is unlikely to fail (esp. under sudo) if the rm doesn't. $SUDO mv -f "$TEMP_DIR/letsencrypt-auto.permission-clone" "$0" - # TODO: Clean up temp dir safely, even if it has quotes in its path. - rm -rf "$TEMP_DIR" fi # A newer version is available. fi # Self-upgrading is allowed. diff --git a/letsencrypt-auto-source/letsencrypt-auto.template b/letsencrypt-auto-source/letsencrypt-auto.template index f1ed82c4c..116894a93 100755 --- a/letsencrypt-auto-source/letsencrypt-auto.template +++ b/letsencrypt-auto-source/letsencrypt-auto.template @@ -291,6 +291,7 @@ else if [ "$NO_SELF_UPGRADE" != 1 ]; then TEMP_DIR=$(TempDir) + trap 'rm -rf "$TEMP_DIR"' EXIT # --------------------------------------------------------------------------- cat << "UNLIKELY_EOF" > "$TEMP_DIR/fetch.py" {{ fetch.py }} @@ -319,8 +320,6 @@ UNLIKELY_EOF # filesystems is non-atomic, doing `rm dest, cp src dest, rm src`, but the # cp is unlikely to fail (esp. under sudo) if the rm doesn't. $SUDO mv -f "$TEMP_DIR/letsencrypt-auto.permission-clone" "$0" - # TODO: Clean up temp dir safely, even if it has quotes in its path. - rm -rf "$TEMP_DIR" fi # A newer version is available. fi # Self-upgrading is allowed. From c606273d1489a27c50376fca6244968a4ccde06a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 May 2016 13:16:21 -0700 Subject: [PATCH 62/71] use TEMP_DIR trap consistently --- letsencrypt-auto-source/letsencrypt-auto | 2 +- letsencrypt-auto-source/letsencrypt-auto.template | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 2de4b053e..b65c29a44 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -532,6 +532,7 @@ if [ "$1" = "--le-auto-phase2" ]; then echo "Installing Python packages..." TEMP_DIR=$(TempDir) + trap 'rm -rf "$TEMP_DIR"' EXIT # There is no $ interpolation due to quotes on starting heredoc delimiter. # ------------------------------------------------------------------------- cat << "UNLIKELY_EOF" > "$TEMP_DIR/letsencrypt-auto-requirements.txt" @@ -889,7 +890,6 @@ UNLIKELY_EOF PIP_OUT=`"$VENV_BIN/pip" install --no-cache-dir --require-hashes -r "$TEMP_DIR/letsencrypt-auto-requirements.txt" 2>&1` PIP_STATUS=$? set -e - rm -rf "$TEMP_DIR" if [ "$PIP_STATUS" != 0 ]; then # Report error. (Otherwise, be quiet.) echo "Had a problem while installing Python packages:" diff --git a/letsencrypt-auto-source/letsencrypt-auto.template b/letsencrypt-auto-source/letsencrypt-auto.template index 116894a93..43d8bc7e1 100755 --- a/letsencrypt-auto-source/letsencrypt-auto.template +++ b/letsencrypt-auto-source/letsencrypt-auto.template @@ -229,6 +229,7 @@ if [ "$1" = "--le-auto-phase2" ]; then echo "Installing Python packages..." TEMP_DIR=$(TempDir) + trap 'rm -rf "$TEMP_DIR"' EXIT # There is no $ interpolation due to quotes on starting heredoc delimiter. # ------------------------------------------------------------------------- cat << "UNLIKELY_EOF" > "$TEMP_DIR/letsencrypt-auto-requirements.txt" @@ -245,7 +246,6 @@ UNLIKELY_EOF PIP_OUT=`"$VENV_BIN/pip" install --no-cache-dir --require-hashes -r "$TEMP_DIR/letsencrypt-auto-requirements.txt" 2>&1` PIP_STATUS=$? set -e - rm -rf "$TEMP_DIR" if [ "$PIP_STATUS" != 0 ]; then # Report error. (Otherwise, be quiet.) echo "Had a problem while installing Python packages:" From 420e64f03951bbce7737d0b0ce05fad9070763db Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 May 2016 13:43:45 -0700 Subject: [PATCH 63/71] Simplify webroot chown and rm errors --- certbot/plugins/webroot.py | 22 +++++---------------- certbot/plugins/webroot_test.py | 35 ++------------------------------- 2 files changed, 7 insertions(+), 50 deletions(-) diff --git a/certbot/plugins/webroot.py b/certbot/plugins/webroot.py index c344954ae..508000d77 100644 --- a/certbot/plugins/webroot.py +++ b/certbot/plugins/webroot.py @@ -181,12 +181,8 @@ to serve all files under specified web root ({0}).""" os.chown(self.full_roots[name], stat_path.st_uid, stat_path.st_gid) except OSError as exception: - if exception.errno == errno.EACCES: - logger.debug("Insufficient permissions to change owner and uid - ignoring") - else: - raise errors.PluginError( - "Couldn't create root for {0} http-01 " - "challenge responses: {1}", name, exception) + logger.debug("Unable to change owner and uid of webroot directory") + logger.debug("Error was: %s", exception) except OSError as exception: if exception.errno != errno.EEXIST: @@ -235,17 +231,9 @@ to serve all files under specified web root ({0}).""" logger.debug("All challenges cleaned up, removing %s", root_path) except OSError as exc: - if exc.errno == errno.ENOTEMPTY: - logger.debug("Challenges cleaned up but %s not empty", - root_path) - elif exc.errno == errno.EACCES: - logger.debug("Challenges cleaned up but no permissions for %s", - root_path) - elif exc.errno == errno.ENOENT: - logger.debug("Challenges cleaned up, %s does not exists", - root_path) - else: - raise + logger.debug( + "Unable to clean up challenge directory %s", root_path) + logger.debug("Error was: %s", exc) class _WebrootMapAction(argparse.Action): diff --git a/certbot/plugins/webroot_test.py b/certbot/plugins/webroot_test.py index ab2a9e9a4..5d784a75c 100644 --- a/certbot/plugins/webroot_test.py +++ b/certbot/plugins/webroot_test.py @@ -138,15 +138,10 @@ class AuthenticatorTest(unittest.TestCase): os.chmod(self.path, 0o700) @mock.patch("certbot.plugins.webroot.os.chown") - def test_failed_chown_eacces(self, mock_chown): + def test_failed_chown(self, mock_chown): mock_chown.side_effect = OSError(errno.EACCES, "msg") self.auth.perform([self.achall]) # exception caught and logged - @mock.patch("certbot.plugins.webroot.os.chown") - def test_failed_chown_not_eacces(self, mock_chown): - mock_chown.side_effect = OSError() - self.assertRaises(errors.PluginError, self.auth.perform, []) - def test_perform_permissions(self): self.auth.prepare() @@ -200,7 +195,7 @@ class AuthenticatorTest(unittest.TestCase): os.rmdir(leftover_path) @mock.patch('os.rmdir') - def test_cleanup_permission_denied(self, mock_rmdir): + def test_cleanup_failure(self, mock_rmdir): self.auth.prepare() self.auth.perform([self.achall]) @@ -212,32 +207,6 @@ class AuthenticatorTest(unittest.TestCase): self.assertFalse(os.path.exists(self.validation_path)) self.assertTrue(os.path.exists(self.root_challenge_path)) - @mock.patch('os.rmdir') - def test_cleanup_oserror(self, mock_rmdir): - self.auth.prepare() - self.auth.perform([self.achall]) - - os_error = OSError() - os_error.errno = errno.EPERM - mock_rmdir.side_effect = os_error - - self.assertRaises(OSError, self.auth.cleanup, [self.achall]) - self.assertFalse(os.path.exists(self.validation_path)) - self.assertTrue(os.path.exists(self.root_challenge_path)) - - @mock.patch('os.rmdir') - def test_cleanup_file_not_exists(self, mock_rmdir): - self.auth.prepare() - self.auth.perform([self.achall]) - - os_error = OSError() - os_error.errno = errno.ENOENT - mock_rmdir.side_effect = os_error - - self.auth.cleanup([self.achall]) - self.assertFalse(os.path.exists(self.validation_path)) - self.assertTrue(os.path.exists(self.root_challenge_path)) - class WebrootActionTest(unittest.TestCase): """Tests for webroot argparse actions.""" From c01e2c259af0e200222e66bdf27434f3dba47613 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 24 May 2016 15:38:03 -0700 Subject: [PATCH 64/71] Check out Boulder master instead of branch. --- tests/boulder-fetch.sh | 2 +- tests/letstest/scripts/boulder_install.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/boulder-fetch.sh b/tests/boulder-fetch.sh index 11e36835a..a09d0adf9 100755 --- a/tests/boulder-fetch.sh +++ b/tests/boulder-fetch.sh @@ -3,7 +3,7 @@ set -xe # Check out special branch until latest docker changes land in Boulder master. -git clone -b rev-rev https://github.com/letsencrypt/boulder $BOULDERPATH +git clone https://github.com/letsencrypt/boulder $BOULDERPATH cd $BOULDERPATH sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml docker-compose up -d diff --git a/tests/letstest/scripts/boulder_install.sh b/tests/letstest/scripts/boulder_install.sh index 936a64802..426642880 100755 --- a/tests/letstest/scripts/boulder_install.sh +++ b/tests/letstest/scripts/boulder_install.sh @@ -3,7 +3,7 @@ # >>>> only tested on Ubuntu 14.04LTS <<<< # Check out special branch until latest docker changes land in Boulder master. -git clone -b rev-rev https://github.com/letsencrypt/boulder $BOULDERPATH +git clone https://github.com/letsencrypt/boulder $BOULDERPATH cd $BOULDERPATH sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml docker-compose up -d From 57e6d1995bebf2a700f83e1dfaff2c626d6d8d18 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 May 2016 16:32:03 -0700 Subject: [PATCH 65/71] log louder --- certbot/plugins/webroot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot/plugins/webroot.py b/certbot/plugins/webroot.py index 508000d77..624ee2ff4 100644 --- a/certbot/plugins/webroot.py +++ b/certbot/plugins/webroot.py @@ -181,7 +181,7 @@ to serve all files under specified web root ({0}).""" os.chown(self.full_roots[name], stat_path.st_uid, stat_path.st_gid) except OSError as exception: - logger.debug("Unable to change owner and uid of webroot directory") + logger.info("Unable to change owner and uid of webroot directory") logger.debug("Error was: %s", exception) except OSError as exception: @@ -231,7 +231,7 @@ to serve all files under specified web root ({0}).""" logger.debug("All challenges cleaned up, removing %s", root_path) except OSError as exc: - logger.debug( + logger.info( "Unable to clean up challenge directory %s", root_path) logger.debug("Error was: %s", exc) From e93aeb88dd4da8e26fd6a4c257234ba990aae403 Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 25 May 2016 04:19:18 +0000 Subject: [PATCH 66/71] Fix docs --- certbot/client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/certbot/client.py b/certbot/client.py index 514da879e..9ce326822 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -580,7 +580,8 @@ def _open_pem_file(cli_arg_path, pem_path): :param str cli_arg_path: the cli arg name, e.g. cert_path :param str pem_path: the pem file path to open - :returns a file object + :returns: a tuple of file object and its absolute file path + """ if cli.set_by_cli(cli_arg_path): return le_util.safe_open(pem_path, chmod=0o644),\ From 13e165e2f9ef3af630d6dbe74082aa689f35be31 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 24 May 2016 21:30:45 -0700 Subject: [PATCH 67/71] add exception type --- certbot/tests/reverter_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/tests/reverter_test.py b/certbot/tests/reverter_test.py index c79c72a48..85234b76a 100644 --- a/certbot/tests/reverter_test.py +++ b/certbot/tests/reverter_test.py @@ -39,7 +39,7 @@ class ReverterCheckpointLocalTest(unittest.TestCase): mock_read.side_effect = OSError("cannot even") try: self.reverter.add_to_checkpoint(self.sets[0], "save1") - except: + except OSError: pass self.reverter.finalize_checkpoint("blah") path = os.listdir(self.reverter.config.backup_dir)[0] From 4a85d59d9353e7a0567c0bc56c9729d75b96d209 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 May 2016 22:03:30 -0700 Subject: [PATCH 68/71] Add test for missing renewal conf version --- certbot/tests/storage_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index b1444f311..f19b7d89d 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -138,6 +138,16 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertRaises(errors.CertStorageError, storage.RenewableCert, config.filename, self.cli_config) + def test_no_renewal_version(self): + from certbot import storage + + self._write_out_ex_kinds() + self.assertTrue("version" not in self.config) + + with mock.patch("certbot.storage.logger") as mock_logger: + storage.RenewableCert(self.config.filename, self.cli_config) + self.assertFalse(mock_logger.warning.called) + def test_renewal_newer_version(self): from certbot import storage From d1df72d63c7eb9e590d05651f0f1a41caf234a1d Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 25 May 2016 19:06:25 +0000 Subject: [PATCH 69/71] Add the chain_cert is None case --- certbot/client.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 9ce326822..ba31f8760 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -328,7 +328,9 @@ class Client(object): logger.info("Server issued certificate; certificate written to %s", abs_cert_path) - if chain_cert: + if not chain_cert: + return abs_cert_path, None, None + else: chain_pem = crypto_util.dump_pyopenssl_chain(chain_cert) chain_file, abs_chain_path =\ @@ -339,7 +341,7 @@ class Client(object): _save_chain(chain_pem, chain_file) _save_chain(cert_pem + chain_pem, fullchain_file) - return abs_cert_path, abs_chain_path, abs_fullchain_path + return abs_cert_path, abs_chain_path, abs_fullchain_path def deploy_certificate(self, domains, privkey_path, cert_path, chain_path, fullchain_path): From 94588b1a9175af5f5943bfb8b9485c3bab376a5c Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 25 May 2016 14:43:10 -0700 Subject: [PATCH 70/71] Check out a specific version of Boulder. A recent Boulder change broke integration tests, this fixes it. --- tests/boulder-fetch.sh | 2 +- tests/letstest/scripts/boulder_install.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/boulder-fetch.sh b/tests/boulder-fetch.sh index a09d0adf9..01f236575 100755 --- a/tests/boulder-fetch.sh +++ b/tests/boulder-fetch.sh @@ -3,7 +3,7 @@ set -xe # Check out special branch until latest docker changes land in Boulder master. -git clone https://github.com/letsencrypt/boulder $BOULDERPATH +git clone -b 71e4af43f792f33e6ab1aa87ddc23a6a368889f2 https://github.com/letsencrypt/boulder $BOULDERPATH cd $BOULDERPATH sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml docker-compose up -d diff --git a/tests/letstest/scripts/boulder_install.sh b/tests/letstest/scripts/boulder_install.sh index 426642880..0d6153a2d 100755 --- a/tests/letstest/scripts/boulder_install.sh +++ b/tests/letstest/scripts/boulder_install.sh @@ -3,7 +3,7 @@ # >>>> only tested on Ubuntu 14.04LTS <<<< # Check out special branch until latest docker changes land in Boulder master. -git clone https://github.com/letsencrypt/boulder $BOULDERPATH +git clone -b 71e4af43f792f33e6ab1aa87ddc23a6a368889f2 https://github.com/letsencrypt/boulder $BOULDERPATH cd $BOULDERPATH sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml docker-compose up -d From 0fb3704dcedf66f67b517b22833564f00cf74c48 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 25 May 2016 15:43:54 -0700 Subject: [PATCH 71/71] Use a real branch name. --- tests/boulder-fetch.sh | 2 +- tests/letstest/scripts/boulder_install.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/boulder-fetch.sh b/tests/boulder-fetch.sh index 01f236575..469c5cd80 100755 --- a/tests/boulder-fetch.sh +++ b/tests/boulder-fetch.sh @@ -3,7 +3,7 @@ set -xe # Check out special branch until latest docker changes land in Boulder master. -git clone -b 71e4af43f792f33e6ab1aa87ddc23a6a368889f2 https://github.com/letsencrypt/boulder $BOULDERPATH +git clone -b docker-integration https://github.com/letsencrypt/boulder $BOULDERPATH cd $BOULDERPATH sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml docker-compose up -d diff --git a/tests/letstest/scripts/boulder_install.sh b/tests/letstest/scripts/boulder_install.sh index 0d6153a2d..7e298783f 100755 --- a/tests/letstest/scripts/boulder_install.sh +++ b/tests/letstest/scripts/boulder_install.sh @@ -3,7 +3,7 @@ # >>>> only tested on Ubuntu 14.04LTS <<<< # Check out special branch until latest docker changes land in Boulder master. -git clone -b 71e4af43f792f33e6ab1aa87ddc23a6a368889f2 https://github.com/letsencrypt/boulder $BOULDERPATH +git clone -b docker-integration https://github.com/letsencrypt/boulder $BOULDERPATH cd $BOULDERPATH sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml docker-compose up -d