From 275f083a33402821f789d8f4ebaff75b796dc0c2 Mon Sep 17 00:00:00 2001 From: Dev & Sec Date: Mon, 2 Nov 2015 00:55:29 +0000 Subject: [PATCH 01/63] Use su if sudo is not available, this fixes #1148 --- letsencrypt-auto | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index d163998aa..e14b099a9 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -14,7 +14,15 @@ VENV_PATH=${VENV_PATH:-"$XDG_DATA_HOME/$VENV_NAME"} VENV_BIN=${VENV_PATH}/bin if test "`id -u`" -ne "0" ; then - SUDO=sudo + if type sudo &>/dev/null; then + SUDO=sudo + else + args=("$@") + for i in "${!args[@]}"; do + args[$i]="'$(printf "%s" "${args[$i]}" | sed -e "s/'/'\"'\"'/g")' " + done + exec su root -c "$0 ${args[*]}" + fi else SUDO= fi From 8bad8de1c673ee21f8cbdb7dcec1bc425586ba6d Mon Sep 17 00:00:00 2001 From: Dev & Sec Date: Mon, 2 Nov 2015 21:49:22 +0000 Subject: [PATCH 02/63] not running the letsencrypt-auto script as root, but use su if sudo not found --- letsencrypt-auto | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index e14b099a9..514f03f46 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -14,15 +14,16 @@ VENV_PATH=${VENV_PATH:-"$XDG_DATA_HOME/$VENV_NAME"} VENV_BIN=${VENV_PATH}/bin if test "`id -u`" -ne "0" ; then - if type sudo &>/dev/null; then - SUDO=sudo - else - args=("$@") - for i in "${!args[@]}"; do - args[$i]="'$(printf "%s" "${args[$i]}" | sed -e "s/'/'\"'\"'/g")' " - done - exec su root -c "$0 ${args[*]}" + if ! type sudo &>/dev/null; then + function sudo (){ + args=("$@") + for i in "${!args[@]}"; do + args[$i]="'$(printf "%s" "${args[$i]}" | sed -e "s/'/'\"'\"'/g")' " + done + su root -c "${args[*]}" + } fi + SUDO=sudo else SUDO= fi From ed173d9c9aac7b0e9b2ffeb782b42ef6663cc58e Mon Sep 17 00:00:00 2001 From: Dev & Sec Date: Tue, 3 Nov 2015 22:22:49 +0000 Subject: [PATCH 03/63] fix sh compatibility --- letsencrypt-auto | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index 514f03f46..cfc830c9f 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -15,12 +15,13 @@ VENV_BIN=${VENV_PATH}/bin if test "`id -u`" -ne "0" ; then if ! type sudo &>/dev/null; then - function sudo (){ - args=("$@") - for i in "${!args[@]}"; do - args[$i]="'$(printf "%s" "${args[$i]}" | sed -e "s/'/'\"'\"'/g")' " + sudo() { + args="" + while [ $# -ne 0 ]; do + args="$args'$(printf "%s" "$1" | sed -e "s/'/'\"'\"'/g")' " + shift done - su root -c "${args[*]}" + su root -c "$args" } fi SUDO=sudo From 5c8ad3666b9bef423e846453c50450b32a3d983d Mon Sep 17 00:00:00 2001 From: Dev & Sec Date: Tue, 3 Nov 2015 22:34:07 +0000 Subject: [PATCH 04/63] fix sudo function name scope issue, it is not a local function --- letsencrypt-auto | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index cfc830c9f..c352482a2 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -14,8 +14,10 @@ VENV_PATH=${VENV_PATH:-"$XDG_DATA_HOME/$VENV_NAME"} VENV_BIN=${VENV_PATH}/bin if test "`id -u`" -ne "0" ; then - if ! type sudo &>/dev/null; then - sudo() { + if type sudo &>/dev/null; then + SUDO=sudo + else + su_sudo() { args="" while [ $# -ne 0 ]; do args="$args'$(printf "%s" "$1" | sed -e "s/'/'\"'\"'/g")' " @@ -23,8 +25,8 @@ if test "`id -u`" -ne "0" ; then done su root -c "$args" } + SUDO=su_sudo fi - SUDO=sudo else SUDO= fi From a7de3d59dadd9d3dfc88c8a684a4f9b939398739 Mon Sep 17 00:00:00 2001 From: Dev & Sec Date: Tue, 3 Nov 2015 22:46:56 +0000 Subject: [PATCH 05/63] fix `dash` compatibility issue caused by `&>` redirect symbol --- letsencrypt-auto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index c352482a2..8626ab329 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -14,7 +14,7 @@ VENV_PATH=${VENV_PATH:-"$XDG_DATA_HOME/$VENV_NAME"} VENV_BIN=${VENV_PATH}/bin if test "`id -u`" -ne "0" ; then - if type sudo &>/dev/null; then + if type sudo 1>/dev/null 2>&1; then SUDO=sudo else su_sudo() { From ec5c28980dbcb787c3c89a40a49a8c421294953d Mon Sep 17 00:00:00 2001 From: John Leach Date: Thu, 5 Nov 2015 18:52:10 +0000 Subject: [PATCH 06/63] docker: Use full filename when copying symlink Works around an upstream bug in Docker: https://github.com/docker/docker/issues/17730 Fixes #1374 --- Dockerfile | 3 ++- Dockerfile-dev | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index b9ea168de..e3be01dee 100644 --- a/Dockerfile +++ b/Dockerfile @@ -21,7 +21,8 @@ WORKDIR /opt/letsencrypt # If doesn't exist, it is created along with all missing # directories in its path. -COPY bootstrap/ubuntu.sh /opt/letsencrypt/src/ + +COPY bootstrap/ubuntu.sh /opt/letsencrypt/src/ubuntu.sh RUN /opt/letsencrypt/src/ubuntu.sh && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* \ diff --git a/Dockerfile-dev b/Dockerfile-dev index daa8e9af0..db9ecf5b8 100644 --- a/Dockerfile-dev +++ b/Dockerfile-dev @@ -22,7 +22,7 @@ WORKDIR /opt/letsencrypt # TODO: Install non-default Python versions for tox. # TODO: Install Apache/Nginx for plugin development. -COPY bootstrap/ubuntu.sh /opt/letsencrypt/src/ +COPY bootstrap/ubuntu.sh /opt/letsencrypt/src/ubuntu.sh RUN /opt/letsencrypt/src/ubuntu.sh && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* \ From 9f8ab4677ef4a8caaca077ae93bb288c6b35bc62 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 5 Nov 2015 14:49:21 +0200 Subject: [PATCH 07/63] Ignoring mod_macro virtualhosts, but displaying a notification why it's done --- .../letsencrypt_apache/configurator.py | 43 +++++++++++++++++-- letsencrypt-apache/letsencrypt_apache/obj.py | 6 ++- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d376fe4b6..e1f53f269 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -348,8 +348,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ all_names = set() + vhost_macro = [] + for vhost in self.vhosts: all_names.update(vhost.get_names()) + if vhost.modmacro: + vhost_macro.append(vhost.filep) for addr in vhost.addrs: if common.hostname_regex.match(addr.get_addr()): @@ -359,6 +363,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if name: all_names.add(name) + if len(vhost_macro) > 0: + zope.component.getUtility(interfaces.IDisplay).notification( + "Apache mod_macro seems to be in use in file(s):\n{0}" + "\n\nUnfortunately mod_macro is not yet supported".format( + "\n ".join(vhost_macro))) + return all_names def get_name_from_ip(self, addr): # pylint: disable=no-self-use @@ -395,11 +405,34 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "ServerAlias", None, start=host.path, exclude=False) for alias in serveralias_match: - host.aliases.add(self.parser.get_arg(alias)) + serveralias = self.parser.get_arg(alias) + if not self._is_mod_macro(serveralias): + host.aliases.add(serveralias) + else: + host.modmacro = True if servername_match: # Get last ServerName as each overwrites the previous - host.name = self.parser.get_arg(servername_match[-1]) + servername = self.parser.get_arg(servername_match[-1]) + if not self._is_mod_macro(servername): + host.name = servername + else: + host.modmacro = True + + def _is_mod_macro(self, name): + """Helper function for _add_servernames(). + Checks if the ServerName / ServerAlias belongs to a macro + + :param str name: Name to check and filter out if it's a variable + + :returns: boolean + :rtype: boolean + + """ + + if "$" in name: + return True + return False def _create_vhost(self, path): """Used by get_virtual_hosts to create vhost objects @@ -1236,7 +1269,7 @@ def get_file_path(vhost_path): avail_fp = vhost_path[6:] # This can be optimized... while True: - # Cast both to lowercase to be case insensitive + # Cast all to lowercase to be case insensitive find_if = avail_fp.lower().find("/ifmodule") if find_if != -1: avail_fp = avail_fp[:find_if] @@ -1245,6 +1278,10 @@ def get_file_path(vhost_path): if find_vh != -1: avail_fp = avail_fp[:find_vh] continue + find_macro = avail_fp.lower().find("/macro") + if find_macro != -1: + avail_fp = avail_fp[:find_macro] + continue break return avail_fp diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index 58a6c740e..d71cfb1ad 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -102,6 +102,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods :ivar bool ssl: SSLEngine on in vhost :ivar bool enabled: Virtual host is enabled + :ivar bool modmacro: VirtualHost is using mod_macro https://httpd.apache.org/docs/2.4/vhosts/details.html @@ -112,7 +113,9 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods # ?: is used for not returning enclosed characters strip_name = re.compile(r"^(?:.+://)?([^ :$]*)") - def __init__(self, filep, path, addrs, ssl, enabled, name=None, aliases=None): + def __init__(self, filep, path, addrs, ssl, enabled, modmacro=False, + name=None, aliases=None): + # pylint: disable=too-many-arguments """Initialize a VH.""" self.filep = filep @@ -122,6 +125,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods self.aliases = aliases if aliases is not None else set() self.ssl = ssl self.enabled = enabled + self.modmacro = modmacro def get_names(self): """Return a set of all names.""" From aa0161fbec624c08c14a0d7aedb4f1cedbb2ad32 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 5 Nov 2015 18:53:26 +0200 Subject: [PATCH 08/63] Let's avoid breaking backwards compatibility --- letsencrypt-apache/letsencrypt_apache/obj.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index d71cfb1ad..80f92cab3 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -113,8 +113,8 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods # ?: is used for not returning enclosed characters strip_name = re.compile(r"^(?:.+://)?([^ :$]*)") - def __init__(self, filep, path, addrs, ssl, enabled, modmacro=False, - name=None, aliases=None): + def __init__(self, filep, path, addrs, ssl, enabled, name=None, + aliases=None, modmacro=False): # pylint: disable=too-many-arguments """Initialize a VH.""" From 4bd0330ae7b6a568350e18b358ce66deceeb0f4f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 10:55:35 +0200 Subject: [PATCH 09/63] Do not suggest mod_macro vhost for the best vhost --- .../letsencrypt_apache/configurator.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index e1f53f269..a1227b0bb 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -307,6 +307,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): best_points = 0 for vhost in self.vhosts: + if vhost.modmacro is True: + continue if target_name in vhost.get_names(): points = 2 elif any(addr.get_addr() == target_name for addr in vhost.addrs): @@ -326,12 +328,27 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # No winners here... is there only one reasonable vhost? if best_candidate is None: # reasonable == Not all _default_ addrs - reasonable_vhosts = self._non_default_vhosts() + # remove mod_macro hosts from reasonable vhosts + reasonable_vhosts = self._without_modmacro( + self._non_default_vhosts()) if len(reasonable_vhosts) == 1: best_candidate = reasonable_vhosts[0] + if best_candidate is not None and best_candidate.modmacro is True: + return None return best_candidate + def _without_modmacro(self, vhosts): + """Return all non mod_macro vhosts + + :param vhosts: List of VirtualHosts + :type vhosts: (:class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost`) + + :returns: List of VirtualHosts without mod_macro + :rtype: (:class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost`) + """ + return [vh for vh in vhosts if vh.modmacro == False] + def _non_default_vhosts(self): """Return all non _default_ only vhosts.""" return [vh for vh in self.vhosts if not all( From ba3db558d5295d0ec2866c40a27a2abb102d7c79 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 10:56:50 +0200 Subject: [PATCH 10/63] Tests taking mod_macro into account --- .../tests/configurator_test.py | 20 ++++++++++++------- .../tests/display_ops_test.py | 4 ++-- .../letsencrypt_apache/tests/parser_test.py | 2 +- .../sites-available/mod_macro-example.conf | 15 ++++++++++++++ .../sites-enabled/mod_macro-example.conf | 1 + .../letsencrypt_apache/tests/util.py | 5 +++++ 6 files changed, 37 insertions(+), 10 deletions(-) create mode 100644 letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf create mode 120000 letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 7c2137c45..c74fa3241 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -59,14 +59,20 @@ class TwoVhost80Test(util.ApacheTest): # Weak test.. ApacheConfigurator.add_parser_arguments(mock.MagicMock()) - def test_get_all_names(self): + @mock.patch("zope.component.getUtility") + def test_get_all_names(self, mock_getutility): + 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"])) + @mock.patch("zope.component.getUtility") @mock.patch("letsencrypt_apache.configurator.socket.gethostbyaddr") - def test_get_all_names_addrs(self, mock_gethost): + def test_get_all_names_addrs(self, mock_gethost, mock_getutility): mock_gethost.side_effect = [("google.com", "", ""), socket.error] + notification = mock.Mock() + notification.notification = mock.Mock(return_value=True) + mock_getutility.return_value = notification vhost = obj.VirtualHost( "fp", "ap", set([obj.Addr(("8.8.8.8", "443")), @@ -97,7 +103,7 @@ class TwoVhost80Test(util.ApacheTest): """ vhs = self.config.get_virtual_hosts() - self.assertEqual(len(vhs), 4) + self.assertEqual(len(vhs), 5) found = 0 for vhost in vhs: @@ -108,7 +114,7 @@ class TwoVhost80Test(util.ApacheTest): else: raise Exception("Missed: %s" % vhost) # pragma: no cover - self.assertEqual(found, 4) + self.assertEqual(found, 5) @mock.patch("letsencrypt_apache.display_ops.select_vhost") def test_choose_vhost_none_avail(self, mock_select): @@ -174,7 +180,7 @@ class TwoVhost80Test(util.ApacheTest): def test_non_default_vhosts(self): # pylint: disable=protected-access - self.assertEqual(len(self.config._non_default_vhosts()), 3) + self.assertEqual(len(self.config._non_default_vhosts()), 4) def test_is_site_enabled(self): """Test if site is enabled. @@ -345,7 +351,7 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(self.config.is_name_vhost(self.vh_truth[0]), self.config.is_name_vhost(ssl_vhost)) - self.assertEqual(len(self.config.vhosts), 5) + self.assertEqual(len(self.config.vhosts), 6) def test_make_vhost_ssl_extra_vhs(self): self.config.aug.match = mock.Mock(return_value=["p1", "p2"]) @@ -587,7 +593,7 @@ class TwoVhost80Test(util.ApacheTest): self.vh_truth[1].aliases = set(["yes.default.com"]) self.config._enable_redirect(self.vh_truth[1], "") # pylint: disable=protected-access - self.assertEqual(len(self.config.vhosts), 5) + self.assertEqual(len(self.config.vhosts), 6) def get_achalls(self): """Return testing achallenges.""" diff --git a/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py b/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py index d7cfb09b3..6db319d87 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py @@ -57,7 +57,7 @@ class SelectVhostTest(unittest.TestCase): @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility") def test_multiple_names(self, mock_util): - mock_util().menu.return_value = (display_util.OK, 4) + mock_util().menu.return_value = (display_util.OK, 5) self.vhosts.append( obj.VirtualHost( @@ -65,7 +65,7 @@ class SelectVhostTest(unittest.TestCase): False, False, "wildcard.com", set(["*.wildcard.com"]))) - self.assertEqual(self.vhosts[4], self._call(self.vhosts)) + self.assertEqual(self.vhosts[5], self._call(self.vhosts)) if __name__ == "__main__": diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index d2e4dec14..bc1f316f9 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -52,7 +52,7 @@ class BasicParserTest(util.ParserTest): test2 = self.parser.find_dir("documentroot") self.assertEqual(len(test), 1) - self.assertEqual(len(test2), 3) + self.assertEqual(len(test2), 4) def test_add_dir(self): aug_default = "/files" + self.parser.loc["default"] diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf new file mode 100644 index 000000000..6a6579007 --- /dev/null +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf @@ -0,0 +1,15 @@ + + + ServerName $domain + ServerAlias www.$domain + DocumentRoot /var/www/html + + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + +Use VHost macro1 test.com +Use VHost macro2 hostname.org +Use VHost macro3 apache.org + +# vim: syntax=apache ts=4 sw=4 sts=4 sr noet diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf new file mode 120000 index 000000000..44f254304 --- /dev/null +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf @@ -0,0 +1 @@ +../sites-available/mod_macro-example.conf \ No newline at end of file diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index 2594ba773..a8bfe0e4b 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -124,6 +124,11 @@ def get_vh_truth(temp_dir, config_name): os.path.join(aug_pre, "letsencrypt.conf/VirtualHost"), set([obj.Addr.fromstring("*:80")]), False, True, "letsencrypt.demo"), + obj.VirtualHost( + os.path.join(prefix, "mod_macro-example.conf"), + os.path.join(aug_pre, + "mod_macro-example.conf/Macro/VirtualHost"), + set([obj.Addr.fromstring("*:80")]), False, True, modmacro=True) ] return vh_truth From 93a53d507821ac0612aadb62d70edf5f3856c468 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 11:11:14 +0200 Subject: [PATCH 11/63] Added test for removing mod_macro vhosts from vhost list --- .../letsencrypt_apache/tests/configurator_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index c74fa3241..4224749bf 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -178,6 +178,10 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual( self.config._find_best_vhost("example.demo"), self.vh_truth[2]) + def test_without_modmacro(self): + self.assertEqual(len(self.vh_truth)-1, + len(self.config._without_modmacro(self.vh_truth))) + def test_non_default_vhosts(self): # pylint: disable=protected-access self.assertEqual(len(self.config._non_default_vhosts()), 4) From 88b89a04b1e2aabdfc692c2143223da31ba5c03d Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 12:08:28 +0200 Subject: [PATCH 12/63] Fix pylint in the new test --- letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 4224749bf..2e335ea00 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -179,6 +179,7 @@ class TwoVhost80Test(util.ApacheTest): self.config._find_best_vhost("example.demo"), self.vh_truth[2]) def test_without_modmacro(self): + # pylint: disable=protected-access self.assertEqual(len(self.vh_truth)-1, len(self.config._without_modmacro(self.vh_truth))) From f0c059752fcd8b36862e12c77a98ce351c6d5442 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 12:15:53 +0200 Subject: [PATCH 13/63] Added test for mod_macro check in domain names --- .../letsencrypt_apache/tests/configurator_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 2e335ea00..963948d6e 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -183,6 +183,11 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(len(self.vh_truth)-1, len(self.config._without_modmacro(self.vh_truth))) + def test_is_mod_macro(self): + # pylint: disable=protected-access + self.assertEqual(self.config._is_mod_macro("$domain"), True) + self.assertEqual(self.config._is_mod_macro("www.example.com"), False) + def test_non_default_vhosts(self): # pylint: disable=protected-access self.assertEqual(len(self.config._non_default_vhosts()), 4) From 8fb3956ecd2b46c42e18d97b19129f4e31266a89 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 12:45:44 +0200 Subject: [PATCH 14/63] Removed dead code --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index a1227b0bb..9da00b5c3 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -334,8 +334,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if len(reasonable_vhosts) == 1: best_candidate = reasonable_vhosts[0] - if best_candidate is not None and best_candidate.modmacro is True: - return None return best_candidate def _without_modmacro(self, vhosts): From effc1ed8e4595f784983c83d9063e5cf8ace8d6f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 19:53:13 +0200 Subject: [PATCH 15/63] Fix VirtualHost magic methods to take mod_macro addition into account --- letsencrypt-apache/letsencrypt_apache/obj.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index 80f92cab3..175ce3f92 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -145,21 +145,25 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods "Name: {name}\n" "Aliases: {aliases}\n" "TLS Enabled: {tls}\n" - "Site Enabled: {active}".format( + "Site Enabled: {active}\n" + "mod_macro Vhost: {modmacro}".format( filename=self.filep, vhpath=self.path, addrs=", ".join(str(addr) for addr in self.addrs), name=self.name if self.name is not None else "", aliases=", ".join(name for name in self.aliases), tls="Yes" if self.ssl else "No", - active="Yes" if self.enabled else "No")) + active="Yes" if self.enabled else "No", + modmacro="Yes" if self.modmacro else "No")) def __eq__(self, other): if isinstance(other, self.__class__): return (self.filep == other.filep and self.path == other.path and self.addrs == other.addrs and self.get_names() == other.get_names() and - self.ssl == other.ssl and self.enabled == other.enabled) + self.ssl == other.ssl and + self.enabled == other.enabled and + self.modmacro == other.modmacro) return False From 980b4ac3fae6ccc9d4def3fdeb35b160af55bdca Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 20:06:56 +0200 Subject: [PATCH 16/63] PEP8 and sphinx documentation fixes --- letsencrypt-apache/letsencrypt_apache/configurator.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 9da00b5c3..d3849b7fd 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -340,12 +340,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """Return all non mod_macro vhosts :param vhosts: List of VirtualHosts - :type vhosts: (:class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost`) + :type vhosts: :class:`list` of + :class:`~letsencrypt_apache.obj.VirtualHost` :returns: List of VirtualHosts without mod_macro - :rtype: (:class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost`) + :rtype: :class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost` """ - return [vh for vh in vhosts if vh.modmacro == False] + return [vh for vh in vhosts if vh.modmacro is False] def _non_default_vhosts(self): """Return all non _default_ only vhosts.""" From ce501f94a38a9ab23a666af34d1390cb4250cc28 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sat, 7 Nov 2015 08:31:46 +0200 Subject: [PATCH 17/63] Simplify the code --- .../letsencrypt_apache/configurator.py | 17 +++-------------- .../tests/configurator_test.py | 5 ----- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d3849b7fd..6b4caef50 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -328,26 +328,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # No winners here... is there only one reasonable vhost? if best_candidate is None: # reasonable == Not all _default_ addrs + vhosts = self._non_default_vhosts() # remove mod_macro hosts from reasonable vhosts - reasonable_vhosts = self._without_modmacro( - self._non_default_vhosts()) + reasonable_vhosts = [vh for vh + in vhosts if vh.modmacro is False] if len(reasonable_vhosts) == 1: best_candidate = reasonable_vhosts[0] return best_candidate - def _without_modmacro(self, vhosts): - """Return all non mod_macro vhosts - - :param vhosts: List of VirtualHosts - :type vhosts: :class:`list` of - :class:`~letsencrypt_apache.obj.VirtualHost` - - :returns: List of VirtualHosts without mod_macro - :rtype: :class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost` - """ - return [vh for vh in vhosts if vh.modmacro is False] - def _non_default_vhosts(self): """Return all non _default_ only vhosts.""" return [vh for vh in self.vhosts if not all( diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 963948d6e..61009b89f 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -178,11 +178,6 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual( self.config._find_best_vhost("example.demo"), self.vh_truth[2]) - def test_without_modmacro(self): - # pylint: disable=protected-access - self.assertEqual(len(self.vh_truth)-1, - len(self.config._without_modmacro(self.vh_truth))) - def test_is_mod_macro(self): # pylint: disable=protected-access self.assertEqual(self.config._is_mod_macro("$domain"), True) From a832070a12caf2fcb5a4d93d829b4d446c1401f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20S=C3=BAkup?= Date: Fri, 6 Nov 2015 12:46:15 +0100 Subject: [PATCH 18/63] Add bootstrap for openSUSE --- bootstrap/_suse_common.sh | 14 ++++++++++++++ bootstrap/install-deps.sh | 3 +++ bootstrap/suse.sh | 1 + letsencrypt-auto | 3 +++ 4 files changed, 21 insertions(+) create mode 100755 bootstrap/_suse_common.sh create mode 120000 bootstrap/suse.sh diff --git a/bootstrap/_suse_common.sh b/bootstrap/_suse_common.sh new file mode 100755 index 000000000..4b41bac36 --- /dev/null +++ b/bootstrap/_suse_common.sh @@ -0,0 +1,14 @@ +#!/bin/sh + +# SLE12 dont have python-virtualenv + +zypper -nq in -l git-core \ + python \ + python-devel \ + python-virtualenv \ + gcc \ + dialog \ + augeas-lenses \ + libopenssl-devel \ + libffi-devel \ + ca-certificates \ diff --git a/bootstrap/install-deps.sh b/bootstrap/install-deps.sh index 3cb0fc274..e907e7035 100755 --- a/bootstrap/install-deps.sh +++ b/bootstrap/install-deps.sh @@ -29,6 +29,9 @@ elif [ -f /etc/gentoo-release ] ; then elif uname | grep -iq FreeBSD ; then echo "Bootstrapping dependencies for FreeBSD..." $SUDO $BOOTSTRAP/freebsd.sh +elif `grep -qs openSUSE /etc/os-release` ; then + echo "Bootstrapping dependencies for openSUSE.." + $SUDO $BOOTSTRAP/suse.sh elif uname | grep -iq Darwin ; then echo "Bootstrapping dependencies for Mac OS X..." echo "WARNING: Mac support is very experimental at present..." diff --git a/bootstrap/suse.sh b/bootstrap/suse.sh new file mode 120000 index 000000000..fc4c1dee4 --- /dev/null +++ b/bootstrap/suse.sh @@ -0,0 +1 @@ +_suse_common.sh \ No newline at end of file diff --git a/letsencrypt-auto b/letsencrypt-auto index 2391a7c0b..5d5e23a04 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -82,6 +82,9 @@ then elif [ -f /etc/redhat-release ] ; then echo "Bootstrapping dependencies for RedHat-based OSes..." $SUDO $BOOTSTRAP/_rpm_common.sh + elif `grep -q openSUSE /etc/os-release` ; then + echo "Bootstrapping dependencies for openSUSE-based OSes..." + $SUDO $BOOTSTRAP/_suse_common.sh elif [ -f /etc/arch-release ] ; then echo "Bootstrapping dependencies for Archlinux..." $SUDO $BOOTSTRAP/archlinux.sh From d0a2b38457abc015c8204cf27002e910607e743c Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 7 Nov 2015 19:36:08 +0000 Subject: [PATCH 19/63] pep8 for docs/conf.py --- acme/docs/conf.py | 26 +++++++-------- docs/conf.py | 26 +++++++-------- letsencrypt-apache/docs/conf.py | 26 +++++++-------- letsencrypt-compatibility-test/docs/conf.py | 37 ++++++++++++--------- letsencrypt-nginx/docs/conf.py | 26 +++++++-------- letshelp-letsencrypt/docs/conf.py | 26 +++++++-------- 6 files changed, 86 insertions(+), 81 deletions(-) diff --git a/acme/docs/conf.py b/acme/docs/conf.py index 1448aaea3..55f5eee3f 100644 --- a/acme/docs/conf.py +++ b/acme/docs/conf.py @@ -227,25 +227,25 @@ htmlhelp_basename = 'acme-pythondoc' # -- Options for LaTeX output --------------------------------------------- latex_elements = { -# The paper size ('letterpaper' or 'a4paper'). -#'papersize': 'letterpaper', + # The paper size ('letterpaper' or 'a4paper'). + #'papersize': 'letterpaper', -# The font size ('10pt', '11pt' or '12pt'). -#'pointsize': '10pt', + # The font size ('10pt', '11pt' or '12pt'). + #'pointsize': '10pt', -# Additional stuff for the LaTeX preamble. -#'preamble': '', + # Additional stuff for the LaTeX preamble. + #'preamble': '', -# Latex figure (float) alignment -#'figure_align': 'htbp', + # Latex figure (float) alignment + #'figure_align': 'htbp', } # Grouping the document tree into LaTeX files. List of tuples # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). latex_documents = [ - (master_doc, 'acme-python.tex', u'acme-python Documentation', - u'Let\'s Encrypt Project', 'manual'), + (master_doc, 'acme-python.tex', u'acme-python Documentation', + u'Let\'s Encrypt Project', 'manual'), ] # The name of an image file (relative to this directory) to place at the top of @@ -289,9 +289,9 @@ man_pages = [ # (source start file, target name, title, author, # dir menu entry, description, category) texinfo_documents = [ - (master_doc, 'acme-python', u'acme-python Documentation', - author, 'acme-python', 'One line description of project.', - 'Miscellaneous'), + (master_doc, 'acme-python', u'acme-python Documentation', + author, 'acme-python', 'One line description of project.', + 'Miscellaneous'), ] # Documents to append as an appendix to all manuals. diff --git a/docs/conf.py b/docs/conf.py index 62a7cea07..21bcc6817 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -230,25 +230,25 @@ htmlhelp_basename = 'LetsEncryptdoc' # -- Options for LaTeX output --------------------------------------------- latex_elements = { -# The paper size ('letterpaper' or 'a4paper'). -#'papersize': 'letterpaper', + # The paper size ('letterpaper' or 'a4paper'). + #'papersize': 'letterpaper', -# The font size ('10pt', '11pt' or '12pt'). -#'pointsize': '10pt', + # The font size ('10pt', '11pt' or '12pt'). + #'pointsize': '10pt', -# Additional stuff for the LaTeX preamble. -#'preamble': '', + # Additional stuff for the LaTeX preamble. + #'preamble': '', -# Latex figure (float) alignment -#'figure_align': 'htbp', + # Latex figure (float) alignment + #'figure_align': 'htbp', } # Grouping the document tree into LaTeX files. List of tuples # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). latex_documents = [ - ('index', 'LetsEncrypt.tex', u'Let\'s Encrypt Documentation', - u'Let\'s Encrypt Project', 'manual'), + ('index', 'LetsEncrypt.tex', u'Let\'s Encrypt Documentation', + u'Let\'s Encrypt Project', 'manual'), ] # The name of an image file (relative to this directory) to place at the top of @@ -295,9 +295,9 @@ man_pages = [ # (source start file, target name, title, author, # dir menu entry, description, category) texinfo_documents = [ - ('index', 'LetsEncrypt', u'Let\'s Encrypt Documentation', - u'Let\'s Encrypt Project', 'LetsEncrypt', 'One line description of project.', - 'Miscellaneous'), + ('index', 'LetsEncrypt', u'Let\'s Encrypt Documentation', + u'Let\'s Encrypt Project', 'LetsEncrypt', 'One line description of project.', + 'Miscellaneous'), ] # Documents to append as an appendix to all manuals. diff --git a/letsencrypt-apache/docs/conf.py b/letsencrypt-apache/docs/conf.py index ddbf09262..aa58038cd 100644 --- a/letsencrypt-apache/docs/conf.py +++ b/letsencrypt-apache/docs/conf.py @@ -232,25 +232,25 @@ htmlhelp_basename = 'letsencrypt-apachedoc' # -- Options for LaTeX output --------------------------------------------- latex_elements = { -# The paper size ('letterpaper' or 'a4paper'). -#'papersize': 'letterpaper', + # The paper size ('letterpaper' or 'a4paper'). + #'papersize': 'letterpaper', -# The font size ('10pt', '11pt' or '12pt'). -#'pointsize': '10pt', + # The font size ('10pt', '11pt' or '12pt'). + #'pointsize': '10pt', -# Additional stuff for the LaTeX preamble. -#'preamble': '', + # Additional stuff for the LaTeX preamble. + #'preamble': '', -# Latex figure (float) alignment -#'figure_align': 'htbp', + # Latex figure (float) alignment + #'figure_align': 'htbp', } # Grouping the document tree into LaTeX files. List of tuples # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). latex_documents = [ - (master_doc, 'letsencrypt-apache.tex', u'letsencrypt-apache Documentation', - u'Let\'s Encrypt Project', 'manual'), + (master_doc, 'letsencrypt-apache.tex', u'letsencrypt-apache Documentation', + u'Let\'s Encrypt Project', 'manual'), ] # The name of an image file (relative to this directory) to place at the top of @@ -293,9 +293,9 @@ man_pages = [ # (source start file, target name, title, author, # dir menu entry, description, category) texinfo_documents = [ - (master_doc, 'letsencrypt-apache', u'letsencrypt-apache Documentation', - author, 'letsencrypt-apache', 'One line description of project.', - 'Miscellaneous'), + (master_doc, 'letsencrypt-apache', u'letsencrypt-apache Documentation', + author, 'letsencrypt-apache', 'One line description of project.', + 'Miscellaneous'), ] # Documents to append as an appendix to all manuals. diff --git a/letsencrypt-compatibility-test/docs/conf.py b/letsencrypt-compatibility-test/docs/conf.py index 7e9f0d5a4..3ee161efb 100644 --- a/letsencrypt-compatibility-test/docs/conf.py +++ b/letsencrypt-compatibility-test/docs/conf.py @@ -226,25 +226,26 @@ htmlhelp_basename = 'letsencrypt-compatibility-testdoc' # -- Options for LaTeX output --------------------------------------------- latex_elements = { -# The paper size ('letterpaper' or 'a4paper'). -#'papersize': 'letterpaper', + # The paper size ('letterpaper' or 'a4paper'). + #'papersize': 'letterpaper', -# The font size ('10pt', '11pt' or '12pt'). -#'pointsize': '10pt', + # The font size ('10pt', '11pt' or '12pt'). + #'pointsize': '10pt', -# Additional stuff for the LaTeX preamble. -#'preamble': '', + # Additional stuff for the LaTeX preamble. + #'preamble': '', -# Latex figure (float) alignment -#'figure_align': 'htbp', + # Latex figure (float) alignment + #'figure_align': 'htbp', } # Grouping the document tree into LaTeX files. List of tuples # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). latex_documents = [ - (master_doc, 'letsencrypt-compatibility-test.tex', u'letsencrypt-compatibility-test Documentation', - u'Let\'s Encrypt Project', 'manual'), + (master_doc, 'letsencrypt-compatibility-test.tex', + u'letsencrypt-compatibility-test Documentation', + u'Let\'s Encrypt Project', 'manual'), ] # The name of an image file (relative to this directory) to place at the top of @@ -273,7 +274,8 @@ latex_documents = [ # One entry per manual page. List of tuples # (source start file, name, description, authors, manual section). man_pages = [ - (master_doc, 'letsencrypt-compatibility-test', u'letsencrypt-compatibility-test Documentation', + (master_doc, 'letsencrypt-compatibility-test', + u'letsencrypt-compatibility-test Documentation', [author], 1) ] @@ -287,9 +289,10 @@ man_pages = [ # (source start file, target name, title, author, # dir menu entry, description, category) texinfo_documents = [ - (master_doc, 'letsencrypt-compatibility-test', u'letsencrypt-compatibility-test Documentation', - author, 'letsencrypt-compatibility-test', 'One line description of project.', - 'Miscellaneous'), + (master_doc, 'letsencrypt-compatibility-test', + u'letsencrypt-compatibility-test Documentation', + author, 'letsencrypt-compatibility-test', + 'One line description of project.', 'Miscellaneous'), ] # Documents to append as an appendix to all manuals. @@ -309,6 +312,8 @@ intersphinx_mapping = { 'python': ('https://docs.python.org/', None), 'acme': ('https://acme-python.readthedocs.org/en/latest/', None), 'letsencrypt': ('https://letsencrypt.readthedocs.org/en/latest/', None), - 'letsencrypt-apache': ('https://letsencrypt-apache.readthedocs.org/en/latest/', None), - 'letsencrypt-nginx': ('https://letsencrypt-nginx.readthedocs.org/en/latest/', None), + 'letsencrypt-apache': ( + 'https://letsencrypt-apache.readthedocs.org/en/latest/', None), + 'letsencrypt-nginx': ( + 'https://letsencrypt-nginx.readthedocs.org/en/latest/', None), } diff --git a/letsencrypt-nginx/docs/conf.py b/letsencrypt-nginx/docs/conf.py index cdb3490a0..14713a4b2 100644 --- a/letsencrypt-nginx/docs/conf.py +++ b/letsencrypt-nginx/docs/conf.py @@ -225,25 +225,25 @@ htmlhelp_basename = 'letsencrypt-nginxdoc' # -- Options for LaTeX output --------------------------------------------- latex_elements = { -# The paper size ('letterpaper' or 'a4paper'). -#'papersize': 'letterpaper', + # The paper size ('letterpaper' or 'a4paper'). + #'papersize': 'letterpaper', -# The font size ('10pt', '11pt' or '12pt'). -#'pointsize': '10pt', + # The font size ('10pt', '11pt' or '12pt'). + #'pointsize': '10pt', -# Additional stuff for the LaTeX preamble. -#'preamble': '', + # Additional stuff for the LaTeX preamble. + #'preamble': '', -# Latex figure (float) alignment -#'figure_align': 'htbp', + # Latex figure (float) alignment + #'figure_align': 'htbp', } # Grouping the document tree into LaTeX files. List of tuples # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). latex_documents = [ - (master_doc, 'letsencrypt-nginx.tex', u'letsencrypt-nginx Documentation', - u'Let\'s Encrypt Project', 'manual'), + (master_doc, 'letsencrypt-nginx.tex', u'letsencrypt-nginx Documentation', + u'Let\'s Encrypt Project', 'manual'), ] # The name of an image file (relative to this directory) to place at the top of @@ -286,9 +286,9 @@ man_pages = [ # (source start file, target name, title, author, # dir menu entry, description, category) texinfo_documents = [ - (master_doc, 'letsencrypt-nginx', u'letsencrypt-nginx Documentation', - author, 'letsencrypt-nginx', 'One line description of project.', - 'Miscellaneous'), + (master_doc, 'letsencrypt-nginx', u'letsencrypt-nginx Documentation', + author, 'letsencrypt-nginx', 'One line description of project.', + 'Miscellaneous'), ] # Documents to append as an appendix to all manuals. diff --git a/letshelp-letsencrypt/docs/conf.py b/letshelp-letsencrypt/docs/conf.py index 206b0b9e2..a84c4c982 100644 --- a/letshelp-letsencrypt/docs/conf.py +++ b/letshelp-letsencrypt/docs/conf.py @@ -225,25 +225,25 @@ htmlhelp_basename = 'letshelp-letsencryptdoc' # -- Options for LaTeX output --------------------------------------------- latex_elements = { -# The paper size ('letterpaper' or 'a4paper'). -#'papersize': 'letterpaper', + # The paper size ('letterpaper' or 'a4paper'). + #'papersize': 'letterpaper', -# The font size ('10pt', '11pt' or '12pt'). -#'pointsize': '10pt', + # The font size ('10pt', '11pt' or '12pt'). + #'pointsize': '10pt', -# Additional stuff for the LaTeX preamble. -#'preamble': '', + # Additional stuff for the LaTeX preamble. + #'preamble': '', -# Latex figure (float) alignment -#'figure_align': 'htbp', + # Latex figure (float) alignment + #'figure_align': 'htbp', } # Grouping the document tree into LaTeX files. List of tuples # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). latex_documents = [ - (master_doc, 'letshelp-letsencrypt.tex', u'letshelp-letsencrypt Documentation', - u'Let\'s Encrypt Project', 'manual'), + (master_doc, 'letshelp-letsencrypt.tex', u'letshelp-letsencrypt Documentation', + u'Let\'s Encrypt Project', 'manual'), ] # The name of an image file (relative to this directory) to place at the top of @@ -286,9 +286,9 @@ man_pages = [ # (source start file, target name, title, author, # dir menu entry, description, category) texinfo_documents = [ - (master_doc, 'letshelp-letsencrypt', u'letshelp-letsencrypt Documentation', - author, 'letshelp-letsencrypt', 'One line description of project.', - 'Miscellaneous'), + (master_doc, 'letshelp-letsencrypt', u'letshelp-letsencrypt Documentation', + author, 'letshelp-letsencrypt', 'One line description of project.', + 'Miscellaneous'), ] # Documents to append as an appendix to all manuals. From 5ee17f698ecf98c86d4789b8e811ea6f6554da56 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 7 Nov 2015 19:37:00 +0000 Subject: [PATCH 20/63] Fix more pep8 --- letsencrypt/cli.py | 1 - letsencrypt/plugins/disco_test.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5757783cd..3130471c6 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -879,7 +879,6 @@ def prepare_and_parse_args(plugins, args): # parser (--help should display plugin-specific options last) _plugins_parsing(helpful, plugins) - return helpful.parse_args() diff --git a/letsencrypt/plugins/disco_test.py b/letsencrypt/plugins/disco_test.py index 41d8cd5fe..0df4f88f1 100644 --- a/letsencrypt/plugins/disco_test.py +++ b/letsencrypt/plugins/disco_test.py @@ -51,8 +51,8 @@ class PluginEntryPointTest(unittest.TestCase): def test_description(self): self.assertEqual( - "Automatically use a temporary webserver", - self.plugin_ep.description) + "Automatically use a temporary webserver", + self.plugin_ep.description) def test_description_with_name(self): self.plugin_ep.plugin_cls = mock.MagicMock(description="Desc") From 9c12102d0bb4ca5d1e002e6e6e09813ff9bf4412 Mon Sep 17 00:00:00 2001 From: Dev & Sec Date: Sun, 8 Nov 2015 10:26:15 +0000 Subject: [PATCH 21/63] use `command -v` instead of `type`, and add comments for the `su_sudo` function --- letsencrypt-auto | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index 8626ab329..ce58488c4 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -14,11 +14,24 @@ VENV_PATH=${VENV_PATH:-"$XDG_DATA_HOME/$VENV_NAME"} VENV_BIN=${VENV_PATH}/bin if test "`id -u`" -ne "0" ; then - if type sudo 1>/dev/null 2>&1; then + if command -v sudo 1>/dev/null 2>&1; then SUDO=sudo else + # `sudo` command does not exist, use `su` instead. + # Because the parameters in `su -c` has to be a string, + # we need properly escape it su_sudo() { args="" + # This `while` loop iterates over all parameters given to this function. + # For each parameter, all `'` will be replace by `'"'"'`, and the escaped string + # will be wrap in a pair of `'`, then append to `$args` string + # For example, `echo "It's only 1\$\!"` will be escaped to: + # 'echo' 'It'"'"'s only 1$!' + # │ │└┼┘│ + # │ │ │ └── `'s only 1$!'` the literal string + # │ │ └── `\"'\"` is a single quote (as a string) + # │ └── `'It'`, to be concatenated with the strings followed it + # └── `echo` wrapped in a pair of `'`, it's totally fine for the shell command itself while [ $# -ne 0 ]; do args="$args'$(printf "%s" "$1" | sed -e "s/'/'\"'\"'/g")' " shift From 0c197c955ef969ffc461f6255f69daed05896561 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 9 Nov 2015 18:44:30 -0800 Subject: [PATCH 22/63] Revert all changes in cleanup, temporary or otherwise --- 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 eee7cdbc5..704ecf870 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1157,7 +1157,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # If all of the challenges have been finished, clean up everything if not self._chall_out: - self.revert_challenge_config() + self.recovery_routine() self.restart() self.parser.init_modules() From 0143d773628fb5d91228eca9f16f8543755b029a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 9 Nov 2015 18:47:38 -0800 Subject: [PATCH 23/63] Removed revert_challenge_config --- .../letsencrypt_apache/augeas_configurator.py | 12 ------------ .../tests/augeas_configurator_test.py | 14 -------------- 2 files changed, 26 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py index b0b15d649..9400336eb 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py @@ -162,18 +162,6 @@ class AugeasConfigurator(common.Plugin): # Need to reload configuration after these changes take effect self.aug.load() - def revert_challenge_config(self): - """Used to cleanup challenge configurations. - - :raises .errors.PluginError: If unable to revert the challenge config. - - """ - try: - self.reverter.revert_temporary_config() - except errors.ReverterError as err: - raise errors.PluginError(str(err)) - self.aug.load() - def rollback_checkpoints(self, rollback=1): """Rollback saved checkpoints. diff --git a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py index 815e6fc44..afa36ba77 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py @@ -76,20 +76,6 @@ class AugeasConfiguratorTest(util.ApacheTest): self.assertRaises( errors.PluginError, self.config.recovery_routine) - def test_revert_challenge_config(self): - mock_load = mock.Mock() - self.config.aug.load = mock_load - - self.config.revert_challenge_config() - self.assertEqual(mock_load.call_count, 1) - - def test_revert_challenge_config_error(self): - self.config.reverter.revert_temporary_config = mock.Mock( - side_effect=errors.ReverterError) - - self.assertRaises( - errors.PluginError, self.config.revert_challenge_config) - def test_rollback_checkpoints(self): mock_load = mock.Mock() self.config.aug.load = mock_load From 747b7ca507aa6c5a9076311635d9b0550d04c529 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 10 Nov 2015 05:34:15 +0200 Subject: [PATCH 24/63] More robust way of detecting the mod_macro vhosts --- .../letsencrypt_apache/configurator.py | 30 +++++-------------- .../tests/configurator_test.py | 5 ---- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 396501c90..32ecb1cdb 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -410,33 +410,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): for alias in serveralias_match: serveralias = self.parser.get_arg(alias) - if not self._is_mod_macro(serveralias): + if not host.modmacro: host.aliases.add(serveralias) - else: - host.modmacro = True if servername_match: # Get last ServerName as each overwrites the previous servername = self.parser.get_arg(servername_match[-1]) - if not self._is_mod_macro(servername): + if not host.modmacro: host.name = servername - else: - host.modmacro = True - - def _is_mod_macro(self, name): - """Helper function for _add_servernames(). - Checks if the ServerName / ServerAlias belongs to a macro - - :param str name: Name to check and filter out if it's a variable - - :returns: boolean - :rtype: boolean - - """ - - if "$" in name: - return True - return False def _create_vhost(self, path): """Used by get_virtual_hosts to create vhost objects @@ -459,7 +440,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): filename = get_file_path(path) is_enabled = self.is_site_enabled(filename) - vhost = obj.VirtualHost(filename, path, addrs, is_ssl, is_enabled) + macro = False + if "/Macro/" in path: + macro = True + + vhost = obj.VirtualHost(filename, path, addrs, is_ssl, + is_enabled, modmacro=macro) self._add_servernames(vhost) return vhost diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 8fc1240ec..0350a32ec 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -178,11 +178,6 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual( self.config._find_best_vhost("example.demo"), self.vh_truth[2]) - def test_is_mod_macro(self): - # pylint: disable=protected-access - self.assertEqual(self.config._is_mod_macro("$domain"), True) - self.assertEqual(self.config._is_mod_macro("www.example.com"), False) - def test_non_default_vhosts(self): # pylint: disable=protected-access self.assertEqual(len(self.config._non_default_vhosts()), 4) From 85675d709ccd10c7a57aa632e5a94e7495b8f1d4 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 10 Nov 2015 11:20:33 +0200 Subject: [PATCH 25/63] Case insensitive matching --- 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 32ecb1cdb..f10f0c241 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -441,7 +441,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): is_enabled = self.is_site_enabled(filename) macro = False - if "/Macro/" in path: + if "/macro/" in path.lower(): macro = True vhost = obj.VirtualHost(filename, path, addrs, is_ssl, From 096c689fba07d10dc592e3d74b4ed63ddd5a2b47 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 10 Nov 2015 12:03:15 +0200 Subject: [PATCH 26/63] Added help text for -d flag --- letsencrypt/cli.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 7227116e3..a41cd3e6d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -811,7 +811,9 @@ def prepare_and_parse_args(plugins, args): # --domains is useful, because it can be stored in config #for subparser in parser_run, parser_auth, parser_install: # subparser.add_argument("domains", nargs="*", metavar="domain") - helpful.add(None, "-d", "--domains", metavar="DOMAIN", action="append") + helpful.add(None, "-d", "--domains", metavar="DOMAIN", action="append", + help="Domain names to apply. Use multiple -d flags if you want " + "to specify multiple domains") helpful.add( None, "--duplicate", dest="duplicate", action="store_true", help="Allow getting a certificate that duplicates an existing one") From 3074ef996a465d3068cba19f59bc4b345ff4291b Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 10 Nov 2015 12:29:08 +0200 Subject: [PATCH 27/63] Refactored the argument and the code to use --domain instead of --domains, which was semantically incorrect --- letsencrypt-nginx/tests/boulder-integration.sh | 2 +- letsencrypt/cli.py | 13 +++++++------ letsencrypt/tests/cli_test.py | 2 +- tests/boulder-integration.sh | 4 ++-- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/letsencrypt-nginx/tests/boulder-integration.sh b/letsencrypt-nginx/tests/boulder-integration.sh index 3cbe9f6b9..0e3e7e77a 100755 --- a/letsencrypt-nginx/tests/boulder-integration.sh +++ b/letsencrypt-nginx/tests/boulder-integration.sh @@ -18,7 +18,7 @@ letsencrypt_test_nginx () { "$@" } -letsencrypt_test_nginx --domains nginx.wtf run +letsencrypt_test_nginx --domain nginx.wtf run echo | openssl s_client -connect localhost:5001 \ | openssl x509 -out $root/nginx.pem diff -q $root/nginx.pem $root/conf/live/nginx.wtf/cert.pem diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index a41cd3e6d..7ce89e184 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -106,7 +106,7 @@ def _find_domains(args, installer): domains = args.domains if not domains: - raise errors.Error("Please specify --domains, or --installer that " + raise errors.Error("Please specify --domain, or --installer that " "will help in domain names autodiscovery") return domains @@ -465,9 +465,9 @@ def obtaincert(args, config, plugins): """Authenticate & obtain cert, but do not install it.""" if args.domains is not None and args.csr is not None: - # TODO: --csr could have a priority, when --domains is + # TODO: --csr could have a priority, when --domain is # supplied, check if CSR matches given domains? - return "--domains and --csr are mutually exclusive" + return "--domain and --csr are mutually exclusive" try: # installers are used in auth mode to determine domain names @@ -807,11 +807,12 @@ def prepare_and_parse_args(plugins, args): None, "-t", "--text", dest="text_mode", action="store_true", help="Use the text output instead of the curses UI.") helpful.add(None, "-m", "--email", help=config_help("email")) - # positional arg shadows --domains, instead of appending, and - # --domains is useful, because it can be stored in config + # positional arg shadows --domain, instead of appending, and + # --domain is useful, because it can be stored in config #for subparser in parser_run, parser_auth, parser_install: # subparser.add_argument("domains", nargs="*", metavar="domain") - helpful.add(None, "-d", "--domains", metavar="DOMAIN", action="append", + helpful.add(None, "-d", "--domain", dest="domains", + metavar="DOMAIN", action="append", help="Domain names to apply. Use multiple -d flags if you want " "to specify multiple domains") helpful.add( diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index da5286a10..31f528cbf 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -170,7 +170,7 @@ class CLITest(unittest.TestCase): def test_certonly_bad_args(self): ret, _, _, _ = self._call(['-d', 'foo.bar', 'certonly', '--csr', CSR]) - self.assertEqual(ret, '--domains and --csr are mutually exclusive') + self.assertEqual(ret, '--domain and --csr are mutually exclusive') ret, _, _, _ = self._call(['-a', 'bad_auth', 'certonly']) self.assertEqual(ret, 'The requested bad_auth plugin does not appear to be installed') diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index d35ecbcff..97babb591 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -27,8 +27,8 @@ common() { "$@" } -common --domains le1.wtf --standalone-supported-challenges tls-sni-01 auth -common --domains le2.wtf --standalone-supported-challenges http-01 run +common --domain le1.wtf --standalone-supported-challenges tls-sni-01 auth +common --domain le2.wtf --standalone-supported-challenges http-01 run common -a manual -d le.wtf auth export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \ From 60147eb5299b5e84a9b6322e51b07850a9d4860b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 10 Nov 2015 14:52:18 -0800 Subject: [PATCH 28/63] Define state of checkpoints when save fails --- .../letsencrypt_apache/augeas_configurator.py | 16 +++++++++------- letsencrypt/interfaces.py | 3 ++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py index 9400336eb..c47981252 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py @@ -73,7 +73,8 @@ class AugeasConfigurator(common.Plugin): This function first checks for save errors, if none are found, all configuration changes made will be saved. According to the - function parameters. + function parameters. If an exception is raised, a new checkpoint + was not created. :param str title: The title of the save. If a title is given, the configuration will be saved as a new checkpoint and put in a @@ -82,8 +83,9 @@ class AugeasConfigurator(common.Plugin): :param bool temporary: Indicates whether the changes made will be quickly reversed in the future (ie. challenges) - :raises .errors.PluginError: If there was an error in Augeas, in an - attempt to save the configuration, or an error creating a checkpoint + :raises .errors.PluginError: If there was an error in Augeas, in + an attempt to save the configuration, or an error creating a + checkpoint """ save_state = self.aug.get("/augeas/save") @@ -122,16 +124,16 @@ class AugeasConfigurator(common.Plugin): except errors.ReverterError as err: raise errors.PluginError(str(err)) + self.aug.set("/augeas/save", save_state) + self.save_notes = "" + self.aug.save() + if title and not temporary: try: self.reverter.finalize_checkpoint(title) except errors.ReverterError as err: raise errors.PluginError(str(err)) - self.aug.set("/augeas/save", save_state) - self.save_notes = "" - self.aug.save() - def _log_save_errors(self, ex_errs): """Log errors due to bad Augeas save. diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index 987fdc25e..c8a725fde 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -298,7 +298,8 @@ class IInstaller(IPlugin): Both title and temporary are needed because a save may be intended to be permanent, but the save is not ready to be a full - checkpoint + checkpoint. If an exception is raised, it is assumed a new + checkpoint was not created. :param str title: The title of the save. If a title is given, the configuration will be saved as a new checkpoint and put in a From 414321fca6073cbd99cad32db1632f6fe178cea2 Mon Sep 17 00:00:00 2001 From: Chhatoi Pritam Baral Date: Wed, 11 Nov 2015 04:50:16 +0530 Subject: [PATCH 29/63] Fix #1281: Check if nginx binary exists --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 0123ac321..d97cf7397 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -107,6 +107,10 @@ class NginxConfigurator(common.Plugin): # This is called in determine_authenticator and determine_installer def prepare(self): """Prepare the authenticator/installer.""" + # Verify Nginx is installed + if not le_util.exe_exists(self.conf('ctl')): + raise errors.NoInstallationError + self.parser = parser.NginxParser( self.conf('server-root'), self.mod_ssl_conf) From 1bb063e87072e756db96c5641538ac6879ef9594 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 10 Nov 2015 16:03:18 -0800 Subject: [PATCH 30/63] Corrected crash recovery in client and added tests --- letsencrypt/client.py | 54 ++++++++++++++------- letsencrypt/tests/client_test.py | 80 ++++++++++++++++++++++++++++++-- 2 files changed, 114 insertions(+), 20 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 8a0ad6af4..452dcac16 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -1,10 +1,12 @@ """Let's Encrypt client API.""" +import functools import logging import os from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives.asymmetric import rsa import OpenSSL +import zope.component from acme import client as acme_client from acme import jose @@ -18,6 +20,7 @@ from letsencrypt import continuity_auth from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import error_handler +from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt import reverter from letsencrypt import storage @@ -331,25 +334,14 @@ class Client(object): self.installer.save("Deployed Let's Encrypt Certificate") # sites may have been enabled / final cleanup - with error_handler.ErrorHandler(self._rollback_and_restart): + restart_error_handler = error_handler.ErrorHandler(functools.partial( + self._rollback_and_restart, + "We were unable to install your certificate, however, we " + "successfully rolled back your server to its previous " + "configuration.")) + with restart_error_handler: self.installer.restart() - def _rollback_and_restart(self): - """Rollback the most recent checkpoint and restart the webserver""" - logger.critical("Rolling back to previous server configuration...") - try: - self.installer.rollback_checkpoints() - self.installer.restart() - except: - # TODO: suggest letshelp-letsencypt here - logger.critical("Failure to rollback config " - "changes and restart your server") - logger.critical("Please submit a bug report to " - "https://github.com/letsencrypt/letsencrypt") - raise - logger.critical("Rollback successful; your server has " - "been restarted with your old configuration") - def enhance_config(self, domains, redirect=None): """Enhance the configuration. @@ -395,8 +387,36 @@ class Client(object): raise self.installer.save("Add Redirects") + + restart_error_handler = error_handler.ErrorHandler(functools.partial( + self._rollback_and_restart, + "We were unable to setup a redirect for your server, however, we " + "successfully installed your certificate. If you'd like to revert " + "these changes as well, run 'letsencrypt rollback'.")) + with restart_error_handler: self.installer.restart() + def _rollback_and_restart(self, reporter_msg): + """Rollback the most recent checkpoint and restart the webserver + + :param str reporter_msg: message to show on successful rollback + + """ + logger.critical("Rolling back to previous server configuration...") + reporter = zope.component.getUtility(interfaces.IReporter) + try: + self.installer.rollback_checkpoints() + self.installer.restart() + except: + # TODO: suggest letshelp-letsencypt here + reporter.add_message( + "An error occured and we failed to rollback your config and " + "restart your server. Please submit a bug report to " + "https://github.com/letsencrypt/letsencrypt", + reporter.HIGH_PRIORITY) + raise + reporter.add_message(reporter_msg, reporter.HIGH_PRIORITY) + def validate_key_csr(privkey, csr=None): """Validate Key and CSR files. diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 4a61194f3..7c53677e4 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -148,7 +148,7 @@ class ClientTest(unittest.TestCase): shutil.rmtree(tmp_path) - def test_deploy_certificate(self): + def test_deploy_certificate_success(self): self.assertRaises(errors.Error, self.client.deploy_certificate, ["foo.bar"], "key", "cert", "chain", "fullchain") @@ -166,17 +166,38 @@ class ClientTest(unittest.TestCase): self.assertEqual(installer.save.call_count, 2) installer.restart.assert_called_once_with() - def test_deploy_certificate_restart_failure_with_recovery(self): + def test_deploy_certificate_failure(self): + installer = mock.MagicMock() + self.client.installer = installer + + installer.deploy_cert.side_effect = errors.PluginError + self.assertRaises(errors.PluginError, self.client.deploy_certificate, + ["foo.bar"], "key", "cert", "chain", "fullchain") + installer.recovery_routine.assert_called_once_with() + + def test_deploy_certificate_save_failure(self): + installer = mock.MagicMock() + self.client.installer = installer + + installer.save.side_effect = errors.PluginError + self.assertRaises(errors.PluginError, self.client.deploy_certificate, + ["foo.bar"], "key", "cert", "chain", "fullchain") + installer.recovery_routine.assert_called_once_with() + + @mock.patch("letsencrypt.client.zope.component.getUtility") + def test_deploy_certificate_restart_failure(self, mock_get_utility): installer = mock.MagicMock() installer.restart.side_effect = [errors.PluginError, None] self.client.installer = installer self.assertRaises(errors.PluginError, self.client.deploy_certificate, ["foo.bar"], "key", "cert", "chain", "fullchain") + self.assertEqual(mock_get_utility().add_message.call_count, 1) installer.rollback_checkpoints.assert_called_once_with() self.assertEqual(installer.restart.call_count, 2) - def test_deploy_certificate_restart_failure_without_recovery(self): + @mock.patch("letsencrypt.client.zope.component.getUtility") + def test_deploy_certificate_restart_failure2(self, mock_get_utility): installer = mock.MagicMock() installer.restart.side_effect = errors.PluginError installer.rollback_checkpoints.side_effect = errors.ReverterError @@ -184,6 +205,7 @@ class ClientTest(unittest.TestCase): self.assertRaises(errors.PluginError, self.client.deploy_certificate, ["foo.bar"], "key", "cert", "chain", "fullchain") + self.assertEqual(mock_get_utility().add_message.call_count, 1) installer.rollback_checkpoints.assert_called_once_with() self.assertEqual(installer.restart.call_count, 1) @@ -201,11 +223,63 @@ class ClientTest(unittest.TestCase): self.assertEqual(installer.save.call_count, 1) installer.restart.assert_called_once_with() + def test_enhance_config_no_installer(self): + self.assertRaises(errors.Error, + self.client.enhance_config, ["foo.bar"]) + + @mock.patch("letsencrypt.client.enhancements") + def test_enhance_config_enhance_failure(self, mock_enhancements): + mock_enhancements.ask.return_value = True + installer = mock.MagicMock() + self.client.installer = installer installer.enhance.side_effect = errors.PluginError + self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], True) installer.recovery_routine.assert_called_once_with() + @mock.patch("letsencrypt.client.enhancements") + def test_enhance_config_save_failure(self, mock_enhancements): + mock_enhancements.ask.return_value = True + installer = mock.MagicMock() + self.client.installer = installer + installer.save.side_effect = errors.PluginError + + self.assertRaises(errors.PluginError, + self.client.enhance_config, ["foo.bar"], True) + installer.recovery_routine.assert_called_once_with() + + @mock.patch("letsencrypt.client.zope.component.getUtility") + @mock.patch("letsencrypt.client.enhancements") + def test_enhance_config_restart_failure(self, mock_enhancements, + mock_get_utility): + mock_enhancements.ask.return_value = True + installer = mock.MagicMock() + self.client.installer = installer + installer.restart.side_effect = [errors.PluginError, None] + + self.assertRaises(errors.PluginError, + self.client.enhance_config, ["foo.bar"], True) + self.assertEqual(mock_get_utility().add_message.call_count, 1) + installer.rollback_checkpoints.assert_called_once_with() + self.assertEqual(installer.restart.call_count, 2) + + @mock.patch("letsencrypt.client.zope.component.getUtility") + @mock.patch("letsencrypt.client.enhancements") + def test_enhance_config_restart_failure2(self, mock_enhancements, + mock_get_utility): + mock_enhancements.ask.return_value = True + installer = mock.MagicMock() + self.client.installer = installer + installer.restart.side_effect = errors.PluginError + installer.rollback_checkpoints.side_effect = errors.ReverterError + + self.assertRaises(errors.PluginError, + self.client.enhance_config, ["foo.bar"], True) + self.assertEqual(mock_get_utility().add_message.call_count, 1) + installer.rollback_checkpoints.assert_called_once_with() + self.assertEqual(installer.restart.call_count, 1) + class RollbackTest(unittest.TestCase): """Tests for letsencrypt.client.rollback.""" From 0bbe69b36a8447dbfddde0b0d6065fd5e47cab94 Mon Sep 17 00:00:00 2001 From: Chhatoi Pritam Baral Date: Wed, 11 Nov 2015 05:49:34 +0530 Subject: [PATCH 31/63] Mock existence of nginx binary --- .../letsencrypt_nginx/tests/util.py | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index cb4e08ddf..e60feb3d3 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -49,21 +49,25 @@ def get_nginx_configurator( backups = os.path.join(work_dir, "backups") - config = configurator.NginxConfigurator( - config=mock.MagicMock( - nginx_server_root=config_path, - le_vhost_ext="-le-ssl.conf", - config_dir=config_dir, - work_dir=work_dir, - backup_dir=backups, - temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), - in_progress_dir=os.path.join(backups, "IN_PROGRESS"), - server="https://acme-server.org:443/new", - tls_sni_01_port=5001, - ), - name="nginx", - version=version) - config.prepare() + with mock.patch("letsencrypt_nginx.configurator.le_util." + "exe_exists") as mock_exe_exists: + mock_exe_exists.return_value = True + + config = configurator.NginxConfigurator( + config=mock.MagicMock( + nginx_server_root=config_path, + le_vhost_ext="-le-ssl.conf", + config_dir=config_dir, + work_dir=work_dir, + backup_dir=backups, + temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), + in_progress_dir=os.path.join(backups, "IN_PROGRESS"), + server="https://acme-server.org:443/new", + tls_sni_01_port=5001, + ), + name="nginx", + version=version) + config.prepare() # Provide general config utility. nsconfig = configuration.NamespaceConfig(config.config) From 3c00afd55c1c5f06ee2400f3b8980ebcbe553dea Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 10 Nov 2015 16:31:52 -0800 Subject: [PATCH 32/63] Revert "Removed revert_challenge_config" This reverts commit 0143d773628fb5d91228eca9f16f8543755b029a. --- .../letsencrypt_apache/augeas_configurator.py | 12 ++++++++++++ .../tests/augeas_configurator_test.py | 14 ++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py index c47981252..9e0948f12 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py @@ -164,6 +164,18 @@ class AugeasConfigurator(common.Plugin): # Need to reload configuration after these changes take effect self.aug.load() + def revert_challenge_config(self): + """Used to cleanup challenge configurations. + + :raises .errors.PluginError: If unable to revert the challenge config. + + """ + try: + self.reverter.revert_temporary_config() + except errors.ReverterError as err: + raise errors.PluginError(str(err)) + self.aug.load() + def rollback_checkpoints(self, rollback=1): """Rollback saved checkpoints. diff --git a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py index afa36ba77..815e6fc44 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py @@ -76,6 +76,20 @@ class AugeasConfiguratorTest(util.ApacheTest): self.assertRaises( errors.PluginError, self.config.recovery_routine) + def test_revert_challenge_config(self): + mock_load = mock.Mock() + self.config.aug.load = mock_load + + self.config.revert_challenge_config() + self.assertEqual(mock_load.call_count, 1) + + def test_revert_challenge_config_error(self): + self.config.reverter.revert_temporary_config = mock.Mock( + side_effect=errors.ReverterError) + + self.assertRaises( + errors.PluginError, self.config.revert_challenge_config) + def test_rollback_checkpoints(self): mock_load = mock.Mock() self.config.aug.load = mock_load From 553592b2c81bd4667ba39c15f180f8a8aabea3a3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 10 Nov 2015 16:32:05 -0800 Subject: [PATCH 33/63] Revert "Revert all changes in cleanup, temporary or otherwise" This reverts commit 0c197c955ef969ffc461f6255f69daed05896561. --- 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 704ecf870..eee7cdbc5 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1157,7 +1157,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # If all of the challenges have been finished, clean up everything if not self._chall_out: - self.recovery_routine() + self.revert_challenge_config() self.restart() self.parser.init_modules() From 7d4beacce8f9bd234a9eabc2e3f062c9f1874942 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 10 Nov 2015 16:59:22 -0800 Subject: [PATCH 34/63] Added better error messages on redirect failure --- letsencrypt/client.py | 35 ++++++++++++++++++++++++-------- letsencrypt/tests/client_test.py | 10 +++++++-- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 452dcac16..9123eaf95 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -120,6 +120,12 @@ class Client(object): """ + # Message to show if enabling a redirect fails, but recovery is successful + _SUCCESSFUL_REDIRECT_RECOVERY_MSG = ( + "We were unable to setup a redirect for your server, however, we " + "successfully installed your certificate. If you'd like to revert " + "these changes as well, run 'letsencrypt rollback'.") + def __init__(self, config, account_, dv_auth, installer, acme=None): """Initialize a client.""" self.config = config @@ -378,7 +384,10 @@ class Client(object): :type vhost: :class:`letsencrypt.interfaces.IInstaller` """ - with error_handler.ErrorHandler(self.installer.recovery_routine): + enhance_error_handler = error_handler.ErrorHandler( + functools.partial(self._recovery_routine_with_msg, + self._SUCCESSFUL_REDIRECT_RECOVERY_MSG)) + with enhance_error_handler: for dom in domains: try: self.installer.enhance(dom, "redirect") @@ -388,18 +397,26 @@ class Client(object): self.installer.save("Add Redirects") - restart_error_handler = error_handler.ErrorHandler(functools.partial( - self._rollback_and_restart, - "We were unable to setup a redirect for your server, however, we " - "successfully installed your certificate. If you'd like to revert " - "these changes as well, run 'letsencrypt rollback'.")) + restart_error_handler = error_handler.ErrorHandler( + functools.partial(self._rollback_and_restart, + self._SUCCESSFUL_REDIRECT_RECOVERY_MSG)) with restart_error_handler: self.installer.restart() - def _rollback_and_restart(self, reporter_msg): + def _recovery_routine_with_msg(self, success_msg): + """Calls the installer's recovery routine and prints success_msg + + :param str success_msg: message to show on successful recovery + + """ + self.installer.recovery_routine() + reporter = zope.component.getUtility(interfaces.IReporter) + reporter.add_message(success_msg, reporter.HIGH_PRIORITY) + + def _rollback_and_restart(self, success_msg): """Rollback the most recent checkpoint and restart the webserver - :param str reporter_msg: message to show on successful rollback + :param str success_msg: message to show on successful rollback """ logger.critical("Rolling back to previous server configuration...") @@ -415,7 +432,7 @@ class Client(object): "https://github.com/letsencrypt/letsencrypt", reporter.HIGH_PRIORITY) raise - reporter.add_message(reporter_msg, reporter.HIGH_PRIORITY) + reporter.add_message(success_msg, reporter.HIGH_PRIORITY) def validate_key_csr(privkey, csr=None): diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 7c53677e4..f8da90f36 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -227,8 +227,10 @@ class ClientTest(unittest.TestCase): self.assertRaises(errors.Error, self.client.enhance_config, ["foo.bar"]) + @mock.patch("letsencrypt.client.zope.component.getUtility") @mock.patch("letsencrypt.client.enhancements") - def test_enhance_config_enhance_failure(self, mock_enhancements): + def test_enhance_config_enhance_failure(self, mock_enhancements, + mock_get_utility): mock_enhancements.ask.return_value = True installer = mock.MagicMock() self.client.installer = installer @@ -237,9 +239,12 @@ class ClientTest(unittest.TestCase): self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], True) installer.recovery_routine.assert_called_once_with() + self.assertEqual(mock_get_utility().add_message.call_count, 1) + @mock.patch("letsencrypt.client.zope.component.getUtility") @mock.patch("letsencrypt.client.enhancements") - def test_enhance_config_save_failure(self, mock_enhancements): + def test_enhance_config_save_failure(self, mock_enhancements, + mock_get_utility): mock_enhancements.ask.return_value = True installer = mock.MagicMock() self.client.installer = installer @@ -248,6 +253,7 @@ class ClientTest(unittest.TestCase): self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], True) installer.recovery_routine.assert_called_once_with() + self.assertEqual(mock_get_utility().add_message.call_count, 1) @mock.patch("letsencrypt.client.zope.component.getUtility") @mock.patch("letsencrypt.client.enhancements") From 9d30a85b298b61f33437dc19144c5f5fd5a74132 Mon Sep 17 00:00:00 2001 From: Chhatoi Pritam Baral Date: Wed, 11 Nov 2015 05:55:41 +0530 Subject: [PATCH 35/63] Add test for nginx not being installed --- .../letsencrypt_nginx/tests/configurator_test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 7000f85dc..913c5de27 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -1,3 +1,4 @@ +# pylint: disable=too-many-public-methods """Test for letsencrypt_nginx.configurator.""" import os import shutil @@ -29,6 +30,12 @@ class NginxConfiguratorTest(util.NginxTest): shutil.rmtree(self.config_dir) shutil.rmtree(self.work_dir) + @mock.patch("letsencrypt_nginx.configurator.le_util.exe_exists") + def test_prepare_no_install(self, mock_exe_exists): + mock_exe_exists.return_value = False + self.assertRaises( + errors.NoInstallationError, self.config.prepare) + def test_prepare(self): self.assertEquals((1, 6, 2), self.config.version) self.assertEquals(5, len(self.config.parser.parsed)) From e64149cae8343efd377ffa256369c16b9152ac66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20L=C3=A9one?= Date: Wed, 11 Nov 2015 13:27:09 +0100 Subject: [PATCH 36/63] Redeclared names without usage --- acme/acme/jose/jwa.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acme/acme/jose/jwa.py b/acme/acme/jose/jwa.py index 4ce5ca3f5..1853e0107 100644 --- a/acme/acme/jose/jwa.py +++ b/acme/acme/jose/jwa.py @@ -176,5 +176,5 @@ PS384 = JWASignature.register(_JWAPS('PS384', hashes.SHA384)) PS512 = JWASignature.register(_JWAPS('PS512', hashes.SHA512)) ES256 = JWASignature.register(_JWAES('ES256')) -ES256 = JWASignature.register(_JWAES('ES384')) -ES256 = JWASignature.register(_JWAES('ES512')) +ES384 = JWASignature.register(_JWAES('ES384')) +ES512 = JWASignature.register(_JWAES('ES512')) From f02dcbbc4cab8c925542b391dd2ce259dfa2f499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20L=C3=A9one?= Date: Wed, 11 Nov 2015 13:29:15 +0100 Subject: [PATCH 37/63] Variable key already existing --- letsencrypt-nginx/letsencrypt_nginx/dvsni.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py b/letsencrypt-nginx/letsencrypt_nginx/dvsni.py index b388c0267..8fd705f08 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py +++ b/letsencrypt-nginx/letsencrypt_nginx/dvsni.py @@ -99,8 +99,8 @@ class NginxDvsni(common.TLSSNI01): for key, body in main: if key == ['http']: found_bucket = False - for key, _ in body: - if key == bucket_directive[0]: + for k, _ in body: + if k == bucket_directive[0]: found_bucket = True if not found_bucket: body.insert(0, bucket_directive) From 5ce92402008f36ab0f5d54e1456a878a6486ee3f Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 11 Nov 2015 12:42:07 -0800 Subject: [PATCH 38/63] Improve comments for letsencrypt-auto --- letsencrypt-auto | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index b9f95ac14..667dcecfd 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -8,11 +8,25 @@ # without requiring specific versions of its dependencies from the operating # system. +# Note: you can set XDG_DATA_HOME or VENV_PATH before running this script, +# if you want to change where the virtual environment will be installed XDG_DATA_HOME=${XDG_DATA_HOME:-~/.local/share} VENV_NAME="letsencrypt" VENV_PATH=${VENV_PATH:-"$XDG_DATA_HOME/$VENV_NAME"} VENV_BIN=${VENV_PATH}/bin +# This script takes the same arguments as the main letsencrypt program, but it +# additionally responds to --verbose (more output) and --debug (allow support +# for experimental platforms) +for arg in "$@" ; do + # This first clause is redundant with the third, but hedging on portability + if [ "$arg" = "-v" ] || [ "$arg" = "--verbose" ] || echo "$arg" | grep -E -- "-v+$" ; then + VERBOSE=1 + elif [ "$arg" = "--debug" ] ; then + DEBUG=1 + fi +done + if test "`id -u`" -ne "0" ; then if command -v sudo 1>/dev/null 2>&1; then SUDO=sudo @@ -44,15 +58,6 @@ else SUDO= fi -for arg in "$@" ; do - # This first clause is redundant with the third, but hedging on portability - if [ "$arg" = "-v" ] || [ "$arg" = "--verbose" ] || echo "$arg" | grep -E -- "-v+$" ; then - VERBOSE=1 - elif [ "$arg" = "--debug" ] ; then - DEBUG=1 - fi -done - ExperimentalBootstrap() { # Arguments: Platform name, boostrap script name, SUDO command (iff needed) if [ "$DEBUG" = 1 ] ; then From b26a87a33c4487c98f7cb8e6a5279feb4c79eb16 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 11 Nov 2015 12:57:32 -0800 Subject: [PATCH 39/63] Comments on SUDO --- letsencrypt-auto | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index 667dcecfd..160de036a 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -27,11 +27,16 @@ for arg in "$@" ; do fi done +# letsencrypt-auto needs root access to bootstrap OS dependencies, and +# letsencrypt itself needs root access for almost all modes of operation +# The "normal" case is that sudo is used for the steps that need root, but +# this script *can* be run as root (not recommended), or fall back to using +# `su` if test "`id -u`" -ne "0" ; then if command -v sudo 1>/dev/null 2>&1; then SUDO=sudo else - # `sudo` command does not exist, use `su` instead. + echo \"sudo\" is not available, will use \"su\" for installation steps... # Because the parameters in `su -c` has to be a string, # we need properly escape it su_sudo() { From 79646dc42d5891ceec138e0819ac59c098a5c18f Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 11 Nov 2015 12:59:36 -0800 Subject: [PATCH 40/63] Fix misplaced verbosity from pip --- letsencrypt-auto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-auto b/letsencrypt-auto index b9f95ac14..6a6254125 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -158,7 +158,7 @@ else $VENV_BIN/pip install -U pip > /dev/null printf . # nginx is buggy / disabled for now... - $VENV_BIN/pip install -r py26reqs.txt + $VENV_BIN/pip install -r py26reqs.txt > /dev/null printf . $VENV_BIN/pip install -U letsencrypt > /dev/null printf . From d2dacef313f98285b6673090bcb883800caed590 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Wed, 11 Nov 2015 17:46:39 -0600 Subject: [PATCH 41/63] Try reenabling container-based infrastructure Try explicitly pulling from backports Try travis's whitelisted Augeas PPA (not ours or backports) --- .travis.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 86a0d3e7d..96e28b1b0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,11 +2,12 @@ language: python services: - rabbitmq + - mariadb # http://docs.travis-ci.com/user/ci-environment/#CI-environment-OS # gimme has to be kept in sync with Boulder's Go version setting in .travis.yml before_install: - - sudo apt-get install -y mariadb-server mariadb-server-10.0 + - 'dpkg -s libaugeas0' - '[ "xxx$BOULDER_INTEGRATION" = "xxx" ] || eval "$(gimme 1.5.1)"' # using separate envs with different TOXENVs creates 4x1 Travis build @@ -31,9 +32,8 @@ branches: - master - /^test-.*$/ -# enable Trusty beta on travis -sudo: required -dist: trusty +# container-based infrastructure +sudo: false addons: # make sure simplehttp simple verification works (custom /etc/hosts) @@ -41,6 +41,8 @@ addons: - le.wtf mariadb: "10.0" apt: + sources: + - augeas packages: # keep in sync with bootstrap/ubuntu.sh and Boulder - python - python-dev From a0129f7818a0c6922b3db9fb29d19b8ee2097b92 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 15:02:30 +0200 Subject: [PATCH 42/63] Fix test broken by #1454 if nginx installation not present in system --- letsencrypt/tests/cli_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 31f528cbf..baa2a6e78 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -121,7 +121,9 @@ class CLITest(unittest.TestCase): '--key-path', 'key', '--chain-path', 'chain']) self.assertEqual(mock_display_ops.pick_installer.call_count, 1) - def test_configurator_selection(self): + @mock.patch('letsencrypt.le_util.exe_exists') + def test_configurator_selection(self, mock_exe_exists): + mock_exe_exists.return_value = True real_plugins = disco.PluginsRegistry.find_all() args = ['--agree-dev-preview', '--apache', '--authenticator', 'standalone'] From 14e03f9af0e8b4cc3c22d7b80c79e2fb04fe7b81 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 16:34:15 +0200 Subject: [PATCH 43/63] Parse possible multiple domain definitions to a list --- letsencrypt/cli.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 999555741..141309174 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -668,8 +668,32 @@ class HelpfulArgumentParser(object): parsed_args = self.parser.parse_args(self.args) parsed_args.func = self.VERBS[self.verb] + parsed_args.domains = self._parse_domains(parsed_args.domains) return parsed_args + def _parse_domains(self, domains): + """Helper function for parse_args() that parses domains from a + (possibly) comma separated list and returns list of unique domains. + + :param domains: List of domain flags + :type domains: `list` of `string` + + :returns: List of unique domains + :rtype: `list` of `string` + + """ + + uniqd = None + + if domains: + dlist = [] + for domain in domains: + dlist.extend([d.strip() for d in domain.split(",")]) + # Make sure we don't have duplicates + uniqd = [d for i,d in enumerate(dlist) if d not in dlist[:i]] + + return uniqd + def determine_verb(self): """Determines the verb/subcommand provided by the user. From 3b8d6ec58b0b355a9c83439626cba5a60d416a19 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 16:42:58 +0200 Subject: [PATCH 44/63] Modified -d help text to reflect the change --- letsencrypt/cli.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 141309174..bd51e4a0f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -837,8 +837,9 @@ def prepare_and_parse_args(plugins, args): # subparser.add_argument("domains", nargs="*", metavar="domain") helpful.add(None, "-d", "--domain", dest="domains", metavar="DOMAIN", action="append", - help="Domain names to apply. Use multiple -d flags if you want " - "to specify multiple domains") + 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, "--duplicate", dest="duplicate", action="store_true", help="Allow getting a certificate that duplicates an existing one") From 56f21e1d35f82750b1cc8115a28182020c673acc Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 16:53:40 +0200 Subject: [PATCH 45/63] Refactor --domain flag back to -- domains --- letsencrypt-nginx/tests/boulder-integration.sh | 2 +- letsencrypt/cli.py | 12 ++++++------ letsencrypt/tests/cli_test.py | 4 ++-- tests/boulder-integration.sh | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/letsencrypt-nginx/tests/boulder-integration.sh b/letsencrypt-nginx/tests/boulder-integration.sh index 0e3e7e77a..3cbe9f6b9 100755 --- a/letsencrypt-nginx/tests/boulder-integration.sh +++ b/letsencrypt-nginx/tests/boulder-integration.sh @@ -18,7 +18,7 @@ letsencrypt_test_nginx () { "$@" } -letsencrypt_test_nginx --domain nginx.wtf run +letsencrypt_test_nginx --domains nginx.wtf run echo | openssl s_client -connect localhost:5001 \ | openssl x509 -out $root/nginx.pem diff -q $root/nginx.pem $root/conf/live/nginx.wtf/cert.pem diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index bd51e4a0f..f01bf0b96 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -106,7 +106,7 @@ def _find_domains(args, installer): domains = args.domains if not domains: - raise errors.Error("Please specify --domain, or --installer that " + raise errors.Error("Please specify --domains, or --installer that " "will help in domain names autodiscovery") return domains @@ -465,9 +465,9 @@ def obtaincert(args, config, plugins): """Authenticate & obtain cert, but do not install it.""" if args.domains is not None and args.csr is not None: - # TODO: --csr could have a priority, when --domain is + # TODO: --csr could have a priority, when --domains is # supplied, check if CSR matches given domains? - return "--domain and --csr are mutually exclusive" + return "--domains and --csr are mutually exclusive" try: # installers are used in auth mode to determine domain names @@ -831,11 +831,11 @@ def prepare_and_parse_args(plugins, args): None, "-t", "--text", dest="text_mode", action="store_true", help="Use the text output instead of the curses UI.") helpful.add(None, "-m", "--email", help=config_help("email")) - # positional arg shadows --domain, instead of appending, and - # --domain is useful, because it can be stored in config + # positional arg shadows --domains, instead of appending, and + # --domains is useful, because it can be stored in config #for subparser in parser_run, parser_auth, parser_install: # subparser.add_argument("domains", nargs="*", metavar="domain") - helpful.add(None, "-d", "--domain", dest="domains", + helpful.add(None, "-d", "--domains", dest="domains", metavar="DOMAIN", action="append", help="Domain names to apply. For multiple domains you can use " "multiple -d flags or enter a comma separated list of domains" diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 31f528cbf..83f78e9ab 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -117,7 +117,7 @@ class CLITest(unittest.TestCase): @mock.patch('letsencrypt.cli.display_ops') def test_installer_selection(self, mock_display_ops): - self._call(['install', '--domain', 'foo.bar', '--cert-path', 'cert', + self._call(['install', '--domains', 'foo.bar', '--cert-path', 'cert', '--key-path', 'key', '--chain-path', 'chain']) self.assertEqual(mock_display_ops.pick_installer.call_count, 1) @@ -170,7 +170,7 @@ class CLITest(unittest.TestCase): def test_certonly_bad_args(self): ret, _, _, _ = self._call(['-d', 'foo.bar', 'certonly', '--csr', CSR]) - self.assertEqual(ret, '--domain and --csr are mutually exclusive') + self.assertEqual(ret, '--domains and --csr are mutually exclusive') ret, _, _, _ = self._call(['-a', 'bad_auth', 'certonly']) self.assertEqual(ret, 'The requested bad_auth plugin does not appear to be installed') diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 97babb591..53996cd20 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -27,8 +27,8 @@ common() { "$@" } -common --domain le1.wtf --standalone-supported-challenges tls-sni-01 auth -common --domain le2.wtf --standalone-supported-challenges http-01 run +common --domains le1.wtf --standalone-supported-challenges tls-sni-01 auth +common --domains le2.wtf --standalone-supported-challenges http-01 run common -a manual -d le.wtf auth export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \ @@ -40,7 +40,7 @@ common auth --csr "$CSR_PATH" \ openssl x509 -in "${root}/csr/0000_cert.pem" -text openssl x509 -in "${root}/csr/0000_chain.pem" -text -common --domain le3.wtf install \ +common --domains le3.wtf install \ --cert-path "${root}/csr/cert.pem" \ --key-path "${root}/csr/key.pem" From 37e94e631d428131131243460bce417531e2a62c Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 17:25:38 +0200 Subject: [PATCH 46/63] Added tests --- letsencrypt/tests/cli_test.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 83f78e9ab..57807555d 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -193,6 +193,27 @@ class CLITest(unittest.TestCase): self._call, ['-d', '*.wildcard.tld']) + def test_parse_domains(self): + from letsencrypt import cli + plugins = disco.PluginsRegistry.find_all() + + short_args = ['-d', 'example.com'] + namespace = cli.prepare_and_parse_args(plugins, short_args) + self.assertEqual(namespace.domains, ['example.com']) + + short_args = ['-d', 'example.com,another.net,third.org,example.com'] + namespace = cli.prepare_and_parse_args(plugins, short_args) + self.assertEqual(namespace.domains, ['example.com', 'another.net', + 'third.org']) + + long_args = ['--domains', 'example.com'] + namespace = cli.prepare_and_parse_args(plugins, long_args) + self.assertEqual(namespace.domains, ['example.com']) + + long_args = ['--domains', 'example.com,another.net,example.com'] + namespace = cli.prepare_and_parse_args(plugins, long_args) + self.assertEqual(namespace.domains, ['example.com', 'another.net']) + @mock.patch('letsencrypt.crypto_util.notAfter') @mock.patch('letsencrypt.cli.zope.component.getUtility') def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter): From 0ccbbdb120de6ce414da6a4951b009620208dbb0 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 12 Nov 2015 17:46:44 +0200 Subject: [PATCH 47/63] Style fix --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f01bf0b96..7848c4afc 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -690,7 +690,7 @@ class HelpfulArgumentParser(object): for domain in domains: dlist.extend([d.strip() for d in domain.split(",")]) # Make sure we don't have duplicates - uniqd = [d for i,d in enumerate(dlist) if d not in dlist[:i]] + uniqd = [d for i, d in enumerate(dlist) if d not in dlist[:i]] return uniqd From 4590adf545b00daaf78df060f924f7059038d771 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 13:23:01 -0800 Subject: [PATCH 48/63] Add *args and **kwargs to ErrorHandler --- letsencrypt/error_handler.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/letsencrypt/error_handler.py b/letsencrypt/error_handler.py index 8b0eb7c8b..431e677a1 100644 --- a/letsencrypt/error_handler.py +++ b/letsencrypt/error_handler.py @@ -1,4 +1,5 @@ """Registers functions to be called if an exception or signal occurs.""" +import functools import logging import os import signal @@ -40,11 +41,11 @@ class ErrorHandler(object): to be called again by the next signal handler. """ - def __init__(self, func=None): + def __init__(self, func=None, *args, **kwargs): self.funcs = [] self.prev_handlers = {} if func is not None: - self.register(func) + self.register(func, *args, **kwargs) def __enter__(self): self.set_signal_handlers() @@ -57,9 +58,13 @@ class ErrorHandler(object): self.call_registered() self.reset_signal_handlers() - def register(self, func): - """Registers func to be called if an error occurs.""" - self.funcs.append(func) + def register(self, func, *args, **kwargs): + """Sets func to be called with *args and **kwargs during cleanup + + :param function func: function to be called in case of an error + + """ + self.funcs.append(functools.partial(func, *args, **kwargs)) def call_registered(self): """Calls all registered functions""" From 2d827464f47a35e25b307588608e2b5db7d94210 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 13:35:30 -0800 Subject: [PATCH 49/63] Updated error_handler tests --- letsencrypt/tests/error_handler_test.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/letsencrypt/tests/error_handler_test.py b/letsencrypt/tests/error_handler_test.py index c92f12435..bf1ffbdad 100644 --- a/letsencrypt/tests/error_handler_test.py +++ b/letsencrypt/tests/error_handler_test.py @@ -13,7 +13,11 @@ class ErrorHandlerTest(unittest.TestCase): from letsencrypt import error_handler self.init_func = mock.MagicMock() - self.handler = error_handler.ErrorHandler(self.init_func) + self.init_args = {42} + self.init_kwargs = {'foo': 'bar'} + self.handler = error_handler.ErrorHandler(self.init_func, + *self.init_args, + **self.init_kwargs) # pylint: disable=protected-access self.signals = error_handler._SIGNALS @@ -23,7 +27,8 @@ class ErrorHandlerTest(unittest.TestCase): raise ValueError except ValueError: pass - self.init_func.assert_called_once_with() + self.init_func.assert_called_once_with(*self.init_args, + **self.init_kwargs) @mock.patch('letsencrypt.error_handler.os') @mock.patch('letsencrypt.error_handler.signal') @@ -37,7 +42,8 @@ class ErrorHandlerTest(unittest.TestCase): signum = self.signals[0] signal_handler(signum, None) - self.init_func.assert_called_once_with() + self.init_func.assert_called_once_with(*self.init_args, + **self.init_kwargs) mock_os.kill.assert_called_once_with(mock_os.getpid(), signum) self.handler.reset_signal_handlers() @@ -48,7 +54,8 @@ class ErrorHandlerTest(unittest.TestCase): bad_func = mock.MagicMock(side_effect=[ValueError]) self.handler.register(bad_func) self.handler.call_registered() - self.init_func.assert_called_once_with() + self.init_func.assert_called_once_with(*self.init_args, + **self.init_kwargs) bad_func.assert_called_once_with() def test_sysexit_ignored(self): From 13a4987f694740884aa6dc9bbbbf5d128c6dc146 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 13:58:55 -0800 Subject: [PATCH 50/63] Incorporated pde's and joohi's feedback --- letsencrypt/client.py | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 9123eaf95..0f0ecdc6d 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -1,5 +1,4 @@ """Let's Encrypt client API.""" -import functools import logging import os @@ -120,12 +119,6 @@ class Client(object): """ - # Message to show if enabling a redirect fails, but recovery is successful - _SUCCESSFUL_REDIRECT_RECOVERY_MSG = ( - "We were unable to setup a redirect for your server, however, we " - "successfully installed your certificate. If you'd like to revert " - "these changes as well, run 'letsencrypt rollback'.") - def __init__(self, config, account_, dv_auth, installer, acme=None): """Initialize a client.""" self.config = config @@ -339,13 +332,11 @@ class Client(object): self.installer.save("Deployed Let's Encrypt Certificate") - # sites may have been enabled / final cleanup - restart_error_handler = error_handler.ErrorHandler(functools.partial( - self._rollback_and_restart, - "We were unable to install your certificate, however, we " - "successfully rolled back your server to its previous " - "configuration.")) - with restart_error_handler: + msg = ("We were unable to install your certificate, " + "however, we successfully restored your " + "server to its prior configuration.") + with error_handler.ErrorHandler(self._rollback_and_restart, msg): + # sites may have been enabled / final cleanup self.installer.restart() def enhance_config(self, domains, redirect=None): @@ -384,10 +375,9 @@ class Client(object): :type vhost: :class:`letsencrypt.interfaces.IInstaller` """ - enhance_error_handler = error_handler.ErrorHandler( - functools.partial(self._recovery_routine_with_msg, - self._SUCCESSFUL_REDIRECT_RECOVERY_MSG)) - with enhance_error_handler: + msg = ("We were unable to set up a redirect for your server, " + "however, we successfully installed your certificate.") + with error_handler.ErrorHandler(self._recovery_routine_with_msg, msg): for dom in domains: try: self.installer.enhance(dom, "redirect") @@ -397,10 +387,7 @@ class Client(object): self.installer.save("Add Redirects") - restart_error_handler = error_handler.ErrorHandler( - functools.partial(self._rollback_and_restart, - self._SUCCESSFUL_REDIRECT_RECOVERY_MSG)) - with restart_error_handler: + with error_handler.ErrorHandler(self._rollback_and_restart, msg): self.installer.restart() def _recovery_routine_with_msg(self, success_msg): @@ -427,7 +414,7 @@ class Client(object): except: # TODO: suggest letshelp-letsencypt here reporter.add_message( - "An error occured and we failed to rollback your config and " + "An error occured and we failed to restore your config and " "restart your server. Please submit a bug report to " "https://github.com/letsencrypt/letsencrypt", reporter.HIGH_PRIORITY) From 4158e3b559d34e0471a5b9e691bcb00b738307d6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 14:22:36 -0800 Subject: [PATCH 51/63] Revert "Removing stray debug statements, fixes #1308" This reverts commit b11e556ae7ba931a9593b0a82623c662eae8c737. --- letsencrypt/cli.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 999555741..7210709c9 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -549,6 +549,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) return filtered.init(config) @@ -556,11 +557,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) return verified.prepare() available = verified.available() logger.debug("Prepared plugins: %s", available) + print str(available) def read_file(filename, mode="rb"): @@ -936,6 +939,7 @@ def _paths_parser(helpful): section = "paths" if verb in ("install", "revoke"): section = verb + print helpful.help_arg, helpful.help_arg == "install" # revoke --key-path reads a file, install --key-path takes a string add(section, "--key-path", type=((verb == "revoke" and read_file) or str), required=(verb == "install"), From 3ef0ed11916dac59398c1223b6eeec80bf34c184 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 14:24:31 -0800 Subject: [PATCH 52/63] Removed debugging statement --- letsencrypt/cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 7210709c9..4c798ee5c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -939,7 +939,6 @@ def _paths_parser(helpful): section = "paths" if verb in ("install", "revoke"): section = verb - print helpful.help_arg, helpful.help_arg == "install" # revoke --key-path reads a file, install --key-path takes a string add(section, "--key-path", type=((verb == "revoke" and read_file) or str), required=(verb == "install"), From a31be6d5679a5387b1a0ebf2c49904ab8e2e9252 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 15:04:41 -0800 Subject: [PATCH 53/63] Added plugin_cmd tests --- letsencrypt/tests/cli_test.py | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 31f528cbf..509b7eb34 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -168,6 +168,48 @@ class CLITest(unittest.TestCase): for r in xrange(len(flags)))): self._call(['plugins'] + list(args)) + @mock.patch('letsencrypt.cli.plugins_disco') + def test_plugins_no_args(self, mock_disco): + ifaces = [] + plugins = mock_disco.PluginsRegistry.find_all() + + _, stdout, _, _ = self._call(['plugins']) + plugins.visible.assert_called_once_with() + plugins.visible().ifaces.assert_called_once_with(ifaces) + filtered = plugins.visible().ifaces() + stdout.write.called_once_with(str(filtered)) + + @mock.patch('letsencrypt.cli.plugins_disco') + def test_plugins_init(self, mock_disco): + ifaces = [] + plugins = mock_disco.PluginsRegistry.find_all() + + _, stdout, _, _ = self._call(['plugins', '--init']) + plugins.visible.assert_called_once_with() + plugins.visible().ifaces.assert_called_once_with(ifaces) + filtered = plugins.visible().ifaces() + self.assertEqual(filtered.init.call_count, 1) + filtered.verify.assert_called_once_with(ifaces) + verified = filtered.verify() + stdout.write.called_once_with(str(verified)) + + @mock.patch('letsencrypt.cli.plugins_disco') + def test_plugins_prepare(self, mock_disco): + ifaces = [] + plugins = mock_disco.PluginsRegistry.find_all() + + _, stdout, _, _ = self._call(['plugins', '--init', '--prepare']) + plugins.visible.assert_called_once_with() + plugins.visible().ifaces.assert_called_once_with(ifaces) + filtered = plugins.visible().ifaces() + self.assertEqual(filtered.init.call_count, 1) + filtered.verify.assert_called_once_with(ifaces) + verified = filtered.verify() + verified.prepare.assert_called_once_with() + verified.available.assert_called_once_with() + available = verified.available() + stdout.write.called_once_with(str(available)) + def test_certonly_bad_args(self): ret, _, _, _ = self._call(['-d', 'foo.bar', 'certonly', '--csr', CSR]) self.assertEqual(ret, '--domain and --csr are mutually exclusive') From 08d7c06a5484e8d23391e6c8c4a9cc9cf8857d39 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 15:19:50 -0800 Subject: [PATCH 54/63] Python2.6: It's called a set --- letsencrypt/tests/error_handler_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/error_handler_test.py b/letsencrypt/tests/error_handler_test.py index bf1ffbdad..7fbdcffd8 100644 --- a/letsencrypt/tests/error_handler_test.py +++ b/letsencrypt/tests/error_handler_test.py @@ -13,7 +13,7 @@ class ErrorHandlerTest(unittest.TestCase): from letsencrypt import error_handler self.init_func = mock.MagicMock() - self.init_args = {42} + self.init_args = set((42,)) self.init_kwargs = {'foo': 'bar'} self.handler = error_handler.ErrorHandler(self.init_func, *self.init_args, From 211ca2420f454cea580f11d90c58c51818c5d274 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 17:19:26 -0800 Subject: [PATCH 55/63] Make read_file use abspath --- letsencrypt/cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4c798ee5c..7411ada63 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -569,7 +569,7 @@ def plugins_cmd(args, config, plugins): # TODO: Use IDisplay rather than print def read_file(filename, mode="rb"): """Returns the given file's contents. - :param str filename: Filename + :param str filename: filename as an absolute path :param str mode: open mode (see `open`) :returns: A tuple of filename and its contents @@ -579,6 +579,7 @@ def read_file(filename, mode="rb"): """ try: + filename = os.path.abspath(filename) return filename, open(filename, mode).read() except IOError as exc: raise argparse.ArgumentTypeError(exc.strerror) From 707df3d2c61179fe89b99d03deb142e316befa92 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 17:27:56 -0800 Subject: [PATCH 56/63] Add read_file tests --- letsencrypt/tests/cli_test.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 509b7eb34..008d239d3 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -1,4 +1,5 @@ """Tests for letsencrypt.cli.""" +import argparse import itertools import os import shutil @@ -356,6 +357,20 @@ class CLITest(unittest.TestCase): mock_sys.exit.assert_called_with(''.join( traceback.format_exception_only(KeyboardInterrupt, interrupt))) + def test_read_file(self): + from letsencrypt import cli + rel_test_path = os.path.relpath(os.path.join(self.tmp_dir, 'foo')) + self.assertRaises( + argparse.ArgumentTypeError, cli.read_file, rel_test_path) + + test_contents = 'bar\n' + with open(rel_test_path, 'w') as f: + f.write(test_contents) + + path, contents = cli.read_file(rel_test_path) + self.assertEqual(path, os.path.abspath(path)) + self.assertEqual(contents, test_contents) + class DetermineAccountTest(unittest.TestCase): """Tests for letsencrypt.cli._determine_account.""" From dc78f811ce18ca1414e202c2e2a24b2a769e45ed Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 17:33:40 -0800 Subject: [PATCH 57/63] Make cert_path,key_path,chain_path,fullchain_path absolute --- letsencrypt/cli.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 7411ada63..b6181bb64 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -931,26 +931,28 @@ def _paths_parser(helpful): if verb in ("install", "revoke", "certonly"): section = verb if verb == "certonly": - add(section, "--cert-path", default=flag_default("auth_cert_path"), help=cph) + add(section, "--cert-path", type=os.path.abspath, + default=flag_default("auth_cert_path"), help=cph) elif verb == "revoke": add(section, "--cert-path", type=read_file, required=True, help=cph) else: - add(section, "--cert-path", help=cph, required=(verb == "install")) + add(section, "--cert-path", type=os.path.abspath, + help=cph, required=(verb == "install")) section = "paths" if verb in ("install", "revoke"): section = verb # revoke --key-path reads a file, install --key-path takes a string - add(section, "--key-path", type=((verb == "revoke" and read_file) or str), - required=(verb == "install"), + add(section, "--key-path", required=(verb == "install"), + type=((verb == "revoke" and read_file) or os.path.abspath), help="Path to private key for cert creation or revocation (if account key is missing)") default_cp = None if verb == "certonly": default_cp = flag_default("auth_chain_path") - add("paths", "--fullchain-path", default=default_cp, + add("paths", "--fullchain-path", default=default_cp, type=os.path.abspath, help="Accompanying path to a full certificate chain (cert plus chain).") - add("paths", "--chain-path", default=default_cp, + add("paths", "--chain-path", default=default_cp, type=os.path.abspath, help="Accompanying path to a certificate chain.") add("paths", "--config-dir", default=flag_default("config_dir"), help=config_help("config_dir")) From 1dd1523680cadc4d8b8fd59cbf31fdc9c37fcfd0 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 17:47:36 -0800 Subject: [PATCH 58/63] Added cert, key, chain, and fullchain abspath tests --- letsencrypt/tests/cli_test.py | 36 ++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 008d239d3..2f729f71d 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -23,7 +23,7 @@ from letsencrypt.tests import test_util CSR = test_util.vector_path('csr.der') -class CLITest(unittest.TestCase): +class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods """Tests for different commands.""" def setUp(self): @@ -116,6 +116,23 @@ class CLITest(unittest.TestCase): from letsencrypt import cli self.assertTrue(cli.usage_strings(plugins)[0] in out) + def test_install_abspath(self): + cert = 'cert' + key = 'key' + chain = 'chain' + fullchain = 'fullchain' + + with MockedVerb('install') as mock_install: + self._call(['install', '--cert-path', cert, '--key-path', 'key', + '--chain-path', 'chain', + '--fullchain-path', 'fullchain']) + + args = mock_install.call_args[0][0] + self.assertEqual(args.cert_path, os.path.abspath(cert)) + self.assertEqual(args.key_path, os.path.abspath(key)) + self.assertEqual(args.chain_path, os.path.abspath(chain)) + self.assertEqual(args.fullchain_path, os.path.abspath(fullchain)) + @mock.patch('letsencrypt.cli.display_ops') def test_installer_selection(self, mock_display_ops): self._call(['install', '--domain', 'foo.bar', '--cert-path', 'cert', @@ -211,6 +228,23 @@ class CLITest(unittest.TestCase): available = verified.available() stdout.write.called_once_with(str(available)) + def test_certonly_abspath(self): + cert = 'cert' + key = 'key' + chain = 'chain' + fullchain = 'fullchain' + + with MockedVerb('certonly') as mock_obtaincert: + self._call(['certonly', '--cert-path', cert, '--key-path', 'key', + '--chain-path', 'chain', + '--fullchain-path', 'fullchain']) + + args = mock_obtaincert.call_args[0][0] + self.assertEqual(args.cert_path, os.path.abspath(cert)) + self.assertEqual(args.key_path, os.path.abspath(key)) + self.assertEqual(args.chain_path, os.path.abspath(chain)) + self.assertEqual(args.fullchain_path, os.path.abspath(fullchain)) + def test_certonly_bad_args(self): ret, _, _, _ = self._call(['-d', 'foo.bar', 'certonly', '--csr', CSR]) self.assertEqual(ret, '--domain and --csr are mutually exclusive') From 778b8797bb3c215a3f0645d7cd11b375ad974054 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 18:17:10 -0800 Subject: [PATCH 59/63] Ensure config_dir, work_dir, and logs_dir have absolute paths --- letsencrypt/configuration.py | 5 +++++ letsencrypt/tests/renewer_test.py | 3 +++ 2 files changed, 8 insertions(+) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index e70171675..4955655f3 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -37,6 +37,11 @@ class NamespaceConfig(object): def __init__(self, namespace): self.namespace = namespace + + self.namespace.config_dir = os.path.abspath(self.namespace.config_dir) + self.namespace.work_dir = os.path.abspath(self.namespace.work_dir) + self.namespace.logs_dir = os.path.abspath(self.namespace.logs_dir) + # Check command line parameters sanity, and error out in case of problem. check_config_sanity(self) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index e76b6eb88..daec9678f 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -692,6 +692,9 @@ class RenewableCertTests(BaseRenewableCertTest): self.test_rc.configfile["renewalparams"]["http01_port"] = "1234" self.test_rc.configfile["renewalparams"]["account"] = "abcde" self.test_rc.configfile["renewalparams"]["domains"] = ["example.com"] + self.test_rc.configfile["renewalparams"]["config_dir"] = "config" + self.test_rc.configfile["renewalparams"]["work_dir"] = "work" + self.test_rc.configfile["renewalparams"]["logs_dir"] = "logs" mock_auth = mock.MagicMock() mock_pd.PluginsRegistry.find_all.return_value = {"apache": mock_auth} # Fails because "fake" != "apache" From b408ec765dd6a0c217783c5f22d41f46efcf730c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 18:20:00 -0800 Subject: [PATCH 60/63] Test absolute paths are used --- letsencrypt/tests/configuration_test.py | 30 +++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/letsencrypt/tests/configuration_test.py b/letsencrypt/tests/configuration_test.py index 3a8bf40cf..be48a7c9c 100644 --- a/letsencrypt/tests/configuration_test.py +++ b/letsencrypt/tests/configuration_test.py @@ -59,6 +59,36 @@ class NamespaceConfigTest(unittest.TestCase): self.namespace.http01_port = None self.assertEqual(80, self.config.http01_port) + def test_absolute_paths(self): + from letsencrypt.configuration import NamespaceConfig + + config_base = "foo" + work_base = "bar" + logs_base = "baz" + + config = NamespaceConfig(mock.MagicMock( + config_dir=config_base, work_dir=work_base, logs_dir=logs_base)) + + self.assertTrue(os.path.isabs(config.config_dir)) + self.assertEqual(config.config_dir, + os.path.join(os.getcwd(), config_base)) + self.assertTrue(os.path.isabs(config.work_dir)) + self.assertEqual(config.work_dir, + os.path.join(os.getcwd(), work_base)) + self.assertTrue(os.path.isabs(config.logs_dir)) + self.assertEqual(config.logs_dir, + os.path.join(os.getcwd(), logs_base)) + self.assertTrue(os.path.isabs(config.cert_path)) + self.assertTrue(os.path.isabs(config.key_path)) + self.assertTrue(os.path.isabs(config.chain_path)) + self.assertTrue(os.path.isabs(config.fullchain_path)) + self.assertTrue(os.path.isabs(config.accounts_dir)) + self.assertTrue(os.path.isabs(config.backup_dir)) + self.assertTrue(os.path.isabs(config.csr_dir)) + self.assertTrue(os.path.isabs(config.in_progress_dir)) + self.assertTrue(os.path.isabs(config.key_dir)) + self.assertTrue(os.path.isabs(config.temp_checkpoint_dir)) + class RenewerConfigurationTest(unittest.TestCase): """Test for letsencrypt.configuration.RenewerConfiguration.""" From 82614df6f0bdd7bf20ea3554780e880f84e80db3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 18:30:37 -0800 Subject: [PATCH 61/63] What do YOU think os.path.isabs(mock.MagicMock()) returns? --- letsencrypt/tests/configuration_test.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/letsencrypt/tests/configuration_test.py b/letsencrypt/tests/configuration_test.py index be48a7c9c..16a4da6e1 100644 --- a/letsencrypt/tests/configuration_test.py +++ b/letsencrypt/tests/configuration_test.py @@ -66,8 +66,14 @@ class NamespaceConfigTest(unittest.TestCase): work_base = "bar" logs_base = "baz" - config = NamespaceConfig(mock.MagicMock( - config_dir=config_base, work_dir=work_base, logs_dir=logs_base)) + mock_namespace = mock.MagicMock(spec=['config_dir', 'work_dir', + 'logs_dir', 'http01_port', + 'tls_sni_01_port', + 'domains', 'server']) + mock_namespace.config_dir = config_base + mock_namespace.work_dir = work_base + mock_namespace.logs_dir = logs_base + config = NamespaceConfig(mock_namespace) self.assertTrue(os.path.isabs(config.config_dir)) self.assertEqual(config.config_dir, @@ -78,10 +84,6 @@ class NamespaceConfigTest(unittest.TestCase): self.assertTrue(os.path.isabs(config.logs_dir)) self.assertEqual(config.logs_dir, os.path.join(os.getcwd(), logs_base)) - self.assertTrue(os.path.isabs(config.cert_path)) - self.assertTrue(os.path.isabs(config.key_path)) - self.assertTrue(os.path.isabs(config.chain_path)) - self.assertTrue(os.path.isabs(config.fullchain_path)) self.assertTrue(os.path.isabs(config.accounts_dir)) self.assertTrue(os.path.isabs(config.backup_dir)) self.assertTrue(os.path.isabs(config.csr_dir)) From 31b984adc520eab2066b41360a9b6da58efe0c25 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Nov 2015 18:34:46 -0800 Subject: [PATCH 62/63] Test RenwerConfiguration --- letsencrypt/tests/configuration_test.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/letsencrypt/tests/configuration_test.py b/letsencrypt/tests/configuration_test.py index 16a4da6e1..c42b99081 100644 --- a/letsencrypt/tests/configuration_test.py +++ b/letsencrypt/tests/configuration_test.py @@ -113,6 +113,28 @@ class RenewerConfigurationTest(unittest.TestCase): self.config.renewal_configs_dir, '/tmp/config/renewal_configs') self.assertEqual(self.config.renewer_config_file, '/tmp/config/r.conf') + def test_absolute_paths(self): + from letsencrypt.configuration import NamespaceConfig + from letsencrypt.configuration import RenewerConfiguration + + config_base = "foo" + work_base = "bar" + logs_base = "baz" + + mock_namespace = mock.MagicMock(spec=['config_dir', 'work_dir', + 'logs_dir', 'http01_port', + 'tls_sni_01_port', + 'domains', 'server']) + mock_namespace.config_dir = config_base + mock_namespace.work_dir = work_base + mock_namespace.logs_dir = logs_base + config = RenewerConfiguration(NamespaceConfig(mock_namespace)) + + self.assertTrue(os.path.isabs(config.archive_dir)) + self.assertTrue(os.path.isabs(config.live_dir)) + self.assertTrue(os.path.isabs(config.renewal_configs_dir)) + self.assertTrue(os.path.isabs(config.renewer_config_file)) + if __name__ == '__main__': unittest.main() # pragma: no cover From 0db3814c130396cedcc6c1f013773e909694e51a Mon Sep 17 00:00:00 2001 From: Lee Watson Date: Fri, 13 Nov 2015 08:15:09 +0000 Subject: [PATCH 63/63] Update readme to reflect current commands auth -> certonly --- README.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index d3e89c939..ce0d1b686 100644 --- a/README.rst +++ b/README.rst @@ -35,11 +35,11 @@ It's all automated: All you need to do to sign a single domain is:: - user@www:~$ sudo letsencrypt -d www.example.org auth + user@www:~$ sudo letsencrypt -d www.example.org certonly For multiple domains (SAN) use:: - user@www:~$ sudo letsencrypt -d www.example.org -d example.org auth + user@www:~$ sudo letsencrypt -d www.example.org -d example.org certonly and if you have a compatible web server (Apache or Nginx), Let's Encrypt can not only get a new certificate, but also deploy it and configure your