From 424acfe16e542b28e2aabe66ccee796b1d548be5 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 21 May 2015 18:58:40 -0700 Subject: [PATCH 1/6] Fixes to running on command line. Use cert_dir instead of cert_path Restore server_url When creating a unique file, only loop for EEXISTS, not other OS errors like permission denied. Pass uid explicitly to make_or_verify_dir. --- letsencrypt/client.py | 12 ++++++------ letsencrypt/configuration.py | 4 ++++ letsencrypt/crypto_util.py | 2 +- letsencrypt/le_util.py | 6 ++++-- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 8b6bbf2e2..e2b5cf2df 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -138,22 +138,22 @@ class Client(object): # Save Certificate cert_path, chain_path = self.save_certificate( - certr, self.config.cert_path, self.config.chain_path) + certr, self.config.cert_dir, self.config.cert_dir) revoker.Revoker.store_cert_key( cert_path, self.account.key.file, self.config) return cert_key, cert_path, chain_path - def save_certificate(self, certr, cert_path, chain_path): + def save_certificate(self, certr, cert_dir, chain_dir): # pylint: disable=no-self-use """Saves the certificate received from the ACME server. :param certr: ACME "certificate" resource. :type certr: :class:`acme.messages.Certificate` - :param str cert_path: Path to attempt to save the cert file - :param str chain_path: Path to attempt to save the chain file + :param str cert_dir: Path to attempt to save the cert file + :param str chain_dir: Path to attempt to save the chain file :returns: cert_path, chain_path (absolute paths to the actual files) :rtype: `tuple` of `str` @@ -163,7 +163,7 @@ class Client(object): """ # try finally close cert_chain_abspath = None - cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) + cert_file, act_cert_path = le_util.unique_file(cert_dir, 0o644) # TODO: Except cert_pem = certr.body.as_pem() try: @@ -178,7 +178,7 @@ class Client(object): chain_cert = self.network.fetch_chain(certr) if chain_cert is not None: chain_file, act_chain_path = le_util.unique_file( - chain_path, 0o644) + chain_dir, 0o644) chain_pem = chain_cert.as_pem() try: chain_file.write(chain_pem) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 6a808a6a9..9fba3047a 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -44,6 +44,10 @@ class NamespaceConfig(object): def in_progress_dir(self): # pylint: disable=missing-docstring return os.path.join(self.namespace.work_dir, constants.IN_PROGRESS_DIR) + @property + def server_url(self): + return self.namespace.server + @property def server_path(self): """File path based on ``server``.""" diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index 94617eef6..6fb6adbdc 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -72,7 +72,7 @@ def init_save_csr(privkey, names, cert_dir, csrname="csr-letsencrypt.pem"): csr_pem, csr_der = make_csr(privkey.pem, names) # Save CSR - le_util.make_or_verify_dir(cert_dir, 0o755) + le_util.make_or_verify_dir(cert_dir, 0o755, os.geteuid()) csr_f, csr_filename = le_util.unique_file( os.path.join(cert_dir, csrname), 0o644) csr_f.write(csr_pem) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 27d795749..cab13965e 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -70,8 +70,10 @@ def unique_file(path, mode=0o777): try: file_d = os.open(fname, os.O_CREAT | os.O_EXCL | os.O_RDWR, mode) return os.fdopen(file_d, "w"), fname - except OSError: - pass + except OSError, e: + # Errno 17, "File exists," is okay. + if e.errno != 17: + raise count += 1 From e1e6135313885e4e8a163cec3f4678c342bd5a07 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 21 May 2015 19:08:05 -0700 Subject: [PATCH 2/6] Use a different port for test mode. --- letsencrypt/plugins/standalone/authenticator.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index 7d5c3ea6e..d93a10008 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -378,7 +378,10 @@ class StandaloneAuthenticator(common.Plugin): results_if_failure.append(False) if not self.tasks: raise ValueError("nothing for .perform() to do") - if self.already_listening(challenges.DVSNI.PORT): + port = challenges.DVSNI.PORT + if self.config.test_mode: + port = 5001 + if self.already_listening(port): # If we know a process is already listening on this port, # tell the user, and don't even attempt to bind it. (This # test is Linux-specific and won't indicate that the port @@ -386,7 +389,7 @@ class StandaloneAuthenticator(common.Plugin): return results_if_failure # Try to do the authentication; note that this creates # the listener subprocess via os.fork() - if self.start_listener(challenges.DVSNI.PORT, key): + if self.start_listener(port, key): return results_if_success else: # TODO: This should probably raise a DVAuthError exception From 969c2c052d2f92bcc9aafd4417d21721b8e7881b Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 21 May 2015 23:44:13 -0700 Subject: [PATCH 3/6] Re-remove server_url. --- letsencrypt/configuration.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 9fba3047a..6a808a6a9 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -44,10 +44,6 @@ class NamespaceConfig(object): def in_progress_dir(self): # pylint: disable=missing-docstring return os.path.join(self.namespace.work_dir, constants.IN_PROGRESS_DIR) - @property - def server_url(self): - return self.namespace.server - @property def server_path(self): """File path based on ``server``.""" From 8562496f82f64771b12b7b4140ea605c94541bf4 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 22 May 2015 13:06:17 -0700 Subject: [PATCH 4/6] Fixes from review comments. --- letsencrypt/client.py | 12 ++++++------ letsencrypt/constants.py | 4 ++++ letsencrypt/le_util.py | 4 ++-- letsencrypt/plugins/standalone/authenticator.py | 3 ++- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 2f4d02f70..ae1667dfa 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -138,22 +138,22 @@ class Client(object): # Save Certificate cert_path, chain_path = self.save_certificate( - certr, self.config.cert_dir, self.config.cert_dir) + certr, self.config.cert_path, self.config.chain_path) revoker.Revoker.store_cert_key( cert_path, self.account.key.file, self.config) return cert_key, cert_path, chain_path - def save_certificate(self, certr, cert_dir, chain_dir): + def save_certificate(self, certr, cert_path, chain_path): # pylint: disable=no-self-use """Saves the certificate received from the ACME server. :param certr: ACME "certificate" resource. :type certr: :class:`acme.messages.Certificate` - :param str cert_dir: Path to attempt to save the cert file - :param str chain_dir: Path to attempt to save the chain file + :param str cert_path: Path to attempt to save the cert file + :param str chain_path: Path to attempt to save the chain file :returns: cert_path, chain_path (absolute paths to the actual files) :rtype: `tuple` of `str` @@ -163,7 +163,7 @@ class Client(object): """ # try finally close cert_chain_abspath = None - cert_file, act_cert_path = le_util.unique_file(cert_dir, 0o644) + cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) # TODO: Except cert_pem = certr.body.as_pem() try: @@ -178,7 +178,7 @@ class Client(object): chain_cert = self.network.fetch_chain(certr) if chain_cert is not None: chain_file, act_chain_path = le_util.unique_file( - chain_dir, 0o644) + chain_path, 0o644) chain_pem = chain_cert.as_pem() try: chain_file.write(chain_pem) diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 9ff0b128c..9bd33726d 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -71,3 +71,7 @@ IConfig.work_dir).""" NETSTAT = "/bin/netstat" """Location of netstat binary for checking whether a listener is already running on the specified port (Linux-specific).""" + +BOULDER_TEST_MODE_CHALLENGE_PORT = 5001 +"""Port that Boulder will connect on for validations in test mode.""" + diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index cab13965e..eacdfe341 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -71,8 +71,8 @@ def unique_file(path, mode=0o777): file_d = os.open(fname, os.O_CREAT | os.O_EXCL | os.O_RDWR, mode) return os.fdopen(file_d, "w"), fname except OSError, e: - # Errno 17, "File exists," is okay. - if e.errno != 17: + # "File exists," is okay, try a different name. + if exception.errno != errno.EEXIST: raise count += 1 diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index d93a10008..b124a490a 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -15,6 +15,7 @@ import zope.interface from acme import challenges from letsencrypt import achallenges +from letsencrypt import constants from letsencrypt import interfaces from letsencrypt.plugins import common @@ -380,7 +381,7 @@ class StandaloneAuthenticator(common.Plugin): raise ValueError("nothing for .perform() to do") port = challenges.DVSNI.PORT if self.config.test_mode: - port = 5001 + port = constants.BOULDER_TEST_MODE_CHALLENGE_PORT if self.already_listening(port): # If we know a process is already listening on this port, # tell the user, and don't even attempt to bind it. (This From 1a5d6ba90d230b0b90efdd7988fc6415fb427316 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 22 May 2015 13:16:55 -0700 Subject: [PATCH 5/6] Use more verbose exception catch. --- letsencrypt/le_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index eacdfe341..2e85de1a0 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -70,7 +70,7 @@ def unique_file(path, mode=0o777): try: file_d = os.open(fname, os.O_CREAT | os.O_EXCL | os.O_RDWR, mode) return os.fdopen(file_d, "w"), fname - except OSError, e: + except OSError as exception: # "File exists," is okay, try a different name. if exception.errno != errno.EEXIST: raise From 958b6b10483c37d8fcd4983e79435157b8b38073 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 22 May 2015 13:57:41 -0700 Subject: [PATCH 6/6] Only check config if it is defined. --- letsencrypt/plugins/standalone/authenticator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index b124a490a..3947c1e3e 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -380,7 +380,7 @@ class StandaloneAuthenticator(common.Plugin): if not self.tasks: raise ValueError("nothing for .perform() to do") port = challenges.DVSNI.PORT - if self.config.test_mode: + if self.config and self.config.test_mode: port = constants.BOULDER_TEST_MODE_CHALLENGE_PORT if self.already_listening(port): # If we know a process is already listening on this port,