From df51f7f50c0737b1453ae293ab509e0eb12eda42 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 2 Dec 2015 15:15:08 -0800 Subject: [PATCH 001/123] Version 0.1.0 for Public Beta! --- letsencrypt/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/__init__.py b/letsencrypt/__init__.py index 1155a5b0c..a2cc7d31a 100644 --- a/letsencrypt/__init__.py +++ b/letsencrypt/__init__.py @@ -1,4 +1,4 @@ """Let's Encrypt client.""" # version number like 1.2.3a0, must have at least 2 parts, like 1.2 -__version__ = '0.1.0.dev0' +__version__ = '0.1.0' From 5747ab7fd9641986833bad474d71b46a8c589247 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 2 Dec 2015 15:55:43 -0800 Subject: [PATCH 002/123] Release 0.1.0 --- acme/setup.py | 2 +- letsencrypt-apache/setup.py | 2 +- letsencrypt-nginx/setup.py | 2 +- letshelp-letsencrypt/setup.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index a6551a023..1889ec020 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.1.0' install_requires = [ # load_pem_private/public_key (>=0.6) diff --git a/letsencrypt-apache/setup.py b/letsencrypt-apache/setup.py index e4dd11935..3b994c0ef 100644 --- a/letsencrypt-apache/setup.py +++ b/letsencrypt-apache/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.1.0' install_requires = [ 'acme=={0}'.format(version), diff --git a/letsencrypt-nginx/setup.py b/letsencrypt-nginx/setup.py index a669ad841..93986c10e 100644 --- a/letsencrypt-nginx/setup.py +++ b/letsencrypt-nginx/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.1.0' install_requires = [ 'acme=={0}'.format(version), diff --git a/letshelp-letsencrypt/setup.py b/letshelp-letsencrypt/setup.py index 04b879e14..a5a069c55 100644 --- a/letshelp-letsencrypt/setup.py +++ b/letshelp-letsencrypt/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.1.0' install_requires = [ 'setuptools', # pkg_resources From 047ae326f6f901fd2c1bc1ab448248da992018c1 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 2 Dec 2015 19:04:31 -0800 Subject: [PATCH 003/123] Bump anticipated release version --- letsencrypt/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/__init__.py b/letsencrypt/__init__.py index a2cc7d31a..e011c3f9b 100644 --- a/letsencrypt/__init__.py +++ b/letsencrypt/__init__.py @@ -1,4 +1,4 @@ """Let's Encrypt client.""" # version number like 1.2.3a0, must have at least 2 parts, like 1.2 -__version__ = '0.1.0' +__version__ = '0.1.1' From 55d4365a46cef56228851ac9f7e6c47846eaff88 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 21:30:05 -0800 Subject: [PATCH 004/123] Expect a fixed standalone challenge preference --- letsencrypt/plugins/standalone_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/standalone_test.py b/letsencrypt/plugins/standalone_test.py index 26a040c2e..91b7b56a0 100644 --- a/letsencrypt/plugins/standalone_test.py +++ b/letsencrypt/plugins/standalone_test.py @@ -104,8 +104,8 @@ class AuthenticatorTest(unittest.TestCase): self.assertTrue(isinstance(self.auth.more_info(), six.string_types)) def test_get_chall_pref(self): - self.assertEqual(set(self.auth.get_chall_pref(domain=None)), - set([challenges.TLSSNI01, challenges.HTTP01])) + self.assertEqual(self.auth.get_chall_pref(domain=None), + [challenges.TLSSNI01, challenges.HTTP01]) @mock.patch("letsencrypt.plugins.standalone.util") def test_perform_alredy_listening(self, mock_util): From 5054a3dd79b752b9839ba85f120dc43d452078d1 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 21:57:31 -0800 Subject: [PATCH 005/123] Fix typo in test name --- letsencrypt/plugins/standalone_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone_test.py b/letsencrypt/plugins/standalone_test.py index 91b7b56a0..b4aac76b2 100644 --- a/letsencrypt/plugins/standalone_test.py +++ b/letsencrypt/plugins/standalone_test.py @@ -108,7 +108,7 @@ class AuthenticatorTest(unittest.TestCase): [challenges.TLSSNI01, challenges.HTTP01]) @mock.patch("letsencrypt.plugins.standalone.util") - def test_perform_alredy_listening(self, mock_util): + def test_perform_already_listening(self, mock_util): for chall, port in ((challenges.TLSSNI01.typ, 1234), (challenges.HTTP01.typ, 4321)): mock_util.already_listening.return_value = True From 144a678473fd3d2634092adcb72b411c1a3274fb Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 22:01:55 -0800 Subject: [PATCH 006/123] Encode challenge preference order in constant --- letsencrypt/plugins/standalone.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 8b8612fd1..9efb5d301 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -108,7 +108,7 @@ class ServerManager(object): in six.iteritems(self._instances)) -SUPPORTED_CHALLENGES = set([challenges.TLSSNI01, challenges.HTTP01]) +SUPPORTED_CHALLENGES = [challenges.TLSSNI01, challenges.HTTP01] def supported_challenges_validator(data): From a0142dbe44e2e8f1530fa35361b044232ec91b2c Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 22:13:05 -0800 Subject: [PATCH 007/123] Don't randomize challenge preference --- letsencrypt/plugins/standalone.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 9efb5d301..496ea7ded 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -198,9 +198,7 @@ class Authenticator(common.Plugin): def get_chall_pref(self, domain): # pylint: disable=unused-argument,missing-docstring - chall_pref = list(self.supported_challenges) - random.shuffle(chall_pref) # 50% for each challenge - return chall_pref + return SUPPORTED_CHALLENGES def perform(self, achalls): # pylint: disable=missing-docstring if any(util.already_listening(port) for port in self._necessary_ports): From fa558715983956eefec8ed41fab3208a390afea4 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 22:14:32 -0800 Subject: [PATCH 008/123] Remove dead import --- letsencrypt/plugins/standalone.py | 1 - 1 file changed, 1 deletion(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 496ea7ded..dc52cf5e7 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -2,7 +2,6 @@ import argparse import collections import logging -import random import socket import threading From d5511971aa4ea64d67c7181aa16720fce51481c4 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 22:50:32 -0800 Subject: [PATCH 009/123] Update plugin help string --- letsencrypt/plugins/standalone.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index dc52cf5e7..3cd8cd95f 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -165,10 +165,10 @@ class Authenticator(common.Plugin): @classmethod def add_parser_arguments(cls, add): - add("supported-challenges", help="Supported challenges, " - "order preferences are randomly chosen.", - type=supported_challenges_validator, default=",".join( - sorted(chall.typ for chall in SUPPORTED_CHALLENGES))) + add("supported-challenges", + help="Supported challenges. Prefers tls-sni-01.", + type=supported_challenges_validator, + default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES)) @property def supported_challenges(self): From 54c74a6d2fa6cc134676758142075db43f738b2a Mon Sep 17 00:00:00 2001 From: Anselm Levskaya Date: Wed, 2 Dec 2015 23:06:11 -0800 Subject: [PATCH 010/123] fix sudo issue on amazon linux instance with letsencrypt-auto the letsencrypt-auto script was missing the sudo parameter on call to ExperimentalBootstrap for amazon linux. also added comment to mac entry to clarify why it lacks the parameter --- letsencrypt-auto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index c88028b72..44c71883c 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -147,9 +147,9 @@ then elif uname | grep -iq FreeBSD ; then ExperimentalBootstrap "FreeBSD" freebsd.sh "$SUDO" elif uname | grep -iq Darwin ; then - ExperimentalBootstrap "Mac OS X" mac.sh + ExperimentalBootstrap "Mac OS X" mac.sh # homebrew doesn't normally run as root elif grep -iq "Amazon Linux" /etc/issue ; then - ExperimentalBootstrap "Amazon Linux" _rpm_common.sh + ExperimentalBootstrap "Amazon Linux" _rpm_common.sh "$SUDO" else echo "Sorry, I don't know how to bootstrap Let's Encrypt on your operating system!" echo From c7dbf8aa24ca08cc977b5bdef3003eeeee3513aa Mon Sep 17 00:00:00 2001 From: Marius Gedminas Date: Wed, 2 Dec 2015 16:12:47 +0200 Subject: [PATCH 011/123] Avoid trailing whitespace in pretty-printed JSON Fixes a failing test on Python 3.3: ====================================================================== FAIL: test_json_dumps_pretty (acme.jose.interfaces_test.JSONDeSerializableTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/mg/src/letsencrypt/acme/acme/jose/interfaces_test.py", line 97, in test_json_dumps_pretty '[\n "foo1",{0}\n "foo2"\n]'.format(filler)) AssertionError: '[\n "foo1", \n "foo2"\n]' != '[\n "foo1",\n "foo2"\n]' [ - "foo1", ? - + "foo1", "foo2" ] ---------------------------------------------------------------------- (The test expected trailing whitespace on Python < 3.0, while it should've been checking for Python < 3.4.) --- acme/acme/jose/interfaces.py | 2 +- acme/acme/jose/interfaces_test.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/acme/acme/jose/interfaces.py b/acme/acme/jose/interfaces.py index f85777a30..f841848b3 100644 --- a/acme/acme/jose/interfaces.py +++ b/acme/acme/jose/interfaces.py @@ -194,7 +194,7 @@ class JSONDeSerializable(object): :rtype: str """ - return self.json_dumps(sort_keys=True, indent=4) + return self.json_dumps(sort_keys=True, indent=4, separators=(',', ': ')) @classmethod def json_dump_default(cls, python_object): diff --git a/acme/acme/jose/interfaces_test.py b/acme/acme/jose/interfaces_test.py index 84dc2a1be..cf98ff371 100644 --- a/acme/acme/jose/interfaces_test.py +++ b/acme/acme/jose/interfaces_test.py @@ -1,8 +1,6 @@ """Tests for acme.jose.interfaces.""" import unittest -import six - class JSONDeSerializableTest(unittest.TestCase): # pylint: disable=too-many-instance-attributes @@ -92,9 +90,8 @@ class JSONDeSerializableTest(unittest.TestCase): self.assertEqual('["foo1", "foo2"]', self.seq.json_dumps()) def test_json_dumps_pretty(self): - filler = ' ' if six.PY2 else '' self.assertEqual(self.seq.json_dumps_pretty(), - '[\n "foo1",{0}\n "foo2"\n]'.format(filler)) + '[\n "foo1",\n "foo2"\n]') def test_json_dump_default(self): from acme.jose.interfaces import JSONDeSerializable From 8cf47e3aba7dba8090ef8c18c325ff5177dc42ed Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 3 Dec 2015 00:13:09 -0800 Subject: [PATCH 012/123] Add tests to check that configuration is used The existing tests use the case in which the (configured) supported challenges are equal to the defaults, and in the same (now-fixed) order. These additional tests check that, if we have configured a subset of the supported challenges, then we actually _use_ that configuration. --- letsencrypt/plugins/standalone_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/letsencrypt/plugins/standalone_test.py b/letsencrypt/plugins/standalone_test.py index b4aac76b2..1833a55fe 100644 --- a/letsencrypt/plugins/standalone_test.py +++ b/letsencrypt/plugins/standalone_test.py @@ -100,6 +100,11 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(self.auth.supported_challenges, set([challenges.TLSSNI01, challenges.HTTP01])) + def test_supported_challenges_configured(self): + self.config.standalone_supported_challenges = "tls-sni-01" + self.assertEqual(self.auth.supported_challenges, + set([challenges.TLSSNI01])) + def test_more_info(self): self.assertTrue(isinstance(self.auth.more_info(), six.string_types)) @@ -107,6 +112,11 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(self.auth.get_chall_pref(domain=None), [challenges.TLSSNI01, challenges.HTTP01]) + def test_get_chall_pref_configured(self): + self.config.standalone_supported_challenges = "tls-sni-01" + self.assertEqual(self.auth.get_chall_pref(domain=None), + [challenges.TLSSNI01]) + @mock.patch("letsencrypt.plugins.standalone.util") def test_perform_already_listening(self, mock_util): for chall, port in ((challenges.TLSSNI01.typ, 1234), From dbf181ebacd9edf00196dbcfc85fdf2794e2f3c9 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 3 Dec 2015 00:28:12 -0800 Subject: [PATCH 013/123] Respect config when stating challenge preferences --- letsencrypt/plugins/standalone.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 3cd8cd95f..1bca3c036 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -197,7 +197,8 @@ class Authenticator(common.Plugin): def get_chall_pref(self, domain): # pylint: disable=unused-argument,missing-docstring - return SUPPORTED_CHALLENGES + return [chall for chall in SUPPORTED_CHALLENGES + if chall in self.supported_challenges] def perform(self, achalls): # pylint: disable=missing-docstring if any(util.already_listening(port) for port in self._necessary_ports): From 7a6e084e3ab4ae82c55acd9535d1a758985e96ec Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 3 Dec 2015 15:55:17 +0000 Subject: [PATCH 014/123] Unbreak master --- acme/setup.py | 2 +- letsencrypt-apache/setup.py | 2 +- letsencrypt-compatibility-test/setup.py | 2 +- letsencrypt-nginx/setup.py | 2 +- letsencrypt/__init__.py | 2 +- letshelp-letsencrypt/setup.py | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index 1889ec020..e35b40d6e 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0' +version = '0.2.0.dev0' install_requires = [ # load_pem_private/public_key (>=0.6) diff --git a/letsencrypt-apache/setup.py b/letsencrypt-apache/setup.py index 3b994c0ef..58008e1e4 100644 --- a/letsencrypt-apache/setup.py +++ b/letsencrypt-apache/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0' +version = '0.2.0.dev0' install_requires = [ 'acme=={0}'.format(version), diff --git a/letsencrypt-compatibility-test/setup.py b/letsencrypt-compatibility-test/setup.py index c791d51c4..eb7e23036 100644 --- a/letsencrypt-compatibility-test/setup.py +++ b/letsencrypt-compatibility-test/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.2.0.dev0' install_requires = [ 'letsencrypt=={0}'.format(version), diff --git a/letsencrypt-nginx/setup.py b/letsencrypt-nginx/setup.py index 93986c10e..1d42fe488 100644 --- a/letsencrypt-nginx/setup.py +++ b/letsencrypt-nginx/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0' +version = '0.2.0.dev0' install_requires = [ 'acme=={0}'.format(version), diff --git a/letsencrypt/__init__.py b/letsencrypt/__init__.py index e011c3f9b..1c7815f78 100644 --- a/letsencrypt/__init__.py +++ b/letsencrypt/__init__.py @@ -1,4 +1,4 @@ """Let's Encrypt client.""" # version number like 1.2.3a0, must have at least 2 parts, like 1.2 -__version__ = '0.1.1' +__version__ = '0.2.0.dev0' diff --git a/letshelp-letsencrypt/setup.py b/letshelp-letsencrypt/setup.py index a5a069c55..d487e556d 100644 --- a/letshelp-letsencrypt/setup.py +++ b/letshelp-letsencrypt/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0' +version = '0.2.0.dev0' install_requires = [ 'setuptools', # pkg_resources From 0004610e610b3491a190b074ddaa20e592ef3d93 Mon Sep 17 00:00:00 2001 From: Gene Wood Date: Thu, 3 Dec 2015 12:40:10 -0800 Subject: [PATCH 015/123] Fixing the grammar of the _suggest_donate message --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 9835fa126..3652f828f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -306,7 +306,7 @@ def _report_new_cert(cert_path, fullchain_path): def _suggest_donate(): "Suggest a donation to support Let's Encrypt" reporter_util = zope.component.getUtility(interfaces.IReporter) - msg = ("If like Let's Encrypt, please consider supporting our work by:\n\n" + msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n" "Donating to ISRG / Let's Encrypt: https://letsencrypt.org/donate\n" "Donating to EFF: https://eff.org/donate-le\n\n") reporter_util.add_message(msg, reporter_util.LOW_PRIORITY) From ad5352e8cced72fd6f208eea4e96575bb60bf4e2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 3 Dec 2015 12:54:32 -0800 Subject: [PATCH 016/123] Upstream augeas fix for backslashes at the start of directive args From: https://github.com/hercules-team/augeas/pull/325 Fixes: #1531 --- letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug index 30d8ca501..83d97f7a4 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug +++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug @@ -59,7 +59,7 @@ let empty = Util.empty_dos let indent = Util.indent (* borrowed from shellvars.aug *) -let char_arg_dir = /([^\\ '"\t\r\n]|[^\\ '"\t\r\n][^ '"\t\r\n]*[^\\ '"\t\r\n])|\\\\"|\\\\'/ +let char_arg_dir = /([^\\ '"\t\r\n]|[^ '"\t\r\n]+[^\\ '"\t\r\n])|\\\\"|\\\\'/ let char_arg_sec = /[^ '"\t\r\n>]|\\\\"|\\\\'/ let cdot = /\\\\./ let cl = /\\\\\n/ From 55d51530d916acc8b418c7b79c3bd8d03d56fa2a Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 3 Dec 2015 12:56:29 -0800 Subject: [PATCH 017/123] This should fix parsing of drupal .htaccess files --- .../{failing => passing}/drupal-htaccess-1531.conf | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/apache-conf-files/{failing => passing}/drupal-htaccess-1531.conf (100%) diff --git a/tests/apache-conf-files/failing/drupal-htaccess-1531.conf b/tests/apache-conf-files/passing/drupal-htaccess-1531.conf similarity index 100% rename from tests/apache-conf-files/failing/drupal-htaccess-1531.conf rename to tests/apache-conf-files/passing/drupal-htaccess-1531.conf From 5a39e833c4982085ed88562dd08050966bf7590e Mon Sep 17 00:00:00 2001 From: Alex Conlin Date: Thu, 3 Dec 2015 22:14:43 +0000 Subject: [PATCH 018/123] Fix typo in README.rst --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 018b343fd..f25dc1956 100644 --- a/README.rst +++ b/README.rst @@ -128,7 +128,7 @@ launch. The client requires root access in order to write to bind to ports 80 and 443 (if you use the ``standalone`` plugin) and to read and modify webserver configurations (if you use the ``apache`` or ``nginx`` plugins). If none of these apply to you, it is theoretically possible to run -without root privilegess, but for most users who want to avoid running an ACME +without root privileges, but for most users who want to avoid running an ACME client as root, either `letsencrypt-nosudo `_ or `simp_le `_ are more appropriate choices. From 3add88c64173b6b551018c9939e89a9153c39955 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 3 Dec 2015 15:25:54 -0800 Subject: [PATCH 019/123] Add another apache conf test case --- .../failing/two-blocks-one-line-1693.conf | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 tests/apache-conf-files/failing/two-blocks-one-line-1693.conf diff --git a/tests/apache-conf-files/failing/two-blocks-one-line-1693.conf b/tests/apache-conf-files/failing/two-blocks-one-line-1693.conf new file mode 100644 index 000000000..5d3cef423 --- /dev/null +++ b/tests/apache-conf-files/failing/two-blocks-one-line-1693.conf @@ -0,0 +1,28 @@ + + + ServerAdmin info@somethingnewentertainment.com + ServerName somethingnewentertainment.com + DocumentRoot /var/www/html + + ErrorLog /var/log/apache2/error.log + CustomLog /var/log/apache2/access.log combined + + SSLEngine on + SSLProtocol all -SSLv2 -SSLv3 + SSLHonorCipherOrder on + SSLCipherSuite "EECDH+ECDSA+AESGCM EECDH+aRSA+AESGCM EECDH+ECDSA+SHA384 EEC DH+ECDSA+SHA256 EECDH+aRSA+SHA384 EECDH+aRSA+SHA256 EECDH+aRSA+RC4 EECDH EDH+aRS A RC4 !aNULL !eNULL !LOW !3DES !MD5 !EXP !PSK !SRP !DSS !RC4" + + SSLCertificateFile /etc/ssl/certs/ssl-cert-snakeoil.pem + SSLCertificateKeyFile /etc/ssl/private/ssl-cert-snakeoil.key + + + SSLOptions +StdEnvVars + + + SSLOptions +StdEnvVars + + BrowserMatch "MSIE [2-6]" \ + nokeepalive ssl-unclean-shutdown \ + downgrade-1.0 force-response-1.0 + BrowserMatch "MSIE [17-9]" ssl-unclean-shutdown + From a4396b89a7ab89635845816d4d05cddccfb963df Mon Sep 17 00:00:00 2001 From: j Date: Thu, 3 Dec 2015 19:14:21 +0100 Subject: [PATCH 020/123] Remove ! at end of url (fixes open url in gnome-terminal) The ! at the end of the url is parsed as part of the url if one uses "Open Link" in gnome-terminal. --- letsencrypt/display/ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 038ad6fdc..5c8c543b0 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -245,7 +245,7 @@ def success_installation(domains): """ util(interfaces.IDisplay).notification( - "Congratulations! You have successfully enabled {0}!{1}{1}" + "Congratulations! You have successfully enabled {0}{1}{1}" "You should test your configuration at:{1}{2}".format( _gen_https_names(domains), os.linesep, From 3a4d36e062c1f1f086685479fc1e0809c196a5b5 Mon Sep 17 00:00:00 2001 From: lord63 Date: Fri, 4 Dec 2015 10:21:07 +0800 Subject: [PATCH 021/123] Fix typo in README.rst and docs/using.rst --- README.rst | 2 +- docs/using.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index f25dc1956..d1f5d3428 100644 --- a/README.rst +++ b/README.rst @@ -27,7 +27,7 @@ If ``letsencrypt`` is packaged for your OS, you can install it from there, and run it by typing ``letsencrypt``. Because not all operating systems have packages yet, we provide a temporary solution via the ``letsencrypt-auto`` wrapper script, which obtains some dependencies from your OS and puts others -in an python virtual environment:: +in a python virtual environment:: user@webserver:~$ git clone https://github.com/letsencrypt/letsencrypt user@webserver:~$ cd letsencrypt diff --git a/docs/using.rst b/docs/using.rst index b546e3005..211eb78c8 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -286,7 +286,7 @@ get support on our `forums `_. If you find a bug in the software, please do report it in our `issue tracker `_. Remember to -give us us as much information as possible: +give us as much information as possible: - copy and paste exact command line used and the output (though mind that the latter might include some personally identifiable From 869c3741c51126a98187eb6cce2d5ea751a35416 Mon Sep 17 00:00:00 2001 From: Marius Gedminas Date: Fri, 4 Dec 2015 12:03:33 +0200 Subject: [PATCH 022/123] Typo: Apacche -> Apache --- letsencrypt-apache/letsencrypt_apache/augeas_lens/README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/README b/letsencrypt-apache/letsencrypt_apache/augeas_lens/README index fc803a776..f801efd43 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/README +++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/README @@ -1,2 +1,2 @@ Let's Encrypt includes the very latest Augeas lenses in order to ship bug fixes -to Apacche configuration handling bugs as quickly as possible +to Apache configuration handling bugs as quickly as possible From b4e0dfe5a8b71f004138ca8fdb7587e325569341 Mon Sep 17 00:00:00 2001 From: Seppe Stas Date: Fri, 4 Dec 2015 11:20:50 +0100 Subject: [PATCH 023/123] Fixed small typo --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index f25dc1956..d1f5d3428 100644 --- a/README.rst +++ b/README.rst @@ -27,7 +27,7 @@ If ``letsencrypt`` is packaged for your OS, you can install it from there, and run it by typing ``letsencrypt``. Because not all operating systems have packages yet, we provide a temporary solution via the ``letsencrypt-auto`` wrapper script, which obtains some dependencies from your OS and puts others -in an python virtual environment:: +in a python virtual environment:: user@webserver:~$ git clone https://github.com/letsencrypt/letsencrypt user@webserver:~$ cd letsencrypt From b9b634b6029012d999ade0e5ed4d8a20eaeb3460 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 4 Dec 2015 11:42:08 -0800 Subject: [PATCH 024/123] Merge another augeas lens fix From: https://github.com/hercules-team/augeas/pull/329 Fixes: https://github.com/letsencrypt/letsencrypt/issues/1294#issuecomment-161805063 --- letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug index 30d8ca501..6d15486f8 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug +++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug @@ -51,7 +51,7 @@ let sep_osp = Sep.opt_space let sep_eq = del /[ \t]*=[ \t]*/ "=" let nmtoken = /[a-zA-Z:_][a-zA-Z0-9:_.-]*/ -let word = /[a-zA-Z][a-zA-Z0-9._-]*/ +let word = /[a-z][a-z0-9._-]*/i let comment = Util.comment let eol = Util.doseol From 5ccfb7c37bc6683a1da0c80fef6261cf96f3013b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 4 Dec 2015 11:50:38 -0800 Subject: [PATCH 025/123] Add another failing case --- .../failing/missing-double-quote-1724.conf | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/apache-conf-files/failing/missing-double-quote-1724.conf diff --git a/tests/apache-conf-files/failing/missing-double-quote-1724.conf b/tests/apache-conf-files/failing/missing-double-quote-1724.conf new file mode 100644 index 000000000..7d97b23d0 --- /dev/null +++ b/tests/apache-conf-files/failing/missing-double-quote-1724.conf @@ -0,0 +1,52 @@ + + ServerAdmin webmaster@localhost + ServerAlias www.example.com + ServerName example.com + DocumentRoot /var/www/example.com/www/ + SSLEngine on + + SSLProtocol all -SSLv2 -SSLv3 + SSLCipherSuite "EECDH+ECDSA+AESGCM EECDH+aRSA+AESGCM EECDH+ECDSA+SHA384 EECDH+ECDSA+SHA256 EECDH+aRS$ + SSLCertificateFile /etc/ssl/certs/ssl-cert-snakeoil.pem + SSLCertificateKeyFile /etc/ssl/private/ssl-cert-snakeoil.key + + + Options FollowSymLinks + AllowOverride All + + + Options Indexes FollowSymLinks MultiViews + AllowOverride All + Order allow,deny + allow from all + # This directive allows us to have apache2's default start page + # in /apache2-default/, but still have / go to the right place + + + ScriptAlias /cgi-bin/ /usr/lib/cgi-bin/ + + AllowOverride None + Options +ExecCGI -MultiViews +SymLinksIfOwnerMatch + Order allow,deny + Allow from all + + + ErrorLog /var/log/apache2/error.log + + # Possible values include: debug, info, notice, warn, error, crit, + # alert, emerg. + LogLevel warn + + CustomLog /var/log/apache2/access.log combined + ServerSignature On + + Alias /apache_doc/ "/usr/share/doc/" + + Options Indexes MultiViews FollowSymLinks + AllowOverride None + Order deny,allow + Deny from all + Allow from 127.0.0.0/255.0.0.0 ::1/128 + + + From ffa4eebd900a3c5ed177933779a006d385a97151 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Fri, 4 Dec 2015 14:11:08 -0600 Subject: [PATCH 026/123] Correct typo in --register-unsafely-without-email --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 3652f828f..348818368 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -855,7 +855,7 @@ def prepare_and_parse_args(plugins, args): "email address. This is strongly discouraged, because in the " "event of key loss or account compromise you will irrevocably " "lose access to your account. You will also be unable to receive " - "notice about impending expiration of revocation of your " + "notice about impending expiration or revocation of your " "certificates. Updates to the Subscriber Agreement will still " "affect you, and will be effective 14 days after posting an " "update to the web site.") From df49c661247ca1f8adb235e654332dc3fbf92616 Mon Sep 17 00:00:00 2001 From: Travis Raines Date: Fri, 4 Dec 2015 22:22:32 -0800 Subject: [PATCH 027/123] Added a descriptive error if domain list includes a Unicode-encoded IDN --- letsencrypt/configuration.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index a2a54d2d0..f2221bfcb 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -144,6 +144,15 @@ def _check_config_domain_sanity(domains): if any("xn--" in d for d in domains): raise errors.ConfigurationError( "Punycode domains are not supported") + + # Unicode + try: + for domain in domains: + domain.encode('ascii',errors='strict') + except UnicodeDecodeError: + raise errors.ConfigurationError( + "Internationalized domain names are not supported") + # FQDN checks from # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/ # Characters used, domain parts < 63 chars, tld > 1 < 64 chars From 2f71b2c0bee4c2abd76f76cacd1a3cf2ac56c1e9 Mon Sep 17 00:00:00 2001 From: Travis Raines Date: Fri, 4 Dec 2015 22:44:17 -0800 Subject: [PATCH 028/123] fixing whitespace lint and version incompatibility at once! --- letsencrypt/configuration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index f2221bfcb..69778f5f0 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -148,7 +148,7 @@ def _check_config_domain_sanity(domains): # Unicode try: for domain in domains: - domain.encode('ascii',errors='strict') + domain.encode('ascii') except UnicodeDecodeError: raise errors.ConfigurationError( "Internationalized domain names are not supported") From 753022d8e36f2794696a9fde17d3f535961fa3eb Mon Sep 17 00:00:00 2001 From: Gene Wood Date: Sat, 5 Dec 2015 11:02:14 -0800 Subject: [PATCH 029/123] Clarify error messages with acronym DV --- acme/acme/messages.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 0b9ea8105..0b73864ec 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -22,12 +22,14 @@ class Error(jose.JSONObjectWithFields, errors.Error): ('urn:acme:error:' + name, description) for name, description in ( ('badCSR', 'The CSR is unacceptable (e.g., due to a short key)'), ('badNonce', 'The client sent an unacceptable anti-replay nonce'), - ('connection', 'The server could not connect to the client for DV'), + ('connection', 'The server could not connect to the client to ' + 'verify the domain'), ('dnssec', 'The server could not validate a DNSSEC signed domain'), ('malformed', 'The request message was malformed'), ('rateLimited', 'There were too many requests of a given type'), ('serverInternal', 'The server experienced an internal error'), - ('tls', 'The server experienced a TLS error during DV'), + ('tls', 'The server experienced a TLS error during domain ' + 'verification'), ('unauthorized', 'The client lacks sufficient authorization'), ('unknownHost', 'The server could not resolve a domain name'), ) From 55097af38abe271521e791559bb24f3adbd56a80 Mon Sep 17 00:00:00 2001 From: Nelson Elhage Date: Sat, 5 Dec 2015 11:03:58 -0800 Subject: [PATCH 030/123] Document passing domains via config file. closes #1771 --- examples/cli.ini | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/cli.ini b/examples/cli.ini index a20764ed8..c8678f89c 100644 --- a/examples/cli.ini +++ b/examples/cli.ini @@ -11,6 +11,10 @@ server = https://acme-staging.api.letsencrypt.org/directory # Uncomment and update to register with the specified e-mail address # email = foo@example.com +# Uncommon and update to generate certificates for the specified +# domains. +# domains = example.com, www.example.com + # Uncomment to use a text interface instead of ncurses # text = True From cb6ecea087e9a83a8bf5e4452c498f8cdb57f9e1 Mon Sep 17 00:00:00 2001 From: Nelson Elhage Date: Sat, 5 Dec 2015 11:35:54 -0800 Subject: [PATCH 031/123] Fix a typo in example config file. --- examples/cli.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/cli.ini b/examples/cli.ini index c8678f89c..6b6b05d7d 100644 --- a/examples/cli.ini +++ b/examples/cli.ini @@ -11,7 +11,7 @@ server = https://acme-staging.api.letsencrypt.org/directory # Uncomment and update to register with the specified e-mail address # email = foo@example.com -# Uncommon and update to generate certificates for the specified +# Uncomment and update to generate certificates for the specified # domains. # domains = example.com, www.example.com From f2a93e00ea023768592be25c62691cef74be8181 Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Sun, 6 Dec 2015 18:20:11 +0800 Subject: [PATCH 032/123] Mention the --renew-by-default flag I was going crazy looking for this flag - I think it's worth a mention in the Renewal section --- docs/using.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index b546e3005..6e15d2cf2 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -173,10 +173,11 @@ Renewal In order to renew certificates simply call the ``letsencrypt`` (or letsencrypt-auto_) again, and use the same values when prompted. You can automate it slightly by passing necessary flags on the CLI (see -`--help all`), or even further using the :ref:`config-file`. If you're -sure that UI doesn't prompt for any details you can add the command to -``crontab`` (make it less than every 90 days to avoid problems, say -every month). +`--help all`), or even further using the :ref:`config-file`. The +``--renew-by-default`` flag may be helpful for automating renewal. If +you're sure that UI doesn't prompt for any details you can add the +command to ``crontab`` (make it less than every 90 days to avoid +problems, say every month). Please note that the CA will send notification emails to the address you provide if you do not renew certificates that are about to expire. From f1a50b08fb86974e0f907453a498ab7526fce906 Mon Sep 17 00:00:00 2001 From: Sveder Date: Mon, 7 Dec 2015 02:02:47 +0200 Subject: [PATCH 033/123] Changed freenode to link straight to the web IRC page for the letsencrypt channel. --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index d1f5d3428..57908e90f 100644 --- a/README.rst +++ b/README.rst @@ -163,5 +163,5 @@ Current Features * Free and Open Source Software, made with Python. -.. _Freenode: https://freenode.net +.. _Freenode: https://webchat.freenode.net?channels=%23letsencrypt .. _client-dev: https://groups.google.com/a/letsencrypt.org/forum/#!forum/client-dev From 16c81250452bb29655d06fbe926594a7cb2183d1 Mon Sep 17 00:00:00 2001 From: Sachi King Date: Mon, 7 Dec 2015 22:01:08 +1300 Subject: [PATCH 034/123] Use print_function not print_statement Change the print statements used into print functions. The print satement is not valid in Python 3, however the print function is valid in at least 2.6+. --- letsencrypt/cli.py | 16 +++++++++------- letsencrypt/renewer.py | 4 +++- letsencrypt/reporter.py | 12 +++++++----- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 348818368..61cde7a3f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1,4 +1,6 @@ """Let's Encrypt CLI.""" +from __future__ import print_function + # TODO: Sanity check all input. Be sure to avoid shell code etc... # pylint: disable=too-many-lines # (TODO: split this file into main.py and cli.py) @@ -574,7 +576,7 @@ def plugins_cmd(args, config, plugins): # TODO: Use IDisplay rather than print logger.debug("Filtered plugins: %r", filtered) if not args.init and not args.prepare: - print str(filtered) + print(str(filtered)) return filtered.init(config) @@ -582,13 +584,13 @@ def plugins_cmd(args, config, plugins): # TODO: Use IDisplay rather than print logger.debug("Verified plugins: %r", verified) if not args.prepare: - print str(verified) + print(str(verified)) return verified.prepare() available = verified.available() logger.debug("Prepared plugins: %s", available) - print str(available) + print(str(available)) def read_file(filename, mode="rb"): @@ -681,7 +683,7 @@ class HelpfulArgumentParser(object): self.help_arg = max(help1, help2) if self.help_arg is True: # just --help with no topic; avoid argparse altogether - print usage + print(usage) sys.exit(0) self.visible_topics = self.determine_help_topics(self.help_arg) self.groups = {} # elements are added by .add_group() @@ -785,12 +787,12 @@ class HelpfulArgumentParser(object): """ if self.visible_topics[topic]: - #print "Adding visible group " + topic + #print("Adding visible group " + topic) group = self.parser.add_argument_group(topic, **kwargs) self.groups[topic] = group return group else: - #print "Invisible group " + topic + #print("Invisible group " + topic) return self.silent_parser def add_plugin_args(self, plugins): @@ -802,7 +804,7 @@ class HelpfulArgumentParser(object): """ for name, plugin_ep in plugins.iteritems(): parser_or_group = self.add_group(name, description=plugin_ep.description) - #print parser_or_group + #print(parser_or_group) plugin_ep.plugin_cls.inject_parser_options(parser_or_group, name) def determine_help_topics(self, chosen_topic): diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 0a490d447..3996cfe67 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -7,6 +7,8 @@ within lineages of successor certificates, according to configuration. .. todo:: Call new installer API to restart servers after deployment """ +from __future__ import print_function + import argparse import logging import os @@ -169,7 +171,7 @@ def main(cli_args=sys.argv[1:]): constants.CONFIG_DIRS_MODE, uid) for renewal_file in os.listdir(cli_config.renewal_configs_dir): - print "Processing", renewal_file + print("Processing " + renewal_file) try: # TODO: Before trying to initialize the RenewableCert object, # we could check here whether the combination of the config diff --git a/letsencrypt/reporter.py b/letsencrypt/reporter.py index 0905dfa54..c0c7856a7 100644 --- a/letsencrypt/reporter.py +++ b/letsencrypt/reporter.py @@ -1,4 +1,6 @@ """Collects and displays information to the user.""" +from __future__ import print_function + import collections import logging import os @@ -75,8 +77,8 @@ class Reporter(object): no_exception = sys.exc_info()[0] is None bold_on = sys.stdout.isatty() if bold_on: - print le_util.ANSI_SGR_BOLD - print 'IMPORTANT NOTES:' + print(le_util.ANSI_SGR_BOLD) + print('IMPORTANT NOTES:') first_wrapper = textwrap.TextWrapper( initial_indent=' - ', subsequent_indent=(' ' * 3)) next_wrapper = textwrap.TextWrapper( @@ -89,9 +91,9 @@ class Reporter(object): sys.stdout.write(le_util.ANSI_SGR_RESET) bold_on = False lines = msg.text.splitlines() - print first_wrapper.fill(lines[0]) + print(first_wrapper.fill(lines[0])) if len(lines) > 1: - print "\n".join( - next_wrapper.fill(line) for line in lines[1:]) + print("\n".join( + next_wrapper.fill(line) for line in lines[1:])) if bold_on: sys.stdout.write(le_util.ANSI_SGR_RESET) From edf3a4ed732e1123cf257bd9c999c7ca6e468fb4 Mon Sep 17 00:00:00 2001 From: Luca Beltrame Date: Mon, 7 Dec 2015 10:49:24 +0100 Subject: [PATCH 035/123] Make webroot usable also when running as non-root (GH #1795) Thanks to @aburch's suggestions, the logic has been changed: - Set umask before creating folders and files - Leverage os.makedirs' mode option in conjunction with umask The program still tries to change owner / group, but in case of errors it continues gracefully. Tests have been updated, and they pass. --- letsencrypt/plugins/webroot.py | 51 +++++++++++++++++++---------- letsencrypt/plugins/webroot_test.py | 13 +++++--- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 0b81d45b5..392d1fc2c 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -60,23 +60,38 @@ to serve all files under specified web root ({0}).""" logger.debug("Creating root challenges validation dir at %s", self.full_roots[name]) try: - os.makedirs(self.full_roots[name]) - # Set permissions as parent directory (GH #1389) - # We don't use the parameters in makedirs because it - # may not always work + # Change the permissiosn to be writable (GH #1389) + # We also set umask because os.makedirs's mode parameter does + # not always work: # https://stackoverflow.com/questions/5231901/permission-problems-when-creating-a-dir-with-os-makedirs-python + # We set the umask instead of going the chmod route to ensure the client + # can also run as non-root (GH #1795) + stat_path = os.stat(path) - filemode = stat.S_IMODE(stat_path.st_mode) - os.chmod(self.full_roots[name], filemode) - # Set owner and group, too - os.chown(self.full_roots[name], stat_path.st_uid, - stat_path.st_gid) + old_umask = os.umask(0o022) + os.makedirs(self.full_roots[name], 0o0755) + + # Set owner as parent directory if possible + + try: + stat_path = os.stat(path) + 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) except OSError as exception: if exception.errno != errno.EEXIST: raise errors.PluginError( "Couldn't create root for {0} http-01 " "challenge responses: {1}", name, exception) + finally: + os.umask(old_umask) def perform(self, achalls): # pylint: disable=missing-docstring assert self.full_roots, "Webroot plugin appears to be missing webroot map" @@ -95,18 +110,18 @@ to serve all files under specified web root ({0}).""" def _perform_single(self, achall): response, validation = achall.response_and_validation() + path = self._path_for_achall(achall) logger.debug("Attempting to save validation to %s", path) - with open(path, "w") as validation_file: - validation_file.write(validation.encode()) - # Set permissions as parent directory (GH #1389) - parent_path = self.full_roots[achall.domain] - stat_parent_path = os.stat(parent_path) - filemode = stat.S_IMODE(stat_parent_path.st_mode) - # Remove execution bit (not needed for this file) - os.chmod(path, filemode & ~stat.S_IEXEC) - os.chown(path, stat_parent_path.st_uid, stat_parent_path.st_gid) + old_umask = os.umask(0o022) + + try: + with open(path, "w") as validation_file: + # Change permissions to be world-readable, owner-writable (GH #1795) + validation_file.write(validation.encode()) + finally: + os.umask(old_umask) return response diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index e7f96b50d..2e88c1756 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -75,12 +75,17 @@ class AuthenticatorTest(unittest.TestCase): # Remove exec bit from permission check, so that it # matches the file self.auth.perform([self.achall]) - parent_permissions = (stat.S_IMODE(os.stat(self.path).st_mode) & - ~stat.S_IEXEC) + path_permissions = stat.S_IMODE(os.stat(self.validation_path).st_mode) + self.assertEqual(path_permissions, 0o644) - actual_permissions = stat.S_IMODE(os.stat(self.validation_path).st_mode) + # Check permissions of the directories + + for dirpath, dirnames, filenames in os.walk(self.path): + for directory in dirnames: + full_path = os.path.join(dirpath, directory) + dir_permissions = stat.S_IMODE(os.stat(full_path).st_mode) + self.assertEqual(dir_permissions, 0o755) - self.assertEqual(parent_permissions, actual_permissions) parent_gid = os.stat(self.path).st_gid parent_uid = os.stat(self.path).st_uid From 2b942d97b27eab9ef8f5fee263a5dbbd95b432f8 Mon Sep 17 00:00:00 2001 From: Luca Beltrame Date: Mon, 7 Dec 2015 11:17:29 +0100 Subject: [PATCH 036/123] Address review comments - move the umask call before the try/except block - move comment in _prepare_single to the umask call Simplify the code comments, too. Tests still pass. --- letsencrypt/plugins/webroot.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 392d1fc2c..c4072c3f9 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -59,16 +59,19 @@ to serve all files under specified web root ({0}).""" logger.debug("Creating root challenges validation dir at %s", self.full_roots[name]) + + # Change the permissiosn to be writable (GH #1389) + # Umask is used instead of chmod to ensure the client can also + # run as non-root (GH #1795) + old_umask = os.umask(0o022) + try: - # Change the permissiosn to be writable (GH #1389) - # We also set umask because os.makedirs's mode parameter does - # not always work: - # https://stackoverflow.com/questions/5231901/permission-problems-when-creating-a-dir-with-os-makedirs-python - # We set the umask instead of going the chmod route to ensure the client - # can also run as non-root (GH #1795) stat_path = os.stat(path) - old_umask = os.umask(0o022) + # This is coupled with the "umask" call above because + # os.makedirs's "mode" parameter may not always work: + # https://stackoverflow.com/questions/5231901/permission-problems-when-creating-a-dir-with-os-makedirs-python + os.makedirs(self.full_roots[name], 0o0755) # Set owner as parent directory if possible @@ -114,11 +117,11 @@ to serve all files under specified web root ({0}).""" path = self._path_for_achall(achall) logger.debug("Attempting to save validation to %s", path) + # Change permissions to be world-readable, owner-writable (GH #1795) old_umask = os.umask(0o022) try: with open(path, "w") as validation_file: - # Change permissions to be world-readable, owner-writable (GH #1795) validation_file.write(validation.encode()) finally: os.umask(old_umask) From 312669c64d1fc05716cd892d3579b0cf6b51d15b Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Mon, 7 Dec 2015 10:20:03 +0000 Subject: [PATCH 037/123] Merge Augeas lens fix for closing multiple sections on one line From https://github.com/hercules-team/augeas/commit/f44a7a55cc7162beced99659234eb078a8d20e1d Closes: #1693 --- letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug | 2 +- .../{failing => passing}/two-blocks-one-line-1693.conf | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename tests/apache-conf-files/{failing => passing}/two-blocks-one-line-1693.conf (100%) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug index 30d8ca501..dc30464a8 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug +++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug @@ -91,7 +91,7 @@ let section (body:lens) = indent . dels "" ">" . eol ] + [ indent . dels "<" . square kword inner dword . del />[ \t\n\r]*/ ">\n" ] let rec content = section (content|directive) diff --git a/tests/apache-conf-files/failing/two-blocks-one-line-1693.conf b/tests/apache-conf-files/passing/two-blocks-one-line-1693.conf similarity index 100% rename from tests/apache-conf-files/failing/two-blocks-one-line-1693.conf rename to tests/apache-conf-files/passing/two-blocks-one-line-1693.conf From 2d5d4a65c45ea379f847bcd9effd9c05a4b50556 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 15:07:27 +0200 Subject: [PATCH 038/123] Moved domain check to le_util --- letsencrypt/configuration.py | 40 +++--------------------------------- letsencrypt/le_util.py | 34 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 69778f5f0..6de529981 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -8,6 +8,7 @@ import zope.interface from letsencrypt import constants from letsencrypt import errors from letsencrypt import interfaces +from letsencrypt import le_util class NamespaceConfig(object): @@ -123,40 +124,5 @@ def check_config_sanity(config): # Domain checks if config.namespace.domains is not None: - _check_config_domain_sanity(config.namespace.domains) - - -def _check_config_domain_sanity(domains): - """Helper method for check_config_sanity which validates - domain flag values and errors out if the requirements are not met. - - :param domains: List of domains - :type domains: `list` of `string` - :raises ConfigurationError: for invalid domains and cases where Let's - Encrypt currently will not issue certificates - - """ - # Check if there's a wildcard domain - if any(d.startswith("*.") for d in domains): - raise errors.ConfigurationError( - "Wildcard domains are not supported") - # Punycode - if any("xn--" in d for d in domains): - raise errors.ConfigurationError( - "Punycode domains are not supported") - - # Unicode - try: - for domain in domains: - domain.encode('ascii') - except UnicodeDecodeError: - raise errors.ConfigurationError( - "Internationalized domain names are not supported") - - # FQDN checks from - # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/ - # Characters used, domain parts < 63 chars, tld > 1 < 64 chars - # first and last char is not "-" - fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? 1 < 64 chars + # first and last char is not "-" + fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? Date: Mon, 7 Dec 2015 15:37:09 +0200 Subject: [PATCH 039/123] Added domain checks for apache installer --- letsencrypt-apache/letsencrypt_apache/configurator.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 98b0b8820..50e5ed6be 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -369,7 +369,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): vhost_macro = [] for vhost in self.vhosts: - all_names.update(vhost.get_names()) + # Check domains for validity + for name in vhost.get_names(): + try: + le_util.check_domain_sanity(name) + all_names.add(name) + except errors.ConfigurationError: + pass if vhost.modmacro: vhost_macro.append(vhost.filep) From 82f71cba9ba0df6f639c996a64dcd4e5122a39a6 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 16:02:27 +0200 Subject: [PATCH 040/123] Linter fixes --- letsencrypt/configuration.py | 1 - letsencrypt/le_util.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 6de529981..afd5edbe4 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -1,7 +1,6 @@ """Let's Encrypt user-supplied configuration.""" import os import urlparse -import re import zope.interface diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 97f983ea2..e5e252871 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -313,4 +313,4 @@ def check_domain_sanity(domain): # first and last char is not "-" fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? Date: Mon, 7 Dec 2015 16:05:53 +0200 Subject: [PATCH 041/123] Corrected tests to reflect the removal of wildcard domains etc. --- .../letsencrypt_apache/tests/configurator_test.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index fcccfaae2..986b060f5 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -64,7 +64,7 @@ class TwoVhost80Test(util.ApacheTest): mock_getutility.notification = mock.MagicMock(return_value=True) names = self.config.get_all_names() self.assertEqual(names, set( - ["letsencrypt.demo", "encryption-example.demo", "ip-172-30-0-17"])) + ["letsencrypt.demo", "encryption-example.demo"])) @mock.patch("zope.component.getUtility") @mock.patch("letsencrypt_apache.configurator.socket.gethostbyaddr") @@ -82,7 +82,7 @@ class TwoVhost80Test(util.ApacheTest): self.config.vhosts.append(vhost) names = self.config.get_all_names() - self.assertEqual(len(names), 5) + self.assertEqual(len(names), 4) self.assertTrue("zombo.com" in names) self.assertTrue("google.com" in names) self.assertTrue("letsencrypt.demo" in names) @@ -90,10 +90,17 @@ class TwoVhost80Test(util.ApacheTest): def test_add_servernames_alias(self): self.config.parser.add_dir( self.vh_truth[2].path, "ServerAlias", ["*.le.co"]) + self.config.parser.add_dir( + self.vh_truth[0].path, "ServerAlias", ["working.example.com"]) + self.config._add_servernames(self.vh_truth[2]) # pylint: disable=protected-access + self.config._add_servernames(self.vh_truth[0]) # pylint: disable=protected-access self.assertEqual( self.vh_truth[2].get_names(), set(["*.le.co", "ip-172-30-0-17"])) + self.assertEqual( + self.vh_truth[0].get_names(), set(["working.example.com", + "encryption-example.demo"])) def test_get_virtual_hosts(self): """Make sure all vhosts are being properly found. From d81620ccdd0e1eb3a07f3a2013bb8108e336d56b Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Tue, 8 Dec 2015 02:49:59 +0100 Subject: [PATCH 042/123] _rpm_common.sh: minor typo --- bootstrap/_rpm_common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap/_rpm_common.sh b/bootstrap/_rpm_common.sh index b975da444..411d7bd92 100755 --- a/bootstrap/_rpm_common.sh +++ b/bootstrap/_rpm_common.sh @@ -2,7 +2,7 @@ # Tested with: # - Fedora 22, 23 (x64) -# - Centos 7 (x64: onD igitalOcean droplet) +# - Centos 7 (x64: on DigitalOcean droplet) if type dnf 2>/dev/null then From 51a5d7ceb085ac03593adafc63bfb6357d96eb34 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:05:11 +0200 Subject: [PATCH 043/123] Move validation code to main client --- letsencrypt/display/ops.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 038ad6fdc..ca9c8c126 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -186,7 +186,8 @@ def choose_names(installer): logger.debug("No installer, picking names manually") return _choose_names_manually() - names = list(installer.get_all_names()) + domains = list(installer.get_all_names()) + names = get_valid_domains(domains) if not names: manual = util(interfaces.IDisplay).yesno( @@ -207,6 +208,22 @@ def choose_names(installer): else: return [] +def get_valid_domains(self, domains): + """Helper method for choose_names that implements basic checks + on domain names + + :param list domains: Domain names to validate + :return: List of valid domains + :rtype: list + """ + valid_domains = [] + for domain in domains: + try: + le_util.check_domain_sanity(domain) + valid_domains.append(domain) + except errors.ConfigurationError: + continue + return valid_domains def _filter_names(names): """Determine which names the user would like to select from a list. From d8b83bc478ac3ff875a604ce315c16c593e36170 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:05:33 +0200 Subject: [PATCH 044/123] Revert "Corrected tests to reflect the removal of wildcard domains etc." This reverts commit 53a4d0725dbf0c5f728094dc318f491c9478effd. --- .../letsencrypt_apache/tests/configurator_test.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 986b060f5..fcccfaae2 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -64,7 +64,7 @@ class TwoVhost80Test(util.ApacheTest): mock_getutility.notification = mock.MagicMock(return_value=True) names = self.config.get_all_names() self.assertEqual(names, set( - ["letsencrypt.demo", "encryption-example.demo"])) + ["letsencrypt.demo", "encryption-example.demo", "ip-172-30-0-17"])) @mock.patch("zope.component.getUtility") @mock.patch("letsencrypt_apache.configurator.socket.gethostbyaddr") @@ -82,7 +82,7 @@ class TwoVhost80Test(util.ApacheTest): self.config.vhosts.append(vhost) names = self.config.get_all_names() - self.assertEqual(len(names), 4) + self.assertEqual(len(names), 5) self.assertTrue("zombo.com" in names) self.assertTrue("google.com" in names) self.assertTrue("letsencrypt.demo" in names) @@ -90,17 +90,10 @@ class TwoVhost80Test(util.ApacheTest): def test_add_servernames_alias(self): self.config.parser.add_dir( self.vh_truth[2].path, "ServerAlias", ["*.le.co"]) - self.config.parser.add_dir( - self.vh_truth[0].path, "ServerAlias", ["working.example.com"]) - self.config._add_servernames(self.vh_truth[2]) # pylint: disable=protected-access - self.config._add_servernames(self.vh_truth[0]) # pylint: disable=protected-access self.assertEqual( self.vh_truth[2].get_names(), set(["*.le.co", "ip-172-30-0-17"])) - self.assertEqual( - self.vh_truth[0].get_names(), set(["working.example.com", - "encryption-example.demo"])) def test_get_virtual_hosts(self): """Make sure all vhosts are being properly found. From e891624cb1159c498167fa76c8225c3176fcf4b4 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:07:17 +0200 Subject: [PATCH 045/123] Revert "Added domain checks for apache installer" This reverts commit 5dcd5088273dcf4dcb402bcd8a69a655d29fa383. --- letsencrypt-apache/letsencrypt_apache/configurator.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 50e5ed6be..98b0b8820 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -369,13 +369,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): vhost_macro = [] for vhost in self.vhosts: - # Check domains for validity - for name in vhost.get_names(): - try: - le_util.check_domain_sanity(name) - all_names.add(name) - except errors.ConfigurationError: - pass + all_names.update(vhost.get_names()) if vhost.modmacro: vhost_macro.append(vhost.filep) From 3c1c3c3e8dbf6b0ae7e50032c76a25a4fba7f992 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:31:47 +0200 Subject: [PATCH 046/123] Import and non-classmethod fix --- letsencrypt/display/ops.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index ca9c8c126..941a0a114 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -4,6 +4,7 @@ import os import zope.component +from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt.display import util as display_util @@ -208,7 +209,7 @@ def choose_names(installer): else: return [] -def get_valid_domains(self, domains): +def get_valid_domains(domains): """Helper method for choose_names that implements basic checks on domain names From 0fb4f7dc8bde20afc6746b14438aa65c356eff07 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:32:02 +0200 Subject: [PATCH 047/123] Tests for domain validation --- letsencrypt/tests/display/ops_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index b0b905c33..60874a007 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -385,7 +385,17 @@ class ChooseNamesTest(unittest.TestCase): self.assertEqual(self._call(self.mock_install), []) + def test_get_valid_domains(self): + from letsencrypt.display_ops import get_valid_domains + all_valid = ["example.com", "second.example.com", + "also.example.com"] + all_invalid = ["xn--ls8h.tld", "*.wildcard.com", "notFQDN"] + two_valid = ["example.com", "xn--ls8h.tld", "also.example.com"] + self.assertEqual(get_valid_domains(all_valid), all_valid) + self.assertEqual(get_valid_domains(all_invalid), []) + self.assertEqual(len(get_valid_domains(two_valid)), 2) + class SuccessInstallationTest(unittest.TestCase): # pylint: disable=too-few-public-methods """Test the success installation message.""" From 5c8b493eda2e5fa707ed6299d2ee932e7921a467 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:54:56 +0200 Subject: [PATCH 048/123] Whitespace and dot fix --- letsencrypt/tests/display/ops_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index 60874a007..5958b37a1 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -386,7 +386,7 @@ class ChooseNamesTest(unittest.TestCase): self.assertEqual(self._call(self.mock_install), []) def test_get_valid_domains(self): - from letsencrypt.display_ops import get_valid_domains + from letsencrypt.display.ops import get_valid_domains all_valid = ["example.com", "second.example.com", "also.example.com"] all_invalid = ["xn--ls8h.tld", "*.wildcard.com", "notFQDN"] @@ -395,7 +395,7 @@ class ChooseNamesTest(unittest.TestCase): self.assertEqual(get_valid_domains(all_invalid), []) self.assertEqual(len(get_valid_domains(two_valid)), 2) - + class SuccessInstallationTest(unittest.TestCase): # pylint: disable=too-few-public-methods """Test the success installation message.""" From f479497d6c7ab44d3b34dc800bf583e46634dcf7 Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Mon, 7 Dec 2015 10:53:15 +0000 Subject: [PATCH 049/123] Add failing test from ticket #1766 Augeas fails to parse the wordlist (args inside braces) in the SSLRequire directive. --- tests/apache-conf-files/failing/sslrequire-wordlist.conf | 1 + 1 file changed, 1 insertion(+) create mode 100644 tests/apache-conf-files/failing/sslrequire-wordlist.conf diff --git a/tests/apache-conf-files/failing/sslrequire-wordlist.conf b/tests/apache-conf-files/failing/sslrequire-wordlist.conf new file mode 100644 index 000000000..1c06d5497 --- /dev/null +++ b/tests/apache-conf-files/failing/sslrequire-wordlist.conf @@ -0,0 +1 @@ +SSLRequire %{SSL_CLIENT_S_DN_CN} in {"foo@bar.com", "bar@foo.com"} From d7616461676d145ed361588499f1b0c96739ed4a Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Tue, 8 Dec 2015 08:04:10 +0000 Subject: [PATCH 050/123] Merge Augeas lens fix for SSLRequire wordlists From https://github.com/hercules-team/augeas/commit/f86a28d03a5c42a6c58293667a95d7794e30a42f Closes: #1766 --- .../letsencrypt_apache/augeas_lens/httpd.aug | 14 ++++++++++++-- .../{failing => passing}/sslrequire-wordlist.conf | 0 2 files changed, 12 insertions(+), 2 deletions(-) rename tests/apache-conf-files/{failing => passing}/sslrequire-wordlist.conf (100%) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug index 83d97f7a4..f54f9fbaa 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug +++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug @@ -59,8 +59,10 @@ let empty = Util.empty_dos let indent = Util.indent (* borrowed from shellvars.aug *) -let char_arg_dir = /([^\\ '"\t\r\n]|[^ '"\t\r\n]+[^\\ '"\t\r\n])|\\\\"|\\\\'/ +let char_arg_dir = /([^\\ '"{\t\r\n]|[^ '"{\t\r\n]+[^\\ '"\t\r\n])|\\\\"|\\\\'/ let char_arg_sec = /[^ '"\t\r\n>]|\\\\"|\\\\'/ +let char_arg_wl = /([^\\ '"},\t\r\n]|[^ '"},\t\r\n]+[^\\ '"},\t\r\n])/ + let cdot = /\\\\./ let cl = /\\\\\n/ let dquot = @@ -77,11 +79,19 @@ let comp = /[<>=]?=/ let arg_dir = [ label "arg" . store (char_arg_dir+|dquot|squot) ] let arg_sec = [ label "arg" . store (char_arg_sec+|comp|dquot|squot) ] +let arg_wl = [ label "arg" . store (char_arg_wl+|dquot|squot) ] + +(* comma-separated wordlist as permitted in the SSLRequire directive *) +let arg_wordlist = + let wl_start = Util.del_str "{" in + let wl_end = Util.del_str "}" in + let wl_sep = del /[ \t]*,[ \t]*/ ", " + in [ label "wordlist" . wl_start . arg_wl . (wl_sep . arg_wl)* . wl_end ] let argv (l:lens) = l . (sep_spc . l)* let directive = [ indent . label "directive" . store word . - (sep_spc . argv arg_dir)? . eol ] + (sep_spc . argv (arg_dir|arg_wordlist))? . eol ] let section (body:lens) = (* opt_eol includes empty lines *) diff --git a/tests/apache-conf-files/failing/sslrequire-wordlist.conf b/tests/apache-conf-files/passing/sslrequire-wordlist.conf similarity index 100% rename from tests/apache-conf-files/failing/sslrequire-wordlist.conf rename to tests/apache-conf-files/passing/sslrequire-wordlist.conf From 0cb80bf7534660d3df30bc12d4b8350e4468dd24 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 14:23:46 +0200 Subject: [PATCH 051/123] Better test coverage --- letsencrypt/tests/display/ops_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index 5958b37a1..30183b955 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -1,3 +1,4 @@ +# coding=utf-8 """Test letsencrypt.display.ops.""" import os import sys @@ -389,7 +390,8 @@ class ChooseNamesTest(unittest.TestCase): from letsencrypt.display.ops import get_valid_domains all_valid = ["example.com", "second.example.com", "also.example.com"] - all_invalid = ["xn--ls8h.tld", "*.wildcard.com", "notFQDN"] + all_invalid = ["xn--ls8h.tld", "*.wildcard.com", "notFQDN", + "uniçodé.com"] two_valid = ["example.com", "xn--ls8h.tld", "also.example.com"] self.assertEqual(get_valid_domains(all_valid), all_valid) self.assertEqual(get_valid_domains(all_invalid), []) From 62ea74b9e4a66afedcf4625667e082497b719eea Mon Sep 17 00:00:00 2001 From: Ingolf Becker Date: Tue, 8 Dec 2015 13:22:52 +0000 Subject: [PATCH 052/123] Modify apache plugin to work on setups where apache listens to a specific ip --- .../letsencrypt_apache/configurator.py | 50 +++++++++++++------ .../tests/configurator_test.py | 33 ++++++++++++ 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 98b0b8820..76045bee1 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -545,21 +545,43 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Check for Listen # Note: This could be made to also look for ip:443 combo - if not self.parser.find_dir("Listen", port): - logger.debug("No Listen %s directive found. Setting the " - "Apache Server to Listen on port %s", port, port) - - if port == "443": - args = [port] + listens = [self.parser.get_arg(x).split()[0] for x in self.parser.find_dir("Listen")] + # In case no Listens are set (which really is a broken apache config) + if not listens: + listens = ["80"] + for listen in listens: + # For any listen statement, check if the machine also listens on Port 443. + # If not, add such a listen statement. + if len(listen.split(":")) == 1: + # Its listening to all interfaces + if port not in listens: + if port == "443": + args = [port] + else: + # Non-standard ports should specify https protocol + args = [port, "https"] + self.parser.add_dir_to_ifmodssl( + parser.get_aug_path( + self.parser.loc["listen"]), "Listen", args) + self.save_notes += "Added Listen %s directive to %s\n" % ( + port, self.parser.loc["listen"]) + listens.append(port) else: - # Non-standard ports should specify https protocol - args = [port, "https"] - - self.parser.add_dir_to_ifmodssl( - parser.get_aug_path( - self.parser.loc["listen"]), "Listen", args) - self.save_notes += "Added Listen %s directive to %s\n" % ( - port, self.parser.loc["listen"]) + # The Listen statement specifies an ip + _, ip = listen[::-1].split(":", 1) + ip = ip[::-1] + if "%s:%s" %(ip, port) not in listens: + if port == "443": + args = ["%s:%s" %(ip, port)] + else: + # Non-standard ports should specify https protocol + args = ["%s:%s" %(ip, port), "https"] + self.parser.add_dir_to_ifmodssl( + parser.get_aug_path( + self.parser.loc["listen"]), "Listen", args) + self.save_notes += "Added Listen %s:%s directive to %s\n" % ( + ip, port, self.parser.loc["listen"]) + listens.append("%s:%s" %(ip, port)) def make_addrs_sni_ready(self, addrs): """Checks to see if the server is ready for SNI challenges. diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index fcccfaae2..991704144 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -391,6 +391,39 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(mock_add_dir.call_count, 2) + def test_prepare_server_https_named_listen(self): + mock_find = mock.Mock() + mock_find.return_value = ["test1", "test2", "test3"] + mock_get = mock.Mock() + mock_get.side_effect = ["1.2.3.4:80", "[::1]:80", "1.1.1.1:443"] + mock_add_dir = mock.Mock() + mock_enable = mock.Mock() + + self.config.parser.find_dir = mock_find + self.config.parser.get_arg = mock_get + self.config.parser.add_dir_to_ifmodssl = mock_add_dir + self.config.enable_mod = mock_enable + + # Test Listen statements with specific ip listeed + self.config.prepare_server_https("443") + # Should only be 2 here, as the third interface already listens to the correct port + self.assertEqual(mock_add_dir.call_count, 2) + + # Check argument to new Listen statements + self.assertEqual(mock_add_dir.call_args_list[0][0][2], ["1.2.3.4:443"]) + self.assertEqual(mock_add_dir.call_args_list[1][0][2], ["[::1]:443"]) + + # Reset return lists and inputs + mock_add_dir.reset_mock() + mock_get.side_effect = ["1.2.3.4:80", "[::1]:80", "1.1.1.1:443"] + + # Test + self.config.prepare_server_https("8080", temp=True) + self.assertEqual(mock_add_dir.call_count, 3) + self.assertEqual(mock_add_dir.call_args_list[0][0][2], ["1.2.3.4:8080", "https"]) + self.assertEqual(mock_add_dir.call_args_list[1][0][2], ["[::1]:8080", "https"]) + self.assertEqual(mock_add_dir.call_args_list[2][0][2], ["1.1.1.1:8080", "https"]) + def test_make_vhost_ssl(self): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) From bbd6534744c7c8d91a1be0836c6e069a16eab71b Mon Sep 17 00:00:00 2001 From: Ingolf Becker Date: Tue, 8 Dec 2015 17:56:16 +0000 Subject: [PATCH 053/123] Fixed some pep8 formatting --- .../letsencrypt_apache/configurator.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 76045bee1..1d39e7fdf 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -120,7 +120,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.version = version self.vhosts = None self._enhance_func = {"redirect": self._enable_redirect, - "ensure-http-header": self._set_http_header} + "ensure-http-header": self._set_http_header} @property def mod_ssl_conf(self): @@ -570,18 +570,18 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # The Listen statement specifies an ip _, ip = listen[::-1].split(":", 1) ip = ip[::-1] - if "%s:%s" %(ip, port) not in listens: + if "%s:%s" % (ip, port) not in listens: if port == "443": - args = ["%s:%s" %(ip, port)] + args = ["%s:%s" % (ip, port)] else: # Non-standard ports should specify https protocol - args = ["%s:%s" %(ip, port), "https"] + args = ["%s:%s" % (ip, port), "https"] self.parser.add_dir_to_ifmodssl( parser.get_aug_path( self.parser.loc["listen"]), "Listen", args) self.save_notes += "Added Listen %s:%s directive to %s\n" % ( - ip, port, self.parser.loc["listen"]) - listens.append("%s:%s" %(ip, port)) + ip, port, self.parser.loc["listen"]) + listens.append("%s:%s" % (ip, port)) def make_addrs_sni_ready(self, addrs): """Checks to see if the server is ready for SNI challenges. From 006edfdbd1953183d09d8624aeb13bba8e07babe Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 21:08:30 +0200 Subject: [PATCH 054/123] Wording change to error messages --- letsencrypt/le_util.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index e5e252871..7c7f0b7f7 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -294,18 +294,18 @@ def check_domain_sanity(domain): # Check if there's a wildcard domain if domain.startswith("*."): raise errors.ConfigurationError( - "Wildcard domains are not supported") + "Wildcard domains are not presently supported") # Punycode if "xn--" in domain: raise errors.ConfigurationError( - "Punycode domains are not supported") + "Punycode domains are not presently supported") # Unicode try: domain.encode('ascii') except UnicodeDecodeError: raise errors.ConfigurationError( - "Internationalized domain names are not supported") + "Internationalized domain names are not presently supported") # FQDN checks from # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/ From 05723cbb4afb275ffd27e090b7801ea6d42ef5cf Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 21:28:23 +0200 Subject: [PATCH 055/123] Fixed wording --- 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 7c7f0b7f7..fe63c70af 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -294,7 +294,7 @@ def check_domain_sanity(domain): # Check if there's a wildcard domain if domain.startswith("*."): raise errors.ConfigurationError( - "Wildcard domains are not presently supported") + "Wildcard domains are not supported") # Punycode if "xn--" in domain: raise errors.ConfigurationError( From 1d3cb57aef450556cadf9bba52b1bdcf39c710cd Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 9 Dec 2015 10:36:22 +0200 Subject: [PATCH 056/123] Display meaningful error messages in manual domain entry, and give user an option to retry --- letsencrypt/display/ops.py | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 102dbe3a0..1e0585879 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -250,7 +250,41 @@ def _choose_names_manually(): "Please enter in your domain name(s) (comma and/or space separated) ") if code == display_util.OK: - return display_util.separate_list_input(input_) + invalid_domains = dict() + retry_message = "" + try: + domain_list = display_util.separate_list_input(input_) + except UnicodeEncodeError: + domain_list = [] + retry_message = ( + "Internationalized domain names are not presently " + "supported.{0}{0}Would you like to re-enter the " + "names?{0}").format(os.linesep) + + for domain in domain_list: + try: + le_util.check_domain_sanity(domain) + except errors.ConfigurationError as e: + invalid_domains[domain] = e.message + + if len(invalid_domains): + retry_message = ( + "One or more of the entered domain names was not valid:" + "{0}{0}").format(os.linesep) + for domain in invalid_domains: + retry_message = retry_message + "{1}: {2}{0}".format( + os.linesep, domain, invalid_domains[domain]) + retry_message = retry_message + ( + "{0}Would you like to re-enter the names?{0}").format( + os.linesep) + + if retry_message: + # We had error in input + retry = util(interfaces.IDisplay).yesno(retry_message) + if retry: + return _choose_names_manually() + else: + return domain_list return [] From 64f3d518a415e0cffe1690e455dda822036ec31b Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 9 Dec 2015 13:22:39 +0200 Subject: [PATCH 057/123] Test cases for manual domain validation --- letsencrypt/tests/display/ops_test.py | 33 +++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index 30183b955..4adc572aa 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -397,6 +397,39 @@ class ChooseNamesTest(unittest.TestCase): self.assertEqual(get_valid_domains(all_invalid), []) self.assertEqual(len(get_valid_domains(two_valid)), 2) + @mock.patch("letsencrypt.display.ops.util") + def test_choose_manually(self, mock_util): + from letsencrypt.display.ops import _choose_names_manually + from letsencrypt.display import util as display_util + #No retry + mock_util().yesno.return_value = False + #IDN and no retry + mock_util().input.return_value = (display_util.OK, + "uniçodé.com") + self.assertEqual(_choose_names_manually(), []) + #Punycode and no retry + mock_util().input.return_value = (display_util.OK, + "xn--ls8h.tld") + self.assertEqual(_choose_names_manually(), []) + #non-FQDN and no retry + mock_util().input.return_value = (display_util.OK, + "notFQDN") + self.assertEqual(_choose_names_manually(), []) + #Two valid domains + mock_util().input.return_value = (display_util.OK, + ("example.com," + "valid.example.com")) + self.assertEqual(_choose_names_manually(), + ["example.com", "valid.example.com"]) + #Three iterations + mock_util().input.return_value = (display_util.OK, + "notFQDN") + yn = mock.MagicMock() + yn.side_effect = [True, True, False] + mock_util().yesno = yn + _choose_names_manually() + self.assertEqual(mock_util().yesno.call_count, 3) + class SuccessInstallationTest(unittest.TestCase): # pylint: disable=too-few-public-methods From 5348af6559c87a8065cee4e66421243265bf94b5 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 9 Dec 2015 15:03:14 +0200 Subject: [PATCH 058/123] Better coverage and test fixes --- letsencrypt/tests/display/ops_test.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index 4adc572aa..31db47cce 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -400,28 +400,33 @@ class ChooseNamesTest(unittest.TestCase): @mock.patch("letsencrypt.display.ops.util") def test_choose_manually(self, mock_util): from letsencrypt.display.ops import _choose_names_manually - from letsencrypt.display import util as display_util - #No retry + # No retry mock_util().yesno.return_value = False - #IDN and no retry + # IDN and no retry mock_util().input.return_value = (display_util.OK, "uniçodé.com") self.assertEqual(_choose_names_manually(), []) - #Punycode and no retry + # IDN exception with previous mocks + with mock.patch("letsencrypt.display.util") as mock_sl: + uerror = UnicodeEncodeError('mock', u'', + 0, 1, 'mock') + mock_sl.separate_list_input.side_effect = uerror + self.assertEqual(_choose_names_manually(), []) + # Punycode and no retry mock_util().input.return_value = (display_util.OK, "xn--ls8h.tld") self.assertEqual(_choose_names_manually(), []) - #non-FQDN and no retry + # non-FQDN and no retry mock_util().input.return_value = (display_util.OK, "notFQDN") self.assertEqual(_choose_names_manually(), []) - #Two valid domains + # Two valid domains mock_util().input.return_value = (display_util.OK, ("example.com," "valid.example.com")) self.assertEqual(_choose_names_manually(), ["example.com", "valid.example.com"]) - #Three iterations + # Three iterations mock_util().input.return_value = (display_util.OK, "notFQDN") yn = mock.MagicMock() From 1ee9435db75092cb0715548bf33f6f6d5a8fe2d2 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 9 Dec 2015 15:18:04 +0200 Subject: [PATCH 059/123] PEP8 --- letsencrypt/display/ops.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 1e0585879..5ceb7fcfc 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -123,6 +123,7 @@ def pick_configurator( config, default, plugins, question, (interfaces.IAuthenticator, interfaces.IInstaller)) + def get_email(more=False, invalid=False): """Prompt for valid email address. @@ -209,6 +210,7 @@ def choose_names(installer): else: return [] + def get_valid_domains(domains): """Helper method for choose_names that implements basic checks on domain names @@ -226,6 +228,7 @@ def get_valid_domains(domains): continue return valid_domains + def _filter_names(names): """Determine which names the user would like to select from a list. From e9a0c90e0f56839b109ccbd2bd34076370ee2036 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 9 Dec 2015 16:37:23 +0200 Subject: [PATCH 060/123] Travis bump --- travis_bump.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 travis_bump.txt diff --git a/travis_bump.txt b/travis_bump.txt new file mode 100644 index 000000000..f43876b53 --- /dev/null +++ b/travis_bump.txt @@ -0,0 +1 @@ +bump From dae52e8db387ba44111340f633ac70d54f4f2c3f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 9 Dec 2015 16:37:36 +0200 Subject: [PATCH 061/123] Revert "Travis bump" This reverts commit e9a0c90e0f56839b109ccbd2bd34076370ee2036. --- travis_bump.txt | 1 - 1 file changed, 1 deletion(-) delete mode 100644 travis_bump.txt diff --git a/travis_bump.txt b/travis_bump.txt deleted file mode 100644 index f43876b53..000000000 --- a/travis_bump.txt +++ /dev/null @@ -1 +0,0 @@ -bump From 80901a52d8fe198467b0fb9ec9e65c7a8ae59e9c Mon Sep 17 00:00:00 2001 From: Remi Rampin Date: Wed, 9 Dec 2015 12:26:23 -0500 Subject: [PATCH 062/123] Fix help string for --apache-dismod --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 1d39e7fdf..0b40a7e38 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -93,7 +93,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): add("enmod", default=constants.CLI_DEFAULTS["enmod"], help="Path to the Apache 'a2enmod' binary.") add("dismod", default=constants.CLI_DEFAULTS["dismod"], - help="Path to the Apache 'a2enmod' binary.") + help="Path to the Apache 'a2dismod' binary.") add("le-vhost-ext", default=constants.CLI_DEFAULTS["le_vhost_ext"], help="SSL vhost configuration extension.") add("server-root", default=constants.CLI_DEFAULTS["server_root"], From d5849c3416087efeefa3fd6f8f71aeac0229cc56 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 9 Dec 2015 10:45:28 -0800 Subject: [PATCH 063/123] WIP on cli.py --- letsencrypt/cli.py | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 348818368..457661d1b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -219,6 +219,17 @@ def _treat_as_renewal(config, domains): :raises .Error: If the user would like to rerun the client again. """ + # TODO: This now needs to return 3 different cases plus the .Error + # case: reinstall, renew, fresh cert + # Probably the return value should be a tuple of (result, cert) + # like ("reinstall", RenewableCert_instance) + # ("renew", RenewableCert_instance) + # ("newcert", None) + # We could also return ("cancel", None) instead of raising the + # error, but the error-handling mechanism seems to work + # TODO: Find out whether a RenewableCert instance is enough information + # for the installer to try to reinstall it when we return "reinstall" + # TODO: Also address superset case renewal = False # Considering the possibility that the requested certificate is @@ -234,9 +245,21 @@ def _treat_as_renewal(config, domains): if ident_names_cert is not None: question = ( "You have an existing certificate that contains exactly the " - "same domains you requested (ref: {0}){br}{br}Do you want to " - "renew and replace this certificate with a newly-issued one?" + "same domains you requested (ref: {0}){br}{br}Note that the " + "Let's Encrypt certificate authority limits the number of " + "certificates that can be issued for the same domain name per " + "week!{br}{br}Do you want to reinstall this existing " + "certificate, or renew and replace this certificate with a " + "newly-issued one?" ).format(ident_names_cert.configfile.filename, br=os.linesep) + print(zope.component.getUtility(interfaces.IDisplay).menu( + question, ["Attempt to reinstall this existing certificate", + "Obtain a new certificate for these domains", + "Cancel this operation and do nothing"], + "OK", "Cancel")) + # TODO: Analyze the result and make a code path that does the + # right thing with it + sys.exit(1) elif subset_names_cert is not None: question = ( "You have an existing certificate that contains a portion of " From fe6e9be6a2d3c0090cdd91c7a19825c980deb1cb Mon Sep 17 00:00:00 2001 From: Alcaro Date: Wed, 9 Dec 2015 22:04:29 +0100 Subject: [PATCH 064/123] Update reference to deprecated directives https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcertificatechainfile > SSLCertificateChainFile became obsolete with version 2.4.8, when SSLCertificateFile was extended to also load intermediate CA certificates from the server certificate file. --- docs/using.rst | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 51426183d..687901191 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -224,21 +224,23 @@ The following files are available: ``cert.pem`` Server certificate only. - This is what Apache needs for `SSLCertificateFile + This is what Apache < 2.4.8 needs for `SSLCertificateFile `_. ``chain.pem`` All certificates that need to be served by the browser **excluding** server certificate, i.e. root and intermediate certificates only. - This is what Apache needs for `SSLCertificateChainFile + This is what Apache < 2.4.8 needs for `SSLCertificateChainFile `_. ``fullchain.pem`` All certificates, **including** server certificate. This is concatenation of ``chain.pem`` and ``cert.pem``. - This is what nginx needs for `ssl_certificate + This is what Apache >= 2.4.8 needs for `SSLCertificateFile + `_, + and what nginx needs for `ssl_certificate `_. From 79c61a80987fd9711324bb536f359ca90c38cdda Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 9 Dec 2015 16:37:58 -0800 Subject: [PATCH 065/123] Basic updated logic for #1546 behavior --- letsencrypt/cli.py | 82 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 457661d1b..c24f870f5 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -219,16 +219,14 @@ def _treat_as_renewal(config, domains): :raises .Error: If the user would like to rerun the client again. """ - # TODO: This now needs to return 3 different cases plus the .Error - # case: reinstall, renew, fresh cert - # Probably the return value should be a tuple of (result, cert) - # like ("reinstall", RenewableCert_instance) - # ("renew", RenewableCert_instance) - # ("newcert", None) + # This will now instead return 3 different cases plus the .Error + # case (which raises an exception): reinstall, renew, newcert + # The return value will be a tuple of (result, cert), viz. + # either ("reinstall", RenewableCert_instance) + # or ("renew", RenewableCert_instance) + # or ("newcert", None) # We could also return ("cancel", None) instead of raising the - # error, but the error-handling mechanism seems to work - # TODO: Find out whether a RenewableCert instance is enough information - # for the installer to try to reinstall it when we return "reinstall" + # error, but the error-handling mechanism seems to work OK. # TODO: Also address superset case renewal = False @@ -243,23 +241,48 @@ def _treat_as_renewal(config, domains): # configuration file. question = None if ident_names_cert is not None: + # TODO: I bet this question is confusing to people who don't know + # how the backend repreentation of certificates work. The + # distinction is renewal updates existing lineage with new + # cert (eventually maybe preserving the privkey), while + # newcert creates a new lineage. And reinstall doesn't cause + # a new issuance at all. question = ( "You have an existing certificate that contains exactly the " "same domains you requested (ref: {0}){br}{br}Note that the " "Let's Encrypt certificate authority limits the number of " "certificates that can be issued for the same domain name per " - "week!{br}{br}Do you want to reinstall this existing " - "certificate, or renew and replace this certificate with a " - "newly-issued one?" + "week, including renewal certificates!{br}{br}Do you want to " + "reinstall this existing certificate, renew and replace this " + "certificate with a newly-issued one, or get a completely new " + "certificate?" ).format(ident_names_cert.configfile.filename, br=os.linesep) - print(zope.component.getUtility(interfaces.IDisplay).menu( + response = zope.component.getUtility(interfaces.IDisplay).menu( question, ["Attempt to reinstall this existing certificate", - "Obtain a new certificate for these domains", + "Renew this certificate, replacing it with the updated one", + "Obtain a completely new certificate for these domains", "Cancel this operation and do nothing"], - "OK", "Cancel")) - # TODO: Analyze the result and make a code path that does the - # right thing with it - sys.exit(1) + "OK", "Cancel") + if response[0] == "cancel" or response[1] == 3: + # TODO: Add notification related to command-line options for + # skipping the menu for this case. + raise errors.Error( + "User did not use proper CLI and would like " + "to reinvoke the client.") + elif response[1] == 0: + # Reinstall + return "reinstall", ident_names_cert + elif response[1] == 1: + # Renew + return "renew", ident_names_cert + elif response[1] == 2: + # New cert + return "newcert", None + else: + assert 0 + # NOTREACHED + # TODO: Since the rest of the function deals only with the subset + # case, we could now simplify it considerably! elif subset_names_cert is not None: question = ( "You have an existing certificate that contains a portion of " @@ -295,9 +318,9 @@ def _treat_as_renewal(config, domains): "to reinvoke the client.") if renewal: - return ident_names_cert if ident_names_cert is not None else subset_names_cert + return "renew", ident_names_cert if ident_names_cert is not None else subset_names_cert - return None + return "newcert", None def _report_new_cert(cert_path, fullchain_path): @@ -337,10 +360,20 @@ def _suggest_donate(): def _auth_from_domains(le_client, config, domains): """Authenticate and enroll certificate.""" - # Note: This can raise errors... caught above us though. - lineage = _treat_as_renewal(config, domains) + # Note: This can raise errors... caught above us though. This is now + # a three-way case: reinstall (which results in a no-op here because + # although there is a relevant lineage, we don't do anything to it + # inside this function -- we don't obtain a new certificate), renew + # (which results in treating the request as a renewal), or newcert + # (which results in treating the request as a new certificate request). - if lineage is not None: + action, lineage = _treat_as_renewal(config, domains) + print action, lineage + if action == "reinstall": + # The lineage already exists; allow the caller to try installing + # it without getting a new certificate at all. + return lineage + elif action == "renew": # TODO: schoen wishes to reuse key - discussion # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574 new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) @@ -354,7 +387,7 @@ def _auth_from_domains(le_client, config, domains): # TODO: Check return value of save_successor # TODO: Also update lineage renewal config with any relevant # configuration values from this attempt? <- Absolutely (jdkasten) - else: + elif action == "newcert": # TREAT AS NEW REQUEST lineage = le_client.obtain_and_enroll_certificate(domains) if not lineage: @@ -508,6 +541,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo def obtain_cert(args, config, plugins): """Authenticate & obtain cert, but do not install it.""" + # TODO: Is this now dead code? What calls it? if args.domains and args.csr is not None: # TODO: --csr could have a priority, when --domains is From 45d20847e64c80e81f14c5f239ec57c095ad7202 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 9 Dec 2015 18:21:27 -0800 Subject: [PATCH 066/123] Implement --staging - Fixes #1667 - Should help with rate limit problems --- letsencrypt/cli.py | 13 +++++++++++++ letsencrypt/tests/cli_test.py | 14 ++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 348818368..f57b1eb0b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -696,6 +696,15 @@ class HelpfulArgumentParser(object): parsed_args = self.parser.parse_args(self.args) parsed_args.func = self.VERBS[self.verb] + # Do any post-parsing homework here + + # argparse seemingly isn't flexible enough to give us this behaviour easily... + staging_uri = 'https://acme-staging.api.letsencrypt.org/directory' + if parsed_args.staging: + if parsed_args.server not in (flag_default("server"), staging_uri): + raise errors.Error("--server value conflicts with --staging") + parsed_args.server = staging_uri + return parsed_args @@ -1037,6 +1046,10 @@ def _paths_parser(helpful): help="Logs directory.") add("paths", "--server", default=flag_default("server"), help=config_help("server")) + # overwrites server, handled in HelpfulArgumentParser.parse_args() + add("testing", "--staging", action='store_true', + help='Use the staging server to obtain test (invalid) certs; equivalent' + ' to --server https://acme-staging.api.letsencrypt.org/directory ') def _plugins_parsing(helpful, plugins): diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 462d37a87..60a8e9440 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -343,6 +343,20 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods namespace = cli.prepare_and_parse_args(plugins, long_args) self.assertEqual(namespace.domains, ['example.com', 'another.net']) + def test_parse_server(self): + plugins = disco.PluginsRegistry.find_all() + short_args = ['--server', 'example.com'] + namespace = cli.prepare_and_parse_args(plugins, short_args) + self.assertEqual(namespace.server, 'example.com') + + short_args = ['--staging'] + namespace = cli.prepare_and_parse_args(plugins, short_args) + self.assertEqual(namespace.server, + 'https://acme-staging.api.letsencrypt.org/directory') + + short_args = ['--staging', '--server', 'example.com'] + self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args) + def test_parse_webroot(self): plugins = disco.PluginsRegistry.find_all() webroot_args = ['--webroot', '-w', '/var/www/example', From 5804d033363d0005134c2298c5c6da1013ad1e3f Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Thu, 10 Dec 2015 14:36:25 +0000 Subject: [PATCH 067/123] Add failing test from ticket #1724 Augeas fails to parse the ErrorDocument argument that starts with a double quote and ends with an EOL. --- .../drupal-errordocument-arg-1724.conf | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf diff --git a/tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf b/tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf new file mode 100644 index 000000000..4733ffa4a --- /dev/null +++ b/tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf @@ -0,0 +1,116 @@ +# +# Apache/PHP/Drupal settings: +# + +# Protect files and directories from prying eyes. + + Order allow,deny + + +# Don't show directory listings for URLs which map to a directory. +Options -Indexes + +# Follow symbolic links in this directory. +Options +FollowSymLinks + +# Make Drupal handle any 404 errors. +ErrorDocument 404 /index.php + +# Force simple error message for requests for non-existent favicon.ico. + + # There is no end quote below, for compatibility with Apache 1.3. + ErrorDocument 404 "The requested file favicon.ico was not found. + + +# Set the default handler. +DirectoryIndex index.php + +# Override PHP settings. More in sites/default/settings.php +# but the following cannot be changed at runtime. + +# PHP 4, Apache 1. + + php_value magic_quotes_gpc 0 + php_value register_globals 0 + php_value session.auto_start 0 + php_value mbstring.http_input pass + php_value mbstring.http_output pass + php_value mbstring.encoding_translation 0 + + +# PHP 4, Apache 2. + + php_value magic_quotes_gpc 0 + php_value register_globals 0 + php_value session.auto_start 0 + php_value mbstring.http_input pass + php_value mbstring.http_output pass + php_value mbstring.encoding_translation 0 + + +# PHP 5, Apache 1 and 2. + + php_value magic_quotes_gpc 0 + php_value register_globals 0 + php_value session.auto_start 0 + php_value mbstring.http_input pass + php_value mbstring.http_output pass + php_value mbstring.encoding_translation 0 + + +# Requires mod_expires to be enabled. + + # Enable expirations. + ExpiresActive On + + # Cache all files for 2 weeks after access (A). + ExpiresDefault A1209600 + + + # Do not allow PHP scripts to be cached unless they explicitly send cache + # headers themselves. Otherwise all scripts would have to overwrite the + # headers set by mod_expires if they want another caching behavior. This may + # fail if an error occurs early in the bootstrap process, and it may cause + # problems if a non-Drupal PHP file is installed in a subdirectory. + ExpiresActive Off + + + +# Various rewrite rules. + + RewriteEngine on + + # If your site can be accessed both with and without the 'www.' prefix, you + # can use one of the following settings to redirect users to your preferred + # URL, either WITH or WITHOUT the 'www.' prefix. Choose ONLY one option: + # + # To redirect all users to access the site WITH the 'www.' prefix, + # (http://example.com/... will be redirected to http://www.example.com/...) + # adapt and uncomment the following: + # RewriteCond %{HTTP_HOST} ^example\.com$ [NC] + # RewriteRule ^(.*)$ http://www.example.com/$1 [L,R=301] + # + # To redirect all users to access the site WITHOUT the 'www.' prefix, + # (http://www.example.com/... will be redirected to http://example.com/...) + # uncomment and adapt the following: + # RewriteCond %{HTTP_HOST} ^www\.example\.com$ [NC] + # RewriteRule ^(.*)$ http://example.com/$1 [L,R=301] + + # Modify the RewriteBase if you are using Drupal in a subdirectory or in a + # VirtualDocumentRoot and the rewrite rules are not working properly. + # For example if your site is at http://example.com/drupal uncomment and + # modify the following line: + # RewriteBase /drupal + # + # If your site is running in a VirtualDocumentRoot at http://example.com/, + # uncomment the following line: + # RewriteBase / + + # Rewrite URLs of the form 'x' to the form 'index.php?q=x'. + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteCond %{REQUEST_URI} !=/favicon.ico + RewriteRule ^(.*)$ index.php?q=$1 [L,QSA] + + +# $Id$ From 6b4031331192c86910714b38ff49454602ff4784 Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Thu, 10 Dec 2015 14:37:58 +0000 Subject: [PATCH 068/123] Merge Augeas lens fix for partially quoted arguments From https://github.com/hercules-team/augeas/commit/50fb756580477e9195946133ec2f0d1f0f6786c7 Closes: #1724 --- .../letsencrypt_apache/augeas_lens/httpd.aug | 11 +++++++++-- .../drupal-errordocument-arg-1724.conf | 0 2 files changed, 9 insertions(+), 2 deletions(-) rename tests/apache-conf-files/{failing => passing}/drupal-errordocument-arg-1724.conf (100%) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug index 0669896a0..732ba5468 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug +++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug @@ -68,6 +68,9 @@ let cl = /\\\\\n/ let dquot = let no_dquot = /[^"\\\r\n]/ in /"/ . (no_dquot|cdot|cl)* . /"/ +let dquot_msg = + let no_dquot = /([^ \t"\\\r\n]|[^"\\\r\n]+[^ \t"\\\r\n])/ + in /"/ . (no_dquot|cdot|cl)* let squot = let no_squot = /[^'\\\r\n]/ in /'/ . (no_squot|cdot|cl)* . /'/ @@ -78,6 +81,8 @@ let comp = /[<>=]?=/ *****************************************************************) let arg_dir = [ label "arg" . store (char_arg_dir+|dquot|squot) ] +(* message argument starts with " but ends at EOL *) +let arg_dir_msg = [ label "arg" . store dquot_msg ] let arg_sec = [ label "arg" . store (char_arg_sec+|comp|dquot|squot) ] let arg_wl = [ label "arg" . store (char_arg_wl+|dquot|squot) ] @@ -90,8 +95,10 @@ let arg_wordlist = let argv (l:lens) = l . (sep_spc . l)* -let directive = [ indent . label "directive" . store word . - (sep_spc . argv (arg_dir|arg_wordlist))? . eol ] +let directive = + (* arg_dir_msg may be the last or only argument *) + let dir_args = (argv (arg_dir|arg_wordlist) . (sep_spc . arg_dir_msg)?) | arg_dir_msg + in [ indent . label "directive" . store word . (sep_spc . dir_args)? . eol ] let section (body:lens) = (* opt_eol includes empty lines *) diff --git a/tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf b/tests/apache-conf-files/passing/drupal-errordocument-arg-1724.conf similarity index 100% rename from tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf rename to tests/apache-conf-files/passing/drupal-errordocument-arg-1724.conf From 3b5810995ddc161c5263cd77d044d2a0be14b5c2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Dec 2015 14:11:26 -0800 Subject: [PATCH 069/123] The flag name --test-cert may make sense to most people --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f57b1eb0b..16283a84a 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1047,7 +1047,7 @@ def _paths_parser(helpful): add("paths", "--server", default=flag_default("server"), help=config_help("server")) # overwrites server, handled in HelpfulArgumentParser.parse_args() - add("testing", "--staging", action='store_true', + add("testing", "--test-cert", "--staging", action='store_true', dest='staging', help='Use the staging server to obtain test (invalid) certs; equivalent' ' to --server https://acme-staging.api.letsencrypt.org/directory ') From ae0f35d48da3a86ae0f264588ffe66495ad1c484 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 10 Dec 2015 15:23:10 -0800 Subject: [PATCH 070/123] Taking "newcert" option out of the menu --- letsencrypt/cli.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index c24f870f5..b1d3f3d87 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -227,6 +227,12 @@ def _treat_as_renewal(config, domains): # or ("newcert", None) # We could also return ("cancel", None) instead of raising the # error, but the error-handling mechanism seems to work OK. + # Note that the "newcert" option is not suggested in the UI menu + # when the requested cert has precisely the same names as an + # existing cert (it's considered an "advanced" option in this + # situation, so it would have to be selected with a command-line + # flag). + # # TODO: Also address superset case renewal = False @@ -250,20 +256,17 @@ def _treat_as_renewal(config, domains): question = ( "You have an existing certificate that contains exactly the " "same domains you requested (ref: {0}){br}{br}Note that the " - "Let's Encrypt certificate authority limits the number of " - "certificates that can be issued for the same domain name per " - "week, including renewal certificates!{br}{br}Do you want to " - "reinstall this existing certificate, renew and replace this " - "certificate with a newly-issued one, or get a completely new " - "certificate?" + "Let's Encrypt CA limits how many certificates can be issued " + "for the same domain name per week, including renewal " + "certificates!{br}{br}What would you like to do?" ).format(ident_names_cert.configfile.filename, br=os.linesep) response = zope.component.getUtility(interfaces.IDisplay).menu( question, ["Attempt to reinstall this existing certificate", "Renew this certificate, replacing it with the updated one", - "Obtain a completely new certificate for these domains", +# "Obtain a completely new certificate for these domains", "Cancel this operation and do nothing"], "OK", "Cancel") - if response[0] == "cancel" or response[1] == 3: + if response[0] == "cancel" or response[1] == 2: # TODO: Add notification related to command-line options for # skipping the menu for this case. raise errors.Error( @@ -275,9 +278,9 @@ def _treat_as_renewal(config, domains): elif response[1] == 1: # Renew return "renew", ident_names_cert - elif response[1] == 2: - # New cert - return "newcert", None +# elif response[1] == 2: +# # New cert +# return "newcert", None else: assert 0 # NOTREACHED From 748a7fe2bcca45906085b85280bbcf678f87d27c Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 10 Dec 2015 15:41:27 -0800 Subject: [PATCH 071/123] Partially update documentation for _treat_as_renewal --- letsencrypt/cli.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index b1d3f3d87..ecee15c2e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -211,10 +211,11 @@ def _find_duplicative_certs(config, domains): def _treat_as_renewal(config, domains): - """Determine whether or not the call should be treated as a renewal. + """Determine whether there are duplicated names and how to handle them. - :returns: RenewableCert or None if renewal shouldn't occur. - :rtype: :class:`.storage.RenewableCert` + :returns: Two-element tuple containing desired new-certificate + behavior as a string token, plus either a RenewableCert + instance or None if renewal shouldn't occur. :raises .Error: If the user would like to rerun the client again. From 95d01130988c88b7f13e9a8f437ec317ef536077 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 10 Dec 2015 15:49:33 -0800 Subject: [PATCH 072/123] Make --renew-by-default apply in exact-duplicate case --- letsencrypt/cli.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ecee15c2e..c3a8f93b2 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -254,6 +254,8 @@ def _treat_as_renewal(config, domains): # cert (eventually maybe preserving the privkey), while # newcert creates a new lineage. And reinstall doesn't cause # a new issuance at all. + if config.renew_by_default: + return "renew", ident_names_cert question = ( "You have an existing certificate that contains exactly the " "same domains you requested (ref: {0}){br}{br}Note that the " From 859fcea4ece78af66d248117569724c6d89cdb88 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Dec 2015 19:08:08 -0800 Subject: [PATCH 073/123] Try to minimise verbiage. --- letsencrypt/cli.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index c3a8f93b2..86910acc7 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -258,14 +258,11 @@ def _treat_as_renewal(config, domains): return "renew", ident_names_cert question = ( "You have an existing certificate that contains exactly the " - "same domains you requested (ref: {0}){br}{br}Note that the " - "Let's Encrypt CA limits how many certificates can be issued " - "for the same domain name per week, including renewal " - "certificates!{br}{br}What would you like to do?" + "same domains you requested.{br}(ref: {0}){br}{br}What would you like to do?" ).format(ident_names_cert.configfile.filename, br=os.linesep) response = zope.component.getUtility(interfaces.IDisplay).menu( question, ["Attempt to reinstall this existing certificate", - "Renew this certificate, replacing it with the updated one", + "Renew & replace the cert (limit 5 per 7 days)", # "Obtain a completely new certificate for these domains", "Cancel this operation and do nothing"], "OK", "Cancel") From d3f5df5b023793183b3150e297d73ed0d94cb576 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Dec 2015 19:12:29 -0800 Subject: [PATCH 074/123] Hedge on the rate limit slightly, because it may change after a release --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 86910acc7..3d24c7a06 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -262,7 +262,7 @@ def _treat_as_renewal(config, domains): ).format(ident_names_cert.configfile.filename, br=os.linesep) response = zope.component.getUtility(interfaces.IDisplay).menu( question, ["Attempt to reinstall this existing certificate", - "Renew & replace the cert (limit 5 per 7 days)", + "Renew & replace the cert (limit ~5 per 7 days)", # "Obtain a completely new certificate for these domains", "Cancel this operation and do nothing"], "OK", "Cancel") From 88956dfba8f5109c5b168b3207b72db9d4ae4e4a Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Dec 2015 19:37:22 -0800 Subject: [PATCH 075/123] Move staging URI into constants.py --- letsencrypt/cli.py | 7 +++---- letsencrypt/constants.py | 3 ++- letsencrypt/tests/cli_test.py | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 16283a84a..5be3c1696 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -699,11 +699,10 @@ class HelpfulArgumentParser(object): # Do any post-parsing homework here # argparse seemingly isn't flexible enough to give us this behaviour easily... - staging_uri = 'https://acme-staging.api.letsencrypt.org/directory' if parsed_args.staging: - if parsed_args.server not in (flag_default("server"), staging_uri): + if parsed_args.server not in (flag_default("server"), constants.STAGING_URI): raise errors.Error("--server value conflicts with --staging") - parsed_args.server = staging_uri + parsed_args.server = constants.STAGING_URI return parsed_args @@ -1049,7 +1048,7 @@ def _paths_parser(helpful): # overwrites server, handled in HelpfulArgumentParser.parse_args() add("testing", "--test-cert", "--staging", action='store_true', dest='staging', help='Use the staging server to obtain test (invalid) certs; equivalent' - ' to --server https://acme-staging.api.letsencrypt.org/directory ') + ' to --server ' + constants.STAGING_URI) def _plugins_parsing(helpful, plugins): diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 40155abd7..a1dccd1ea 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -30,8 +30,9 @@ CLI_DEFAULTS = dict( auth_chain_path="./chain.pem", strict_permissions=False, ) -"""Defaults for CLI flags and `.IConfig` attributes.""" +STAGING_URI = "https://acme-staging.api.letsencrypt.org/directory" +"""Defaults for CLI flags and `.IConfig` attributes.""" RENEWER_DEFAULTS = dict( renewer_enabled="yes", diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 60a8e9440..e7ae5de23 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -15,6 +15,7 @@ from acme import jose from letsencrypt import account from letsencrypt import cli from letsencrypt import configuration +from letsencrypt import constants from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import le_util @@ -351,8 +352,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods short_args = ['--staging'] namespace = cli.prepare_and_parse_args(plugins, short_args) - self.assertEqual(namespace.server, - 'https://acme-staging.api.letsencrypt.org/directory') + self.assertEqual(namespace.server, constants.STAGING_URI) short_args = ['--staging', '--server', 'example.com'] self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args) From a878e48624e4d6c6bd4352e300433beb32b04acc Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 4 Dec 2015 11:50:38 -0800 Subject: [PATCH 076/123] Add another failing case --- .../failing/missing-double-quote-1724.conf | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/apache-conf-files/failing/missing-double-quote-1724.conf diff --git a/tests/apache-conf-files/failing/missing-double-quote-1724.conf b/tests/apache-conf-files/failing/missing-double-quote-1724.conf new file mode 100644 index 000000000..7d97b23d0 --- /dev/null +++ b/tests/apache-conf-files/failing/missing-double-quote-1724.conf @@ -0,0 +1,52 @@ + + ServerAdmin webmaster@localhost + ServerAlias www.example.com + ServerName example.com + DocumentRoot /var/www/example.com/www/ + SSLEngine on + + SSLProtocol all -SSLv2 -SSLv3 + SSLCipherSuite "EECDH+ECDSA+AESGCM EECDH+aRSA+AESGCM EECDH+ECDSA+SHA384 EECDH+ECDSA+SHA256 EECDH+aRS$ + SSLCertificateFile /etc/ssl/certs/ssl-cert-snakeoil.pem + SSLCertificateKeyFile /etc/ssl/private/ssl-cert-snakeoil.key + + + Options FollowSymLinks + AllowOverride All + + + Options Indexes FollowSymLinks MultiViews + AllowOverride All + Order allow,deny + allow from all + # This directive allows us to have apache2's default start page + # in /apache2-default/, but still have / go to the right place + + + ScriptAlias /cgi-bin/ /usr/lib/cgi-bin/ + + AllowOverride None + Options +ExecCGI -MultiViews +SymLinksIfOwnerMatch + Order allow,deny + Allow from all + + + ErrorLog /var/log/apache2/error.log + + # Possible values include: debug, info, notice, warn, error, crit, + # alert, emerg. + LogLevel warn + + CustomLog /var/log/apache2/access.log combined + ServerSignature On + + Alias /apache_doc/ "/usr/share/doc/" + + Options Indexes MultiViews FollowSymLinks + AllowOverride None + Order deny,allow + Deny from all + Allow from 127.0.0.0/255.0.0.0 ::1/128 + + + From 2321237d1ece6dbfcdf0293f338b2b8a7c7211ef Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 8 Dec 2015 22:30:17 -0800 Subject: [PATCH 077/123] Embodiement of Apache bug #1755 --- .../passing/example-1755.conf | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tests/apache-conf-files/passing/example-1755.conf diff --git a/tests/apache-conf-files/passing/example-1755.conf b/tests/apache-conf-files/passing/example-1755.conf new file mode 100644 index 000000000..260029576 --- /dev/null +++ b/tests/apache-conf-files/passing/example-1755.conf @@ -0,0 +1,36 @@ + + # The ServerName directive sets the request scheme, hostname and port that + # the server uses to identify itself. This is used when creating + # redirection URLs. In the context of virtual hosts, the ServerName + # specifies what hostname must appear in the request's Host: header to + # match this virtual host. For the default virtual host (this file) this + # value is not decisive as it is used as a last resort host regardless. + # However, you must set it for any further virtual host explicitly. + ServerName www.example.com + ServerAlias example.com +SetOutputFilter DEFLATE +# Do not attempt to compress the following extensions +SetEnvIfNoCase Request_URI \ +\.(?:gif|jpe?g|png|swf|flv|zip|gz|tar|mp3|mp4|m4v)$ no-gzip dont-vary + + ServerAdmin webmaster@localhost + DocumentRoot /var/www/proof + + # Available loglevels: trace8, ..., trace1, debug, info, notice, warn, + # error, crit, alert, emerg. + # It is also possible to configure the loglevel for particular + # modules, e.g. + #LogLevel info ssl:warn + + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + # For most configuration files from conf-available/, which are + # enabled or disabled at a global level, it is possible to + # include a line for only one particular virtual host. For example the + # following line enables the CGI configuration for this host only + # after it has been globally disabled with "a2disconf". + #Include conf-available/serve-cgi-bin.conf + + +# vim: syntax=apache ts=4 sw=4 sts=4 sr noet From bdfca70d55a657ba7f79b63f01e89797229ba43e Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 8 Dec 2015 23:04:13 -0800 Subject: [PATCH 078/123] Another #1531 --- .../apache-conf-files/passing/1626-1531.conf | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/apache-conf-files/passing/1626-1531.conf diff --git a/tests/apache-conf-files/passing/1626-1531.conf b/tests/apache-conf-files/passing/1626-1531.conf new file mode 100644 index 000000000..4a298857e --- /dev/null +++ b/tests/apache-conf-files/passing/1626-1531.conf @@ -0,0 +1,37 @@ + + ServerAdmin denver@ossguy.com + ServerName c-beta.ossguy.com + + Alias /robots.txt /home/denver/www/c-beta.ossguy.com/static/robots.txt + Alias /favicon.ico /home/denver/www/c-beta.ossguy.com/static/favicon.ico + + AliasMatch /(.*\.css) /home/denver/www/c-beta.ossguy.com/static/$1 + AliasMatch /(.*\.js) /home/denver/www/c-beta.ossguy.com/static/$1 + AliasMatch /(.*\.png) /home/denver/www/c-beta.ossguy.com/static/$1 + AliasMatch /(.*\.gif) /home/denver/www/c-beta.ossguy.com/static/$1 + AliasMatch /(.*\.jpg) /home/denver/www/c-beta.ossguy.com/static/$1 + + WSGIScriptAlias / /home/denver/www/c-beta.ossguy.com/django.wsgi + WSGIDaemonProcess c-beta-ossguy user=www-data group=www-data home=/var/www processes=5 threads=10 maximum-requests=1000 umask=0007 display-name=c-beta-ossguy + WSGIProcessGroup c-beta-ossguy + WSGIApplicationGroup %{GLOBAL} + + DocumentRoot /home/denver/www/c-beta.ossguy.com/static + + + Options -Indexes +FollowSymLinks -MultiViews + Require all granted + AllowOverride None + + + + Options +Indexes +FollowSymLinks -MultiViews + Require all granted + AllowOverride None + + + # Custom log file locations + LogLevel warn + ErrorLog /home/denver/www/logs/c-beta.ossguy.com/error.log + CustomLog /home/denver/www/logs/c-beta.ossguy.com/access.log combined + From de9e43de0cf35b5eecb0f91ba043cd6b285f3bff Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 9 Dec 2015 17:03:01 -0800 Subject: [PATCH 079/123] Update apache conf library --- tests/apache-conf-files/passing/1626-1531.conf | 4 ++-- tests/apache-conf-files/passing/README.modules | 2 ++ ...equire-wordlist.conf => sslrequire-wordlist-1827.htaccess} | 0 3 files changed, 4 insertions(+), 2 deletions(-) rename tests/apache-conf-files/passing/{sslrequire-wordlist.conf => sslrequire-wordlist-1827.htaccess} (100%) diff --git a/tests/apache-conf-files/passing/1626-1531.conf b/tests/apache-conf-files/passing/1626-1531.conf index 4a298857e..1622a57df 100644 --- a/tests/apache-conf-files/passing/1626-1531.conf +++ b/tests/apache-conf-files/passing/1626-1531.conf @@ -32,6 +32,6 @@ # Custom log file locations LogLevel warn - ErrorLog /home/denver/www/logs/c-beta.ossguy.com/error.log - CustomLog /home/denver/www/logs/c-beta.ossguy.com/access.log combined + ErrorLog /tmp/error.log + CustomLog /tmp/access.log combined diff --git a/tests/apache-conf-files/passing/README.modules b/tests/apache-conf-files/passing/README.modules index 9c5853061..7edbd3e84 100644 --- a/tests/apache-conf-files/passing/README.modules +++ b/tests/apache-conf-files/passing/README.modules @@ -3,3 +3,5 @@ Modules required to parse these conf files: ssl rewrite macro +wsgi +deflate diff --git a/tests/apache-conf-files/passing/sslrequire-wordlist.conf b/tests/apache-conf-files/passing/sslrequire-wordlist-1827.htaccess similarity index 100% rename from tests/apache-conf-files/passing/sslrequire-wordlist.conf rename to tests/apache-conf-files/passing/sslrequire-wordlist-1827.htaccess From 6fc65505f770915cb6071011170cc0bdeabd241c Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Dec 2015 12:18:27 -0800 Subject: [PATCH 080/123] Add test case for #1724 --- .../passing/missing-quote-1724.conf | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/apache-conf-files/passing/missing-quote-1724.conf diff --git a/tests/apache-conf-files/passing/missing-quote-1724.conf b/tests/apache-conf-files/passing/missing-quote-1724.conf new file mode 100644 index 000000000..7d97b23d0 --- /dev/null +++ b/tests/apache-conf-files/passing/missing-quote-1724.conf @@ -0,0 +1,52 @@ + + ServerAdmin webmaster@localhost + ServerAlias www.example.com + ServerName example.com + DocumentRoot /var/www/example.com/www/ + SSLEngine on + + SSLProtocol all -SSLv2 -SSLv3 + SSLCipherSuite "EECDH+ECDSA+AESGCM EECDH+aRSA+AESGCM EECDH+ECDSA+SHA384 EECDH+ECDSA+SHA256 EECDH+aRS$ + SSLCertificateFile /etc/ssl/certs/ssl-cert-snakeoil.pem + SSLCertificateKeyFile /etc/ssl/private/ssl-cert-snakeoil.key + + + Options FollowSymLinks + AllowOverride All + + + Options Indexes FollowSymLinks MultiViews + AllowOverride All + Order allow,deny + allow from all + # This directive allows us to have apache2's default start page + # in /apache2-default/, but still have / go to the right place + + + ScriptAlias /cgi-bin/ /usr/lib/cgi-bin/ + + AllowOverride None + Options +ExecCGI -MultiViews +SymLinksIfOwnerMatch + Order allow,deny + Allow from all + + + ErrorLog /var/log/apache2/error.log + + # Possible values include: debug, info, notice, warn, error, crit, + # alert, emerg. + LogLevel warn + + CustomLog /var/log/apache2/access.log combined + ServerSignature On + + Alias /apache_doc/ "/usr/share/doc/" + + Options Indexes MultiViews FollowSymLinks + AllowOverride None + Order deny,allow + Deny from all + Allow from 127.0.0.0/255.0.0.0 ::1/128 + + + From 0fa4b4c93fb6db5145e65d9470021d79cf4ab0fb Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Dec 2015 12:18:41 -0800 Subject: [PATCH 081/123] This is a hackish script to run all of these "tests". --- tests/apache-conf-files/hackish-apache-test | 28 +++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100755 tests/apache-conf-files/hackish-apache-test diff --git a/tests/apache-conf-files/hackish-apache-test b/tests/apache-conf-files/hackish-apache-test new file mode 100755 index 000000000..c6663551e --- /dev/null +++ b/tests/apache-conf-files/hackish-apache-test @@ -0,0 +1,28 @@ +#!/bin/bash + +# A hackish script to see if the client is behaving as expected +# with each of the "passing" conf files. + +# TODO presently this requires interaction and human judgement to +# assess, but it should be automated +export EA=/etc/apache2/ +TESTDIR="`dirname $0`" +LEROOT="`realpath \"$TESTDIR/../../\"`" +cd $TESTDIR/passing + +function CleanupExit() { + echo control c, exiting tests... + if [ "$f" != "" ] ; then + sudo rm /etc/apache2/sites-{enabled,available}/"$f" + fi + exit 1 +} + +trap CleanupExit INT +for f in *.conf ; do + echo testing "$f" + sudo cp "$f" "$EA"/sites-available/ + sudo ln -s "$EA/sites-available/$f" "$EA/sites-enabled/$f" + sudo "$LEROOT"/venv/bin/letsencrypt --apache certonly -t + sudo rm /etc/apache2/sites-{enabled,available}/"$f" +done From 0bde2007d0bb949b6cd9f6f2119868dca5e68590 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 15:11:16 -0800 Subject: [PATCH 082/123] Remove closing tag --- .../two_vhost_80/apache2/conf-available/bad_conf_file.conf | 2 -- 1 file changed, 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf index 1aad6a9f4..8e9178803 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf @@ -1,5 +1,3 @@ ServerName invalid.net - - From 090ada2aca22d383db1255362aecf2792823e810 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Fri, 11 Dec 2015 15:46:35 -0800 Subject: [PATCH 083/123] fix issue with mocked _treat_as_renewal code --- letsencrypt/tests/cli_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 462d37a87..433671b38 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -389,7 +389,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def _certonly_new_request_common(self, mock_client): with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal: - mock_renewal.return_value = None + mock_renewal.return_value = ("newcert", None) with mock.patch('letsencrypt.cli._init_le_client') as mock_init: mock_init.return_value = mock_client self._call(['-d', 'foo.bar', '-a', 'standalone', 'certonly']) @@ -405,7 +405,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) mock_cert = mock.MagicMock(body='body') mock_key = mock.MagicMock(pem='pem_key') - mock_renewal.return_value = mock_lineage + mock_renewal.return_value = ("renew", mock_lineage) mock_client = mock.MagicMock() mock_client.obtain_certificate.return_value = (mock_cert, 'chain', mock_key, 'csr') From 9248ba1e9696e4f7b3d79fdda40edce5494a0a26 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 16:06:12 -0800 Subject: [PATCH 084/123] Fix deprecation bug --- letsencrypt/le_util.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index fe63c70af..c630f4fe7 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -1,6 +1,7 @@ """Utilities for all Let's Encrypt.""" import argparse import collections +import configargparse import errno import logging import os @@ -278,9 +279,11 @@ def add_deprecated_argument(add_argument, argument_name, nargs): sys.stderr.write( "Use of {0} is deprecated.\n".format(option_string)) + configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE.add(ShowWarning) add_argument(argument_name, action=ShowWarning, help=argparse.SUPPRESS, nargs=nargs) + def check_domain_sanity(domain): """Method which validates domain value and errors out if the requirements are not met. From 75bc227cfac6bb4de7246597d78a400d1b788b1a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 16:13:06 -0800 Subject: [PATCH 085/123] Test setting agree-dev-preview in config file --- letsencrypt/tests/cli_test.py | 5 +++++ letsencrypt/tests/testdata/cli.ini | 1 + 2 files changed, 6 insertions(+) create mode 100644 letsencrypt/tests/testdata/cli.ini diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index e7ae5de23..2b56c5abe 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -551,6 +551,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(path, os.path.abspath(path)) self.assertEqual(contents, test_contents) + def test_agree_dev_preview_config(self): + with MockedVerb('run') as mocked_run: + self._call(['-c', test_util.vector_path('cli.ini')]) + self.assertTrue(mocked_run.called) + class DetermineAccountTest(unittest.TestCase): """Tests for letsencrypt.cli._determine_account.""" diff --git a/letsencrypt/tests/testdata/cli.ini b/letsencrypt/tests/testdata/cli.ini new file mode 100644 index 000000000..8ef506071 --- /dev/null +++ b/letsencrypt/tests/testdata/cli.ini @@ -0,0 +1 @@ +agree-dev-preview = True From 3521c92be3b2c186ea56fdd3e57b21997ba1dd4a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 16:14:36 -0800 Subject: [PATCH 086/123] Fixed import ordering --- letsencrypt/le_util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index c630f4fe7..64295a80f 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -1,7 +1,6 @@ """Utilities for all Let's Encrypt.""" import argparse import collections -import configargparse import errno import logging import os @@ -11,6 +10,8 @@ import stat import subprocess import sys +import configargparse + from letsencrypt import errors From c66c6bae1d5edcfbe0716b7e5b3f870e8595e0da Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 11 Dec 2015 18:00:33 -0800 Subject: [PATCH 087/123] Make `supported_challenges` return a list, not set --- letsencrypt/plugins/standalone.py | 4 ++-- letsencrypt/plugins/standalone_test.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 1bca3c036..139764601 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -173,8 +173,8 @@ class Authenticator(common.Plugin): @property def supported_challenges(self): """Challenges supported by this plugin.""" - return set(challenges.Challenge.TYPES[name] for name in - self.conf("supported-challenges").split(",")) + return [challenges.Challenge.TYPES[name] for name in + self.conf("supported-challenges").split(",")] @property def _necessary_ports(self): diff --git a/letsencrypt/plugins/standalone_test.py b/letsencrypt/plugins/standalone_test.py index 1833a55fe..1e39dee57 100644 --- a/letsencrypt/plugins/standalone_test.py +++ b/letsencrypt/plugins/standalone_test.py @@ -98,12 +98,12 @@ class AuthenticatorTest(unittest.TestCase): def test_supported_challenges(self): self.assertEqual(self.auth.supported_challenges, - set([challenges.TLSSNI01, challenges.HTTP01])) + [challenges.TLSSNI01, challenges.HTTP01]) def test_supported_challenges_configured(self): self.config.standalone_supported_challenges = "tls-sni-01" self.assertEqual(self.auth.supported_challenges, - set([challenges.TLSSNI01])) + [challenges.TLSSNI01]) def test_more_info(self): self.assertTrue(isinstance(self.auth.more_info(), six.string_types)) From 74927613e9e9f9fa02f0dfbe2a2f75707a850b5d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 18:03:52 -0800 Subject: [PATCH 088/123] Fixed lint issues --- letsencrypt/plugins/webroot.py | 5 ++--- letsencrypt/plugins/webroot_test.py | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index c4072c3f9..b4c877c78 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -2,7 +2,6 @@ import errno import logging import os -import stat import zope.interface @@ -105,10 +104,10 @@ to serve all files under specified web root ({0}).""" path = self.full_roots[achall.domain] except IndexError: raise errors.PluginError("Missing --webroot-path for domain: {1}" - .format(achall.domain)) + .format(achall.domain)) if not os.path.exists(path): raise errors.PluginError("Mysteriously missing path {0} for domain: {1}" - .format(path, achall.domain)) + .format(path, achall.domain)) return os.path.join(path, achall.chall.encode("token")) def _perform_single(self, achall): diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 2e88c1756..2ebbf01a6 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -48,7 +48,7 @@ class AuthenticatorTest(unittest.TestCase): def test_add_parser_arguments(self): add = mock.MagicMock() self.auth.add_parser_arguments(add) - self.assertEqual(0, add.call_count) # became 0 when we moved the args to cli.py! + self.assertEqual(0, add.call_count) # args moved to cli.py! def test_prepare_bad_root(self): self.config.webroot_path = os.path.join(self.path, "null") @@ -80,7 +80,7 @@ class AuthenticatorTest(unittest.TestCase): # Check permissions of the directories - for dirpath, dirnames, filenames in os.walk(self.path): + for dirpath, dirnames, _ in os.walk(self.path): for directory in dirnames: full_path = os.path.join(dirpath, directory) dir_permissions = stat.S_IMODE(os.stat(full_path).st_mode) From 2f904a41e060e468891df53114b5df2ce735fba3 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 11 Dec 2015 18:06:11 -0800 Subject: [PATCH 089/123] Derive preference order from `supported_challenges` order --- letsencrypt/plugins/standalone.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 139764601..70bf92dbb 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -197,8 +197,7 @@ class Authenticator(common.Plugin): def get_chall_pref(self, domain): # pylint: disable=unused-argument,missing-docstring - return [chall for chall in SUPPORTED_CHALLENGES - if chall in self.supported_challenges] + return self.supported_challenges def perform(self, achalls): # pylint: disable=missing-docstring if any(util.already_listening(port) for port in self._necessary_ports): From f4d499dbad41ea16759deffd95290a01836bd914 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 11 Dec 2015 18:07:25 -0800 Subject: [PATCH 090/123] Make help message indicate derived challenge preference --- letsencrypt/plugins/standalone.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 70bf92dbb..4319e51f9 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -166,7 +166,7 @@ class Authenticator(common.Plugin): @classmethod def add_parser_arguments(cls, add): add("supported-challenges", - help="Supported challenges. Prefers tls-sni-01.", + help="Supported challenges. Preferred in the order they are listed.", type=supported_challenges_validator, default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES)) From 2d525594663ecf5037cd31a4b6f67bcb54d2e516 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 18:12:46 -0800 Subject: [PATCH 091/123] Cleanup comment --- letsencrypt/plugins/webroot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index b4c877c78..075930c8b 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -59,8 +59,8 @@ to serve all files under specified web root ({0}).""" logger.debug("Creating root challenges validation dir at %s", self.full_roots[name]) - # Change the permissiosn to be writable (GH #1389) - # Umask is used instead of chmod to ensure the client can also + # Change the permissions to be writable (GH #1389) + # Umask is used instead of chmod to ensure the client can also # run as non-root (GH #1795) old_umask = os.umask(0o022) From d45865a60110f9df383b58d307b06cea6f7cb42a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 19:14:23 -0800 Subject: [PATCH 092/123] Cleanup --- letsencrypt/plugins/webroot.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 075930c8b..0679bc349 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -65,16 +65,12 @@ to serve all files under specified web root ({0}).""" old_umask = os.umask(0o022) try: - - stat_path = os.stat(path) # This is coupled with the "umask" call above because # os.makedirs's "mode" parameter may not always work: # https://stackoverflow.com/questions/5231901/permission-problems-when-creating-a-dir-with-os-makedirs-python - os.makedirs(self.full_roots[name], 0o0755) # Set owner as parent directory if possible - try: stat_path = os.stat(path) os.chown(self.full_roots[name], stat_path.st_uid, From 1a7dd76288204a6bc1c979149162fa5440b04156 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 19:31:50 -0800 Subject: [PATCH 093/123] Added test coverage --- letsencrypt/plugins/webroot_test.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 2ebbf01a6..9f5b6bba8 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -1,9 +1,10 @@ """Tests for letsencrypt.plugins.webroot.""" +import errno import os import shutil +import stat import tempfile import unittest -import stat import mock @@ -35,7 +36,6 @@ class AuthenticatorTest(unittest.TestCase): self.config = mock.MagicMock(webroot_path=self.path, webroot_map={"thing.com": self.path}) self.auth = Authenticator(self.config, "webroot") - self.auth.prepare() def tearDown(self): shutil.rmtree(self.path) @@ -70,7 +70,18 @@ class AuthenticatorTest(unittest.TestCase): self.assertRaises(errors.PluginError, self.auth.prepare) os.chmod(self.path, 0o700) + @mock.patch("letsencrypt.plugins.webroot.os.chown") + def test_failed_chown_eacces(self, mock_chown): + mock_chown.side_effect = OSError(errno.EACCES, "msg") + self.auth.prepare() # exception caught and logged + + @mock.patch("letsencrypt.plugins.webroot.os.chown") + def test_failed_chown_not_eacces(self, mock_chown): + mock_chown.side_effect = OSError() + self.assertRaises(errors.PluginError, self.auth.prepare) + def test_prepare_permissions(self): + self.auth.prepare() # Remove exec bit from permission check, so that it # matches the file @@ -93,6 +104,7 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(os.stat(self.validation_path).st_uid, parent_uid) def test_perform_cleanup(self): + self.auth.prepare() responses = self.auth.perform([self.achall]) self.assertEqual(1, len(responses)) self.assertTrue(os.path.exists(self.validation_path)) From 8fdff540b5ab46fc1d6fd66f54c209eecb8ac6eb Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 11 Dec 2015 22:02:46 -0800 Subject: [PATCH 094/123] Add interactive flag to should_auto{renew,deploy} --- letsencrypt/storage.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 7e2802b14..f457fe13e 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -471,7 +471,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes return ("autodeploy" not in self.configuration or self.configuration.as_bool("autodeploy")) - def should_autodeploy(self): + def should_autodeploy(self, interactive=False): """Should this lineage now automatically deploy a newer version? This is a policy question and does not only depend on whether @@ -480,12 +480,16 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes exists, and whether the time interval for autodeployment has been reached.) + :param bool interactive: set to True to examine the question + regardless of whether the renewal configuration allows + automated deployment (for interactive use). Default False. + :returns: whether the lineage now ought to autodeploy an existing newer cert version :rtype: bool """ - if self.autodeployment_is_enabled(): + if interactive or self.autodeployment_is_enabled(): if self.has_pending_deployment(): interval = self.configuration.get("deploy_before_expiry", "5 days") @@ -529,7 +533,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes return ("autorenew" not in self.configuration or self.configuration.as_bool("autorenew")) - def should_autorenew(self): + def should_autorenew(self, interactive=False): """Should we now try to autorenew the most recent cert version? This is a policy question and does not only depend on whether @@ -540,12 +544,16 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes Note that this examines the numerically most recent cert version, not the currently deployed version. + :param bool interactive: set to True to examine the question + regardless of whether the renewal configuration allows + automated renewal (for interactive use). Default False. + :returns: whether an attempt should now be made to autorenew the most current cert version in this lineage :rtype: bool """ - if self.autorenewal_is_enabled(): + if interactive or self.autorenewal_is_enabled(): # Consider whether to attempt to autorenew this cert now # Renewals on the basis of revocation From 621bef35c966a7bb912d8b83c173a03dbce656b7 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 10:11:28 -0800 Subject: [PATCH 095/123] Close to expiry, default to renewal - Also move the identical-cert-names case into its own function --- letsencrypt/cli.py | 69 ++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 3d24c7a06..135bb7255 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -248,42 +248,7 @@ def _treat_as_renewal(config, domains): # configuration file. question = None if ident_names_cert is not None: - # TODO: I bet this question is confusing to people who don't know - # how the backend repreentation of certificates work. The - # distinction is renewal updates existing lineage with new - # cert (eventually maybe preserving the privkey), while - # newcert creates a new lineage. And reinstall doesn't cause - # a new issuance at all. - if config.renew_by_default: - return "renew", ident_names_cert - question = ( - "You have an existing certificate that contains exactly the " - "same domains you requested.{br}(ref: {0}){br}{br}What would you like to do?" - ).format(ident_names_cert.configfile.filename, br=os.linesep) - response = zope.component.getUtility(interfaces.IDisplay).menu( - question, ["Attempt to reinstall this existing certificate", - "Renew & replace the cert (limit ~5 per 7 days)", -# "Obtain a completely new certificate for these domains", - "Cancel this operation and do nothing"], - "OK", "Cancel") - if response[0] == "cancel" or response[1] == 2: - # TODO: Add notification related to command-line options for - # skipping the menu for this case. - raise errors.Error( - "User did not use proper CLI and would like " - "to reinvoke the client.") - elif response[1] == 0: - # Reinstall - return "reinstall", ident_names_cert - elif response[1] == 1: - # Renew - return "renew", ident_names_cert -# elif response[1] == 2: -# # New cert -# return "newcert", None - else: - assert 0 - # NOTREACHED + return _handle_identical_cert_request(ident_names_cert) # TODO: Since the rest of the function deals only with the subset # case, we could now simplify it considerably! elif subset_names_cert is not None: @@ -325,6 +290,38 @@ def _treat_as_renewal(config, domains): return "newcert", None +def _handle_identical_cert_request(cert): + """Figure out what to do if a cert has the same names as a perviously obtained one + + :param storage.RenewableCert cert: + + :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal + """ + if config.renew_by_default or cert.should_autorenew(interactive=True): + return "renew", cert + display = zope.component.getUtility(interfaces.IDisplay) + question = ( + "You have an existing certificate that contains exactly the same " + "domains you requested.{br}(ref: {0}){br}{br}What would you like to do?" + ).format(cert.configfile.filename, br=os.linesep) + response = display.menu( + question, ["Attempt to reinstall this existing certificate", + "Renew & replace the cert (limit ~5 per 7 days)", + "Cancel this operation and do nothing"], + "OK", "Cancel") + if response[0] == "cancel" or response[1] == 2: + # TODO: Add notification related to command-line options for + # skipping the menu for this case. + raise errors.Error( + "User did not use proper CLI and would like " + "to reinvoke the client.") + elif response[1] == 0: + return "reinstall", cert + elif response[1] == 1: + return "renew", cert + else: + assert False, "This is impossible" + def _report_new_cert(cert_path, fullchain_path): """Reports the creation of a new certificate to the user. From 6351194fc2936bad1a1e2a031831ae900e5250d3 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 10:20:34 -0800 Subject: [PATCH 096/123] Simplify construction of the "renew" case --- letsencrypt/cli.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 135bb7255..8aac90c6d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -235,7 +235,6 @@ def _treat_as_renewal(config, domains): # flag). # # TODO: Also address superset case - renewal = False # Considering the possibility that the requested certificate is # related to an existing certificate. (config.duplicate, which @@ -249,8 +248,6 @@ def _treat_as_renewal(config, domains): question = None if ident_names_cert is not None: return _handle_identical_cert_request(ident_names_cert) - # TODO: Since the rest of the function deals only with the subset - # case, we could now simplify it considerably! elif subset_names_cert is not None: question = ( "You have an existing certificate that contains a portion of " @@ -268,7 +265,7 @@ def _treat_as_renewal(config, domains): pass elif config.renew_by_default or zope.component.getUtility( interfaces.IDisplay).yesno(question, "Replace", "Cancel"): - renewal = True + return "renew", subset_names_cert else: reporter_util = zope.component.getUtility(interfaces.IReporter) reporter_util.add_message( @@ -285,9 +282,6 @@ def _treat_as_renewal(config, domains): "User did not use proper CLI and would like " "to reinvoke the client.") - if renewal: - return "renew", ident_names_cert if ident_names_cert is not None else subset_names_cert - return "newcert", None def _handle_identical_cert_request(cert): From 439a2d05eb89ff91441a05a6cec122c3b4dc10ab Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 10:37:11 -0800 Subject: [PATCH 097/123] Simplifications: - Handle the base "newcert" cases at the top, rather than with a very long if statement - Avoid having a state-tracking "question variable" --- letsencrypt/cli.py | 48 +++++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 8aac90c6d..acba13441 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -240,30 +240,28 @@ def _treat_as_renewal(config, domains): # related to an existing certificate. (config.duplicate, which # is set with --duplicate, skips all of this logic and forces any # kind of certificate to be obtained with renewal = False.) - if not config.duplicate: - ident_names_cert, subset_names_cert = _find_duplicative_certs( - config, domains) - # I am not sure whether that correctly reads the systemwide - # configuration file. - question = None - if ident_names_cert is not None: - return _handle_identical_cert_request(ident_names_cert) - elif subset_names_cert is not None: - question = ( - "You have an existing certificate that contains a portion of " - "the domains you requested (ref: {0}){br}{br}It contains these " - "names: {1}{br}{br}You requested these names for the new " - "certificate: {2}.{br}{br}Do you want to replace this existing " - "certificate with the new certificate?" - ).format(subset_names_cert.configfile.filename, - ", ".join(subset_names_cert.names()), - ", ".join(domains), - br=os.linesep) - if question is None: - # We aren't in a duplicative-names situation at all, so we don't - # have to tell or ask the user anything about this. - pass - elif config.renew_by_default or zope.component.getUtility( + if config.duplicate: + return "newcert", None + ident_names_cert, subset_names_cert = _find_duplicative_certs(config, domains) + # I am not sure whether that correctly reads the systemwide + # configuration file. + if not (ident_names_cert or subset_names_cert): + return "newcert", None + + if ident_names_cert is not None: + return _handle_identical_cert_request(ident_names_cert) + elif subset_names_cert is not None: + question = ( + "You have an existing certificate that contains a portion of " + "the domains you requested (ref: {0}){br}{br}It contains these " + "names: {1}{br}{br}You requested these names for the new " + "certificate: {2}.{br}{br}Do you want to replace this existing " + "certificate with the new certificate?" + ).format(subset_names_cert.configfile.filename, + ", ".join(subset_names_cert.names()), + ", ".join(domains), + br=os.linesep) + if config.renew_by_default or zope.component.getUtility( interfaces.IDisplay).yesno(question, "Replace", "Cancel"): return "renew", subset_names_cert else: @@ -282,8 +280,6 @@ def _treat_as_renewal(config, domains): "User did not use proper CLI and would like " "to reinvoke the client.") - return "newcert", None - def _handle_identical_cert_request(cert): """Figure out what to do if a cert has the same names as a perviously obtained one From f9e7d880bf8501703f8288a9eaded26abc2d0465 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 11:00:38 -0800 Subject: [PATCH 098/123] Remove transient commentary --- letsencrypt/cli.py | 97 ++++++++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 50 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index acba13441..f88a053c8 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -213,79 +213,38 @@ def _find_duplicative_certs(config, domains): def _treat_as_renewal(config, domains): """Determine whether there are duplicated names and how to handle them. - :returns: Two-element tuple containing desired new-certificate - behavior as a string token, plus either a RenewableCert - instance or None if renewal shouldn't occur. + :returns: Two-element tuple containing desired new-certificate behavior as + a string token ("reinstall", "renew", or "newcert), plus either a + RenewableCert instance or None if renewal shouldn't occur. :raises .Error: If the user would like to rerun the client again. """ - # This will now instead return 3 different cases plus the .Error - # case (which raises an exception): reinstall, renew, newcert - # The return value will be a tuple of (result, cert), viz. - # either ("reinstall", RenewableCert_instance) - # or ("renew", RenewableCert_instance) - # or ("newcert", None) - # We could also return ("cancel", None) instead of raising the - # error, but the error-handling mechanism seems to work OK. - # Note that the "newcert" option is not suggested in the UI menu - # when the requested cert has precisely the same names as an - # existing cert (it's considered an "advanced" option in this - # situation, so it would have to be selected with a command-line - # flag). - # - # TODO: Also address superset case - # Considering the possibility that the requested certificate is # related to an existing certificate. (config.duplicate, which # is set with --duplicate, skips all of this logic and forces any # kind of certificate to be obtained with renewal = False.) if config.duplicate: return "newcert", None + # TODO: Also address superset case ident_names_cert, subset_names_cert = _find_duplicative_certs(config, domains) - # I am not sure whether that correctly reads the systemwide + # XXX ^ schoen is not sure whether that correctly reads the systemwide # configuration file. if not (ident_names_cert or subset_names_cert): return "newcert", None if ident_names_cert is not None: - return _handle_identical_cert_request(ident_names_cert) + return _handle_identical_cert_request(config, ident_names_cert) elif subset_names_cert is not None: - question = ( - "You have an existing certificate that contains a portion of " - "the domains you requested (ref: {0}){br}{br}It contains these " - "names: {1}{br}{br}You requested these names for the new " - "certificate: {2}.{br}{br}Do you want to replace this existing " - "certificate with the new certificate?" - ).format(subset_names_cert.configfile.filename, - ", ".join(subset_names_cert.names()), - ", ".join(domains), - br=os.linesep) - if config.renew_by_default or zope.component.getUtility( - interfaces.IDisplay).yesno(question, "Replace", "Cancel"): - return "renew", subset_names_cert - else: - reporter_util = zope.component.getUtility(interfaces.IReporter) - reporter_util.add_message( - "To obtain a new certificate that {0} an existing certificate " - "in its domain-name coverage, you must use the --duplicate " - "option.{br}{br}For example:{br}{br}{1} --duplicate {2}".format( - "duplicates" if ident_names_cert is not None else - "overlaps with", - sys.argv[0], " ".join(sys.argv[1:]), - br=os.linesep - ), - reporter_util.HIGH_PRIORITY) - raise errors.Error( - "User did not use proper CLI and would like " - "to reinvoke the client.") + return _handle_subset_cert_request(config, domains, subset_names_cert) -def _handle_identical_cert_request(cert): +def _handle_identical_cert_request(config, cert): """Figure out what to do if a cert has the same names as a perviously obtained one :param storage.RenewableCert cert: :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal + """ if config.renew_by_default or cert.should_autorenew(interactive=True): return "renew", cert @@ -312,6 +271,44 @@ def _handle_identical_cert_request(cert): else: assert False, "This is impossible" +def _handle_subset_cert_request(config, domains, cert): + """Figure out what to do if a previous cert had a subset of the names now requested + + :param storage.RenewableCert cert: + + :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal + + """ + existing = ", ".join(cert.names()) + question = ( + "You have an existing certificate that contains a portion of " + "the domains you requested (ref: {0}){br}{br}It contains these " + "names: {1}{br}{br}You requested these names for the new " + "certificate: {2}.{br}{br}Do you want to replace this existing " + "certificate with the new certificate?" + ).format(cert.configfile.filename, + existing, + ", ".join(domains), + br=os.linesep) + if config.renew_by_default or zope.component.getUtility( + interfaces.IDisplay).yesno(question, "Replace", "Cancel"): + return "renew", cert + else: + reporter_util = zope.component.getUtility(interfaces.IReporter) + reporter_util.add_message( + "To obtain a new certificate that contains these names without " + "replacing your existing certificate for {0}, you must use the " + "--duplicate option.{br}{br}" + "For example:{br}{br}{1} --duplicate {2}".format( + existing, + sys.argv[0], " ".join(sys.argv[1:]), + br=os.linesep + ), + reporter_util.HIGH_PRIORITY) + raise errors.Error( + "User did not use proper CLI and would like " + "to reinvoke the client.") + def _report_new_cert(cert_path, fullchain_path): """Reports the creation of a new certificate to the user. From 2b3f217ce2d44c55ce46579604dbdbc2fb6101b1 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Sat, 12 Dec 2015 11:03:19 -0800 Subject: [PATCH 099/123] Attempt to fix #1856 --- letsencrypt/cli.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f88a053c8..bebbd24ad 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -205,7 +205,12 @@ def _find_duplicative_certs(config, domains): if candidate_names == set(domains): identical_names_cert = candidate_lineage elif candidate_names.issubset(set(domains)): - subset_names_cert = candidate_lineage + # This logic finds and returns the largest subset-names cert + # in the case where there are several available. + if subset_names_cert is None: + subset_names_cert = candidate_lineage + elif len(candidate_names) > len(subset_names_cert.names()): + subset_names_cert = candidate_lineage return identical_names_cert, subset_names_cert From dfbf572bc418393ae0bd5cbe6ad7b85f13b7ad52 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Sat, 12 Dec 2015 11:06:20 -0800 Subject: [PATCH 100/123] Add missing quotation mark in documentation --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index bebbd24ad..6c0209660 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -219,8 +219,8 @@ def _treat_as_renewal(config, domains): """Determine whether there are duplicated names and how to handle them. :returns: Two-element tuple containing desired new-certificate behavior as - a string token ("reinstall", "renew", or "newcert), plus either a - RenewableCert instance or None if renewal shouldn't occur. + a string token ("reinstall", "renew", or "newcert"), plus either + a RenewableCert instance or None if renewal shouldn't occur. :raises .Error: If the user would like to rerun the client again. From f5fde98ab6925ed8f61352876fb4f6f5e55754a2 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sat, 12 Dec 2015 14:38:21 -0500 Subject: [PATCH 101/123] Fixed an inaccurate comment While it's true that older Pythons do not do (critical) TLS validation by default, that's not what this warning is about. --- acme/acme/client.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index 08d476783..776ddb38a 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -20,7 +20,9 @@ from acme import messages logger = logging.getLogger(__name__) -# Python does not validate certificates by default before version 2.7.9 +# Prior to Python 2.7.9 the stdlib SSL module did not allow a user to configure +# many important security related options. On these platforms we use PyOpenSSL +# for SSL, which does allow these options to be configured. # https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning if sys.version_info < (2, 7, 9): # pragma: no cover requests.packages.urllib3.contrib.pyopenssl.inject_into_urllib3() From 916a946bcd21f3ac872edd4d9cd42da656f49b11 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sat, 12 Dec 2015 14:50:26 -0500 Subject: [PATCH 102/123] Simplify the ACME example client by using an existing method --- acme/examples/example_client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/acme/examples/example_client.py b/acme/examples/example_client.py index b4b5ad010..f6b0329f5 100644 --- a/acme/examples/example_client.py +++ b/acme/examples/example_client.py @@ -28,8 +28,7 @@ acme = client.Client(DIRECTORY_URL, key) regr = acme.register() logging.info('Auto-accepting TOS: %s', regr.terms_of_service) -acme.update_registration(regr.update( - body=regr.body.update(agreement=regr.terms_of_service))) +acme.agree_to_tos(regr) logging.debug(regr) authzr = acme.request_challenges( From d92a32b9d7156f071a74d8fc479330e1e0430a9a Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 12:09:36 -0800 Subject: [PATCH 103/123] Refuse to update valid lineages with staging certs --- letsencrypt/cli.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 6c0209660..950629282 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -366,6 +366,8 @@ def _auth_from_domains(le_client, config, domains): # it without getting a new certificate at all. return lineage elif action == "renew": + orginal_server = lineage.configuration["renewalparams"]["server"] + _avoid_invalidating_lineage(config, lineage, original_server) # TODO: schoen wishes to reuse key - discussion # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574 new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) @@ -389,6 +391,17 @@ def _auth_from_domains(le_client, config, domains): return lineage +def _avoid_invalidating_lineage(config, lineage, original_server): + "Do not renew a valid cert with one from a staging server!" + def is_staging(srv): + return (srv == constants.STAGING_URI or "staging" in srv) + + if is_staging(config.server) and not is_staging(original_server): + if not config.break_my_certs: + raise errors.Error( + "You're trying to renew/replace a valid certificiate with " + "a test certificate. We will not do that unless you use the " + "--break-my-certs flag!") def set_configurator(previously, now): """ @@ -959,7 +972,10 @@ def prepare_and_parse_args(plugins, args): helpful.add( "testing", "--http-01-port", type=int, dest="http01_port", default=flag_default("http01_port"), help=config_help("http01_port")) - + helpful.add( + "testing", "--break-my-certs", action="store_true", + help="Be willing to replace or renew valid certs with invalid " + "(testing/staging) certs") helpful.add_group( "security", description="Security parameters & server settings") helpful.add( From 1cbf78284f7aa4fe05666ad8f15fe235a9be483e Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 12:14:52 -0800 Subject: [PATCH 104/123] Lint, bugfix, helpful domain list --- letsencrypt/cli.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 950629282..69a259d5b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -366,7 +366,7 @@ def _auth_from_domains(le_client, config, domains): # it without getting a new certificate at all. return lineage elif action == "renew": - orginal_server = lineage.configuration["renewalparams"]["server"] + original_server = lineage.configuration["renewalparams"]["server"] _avoid_invalidating_lineage(config, lineage, original_server) # TODO: schoen wishes to reuse key - discussion # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574 @@ -393,15 +393,16 @@ def _auth_from_domains(le_client, config, domains): def _avoid_invalidating_lineage(config, lineage, original_server): "Do not renew a valid cert with one from a staging server!" - def is_staging(srv): - return (srv == constants.STAGING_URI or "staging" in srv) + def _is_staging(srv): + return srv == constants.STAGING_URI or "staging" in srv - if is_staging(config.server) and not is_staging(original_server): + if _is_staging(config.server) and not _is_staging(original_server): if not config.break_my_certs: + names = ", ".join(lineage.names()) raise errors.Error( - "You're trying to renew/replace a valid certificiate with " - "a test certificate. We will not do that unless you use the " - "--break-my-certs flag!") + "You've asked to renew/replace a valid certificiate with " + "a test certificate (domains: {0}). We will not do that " + "unless you use the --break-my-certs flag!".format(names)) def set_configurator(previously, now): """ From d290cd36d59f0b922b0ab9f6effbd4acde2289ca Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Sat, 12 Dec 2015 12:18:01 -0800 Subject: [PATCH 105/123] Correct typo --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ca92595d7..4cf27aa81 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -400,7 +400,7 @@ def _avoid_invalidating_lineage(config, lineage, original_server): if not config.break_my_certs: names = ", ".join(lineage.names()) raise errors.Error( - "You've asked to renew/replace a valid certificiate with " + "You've asked to renew/replace a valid certificate with " "a test certificate (domains: {0}). We will not do that " "unless you use the --break-my-certs flag!".format(names)) From 411b5093e972edf532a3845e16093f42c0aebfc0 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Sat, 12 Dec 2015 12:23:16 -0800 Subject: [PATCH 106/123] Add parallel --reinstall command-line flag --- letsencrypt/cli.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4cf27aa81..c4e0dd2de 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -252,7 +252,13 @@ def _handle_identical_cert_request(config, cert): """ if config.renew_by_default or cert.should_autorenew(interactive=True): + # Set with --renew-by-default, force an identical certificate to + # be renewed without further prompting. return "renew", cert + if config.reinstall: + # Set with --reinstall, force an identical certificate to be + # reinstalled without further prompting. + return "reinstall", cert display = zope.component.getUtility(interfaces.IDisplay) question = ( "You have an existing certificate that contains exactly the same " @@ -943,6 +949,9 @@ def prepare_and_parse_args(plugins, args): helpful.add( None, "--duplicate", dest="duplicate", action="store_true", help="Allow getting a certificate that duplicates an existing one") + helpful.add( + None, "--reinstall", dest="reinstall", action="store_true", + help="Try to reinstall an existing certificate without prompting") helpful.add_group( "automation", From 0e19f43757eb90189cdfc72dc996ec2f1c06863d Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Sat, 12 Dec 2015 12:35:28 -0800 Subject: [PATCH 107/123] Let's not blame the user in the cancel message --- letsencrypt/cli.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index c4e0dd2de..3cf01d3c1 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -273,8 +273,8 @@ def _handle_identical_cert_request(config, cert): # TODO: Add notification related to command-line options for # skipping the menu for this case. raise errors.Error( - "User did not use proper CLI and would like " - "to reinvoke the client.") + "User chose to cancel the operation and may " + "reinvoke the client.") elif response[1] == 0: return "reinstall", cert elif response[1] == 1: @@ -317,8 +317,8 @@ def _handle_subset_cert_request(config, domains, cert): ), reporter_util.HIGH_PRIORITY) raise errors.Error( - "User did not use proper CLI and would like " - "to reinvoke the client.") + "User chose to cancel the operation and may " + "reinvoke the client.") def _report_new_cert(cert_path, fullchain_path): From 723d9fe048184b195ed6bcf4230fe045c345f6b0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 12:44:24 -0800 Subject: [PATCH 108/123] Handle lineages that were upgraded from staging -> production --- letsencrypt/cli.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ca92595d7..953fb8700 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -396,13 +396,22 @@ def _avoid_invalidating_lineage(config, lineage, original_server): def _is_staging(srv): return srv == constants.STAGING_URI or "staging" in srv - if _is_staging(config.server) and not _is_staging(original_server): - if not config.break_my_certs: - names = ", ".join(lineage.names()) - raise errors.Error( - "You've asked to renew/replace a valid certificiate with " - "a test certificate (domains: {0}). We will not do that " - "unless you use the --break-my-certs flag!".format(names)) + # Some lineages may have begun with --staging, but then had production certs + # added to them + latest_cert = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, + open(lineage.cert).read()) + # all our test certs are from happy hacker fake CA, though maybe one day + # we should test more methodically + now_valid = not ("fake" in repr(latest_cert.get_issuer()).lower()) + + if _is_staging(config.server): + if not _is_staging(original_server) or now_valid: + if not config.break_my_certs: + names = ", ".join(lineage.names()) + raise errors.Error( + "You've asked to renew/replace a seemingly valid certificiate with " + "a test certificate (domains: {0}). We will not do that " + "unless you use the --break-my-certs flag!".format(names)) def set_configurator(previously, now): """ From cd1c4e1e8e4038a6eb1803e968149304b29b07b8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 13:02:22 -0800 Subject: [PATCH 109/123] Fix existing test --- letsencrypt/tests/cli_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index e865099e8..ccf16f5b5 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -413,7 +413,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client') def test_certonly_renewal(self, mock_init, mock_renewal, mock_get_utility, _suggest): - cert_path = '/etc/letsencrypt/live/foo.bar/cert.pem' + cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) From eb289bcf8158be2bc1ff9f98cc88a01133a80ec3 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 13:06:58 -0800 Subject: [PATCH 110/123] Remove debugging printf --- letsencrypt/cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 3cda04009..6ea7e9b55 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -366,7 +366,6 @@ def _auth_from_domains(le_client, config, domains): # (which results in treating the request as a new certificate request). action, lineage = _treat_as_renewal(config, domains) - print action, lineage if action == "reinstall": # The lineage already exists; allow the caller to try installing # it without getting a new certificate at all. From d983429f82edd0911893a82c19211306809d39d3 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sat, 12 Dec 2015 16:12:10 -0500 Subject: [PATCH 111/123] Fixed a type in a docstring --- acme/acme/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index 776ddb38a..c3e28ef47 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -340,7 +340,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes `PollError` with non-empty ``waiting`` is raised. :returns: ``(cert, updated_authzrs)`` `tuple` where ``cert`` is - the issued certificate (`.messages.CertificateResource.), + the issued certificate (`.messages.CertificateResource`), and ``updated_authzrs`` is a `tuple` consisting of updated Authorization Resources (`.AuthorizationResource`) as present in the responses from server, and in the same order From 97f987ca41afd58e37867943d761148c355819ce Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 13:14:48 -0800 Subject: [PATCH 112/123] lint --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 6ea7e9b55..fe36b0007 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -407,7 +407,7 @@ def _avoid_invalidating_lineage(config, lineage, original_server): open(lineage.cert).read()) # all our test certs are from happy hacker fake CA, though maybe one day # we should test more methodically - now_valid = not ("fake" in repr(latest_cert.get_issuer()).lower()) + now_valid = not "fake" in repr(latest_cert.get_issuer()).lower() if _is_staging(config.server): if not _is_staging(original_server) or now_valid: From 49e596785860674159c6584c803f850b0fbd31df Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 14:09:33 -0800 Subject: [PATCH 113/123] UI and flag tuning * Add --expand or --replace, which in most cases is what people want to add new names to lineages without always forcing renewal * --reinstall is now also --keep, and the UI is aware if it's in certonly or run mode * Explain --duplicate better --- letsencrypt/cli.py | 54 ++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index fe36b0007..ae76fcae6 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -251,24 +251,33 @@ def _handle_identical_cert_request(config, cert): :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal """ - if config.renew_by_default or cert.should_autorenew(interactive=True): - # Set with --renew-by-default, force an identical certificate to - # be renewed without further prompting. + if config.renew_by_default: + logger.info("Auto-renewal forced with --renew-by-default...") + return "renew", cert + if cert.should_autorenew(interactive=True): + logger.info("Cert is due for renewal, auto-renewing...") return "renew", cert if config.reinstall: # Set with --reinstall, force an identical certificate to be # reinstalled without further prompting. return "reinstall", cert - display = zope.component.getUtility(interfaces.IDisplay) + question = ( "You have an existing certificate that contains exactly the same " - "domains you requested.{br}(ref: {0}){br}{br}What would you like to do?" + "domains you requested and isn't close to expiry." + "{br}(ref: {0}){br}{br}What would you like to do?" ).format(cert.configfile.filename, br=os.linesep) - response = display.menu( - question, ["Attempt to reinstall this existing certificate", - "Renew & replace the cert (limit ~5 per 7 days)", - "Cancel this operation and do nothing"], - "OK", "Cancel") + + if config.verb == "run": + keep_opt = "Attempt to reinstall this existing certificate" + elif config.verb == "certonly": + keep_opt = "Keep the existing certificate for now" + choices = [keep_opt, + "Renew & replace the cert (limit ~5 per 7 days)", + "Cancel this operation and do nothing"] + + display = zope.component.getUtility(interfaces.IDisplay) + response = display.menu(question, choices, "OK", "Cancel") if response[0] == "cancel" or response[1] == 2: # TODO: Add notification related to command-line options for # skipping the menu for this case. @@ -301,7 +310,7 @@ def _handle_subset_cert_request(config, domains, cert): existing, ", ".join(domains), br=os.linesep) - if config.renew_by_default or zope.component.getUtility( + if config.expand or config.renew_by_default or zope.component.getUtility( interfaces.IDisplay).yesno(question, "Replace", "Cancel"): return "renew", cert else: @@ -772,6 +781,7 @@ class HelpfulArgumentParser(object): """ parsed_args = self.parser.parse_args(self.args) parsed_args.func = self.VERBS[self.verb] + parsed_args.verb = self.verb # Do any post-parsing homework here @@ -955,12 +965,11 @@ def prepare_and_parse_args(plugins, args): "multiple -d flags or enter a comma separated list of domains " "as a parameter.") helpful.add( - None, "--duplicate", dest="duplicate", action="store_true", - help="Allow getting a certificate that duplicates an existing one") - helpful.add( - None, "--reinstall", dest="reinstall", action="store_true", - help="Try to reinstall an existing certificate without prompting") - + None, "--keep-until-expiring", "--keep", "--reinstall", + dest="reinstall", action="store_true", + help="If the requested cert matches an existing cert, keep the " + "existing one by default until it is due for renewal (for the " + "'run' subcommand this means reinstall the existing cert)") helpful.add_group( "automation", description="Arguments for automating execution & other tweaks") @@ -968,16 +977,25 @@ def prepare_and_parse_args(plugins, args): "automation", "--version", action="version", version="%(prog)s {0}".format(letsencrypt.__version__), help="show program's version number and exit") + helpful.add( + "automation", "--expand", "--expand-existing-certs", "--replace", action="store_true", + help="If an existing cert covers some subset of the requested names, " + "expand and replace it with the additional names.") helpful.add( "automation", "--renew-by-default", action="store_true", help="Select renewal by default when domains are a superset of a " - "previously attained cert") + "previously attained cert (often --keep-until-expiring is " + "more appropriate). Implies --expand.") helpful.add( "automation", "--agree-tos", dest="tos", action="store_true", help="Agree to the Let's Encrypt Subscriber Agreement") helpful.add( "automation", "--account", metavar="ACCOUNT_ID", help="Account ID to use") + helpful.add( + "automation", "--duplicate", dest="duplicate", action="store_true", + help="Allow making a certificate lineage that duplicates an existing one " + "(mostly useful for multiple webservers with distinct keys)") helpful.add_group( "testing", description="The following flags are meant for " From 8ed70c18cc46f4cc54b5c69dcd7f85f31996443f Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 14:14:18 -0800 Subject: [PATCH 114/123] Tweak docs for --duplicate --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ae76fcae6..81a5f4f65 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -995,7 +995,7 @@ def prepare_and_parse_args(plugins, args): helpful.add( "automation", "--duplicate", dest="duplicate", action="store_true", help="Allow making a certificate lineage that duplicates an existing one " - "(mostly useful for multiple webservers with distinct keys)") + "(both can be renewed in parallel)") helpful.add_group( "testing", description="The following flags are meant for " From 685ec6b5398342b99422cf50b37e78dd61aedf1d Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 14:21:05 -0800 Subject: [PATCH 115/123] obtain_cert isn't dead; maybe it should be called certonly though... --- letsencrypt/cli.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 81a5f4f65..ed0bc230d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -570,8 +570,6 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo def obtain_cert(args, config, plugins): """Authenticate & obtain cert, but do not install it.""" - # TODO: Is this now dead code? What calls it? - if args.domains and args.csr is not None: # TODO: --csr could have a priority, when --domains is # supplied, check if CSR matches given domains? From b34887437246b372a9687ff9222a1c7bed77b02e Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 16:39:52 -0800 Subject: [PATCH 116/123] Address review comments, fine tune flag names --- letsencrypt/cli.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ed0bc230d..317702497 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -235,7 +235,7 @@ def _treat_as_renewal(config, domains): ident_names_cert, subset_names_cert = _find_duplicative_certs(config, domains) # XXX ^ schoen is not sure whether that correctly reads the systemwide # configuration file. - if not (ident_names_cert or subset_names_cert): + if ident_names_cert is None and subset_names_cert is None: return "newcert", None if ident_names_cert is not None: @@ -249,6 +249,7 @@ def _handle_identical_cert_request(config, cert): :param storage.RenewableCert cert: :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal + :rtype: tuple """ if config.renew_by_default: @@ -297,6 +298,7 @@ def _handle_subset_cert_request(config, domains, cert): :param storage.RenewableCert cert: :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal + :rtype: tuple """ existing = ", ".join(cert.names()) @@ -962,23 +964,23 @@ def prepare_and_parse_args(plugins, args): help="Domain names to apply. For multiple domains you can use " "multiple -d flags or enter a comma separated list of domains " "as a parameter.") - helpful.add( - None, "--keep-until-expiring", "--keep", "--reinstall", - dest="reinstall", action="store_true", - help="If the requested cert matches an existing cert, keep the " - "existing one by default until it is due for renewal (for the " - "'run' subcommand this means reinstall the existing cert)") helpful.add_group( "automation", description="Arguments for automating execution & other tweaks") + helpful.add( + "automation", "--keep-until-expiring", "-k", "--reinstall", + dest="reinstall", action="store_true", + help="If the requested cert matches an existing cert, always keep the " + "existing one until it is due for renewal (for the " + "'run' subcommand this means reinstall the existing cert)") + helpful.add( + "automation", "--expand", action="store_true", + help="If an existing cert covers some subset of the requested names, " + "always expand and replace it with the additional names.") helpful.add( "automation", "--version", action="version", version="%(prog)s {0}".format(letsencrypt.__version__), help="show program's version number and exit") - helpful.add( - "automation", "--expand", "--expand-existing-certs", "--replace", action="store_true", - help="If an existing cert covers some subset of the requested names, " - "expand and replace it with the additional names.") helpful.add( "automation", "--renew-by-default", action="store_true", help="Select renewal by default when domains are a superset of a " From 173ea94e17bbcdc321ab4607e78491a2f0481753 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 17:01:43 -0800 Subject: [PATCH 117/123] Further flag tweaking --- letsencrypt/cli.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 317702497..5e06d00d6 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -306,14 +306,14 @@ def _handle_subset_cert_request(config, domains, cert): "You have an existing certificate that contains a portion of " "the domains you requested (ref: {0}){br}{br}It contains these " "names: {1}{br}{br}You requested these names for the new " - "certificate: {2}.{br}{br}Do you want to replace this existing " + "certificate: {2}.{br}{br}Do you want to expand and replace this existing " "certificate with the new certificate?" ).format(cert.configfile.filename, existing, ", ".join(domains), br=os.linesep) if config.expand or config.renew_by_default or zope.component.getUtility( - interfaces.IDisplay).yesno(question, "Replace", "Cancel"): + interfaces.IDisplay).yesno(question, "Expand", "Cancel"): return "renew", cert else: reporter_util = zope.component.getUtility(interfaces.IReporter) @@ -968,7 +968,7 @@ def prepare_and_parse_args(plugins, args): "automation", description="Arguments for automating execution & other tweaks") helpful.add( - "automation", "--keep-until-expiring", "-k", "--reinstall", + "automation", "--keep-until-expiring", "--keep", "--reinstall", dest="reinstall", action="store_true", help="If the requested cert matches an existing cert, always keep the " "existing one until it is due for renewal (for the " From aee25fb05ee63ce42ed23beef08a30904c9039c8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 17:21:24 -0800 Subject: [PATCH 118/123] Attempt to fix ridiculous log message --- letsencrypt/storage.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index f457fe13e..01ff0677f 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -567,8 +567,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes "cert", self.latest_common_version())) now = pytz.UTC.fromutc(datetime.datetime.utcnow()) if expiry < add_time_interval(now, interval): - logger.debug("Should renew, certificate " - "has been expired since %s.", + logger.debug("Should renew, less than %r days before certificate " + "expiry %s.", interval, expiry.strftime("%Y-%m-%d %H:%M:%S %Z")) return True return False From a99f1d1395d6ffab7b0b41cc8a9b6a4bb3386890 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 12 Dec 2015 17:24:05 -0800 Subject: [PATCH 119/123] This should be maximally legible. --- letsencrypt/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 01ff0677f..3b2b548b0 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -567,7 +567,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes "cert", self.latest_common_version())) now = pytz.UTC.fromutc(datetime.datetime.utcnow()) if expiry < add_time_interval(now, interval): - logger.debug("Should renew, less than %r days before certificate " + logger.debug("Should renew, less than %s before certificate " "expiry %s.", interval, expiry.strftime("%Y-%m-%d %H:%M:%S %Z")) return True From f299646bdb75038cc855efa069580913bb1dca9c Mon Sep 17 00:00:00 2001 From: Patrick Figel Date: Wed, 9 Dec 2015 23:11:55 +0100 Subject: [PATCH 120/123] Add staging server hint to avoid rate limit issues --- docs/using.rst | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 51426183d..694eac9fe 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -31,16 +31,21 @@ Firstly, please `install Git`_ and run the following commands: .. _`install Git`: https://git-scm.com/book/en/v2/Getting-Started-Installing-Git +.. note:: On RedHat/CentOS 6 you will need to enable the EPEL_ + repository before install. + +.. _EPEL: http://fedoraproject.org/wiki/EPEL + To install and run the client you just need to type: .. code-block:: shell ./letsencrypt-auto -.. note:: On RedHat/CentOS 6 you will need to enable the EPEL_ - repository before install. - -.. _EPEL: http://fedoraproject.org/wiki/EPEL +.. hint:: During the beta phase, Let's Encrypt enforces strict rate limits on + the number of certificates issued for one domain. It is recommended to + initially use the test server via `--test-cert` until you get the desired + certificates. Throughout the documentation, whenever you see references to ``letsencrypt`` script/binary, you can substitute in From faa7946a3acd27b8c4c231fe32c8e59aae663339 Mon Sep 17 00:00:00 2001 From: Harlan Lieberman-Berg Date: Sun, 13 Dec 2015 18:14:12 -0500 Subject: [PATCH 121/123] Update Debian using instructions. --- docs/using.rst | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 687901191..1d947f00c 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -58,8 +58,8 @@ or for full help, type: ``letsencrypt-auto`` is the recommended method of running the Let's Encrypt -client beta releases on systems that don't have a packaged version. Debian -experimental, Arch linux and FreeBSD now have native packages, so on those +client beta releases on systems that don't have a packaged version. Debian, +Arch linux and FreeBSD now have native packages, so on those systems you can just install ``letsencrypt`` (and perhaps ``letsencrypt-apache``). If you'd like to run the latest copy from Git, or run your own locally modified copy of the client, follow the instructions in @@ -173,10 +173,10 @@ Renewal In order to renew certificates simply call the ``letsencrypt`` (or letsencrypt-auto_) again, and use the same values when prompted. You can automate it slightly by passing necessary flags on the CLI (see -`--help all`), or even further using the :ref:`config-file`. The -``--renew-by-default`` flag may be helpful for automating renewal. If -you're sure that UI doesn't prompt for any details you can add the -command to ``crontab`` (make it less than every 90 days to avoid +`--help all`), or even further using the :ref:`config-file`. The +``--renew-by-default`` flag may be helpful for automating renewal. If +you're sure that UI doesn't prompt for any details you can add the +command to ``crontab`` (make it less than every 90 days to avoid problems, say every month). Please note that the CA will send notification emails to the address @@ -352,20 +352,20 @@ Operating System Packages sudo pacman -S letsencrypt letsencrypt-apache -**Debian Experimental** +**Debian** -If you run Debian unstable, you can install experimental letsencrypt packages. -Add the line ``deb http://ftp.us.debian.org/debian/ experimental main`` (or -the equivalent for your country) to ``/etc/apt/sources.list``, then run +If you run Debian Stretch or Debian Sid, you can install letsencrypt packages. .. code-block:: shell sudo apt-get update - sudo apt-get -t experimental install letsencrypt python-letsencrypt-apache + sudo apt-get install letsencrypt python-letsencrypt-apache If you don't want to use the Apache plugin, you can ommit the ``python-letsencrypt-apache`` package. +Packages for Debian Jessie are coming in the next few weeks. + **Other Operating Systems** OS packaging is an ongoing effort. If you'd like to package From bbd2d0b996d8b7ae453a2653c453101605b32fee Mon Sep 17 00:00:00 2001 From: bmw Date: Tue, 15 Dec 2015 11:28:02 -0800 Subject: [PATCH 122/123] Revert "Add staging server hint to avoid rate limit issues" --- docs/using.rst | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 931b56164..687901191 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -31,21 +31,16 @@ Firstly, please `install Git`_ and run the following commands: .. _`install Git`: https://git-scm.com/book/en/v2/Getting-Started-Installing-Git -.. note:: On RedHat/CentOS 6 you will need to enable the EPEL_ - repository before install. - -.. _EPEL: http://fedoraproject.org/wiki/EPEL - To install and run the client you just need to type: .. code-block:: shell ./letsencrypt-auto -.. hint:: During the beta phase, Let's Encrypt enforces strict rate limits on - the number of certificates issued for one domain. It is recommended to - initially use the test server via `--test-cert` until you get the desired - certificates. +.. note:: On RedHat/CentOS 6 you will need to enable the EPEL_ + repository before install. + +.. _EPEL: http://fedoraproject.org/wiki/EPEL Throughout the documentation, whenever you see references to ``letsencrypt`` script/binary, you can substitute in From 353ae045e8134870eb74f7a1a2ddad1ff7787577 Mon Sep 17 00:00:00 2001 From: bmw Date: Tue, 15 Dec 2015 17:15:37 -0800 Subject: [PATCH 123/123] Revert "Revert "Add staging server hint to avoid rate limit issues"" --- docs/using.rst | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 1d947f00c..1423d6eba 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -31,16 +31,21 @@ Firstly, please `install Git`_ and run the following commands: .. _`install Git`: https://git-scm.com/book/en/v2/Getting-Started-Installing-Git +.. note:: On RedHat/CentOS 6 you will need to enable the EPEL_ + repository before install. + +.. _EPEL: http://fedoraproject.org/wiki/EPEL + To install and run the client you just need to type: .. code-block:: shell ./letsencrypt-auto -.. note:: On RedHat/CentOS 6 you will need to enable the EPEL_ - repository before install. - -.. _EPEL: http://fedoraproject.org/wiki/EPEL +.. hint:: During the beta phase, Let's Encrypt enforces strict rate limits on + the number of certificates issued for one domain. It is recommended to + initially use the test server via `--test-cert` until you get the desired + certificates. Throughout the documentation, whenever you see references to ``letsencrypt`` script/binary, you can substitute in