From 5b4bf427b81af5094abdb8dd6123df750425274e Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 3 Dec 2015 12:22:28 +0200 Subject: [PATCH 01/90] Added apache mod_ssl check & installation to bootstrap --- bootstrap/_rpm_common.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bootstrap/_rpm_common.sh b/bootstrap/_rpm_common.sh index b975da444..b80a9555b 100755 --- a/bootstrap/_rpm_common.sh +++ b/bootstrap/_rpm_common.sh @@ -47,3 +47,11 @@ then echo "Could not install additional dependencies. Aborting bootstrap!" exit 1 fi + + +if yum list installed "httpd" >/dev/null 2>&1; then + if ! $tool install -y mod_ssl + then + echo "Apache found, but mod_ssl could not be installed." + fi +fi From 7d307d1cf4c4ce5fbb5f41e408893e87e051d84c Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 3 Dec 2015 14:14:02 +0200 Subject: [PATCH 02/90] Per-OS constants --- .../letsencrypt_apache/configurator.py | 12 +++++----- .../letsencrypt_apache/constants.py | 22 ++++++++++++++++++- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 98b0b8820..ba3bdcd7d 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -86,17 +86,17 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): @classmethod def add_parser_arguments(cls, add): - add("ctl", default=constants.CLI_DEFAULTS["ctl"], + add("ctl", default=constants.os_constant("ctl"), help="Path to the 'apache2ctl' binary, used for 'configtest', " "retrieving the Apache2 version number, and initialization " "parameters.") - add("enmod", default=constants.CLI_DEFAULTS["enmod"], + add("enmod", default=constants.os_constant("enmod"), help="Path to the Apache 'a2enmod' binary.") - add("dismod", default=constants.CLI_DEFAULTS["dismod"], + add("dismod", default=constants.os_constant("dismod"), help="Path to the Apache 'a2enmod' binary.") - add("le-vhost-ext", default=constants.CLI_DEFAULTS["le_vhost_ext"], + add("le-vhost-ext", default=constants.os_constant("le_vhost_ext"), help="SSL vhost configuration extension.") - add("server-root", default=constants.CLI_DEFAULTS["server_root"], + add("server-root", default=constants.os_constant("server_root"), help="Apache server root directory.") le_util.add_deprecated_argument(add, "init-script", 1) @@ -583,7 +583,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): Duplicates vhost and adds default ssl options New vhost will reside as (nonssl_vhost.path) + - ``letsencrypt_apache.constants.CLI_DEFAULTS["le_vhost_ext"]`` + ``letsencrypt_apache.constants.os_constant("le_vhost_ext")`` .. note:: This function saves the configuration diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 202fc3e21..e302a29fe 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -1,14 +1,27 @@ """Apache plugin constants.""" import pkg_resources +from letsencrypt import le_util -CLI_DEFAULTS = dict( +CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", ctl="apache2ctl", enmod="a2enmod", dismod="a2dismod", le_vhost_ext="-le-ssl.conf", ) +CLI_DEFAULTS_CENTOS = dict( + server_root="/etc/httpd", + ctl="apachectl", + enmod=None, + dismod=None, + le_vhost_ext="-le-ssl.conf", +) +CLI_DEFAULTS = { + "debian": CLI_DEFAULTS_DEBIAN, + "ubuntu": CLI_DEFAULTS_DEBIAN, + "centos": CLI_DEFAULTS_CENTOS +} """CLI defaults.""" MOD_SSL_CONF_DEST = "options-ssl-apache.conf" @@ -38,3 +51,10 @@ UIR_ARGS = ["always", "set", "Content-Security-Policy", HEADER_ARGS = {"Strict-Transport-Security": HSTS_ARGS, "Upgrade-Insecure-Requests": UIR_ARGS} +def os_constant(key): + os_info = le_util.get_os_info() + try: + constants = CLI_DEFAULTS[os_info[0].lower()] + except KeyError: + constants = CLI_DEFAULTS["debian"] + return constants[key] From 6c905056d23be6c75598c59511bdcf32cdef05db Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sat, 5 Dec 2015 18:41:57 +0200 Subject: [PATCH 03/90] Added option to disable apache module handling per OS basis --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 +++-- letsencrypt-apache/letsencrypt_apache/constants.py | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ba3bdcd7d..1ca7e13dc 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -540,8 +540,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :param str port: Port to listen on """ - if "ssl_module" not in self.parser.modules: - self.enable_mod("ssl", temp=temp) + if constants.os_constant("handle_mods"): + if "ssl_module" not in self.parser.modules: + self.enable_mod("ssl", temp=temp) # Check for Listen # Note: This could be made to also look for ip:443 combo diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index e302a29fe..ee45cac70 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -9,6 +9,7 @@ CLI_DEFAULTS_DEBIAN = dict( enmod="a2enmod", dismod="a2dismod", le_vhost_ext="-le-ssl.conf", + handle_mods=True ) CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", @@ -16,6 +17,7 @@ CLI_DEFAULTS_CENTOS = dict( enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", + handle_mods=False ) CLI_DEFAULTS = { "debian": CLI_DEFAULTS_DEBIAN, From da5b980674c898457e2ff722738eaf67e5134961 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sat, 5 Dec 2015 18:53:13 +0200 Subject: [PATCH 04/90] Handle exe checks for distros missing some of the helper scripts --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 1ca7e13dc..34512be72 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -138,8 +138,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # Verify Apache is installed for exe in (self.conf("ctl"), self.conf("enmod"), self.conf("dismod")): - if not le_util.exe_exists(exe): - raise errors.NoInstallationError + if exe != None: + if not le_util.exe_exists(exe): + raise errors.NoInstallationError # Make sure configuration is valid self.config_test() From b856fc58af56672bfc7fea0b45b6d2f791cf1d45 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sat, 5 Dec 2015 19:10:40 +0200 Subject: [PATCH 05/90] Added correct os_info string for CentOS --- letsencrypt-apache/letsencrypt_apache/constants.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index ee45cac70..658fcc70f 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -22,7 +22,8 @@ CLI_DEFAULTS_CENTOS = dict( CLI_DEFAULTS = { "debian": CLI_DEFAULTS_DEBIAN, "ubuntu": CLI_DEFAULTS_DEBIAN, - "centos": CLI_DEFAULTS_CENTOS + "centos": CLI_DEFAULTS_CENTOS, + "centos linux": CLI_DEFAULTS_CENTOS } """CLI defaults.""" From b723de431d2f419b83271f3c5c736ccf5e482716 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sun, 6 Dec 2015 11:33:57 +0200 Subject: [PATCH 06/90] Config parameter for configuration location that != server root --- letsencrypt-apache/letsencrypt_apache/configurator.py | 4 +++- letsencrypt-apache/letsencrypt_apache/constants.py | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 34512be72..73af842d9 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -98,6 +98,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): help="SSL vhost configuration extension.") add("server-root", default=constants.os_constant("server_root"), help="Apache server root directory.") + add("config-root", default=constants.os_constant("config_root"), + help="Apache server configuration root") le_util.add_deprecated_argument(add, "init-script", 1) def __init__(self, *args, **kwargs): @@ -146,7 +148,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.config_test() self.parser = parser.ApacheParser( - self.aug, self.conf("server-root"), self.conf("ctl")) + self.aug, self.conf("config-root"), self.conf("ctl")) # Check for errors in parsing files with Augeas self.check_parsing_errors("httpd.aug") diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 658fcc70f..09fb8ba59 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -5,6 +5,7 @@ from letsencrypt import le_util CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", + config_root="/etc/apache2", ctl="apache2ctl", enmod="a2enmod", dismod="a2dismod", @@ -13,6 +14,7 @@ CLI_DEFAULTS_DEBIAN = dict( ) CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", + config_root="/etc/httpd/conf.d", ctl="apachectl", enmod=None, dismod=None, From e9c389f125e4a7e51a80133403cad4a981156162 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sun, 6 Dec 2015 22:40:51 +0200 Subject: [PATCH 07/90] New constant for VirtualHost - configuration directory and changing the configurator to use it --- .../letsencrypt_apache/configurator.py | 15 +++++++-------- .../letsencrypt_apache/constants.py | 4 ++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 73af842d9..353e6c68a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -98,8 +98,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): help="SSL vhost configuration extension.") add("server-root", default=constants.os_constant("server_root"), help="Apache server root directory.") - add("config-root", default=constants.os_constant("config_root"), - help="Apache server configuration root") + add("vhost-root", default=constants.os_constant("vhost_root"), + help="Apache server VirtualHost configuration root") le_util.add_deprecated_argument(add, "init-script", 1) def __init__(self, *args, **kwargs): @@ -148,7 +148,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.config_test() self.parser = parser.ApacheParser( - self.aug, self.conf("config-root"), self.conf("ctl")) + self.aug, self.conf("server-root"), self.conf("ctl")) # Check for errors in parsing files with Augeas self.check_parsing_errors("httpd.aug") @@ -481,10 +481,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :rtype: list """ - # Search sites-available, httpd.conf for possible virtual hosts + # Search vhost-root, httpd.conf for possible virtual hosts paths = self.aug.match( - ("/files%s/sites-available//*[label()=~regexp('%s')]" % - (self.parser.root, parser.case_i("VirtualHost")))) + ("/files%s//*[label()=~regexp('%s')]" % + (self.conf("vhost-root"), parser.case_i("VirtualHost")))) vhs = [] @@ -997,8 +997,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if len(ssl_vhost.name) < (255 - (len(redirect_filename) + 1)): redirect_filename = "le-redirect-%s.conf" % ssl_vhost.name - redirect_filepath = os.path.join( - self.parser.root, "sites-available", redirect_filename) + redirect_filepath = os.path.join(self.conf("vhost-root"), redirect_filename) # Register the new file that will be created # Note: always register the creation before writing to ensure file will diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 09fb8ba59..cf6351f22 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -5,7 +5,7 @@ from letsencrypt import le_util CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", - config_root="/etc/apache2", + vhost_root="/etc/apache2/sites-available", ctl="apache2ctl", enmod="a2enmod", dismod="a2dismod", @@ -14,7 +14,7 @@ CLI_DEFAULTS_DEBIAN = dict( ) CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", - config_root="/etc/httpd/conf.d", + vhost_root="/etc/httpd/conf.d", ctl="apachectl", enmod=None, dismod=None, From db9bf90cf9aaa779cddd186f2f9cf234b1239e06 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sun, 6 Dec 2015 22:42:51 +0200 Subject: [PATCH 08/90] Change apachectl parameters to work with other distros too --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 353e6c68a..7ad9f0ff8 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1083,6 +1083,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :rtype: bool """ + # Always return true for distros without enabled / available + if self.conf("enmod") == None: + return True enabled_dir = os.path.join(self.parser.root, "sites-enabled") for entry in os.listdir(enabled_dir): try: @@ -1209,7 +1212,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ try: - le_util.run_script([self.conf("ctl"), "-k", "graceful"]) + le_util.run_script([self.conf("ctl"), "graceful"]) except errors.SubprocessError as err: raise errors.MisconfigurationError(str(err)) From 103454e4a304a1a3062dc51dddddcdb69b947cfe Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sun, 6 Dec 2015 22:46:28 +0200 Subject: [PATCH 09/90] Support CentOS configuration layout in parser --- letsencrypt-apache/letsencrypt_apache/parser.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index ec5211ae4..c47dbd99f 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -546,8 +546,7 @@ class ApacheParser(object): def _find_config_root(self): """Find the Apache Configuration Root file.""" - location = ["apache2.conf", "httpd.conf"] - + location = ["apache2.conf", "httpd.conf", "conf/httpd.conf"] for name in location: if os.path.isfile(os.path.join(self.root, name)): return os.path.join(self.root, name) From b11f091023d7e1fc63b3cce66347540008ac0da9 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sun, 6 Dec 2015 23:06:56 +0200 Subject: [PATCH 10/90] Cleanup --- letsencrypt-apache/letsencrypt_apache/configurator.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 7ad9f0ff8..ac7ce76d4 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -140,7 +140,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # Verify Apache is installed for exe in (self.conf("ctl"), self.conf("enmod"), self.conf("dismod")): - if exe != None: + if exe is not None: if not le_util.exe_exists(exe): raise errors.NoInstallationError @@ -472,7 +472,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self._add_servernames(vhost) return vhost - # TODO: make "sites-available" a configurable directory def get_virtual_hosts(self): """Returns list of virtual hosts found in the Apache configuration. From e58c0a530fa180438392adb7aa9e205a1931cbdb Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 01:15:29 +0200 Subject: [PATCH 11/90] Call paper with customizable vhost root parameter --- .../letsencrypt_apache/configurator.py | 2 +- letsencrypt-apache/letsencrypt_apache/parser.py | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ac7ce76d4..ce0714668 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -148,7 +148,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.config_test() self.parser = parser.ApacheParser( - self.aug, self.conf("server-root"), self.conf("ctl")) + self.aug, self.conf("server-root"), self.conf("vhost-root"), self.conf("ctl")) # Check for errors in parsing files with Augeas self.check_parsing_errors("httpd.aug") diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index c47dbd99f..2cd258a1b 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -28,7 +28,7 @@ class ApacheParser(object): arg_var_interpreter = re.compile(r"\$\{[^ \}]*}") fnmatch_chars = set(["*", "?", "\\", "[", "]"]) - def __init__(self, aug, root, ctl): + def __init__(self, aug, root, vhostroot, ctl): # Note: Order is important here. # This uses the binary, so it can be done first. @@ -44,6 +44,8 @@ class ApacheParser(object): self.loc = {"root": self._find_config_root()} self._parse_file(self.loc["root"]) + self.vhostroot = os.path.abspath(vhostroot) + # This problem has been fixed in Augeas 1.0 self.standardize_excl() @@ -56,9 +58,12 @@ class ApacheParser(object): # Set up rest of locations self.loc.update(self._set_locations()) - # Must also attempt to parse sites-available or equivalent - # Sites-available is not included naturally in configuration - self._parse_file(os.path.join(self.root, "sites-available") + "/*") + # Take the CentOS layout into account, httpd.conf not in httpd root + self._parse_file(os.path.join(self.root, "conf") + "/httpd.conf") + + # Must also attempt to parse virtual host root + self._parse_file(self.vhostroot + "/*") + def init_modules(self): """Iterates on the configuration until no new modules are loaded. From 651c8702cbd81a35fcfab081cf4051e7062b1e3b Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 01:16:07 +0200 Subject: [PATCH 12/90] Commented out TLS SNI apache extra config file logging options, because of distro specific paths --- .../letsencrypt_apache/options-ssl-apache.conf | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/options-ssl-apache.conf b/letsencrypt-apache/letsencrypt_apache/options-ssl-apache.conf index 2a724d7ec..ec07a4ba3 100644 --- a/letsencrypt-apache/letsencrypt_apache/options-ssl-apache.conf +++ b/letsencrypt-apache/letsencrypt_apache/options-ssl-apache.conf @@ -14,9 +14,9 @@ SSLOptions +StrictRequire LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common -CustomLog /var/log/apache2/access.log vhost_combined -LogLevel warn -ErrorLog /var/log/apache2/error.log +#CustomLog /var/log/apache2/access.log vhost_combined +#LogLevel warn +#ErrorLog /var/log/apache2/error.log # Always ensure Cookies have "Secure" set (JAH 2012/1) #Header edit Set-Cookie (?i)^(.*)(;\s*secure)??((\s*;)?(.*)) "$1; Secure$3$4" From cd0ae93ddc93fa02acde43d7f91ff65d5659c12b Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 11:06:32 +0200 Subject: [PATCH 13/90] Be more explicit in the configuration file parsing in vhost directory, augeas will silently fail if it encounters something funny --- letsencrypt-apache/letsencrypt_apache/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 2cd258a1b..abdf6e449 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -62,7 +62,7 @@ class ApacheParser(object): self._parse_file(os.path.join(self.root, "conf") + "/httpd.conf") # Must also attempt to parse virtual host root - self._parse_file(self.vhostroot + "/*") + self._parse_file(self.vhostroot + "/*.conf") def init_modules(self): From 6e3da9e0438d26722623fdfb838b8e00cfeb36dc Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 11:07:31 +0200 Subject: [PATCH 14/90] Configurable directory path for challenge configuration file --- letsencrypt-apache/letsencrypt_apache/configurator.py | 3 +++ letsencrypt-apache/letsencrypt_apache/constants.py | 6 ++++-- letsencrypt-apache/letsencrypt_apache/tls_sni_01.py | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ce0714668..642a46696 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -100,6 +100,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): help="Apache server root directory.") add("vhost-root", default=constants.os_constant("vhost_root"), help="Apache server VirtualHost configuration root") + add("challenge-location", + default=constants.os_constant("challenge_location"), + help="Directory path for challenge configuration.") le_util.add_deprecated_argument(add, "init-script", 1) def __init__(self, *args, **kwargs): diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index cf6351f22..0fd9a25dc 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -10,7 +10,8 @@ CLI_DEFAULTS_DEBIAN = dict( enmod="a2enmod", dismod="a2dismod", le_vhost_ext="-le-ssl.conf", - handle_mods=True + handle_mods=True, + challenge_location="/etc/apache2" ) CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", @@ -19,7 +20,8 @@ CLI_DEFAULTS_CENTOS = dict( enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", - handle_mods=False + handle_mods=False, + challenge_location="/etc/httpd/conf.d" ) CLI_DEFAULTS = { "debian": CLI_DEFAULTS_DEBIAN, diff --git a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py index 4284e240c..2049eb574 100644 --- a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py +++ b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py @@ -50,7 +50,7 @@ class ApacheTlsSni01(common.TLSSNI01): super(ApacheTlsSni01, self).__init__(*args, **kwargs) self.challenge_conf = os.path.join( - self.configurator.conf("server-root"), + self.configurator.conf("challenge-location"), "le_tls_sni_01_cert_challenge.conf") def perform(self): From 3d9f8c9748edf6c36a8a40cac8be45ec933ced44 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 12:01:35 +0200 Subject: [PATCH 15/90] Cleanup & pydoc --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- letsencrypt-apache/letsencrypt_apache/constants.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 642a46696..c8b42bd5f 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1087,7 +1087,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # Always return true for distros without enabled / available if self.conf("enmod") == None: - return True + return True enabled_dir = os.path.join(self.parser.root, "sites-enabled") for entry in os.listdir(enabled_dir): try: diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 0fd9a25dc..24cf7373a 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -59,6 +59,10 @@ HEADER_ARGS = {"Strict-Transport-Security": HSTS_ARGS, "Upgrade-Insecure-Requests": UIR_ARGS} def os_constant(key): + """Get a constant value for operating system + :param key: name of cli constant + :return: value of constant for active os + """ os_info = le_util.get_os_info() try: constants = CLI_DEFAULTS[os_info[0].lower()] From 3701560a88279e435e9a49faac538deaa8cc9ced Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 12:03:54 +0200 Subject: [PATCH 16/90] Fix existing tests --- .../tests/configurator_test.py | 8 ++++---- .../letsencrypt_apache/tests/parser_test.py | 10 +++++++--- .../letsencrypt_apache/tests/tls_sni_01_test.py | 2 +- .../letsencrypt_apache/tests/util.py | 17 +++++++++++------ 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index fcccfaae2..50b23b815 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -27,7 +27,7 @@ class TwoVhost80Test(util.ApacheTest): super(TwoVhost80Test, self).setUp() self.config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir) + self.config_path, self.vhost_path, self.config_dir, self.work_dir) self.vh_truth = util.get_vh_truth( self.temp_dir, "debian_apache_2_4/two_vhost_80") @@ -244,7 +244,7 @@ class TwoVhost80Test(util.ApacheTest): def test_deploy_cert_newssl(self): self.config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir, version=(2, 4, 16)) + self.config_path, self.vhost_path, self.config_dir, self.work_dir, version=(2, 4, 16)) self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") @@ -276,7 +276,7 @@ class TwoVhost80Test(util.ApacheTest): def test_deploy_cert_newssl_no_fullchain(self): self.config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir, version=(2, 4, 16)) + self.config_path, self.vhost_path, self.config_dir, self.work_dir, version=(2, 4, 16)) self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") @@ -289,7 +289,7 @@ class TwoVhost80Test(util.ApacheTest): def test_deploy_cert_old_apache_no_chain(self): self.config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir, version=(2, 4, 7)) + self.config_path, self.vhost_path, self.config_dir, self.work_dir, version=(2, 4, 7)) self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index bc1f316f9..ca51c6fd8 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -193,7 +193,9 @@ class ParserInitTest(util.ApacheTest): path = os.path.join( self.temp_dir, "debian_apache_2_4/////two_vhost_80/../two_vhost_80/apache2") - parser = ApacheParser(self.aug, path, "dummy_ctl") + + parser = ApacheParser(self.aug, path, + "/dummy/vhostpath", "dummy_ctl") self.assertEqual(parser.root, self.config_path) @@ -202,7 +204,8 @@ class ParserInitTest(util.ApacheTest): with mock.patch("letsencrypt_apache.parser.ApacheParser." "update_runtime_variables"): parser = ApacheParser( - self.aug, os.path.relpath(self.config_path), "dummy_ctl") + self.aug, os.path.relpath(self.config_path), + "/dummy/vhostpath", "dummy_ctl") self.assertEqual(parser.root, self.config_path) @@ -211,7 +214,8 @@ class ParserInitTest(util.ApacheTest): with mock.patch("letsencrypt_apache.parser.ApacheParser." "update_runtime_variables"): parser = ApacheParser( - self.aug, self.config_path + os.path.sep, "dummy_ctl") + self.aug, self.config_path + os.path.sep, + "/dummy/vhostpath", "dummy_ctl") self.assertEqual(parser.root, self.config_path) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py b/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py index f4dff7734..6caca2520 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py @@ -20,7 +20,7 @@ class TlsSniPerformTest(util.ApacheTest): super(TlsSniPerformTest, self).setUp() config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir) + self.config_path, self.vhost_path, self.config_dir, self.work_dir) config.config.tls_sni_01_port = 443 from letsencrypt_apache import tls_sni_01 diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index 0c60373f2..95c95e6a9 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -23,7 +23,8 @@ from letsencrypt_apache import obj class ApacheTest(unittest.TestCase): # pylint: disable=too-few-public-methods def setUp(self, test_dir="debian_apache_2_4/two_vhost_80", - config_root="debian_apache_2_4/two_vhost_80/apache2"): + config_root="debian_apache_2_4/two_vhost_80/apache2", + vhost_root="debian_apache_2_4/two_vhost_80/apache2/sites-available"): # pylint: disable=arguments-differ super(ApacheTest, self).setUp() @@ -36,6 +37,7 @@ class ApacheTest(unittest.TestCase): # pylint: disable=too-few-public-methods constants.MOD_SSL_CONF_DEST) self.config_path = os.path.join(self.temp_dir, config_root) + self.vhost_path = os.path.join(self.temp_dir, vhost_root) self.rsa512jwk = jose.JWKRSA.load(test_util.load_vector( "rsa512_key.pem")) @@ -44,8 +46,9 @@ class ApacheTest(unittest.TestCase): # pylint: disable=too-few-public-methods class ParserTest(ApacheTest): # pytlint: disable=too-few-public-methods def setUp(self, test_dir="debian_apache_2_4/two_vhost_80", - config_root="debian_apache_2_4/two_vhost_80/apache2"): - super(ParserTest, self).setUp(test_dir, config_root) + config_root="debian_apache_2_4/two_vhost_80/apache2", + vhost_root="debian_apache_2_4/two_vhost_80/apache2/sites-available"): + super(ParserTest, self).setUp(test_dir, config_root, vhost_root) zope.component.provideUtility(display_util.FileDisplay(sys.stdout)) @@ -55,11 +58,11 @@ class ParserTest(ApacheTest): # pytlint: disable=too-few-public-methods with mock.patch("letsencrypt_apache.parser.ApacheParser." "update_runtime_variables"): self.parser = ApacheParser( - self.aug, self.config_path, "dummy_ctl_path") + self.aug, self.config_path, self.vhost_path, "dummy_ctl_path") def get_apache_configurator( - config_path, config_dir, work_dir, version=(2, 4, 7), conf=None): + config_path, vhost_path, config_dir, work_dir, version=(2, 4, 7), conf=None): """Create an Apache Configurator with the specified options. :param conf: Function that returns binary paths. self.conf in Configurator @@ -68,7 +71,9 @@ def get_apache_configurator( backups = os.path.join(work_dir, "backups") mock_le_config = mock.MagicMock( apache_server_root=config_path, - apache_le_vhost_ext=constants.CLI_DEFAULTS["le_vhost_ext"], + apache_vhost_root=vhost_path, + apache_le_vhost_ext=constants.os_constant("le_vhost_ext"), + apache_challenge_location=config_path, backup_dir=backups, config_dir=config_dir, temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), From c9f14e04618f10e8240c5fbbd39ade1c8699c2de Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 12:13:24 +0200 Subject: [PATCH 17/90] Augeas test fix --- .../letsencrypt_apache/tests/augeas_configurator_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py index 815e6fc44..b70e1c7f1 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py @@ -17,7 +17,7 @@ class AugeasConfiguratorTest(util.ApacheTest): super(AugeasConfiguratorTest, self).setUp() self.config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir) + self.config_path, self.vhost_path, self.config_dir, self.work_dir) self.vh_truth = util.get_vh_truth( self.temp_dir, "debian_apache_2_4/two_vhost_80") From 9e0bcf9f3c4593656f4a305ee7e55c439c686fa8 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 12:38:52 +0200 Subject: [PATCH 18/90] Add cli parameter for handle_mods --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index c8b42bd5f..9acc5cad6 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -103,6 +103,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): add("challenge-location", default=constants.os_constant("challenge_location"), help="Directory path for challenge configuration.") + add("handle-modules", default=constants.os_constant("handle_mods"), + help="Let installer handle enabling required modules for you."+ + "(Only Ubuntu/Debian currently)") le_util.add_deprecated_argument(add, "init-script", 1) def __init__(self, *args, **kwargs): @@ -545,7 +548,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :param str port: Port to listen on """ - if constants.os_constant("handle_mods"): + if self.conf("handle_mods"): if "ssl_module" not in self.parser.modules: self.enable_mod("ssl", temp=temp) From b02a881c6e07ae289564be4dc94b976e629504e4 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 12:42:40 +0200 Subject: [PATCH 19/90] Constant value per OS basis if installer should handle enabling / disabling sites --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- letsencrypt-apache/letsencrypt_apache/constants.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 9acc5cad6..30be653a1 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1089,7 +1089,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # Always return true for distros without enabled / available - if self.conf("enmod") == None: + if not constants.os_constant("handle_sites"): return True enabled_dir = os.path.join(self.parser.root, "sites-enabled") for entry in os.listdir(enabled_dir): diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 24cf7373a..70d0beabb 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -11,6 +11,7 @@ CLI_DEFAULTS_DEBIAN = dict( dismod="a2dismod", le_vhost_ext="-le-ssl.conf", handle_mods=True, + handle_sites=True, challenge_location="/etc/apache2" ) CLI_DEFAULTS_CENTOS = dict( @@ -21,6 +22,7 @@ CLI_DEFAULTS_CENTOS = dict( dismod=None, le_vhost_ext="-le-ssl.conf", handle_mods=False, + handle_sites=False, challenge_location="/etc/httpd/conf.d" ) CLI_DEFAULTS = { From 43473827802a2756dadcce2ce15228db5e876c48 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 12:46:56 +0200 Subject: [PATCH 20/90] Moved enable_site check to correct place --- letsencrypt-apache/letsencrypt_apache/configurator.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 30be653a1..c4ec3c2e7 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -245,9 +245,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if chain_path is not None: self.save_notes += "\tSSLCertificateChainFile %s\n" % chain_path - # Make sure vhost is enabled - if not vhost.enabled: - self.enable_site(vhost) + # Make sure vhost is enabled if distro with enabled / available + if constants.os_constant("handle_sites"): + if not vhost.enabled: + self.enable_site(vhost) def choose_vhost(self, target_name, temp=False): """Chooses a virtual host based on the given domain name. @@ -1088,9 +1089,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :rtype: bool """ - # Always return true for distros without enabled / available - if not constants.os_constant("handle_sites"): - return True + enabled_dir = os.path.join(self.parser.root, "sites-enabled") for entry in os.listdir(enabled_dir): try: From 718a6481f1c2f72139d78ec5fa63a1c817f52bb7 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 13:16:48 +0200 Subject: [PATCH 21/90] Tests for the lone function in constants --- .../tests/constants_test.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 letsencrypt-apache/letsencrypt_apache/tests/constants_test.py diff --git a/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py b/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py new file mode 100644 index 000000000..478debb59 --- /dev/null +++ b/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py @@ -0,0 +1,22 @@ +import mock +import unittest + +from letsencrypt_apache import constants + + +class ConstantsTest(unittest.TestCase): + + @mock.patch("letsencrypt.le_util.get_os_info") + def test_get_debian_value(self, os_info): + os_info.return_value = ('Debian','','') + self.assertEqual(constants.os_constant("ctl"), "apache2ctl") + + @mock.patch("letsencrypt.le_util.get_os_info") + def test_get_centos_value(self, os_info): + os_info.return_value = ('CentOS Linux','','') + self.assertEqual(constants.os_constant("ctl"), "apachectl") + + @mock.patch("letsencrypt.le_util.get_os_info") + def test_get_default_value(self, os_info): + os_info.return_value = ('Nonexistent Linux','','') + self.assertEqual(constants.os_constant("ctl"), "apache2ctl") From 47eb7d76cdd25405641b8d7db3349a33c6f5beb8 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 13:37:58 +0200 Subject: [PATCH 22/90] PEP8 & linter love --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- letsencrypt-apache/letsencrypt_apache/constants.py | 1 + letsencrypt-apache/letsencrypt_apache/parser.py | 1 - .../letsencrypt_apache/tests/constants_test.py | 8 +++++--- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index c4ec3c2e7..549a19188 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -104,7 +104,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): default=constants.os_constant("challenge_location"), help="Directory path for challenge configuration.") add("handle-modules", default=constants.os_constant("handle_mods"), - help="Let installer handle enabling required modules for you."+ + help="Let installer handle enabling required modules for you." + "(Only Ubuntu/Debian currently)") le_util.add_deprecated_argument(add, "init-script", 1) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 70d0beabb..773ceb266 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -60,6 +60,7 @@ UIR_ARGS = ["always", "set", "Content-Security-Policy", HEADER_ARGS = {"Strict-Transport-Security": HSTS_ARGS, "Upgrade-Insecure-Requests": UIR_ARGS} + def os_constant(key): """Get a constant value for operating system :param key: name of cli constant diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index abdf6e449..4ab2b82a0 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -64,7 +64,6 @@ class ApacheParser(object): # Must also attempt to parse virtual host root self._parse_file(self.vhostroot + "/*.conf") - def init_modules(self): """Iterates on the configuration until no new modules are loaded. diff --git a/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py b/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py index 478debb59..63eb5c783 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py @@ -1,3 +1,5 @@ +"""Test for letsencrypt_apache.configurator.""" + import mock import unittest @@ -8,15 +10,15 @@ class ConstantsTest(unittest.TestCase): @mock.patch("letsencrypt.le_util.get_os_info") def test_get_debian_value(self, os_info): - os_info.return_value = ('Debian','','') + os_info.return_value = ('Debian', '', '') self.assertEqual(constants.os_constant("ctl"), "apache2ctl") @mock.patch("letsencrypt.le_util.get_os_info") def test_get_centos_value(self, os_info): - os_info.return_value = ('CentOS Linux','','') + os_info.return_value = ('CentOS Linux', '', '') self.assertEqual(constants.os_constant("ctl"), "apachectl") @mock.patch("letsencrypt.le_util.get_os_info") def test_get_default_value(self, os_info): - os_info.return_value = ('Nonexistent Linux','','') + os_info.return_value = ('Nonexistent Linux', '', '') self.assertEqual(constants.os_constant("ctl"), "apache2ctl") From f479a9b894d1a55f6319ef98b837a442188d1f91 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 14:22:56 +0200 Subject: [PATCH 23/90] Added fedora layout --- letsencrypt-apache/letsencrypt_apache/constants.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 773ceb266..87c3e6e66 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -29,7 +29,8 @@ CLI_DEFAULTS = { "debian": CLI_DEFAULTS_DEBIAN, "ubuntu": CLI_DEFAULTS_DEBIAN, "centos": CLI_DEFAULTS_CENTOS, - "centos linux": CLI_DEFAULTS_CENTOS + "centos linux": CLI_DEFAULTS_CENTOS, + "fedora": CLI_DEFAULTS_CENTOS } """CLI defaults.""" From 5dd3ecfb5324ac424b17663ded13830a48b70f06 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 14:42:14 +0200 Subject: [PATCH 24/90] Respect tool setting in rpm bootstrap --- bootstrap/_rpm_common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap/_rpm_common.sh b/bootstrap/_rpm_common.sh index b80a9555b..57b28240e 100755 --- a/bootstrap/_rpm_common.sh +++ b/bootstrap/_rpm_common.sh @@ -49,7 +49,7 @@ then fi -if yum list installed "httpd" >/dev/null 2>&1; then +if $tool list installed "httpd" >/dev/null 2>&1; then if ! $tool install -y mod_ssl then echo "Apache found, but mod_ssl could not be installed." From 0ce4fa4c6732a0d5ca38a80b47c070ac00b85885 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 17:04:22 +0200 Subject: [PATCH 25/90] Was using constant name instead of conf name --- 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 549a19188..e99f5cbe0 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -549,7 +549,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :param str port: Port to listen on """ - if self.conf("handle_mods"): + if self.conf("handle-modules"): if "ssl_module" not in self.parser.modules: self.enable_mod("ssl", temp=temp) From d4337f3936031169b4a8afeed29aa99d59a05841 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 17:12:42 +0200 Subject: [PATCH 26/90] Give user command line control on using enable site helper script --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index e99f5cbe0..52635ca11 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -106,6 +106,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): add("handle-modules", default=constants.os_constant("handle_mods"), help="Let installer handle enabling required modules for you." + "(Only Ubuntu/Debian currently)") + add("handle-sites", default=constants.os_constant("handle_sites"), + help="Let installer handle enabling sites for you." + + "(Only Ubuntu/Debian currently)") le_util.add_deprecated_argument(add, "init-script", 1) def __init__(self, *args, **kwargs): @@ -246,7 +249,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.save_notes += "\tSSLCertificateChainFile %s\n" % chain_path # Make sure vhost is enabled if distro with enabled / available - if constants.os_constant("handle_sites"): + if self.conf("handle-sites"): if not vhost.enabled: self.enable_site(vhost) From 55cac0dc9f9cf3db175bcf79666963e148b11871 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 9 Dec 2015 17:05:31 +0200 Subject: [PATCH 27/90] Fixed a2dismod help text --- 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 3ad6032f1..be9825304 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -93,7 +93,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): add("enmod", default=constants.os_constant("enmod"), help="Path to the Apache 'a2enmod' binary.") add("dismod", default=constants.os_constant("dismod"), - help="Path to the Apache 'a2enmod' binary.") + help="Path to the Apache 'a2dismod' binary.") add("le-vhost-ext", default=constants.os_constant("le_vhost_ext"), help="SSL vhost configuration extension.") add("server-root", default=constants.os_constant("server_root"), From 54127eef5a6286959fa9fe424a776f1faa5b61fb Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 10 Dec 2015 18:12:07 +0200 Subject: [PATCH 28/90] Don't call site enable methods when handle-sites is false for the operating system --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index be9825304..3e6881739 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -471,7 +471,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): is_ssl = True filename = get_file_path(path) - is_enabled = self.is_site_enabled(filename) + if self.conf("handle-sites"): + is_enabled = self.is_site_enabled(filename) + else: + is_enabled = True macro = False if "/macro/" in path.lower(): From 509c192ab8e3515e3733a91769e2ec42b2b7acc0 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 10 Dec 2015 18:17:12 +0200 Subject: [PATCH 29/90] Add RedHat Enterprise defaults to constants --- letsencrypt-apache/letsencrypt_apache/constants.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 87c3e6e66..049ddce4d 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -30,7 +30,8 @@ CLI_DEFAULTS = { "ubuntu": CLI_DEFAULTS_DEBIAN, "centos": CLI_DEFAULTS_CENTOS, "centos linux": CLI_DEFAULTS_CENTOS, - "fedora": CLI_DEFAULTS_CENTOS + "fedora": CLI_DEFAULTS_CENTOS, + "red hat enterprise linux server": CLI_DEFAULTS_CENTOS } """CLI defaults.""" From 7bf8842ba7107a86318c7a954e7ed00b3fc904f5 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 11 Dec 2015 00:03:00 +0200 Subject: [PATCH 30/90] Added missing test for full coverage --- .../tests/configurator_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index b93034cd9..e16dff173 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -116,6 +116,24 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(found, 6) + # Handle case of non-debian layout get_virtual_hosts + orig_conf = self.config.conf + with mock.patch( + "letsencrypt_apache.configurator.ApacheConfigurator.conf" + ) as mock_conf: + def conf_sideeffect(key): + """Handle calls to configurator.conf() + :param key: configuration key + :return: configuration value + """ + if key == "handle-sites": + return False + else: + return orig_conf(key) + mock_conf.side_effect = conf_sideeffect + vhs = self.config.get_virtual_hosts() + self.assertEqual(len(vhs), 6) + @mock.patch("letsencrypt_apache.display_ops.select_vhost") def test_choose_vhost_none_avail(self, mock_select): mock_select.return_value = None From 48b1240451b3dafd9228baa400633317df784654 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 11 Dec 2015 09:18:00 +0200 Subject: [PATCH 31/90] Removed redundant config parsing directive. --- letsencrypt-apache/letsencrypt_apache/parser.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 4ab2b82a0..0289c57d8 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -58,9 +58,6 @@ class ApacheParser(object): # Set up rest of locations self.loc.update(self._set_locations()) - # Take the CentOS layout into account, httpd.conf not in httpd root - self._parse_file(os.path.join(self.root, "conf") + "/httpd.conf") - # Must also attempt to parse virtual host root self._parse_file(self.vhostroot + "/*.conf") From 4d31a8cb390778a35207bb0133b21a46c91de2dc Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 11 Dec 2015 12:41:21 +0200 Subject: [PATCH 32/90] Sane error message in case user tries to use apache-handle-sites functionality in non-supported environment. --- letsencrypt-apache/letsencrypt_apache/configurator.py | 6 ++++++ .../letsencrypt_apache/tests/configurator_test.py | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 3e6881739..d67e7bc18 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1119,6 +1119,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ enabled_dir = os.path.join(self.parser.root, "sites-enabled") + if not os.path.isdir(enabled_dir): + error_msg = ("Directory '{0}' does not exist. Please ensure " + "that the values for --apache-handle-sites and " + "--apache-server-root are correct for your " + "environment.".format(enabled_dir)) + raise errors.ConfigurationError(error_msg) for entry in os.listdir(enabled_dir): try: if filecmp.cmp(avail_fp, os.path.join(enabled_dir, entry)): diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index e16dff173..064111d4a 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -219,6 +219,10 @@ class TwoVhost80Test(util.ApacheTest): self.assertFalse(self.config.is_site_enabled(self.vh_truth[1].filep)) self.assertTrue(self.config.is_site_enabled(self.vh_truth[2].filep)) self.assertTrue(self.config.is_site_enabled(self.vh_truth[3].filep)) + with mock.patch("os.path.isdir") as mock_isdir: + mock_isdir.return_value = False + with (self.assertRaises(errors.ConfigurationError)): + self.config.is_site_enabled("irrelevant") @mock.patch("letsencrypt.le_util.run_script") @mock.patch("letsencrypt.le_util.exe_exists") From 0805b08162ca777062b26c136bebd2e48cea322e Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 11 Dec 2015 13:17:21 +0200 Subject: [PATCH 33/90] Make python 2.6 happy with the assertRaises --- .../letsencrypt_apache/tests/configurator_test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 064111d4a..9ff6456be 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -221,8 +221,9 @@ class TwoVhost80Test(util.ApacheTest): self.assertTrue(self.config.is_site_enabled(self.vh_truth[3].filep)) with mock.patch("os.path.isdir") as mock_isdir: mock_isdir.return_value = False - with (self.assertRaises(errors.ConfigurationError)): - self.config.is_site_enabled("irrelevant") + self.assertRaises(errors.ConfigurationError, + self.config.is_site_enabled, + "irrelevant") @mock.patch("letsencrypt.le_util.run_script") @mock.patch("letsencrypt.le_util.exe_exists") From 77c67f1fa9ab7a6e3fd8519b252f6abb9d54e89c Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 14 Dec 2015 09:27:16 +0200 Subject: [PATCH 34/90] Gentoo constants, still missing gentoo fingerprint though --- letsencrypt-apache/letsencrypt_apache/constants.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 049ddce4d..989106e39 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -25,6 +25,17 @@ CLI_DEFAULTS_CENTOS = dict( handle_sites=False, challenge_location="/etc/httpd/conf.d" ) +CLI_DEFAULTS_GENTOO = dict( + server_root="/etc/apache2", + vhost_root="/etc/apache2/vhosts.d", + ctl="apache2ctl", + enmod=None, + dismod=None, + le_vhost_ext="-le-ssl.conf", + handle_mods=False, + handle_sites=False, + challenge_location="/etc/apache2/vhosts.d" +) CLI_DEFAULTS = { "debian": CLI_DEFAULTS_DEBIAN, "ubuntu": CLI_DEFAULTS_DEBIAN, From e7226d28041a03a8f2d6d61ac84e765e8235a180 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Dec 2015 16:43:24 -0800 Subject: [PATCH 35/90] Automate testing with the apache-conf-library --- tests/apache-conf-files/hackish-apache-test | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/apache-conf-files/hackish-apache-test b/tests/apache-conf-files/hackish-apache-test index c6663551e..8efe65e42 100755 --- a/tests/apache-conf-files/hackish-apache-test +++ b/tests/apache-conf-files/hackish-apache-test @@ -18,11 +18,25 @@ function CleanupExit() { exit 1 } +FAILS=0 trap CleanupExit INT for f in *.conf ; do - echo testing "$f" + echo -n testing "$f"... sudo cp "$f" "$EA"/sites-available/ sudo ln -s "$EA/sites-available/$f" "$EA/sites-enabled/$f" - sudo "$LEROOT"/venv/bin/letsencrypt --apache certonly -t + RESULT=`echo c | sudo "$LEROOT"/venv/bin/letsencrypt --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` + if echo $RESULT | grep -Eq \("Please specify --domains"\|"mod_macro is not yet"\) ; then + echo passed + else + echo failed + echo $RESULT + echo + echo + FAILS=`expr $FAILS + 1` + fi sudo rm /etc/apache2/sites-{enabled,available}/"$f" done +if [ "$FAILS" -ne 0 ] ; then + return 1 +fi +return 0 From 52705107ff76f1e713b9b903837e7c38c230f411 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Dec 2015 17:01:36 -0800 Subject: [PATCH 36/90] let the environment determine how letsencrypt is run --- tests/apache-conf-files/hackish-apache-test | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/apache-conf-files/hackish-apache-test b/tests/apache-conf-files/hackish-apache-test index 8efe65e42..e6f25b15f 100755 --- a/tests/apache-conf-files/hackish-apache-test +++ b/tests/apache-conf-files/hackish-apache-test @@ -9,6 +9,7 @@ export EA=/etc/apache2/ TESTDIR="`dirname $0`" LEROOT="`realpath \"$TESTDIR/../../\"`" cd $TESTDIR/passing +LETSENCRYPT="${LETSENCRYPT:-$LEROOT/venv/bin/letsencrypt}" function CleanupExit() { echo control c, exiting tests... @@ -24,7 +25,7 @@ for f in *.conf ; do echo -n testing "$f"... sudo cp "$f" "$EA"/sites-available/ sudo ln -s "$EA/sites-available/$f" "$EA/sites-enabled/$f" - RESULT=`echo c | sudo "$LEROOT"/venv/bin/letsencrypt --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` + RESULT=`echo c | sudo "$LETSENCRYPT" --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` if echo $RESULT | grep -Eq \("Please specify --domains"\|"mod_macro is not yet"\) ; then echo passed else From 3c6af7094c67b37e8b815309fe0a492e232fccbf Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Dec 2015 17:21:47 -0800 Subject: [PATCH 37/90] Bugfix, and use --staging --- tests/apache-conf-files/hackish-apache-test | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/apache-conf-files/hackish-apache-test b/tests/apache-conf-files/hackish-apache-test index e6f25b15f..c4df319ed 100755 --- a/tests/apache-conf-files/hackish-apache-test +++ b/tests/apache-conf-files/hackish-apache-test @@ -25,7 +25,7 @@ for f in *.conf ; do echo -n testing "$f"... sudo cp "$f" "$EA"/sites-available/ sudo ln -s "$EA/sites-available/$f" "$EA/sites-enabled/$f" - RESULT=`echo c | sudo "$LETSENCRYPT" --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` + RESULT=`echo c | sudo "$LETSENCRYPT" --staging --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` if echo $RESULT | grep -Eq \("Please specify --domains"\|"mod_macro is not yet"\) ; then echo passed else @@ -38,6 +38,6 @@ for f in *.conf ; do sudo rm /etc/apache2/sites-{enabled,available}/"$f" done if [ "$FAILS" -ne 0 ] ; then - return 1 + exit 1 fi -return 0 +exit 0 From bf764e4852dcf061e586789455afcb9785f39e3f Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Dec 2015 18:01:49 -0800 Subject: [PATCH 38/90] Support appending to non-Debianish Apache setups --- tests/apache-conf-files/hackish-apache-test | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/apache-conf-files/hackish-apache-test b/tests/apache-conf-files/hackish-apache-test index c4df319ed..99fa123f7 100755 --- a/tests/apache-conf-files/hackish-apache-test +++ b/tests/apache-conf-files/hackish-apache-test @@ -23,8 +23,14 @@ FAILS=0 trap CleanupExit INT for f in *.conf ; do echo -n testing "$f"... - sudo cp "$f" "$EA"/sites-available/ - sudo ln -s "$EA/sites-available/$f" "$EA/sites-enabled/$f" + if [ "$APPEND_APACHECONF" = "" ] ; then + sudo cp "$f" "$EA"/sites-available/ + sudo ln -sf "$EA/sites-available/$f" "$EA/sites-enabled/$f" + else + TMP="/tmp/`basename \"$APPEND_APACHECONF\"`.$$" + sudo cp -a "$APPEND_APACHECONF" "$TMP" + sudo bash -c "cat \"$f\" >> \"$APPEND_APACHECONF\"" + fi RESULT=`echo c | sudo "$LETSENCRYPT" --staging --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` if echo $RESULT | grep -Eq \("Please specify --domains"\|"mod_macro is not yet"\) ; then echo passed @@ -35,7 +41,11 @@ for f in *.conf ; do echo FAILS=`expr $FAILS + 1` fi - sudo rm /etc/apache2/sites-{enabled,available}/"$f" + if [ "$APPEND_APACHECONF" = "" ] ; then + sudo rm /etc/apache2/sites-{enabled,available}/"$f" + else + sudo mv "$TMP" "$APPEND_APACHECONF" + fi done if [ "$FAILS" -ne 0 ] ; then exit 1 From 9013fecc9cea8bfca8e29789520772c3bd96cd51 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Dec 2015 19:41:35 -0800 Subject: [PATCH 39/90] Prep for testfarming. --- tests/apache-conf-files/hackish-apache-test | 16 ++++++++++------ tests/apache-conf-files/passing/README.modules | 3 +-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/apache-conf-files/hackish-apache-test b/tests/apache-conf-files/hackish-apache-test index 99fa123f7..3d4336579 100755 --- a/tests/apache-conf-files/hackish-apache-test +++ b/tests/apache-conf-files/hackish-apache-test @@ -14,11 +14,19 @@ LETSENCRYPT="${LETSENCRYPT:-$LEROOT/venv/bin/letsencrypt}" function CleanupExit() { echo control c, exiting tests... if [ "$f" != "" ] ; then - sudo rm /etc/apache2/sites-{enabled,available}/"$f" + Cleanup fi exit 1 } +function Cleanup() { + if [ "$APPEND_APACHECONF" = "" ] ; then + sudo rm /etc/apache2/sites-{enabled,available}/"$f" + else + sudo mv "$TMP" "$APPEND_APACHECONF" + fi +} + FAILS=0 trap CleanupExit INT for f in *.conf ; do @@ -41,11 +49,7 @@ for f in *.conf ; do echo FAILS=`expr $FAILS + 1` fi - if [ "$APPEND_APACHECONF" = "" ] ; then - sudo rm /etc/apache2/sites-{enabled,available}/"$f" - else - sudo mv "$TMP" "$APPEND_APACHECONF" - fi + Cleanup done if [ "$FAILS" -ne 0 ] ; then exit 1 diff --git a/tests/apache-conf-files/passing/README.modules b/tests/apache-conf-files/passing/README.modules index 7edbd3e84..32c3ef019 100644 --- a/tests/apache-conf-files/passing/README.modules +++ b/tests/apache-conf-files/passing/README.modules @@ -1,5 +1,4 @@ -Modules required to parse these conf files: - +# Modules required to parse these conf files: ssl rewrite macro From babb33991bffb51fd82acc75afa87ce4774168c4 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Dec 2015 19:51:45 -0800 Subject: [PATCH 40/90] Neaten things with a Setup() function --- tests/apache-conf-files/hackish-apache-test | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/apache-conf-files/hackish-apache-test b/tests/apache-conf-files/hackish-apache-test index 3d4336579..b8caaadc0 100755 --- a/tests/apache-conf-files/hackish-apache-test +++ b/tests/apache-conf-files/hackish-apache-test @@ -19,6 +19,17 @@ function CleanupExit() { exit 1 } +function Setup() { + if [ "$APPEND_APACHECONF" = "" ] ; then + sudo cp "$f" "$EA"/sites-available/ + sudo ln -sf "$EA/sites-available/$f" "$EA/sites-enabled/$f" + else + TMP="/tmp/`basename \"$APPEND_APACHECONF\"`.$$" + sudo cp -a "$APPEND_APACHECONF" "$TMP" + sudo bash -c "cat \"$f\" >> \"$APPEND_APACHECONF\"" + fi +} + function Cleanup() { if [ "$APPEND_APACHECONF" = "" ] ; then sudo rm /etc/apache2/sites-{enabled,available}/"$f" @@ -31,14 +42,7 @@ FAILS=0 trap CleanupExit INT for f in *.conf ; do echo -n testing "$f"... - if [ "$APPEND_APACHECONF" = "" ] ; then - sudo cp "$f" "$EA"/sites-available/ - sudo ln -sf "$EA/sites-available/$f" "$EA/sites-enabled/$f" - else - TMP="/tmp/`basename \"$APPEND_APACHECONF\"`.$$" - sudo cp -a "$APPEND_APACHECONF" "$TMP" - sudo bash -c "cat \"$f\" >> \"$APPEND_APACHECONF\"" - fi + Setup RESULT=`echo c | sudo "$LETSENCRYPT" --staging --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` if echo $RESULT | grep -Eq \("Please specify --domains"\|"mod_macro is not yet"\) ; then echo passed From 9e8f3cc644e7deb388361dc1f3e7430880a0609a Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 21 Dec 2015 22:52:32 +0200 Subject: [PATCH 41/90] Add gentoo fingerprint --- letsencrypt-apache/letsencrypt_apache/constants.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 989106e39..8f3c6a3f6 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -42,7 +42,8 @@ CLI_DEFAULTS = { "centos": CLI_DEFAULTS_CENTOS, "centos linux": CLI_DEFAULTS_CENTOS, "fedora": CLI_DEFAULTS_CENTOS, - "red hat enterprise linux server": CLI_DEFAULTS_CENTOS + "red hat enterprise linux server": CLI_DEFAULTS_CENTOS, + "gentoo base system": CLI_DEFAULTS_GENTOO } """CLI defaults.""" From 52167475dfb8f92371fd564f7c35d51651180d17 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 21 Dec 2015 23:51:22 +0200 Subject: [PATCH 42/90] Get pass version to parser and ignore CLI checks for define for 2.2 as it returns empty results anyway --- .../letsencrypt_apache/configurator.py | 14 ++++++++------ letsencrypt-apache/letsencrypt_apache/constants.py | 3 +++ letsencrypt-apache/letsencrypt_apache/parser.py | 5 +++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d67e7bc18..5c71ee1fe 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -156,11 +156,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Make sure configuration is valid self.config_test() - self.parser = parser.ApacheParser( - self.aug, self.conf("server-root"), self.conf("vhost-root"), self.conf("ctl")) - # Check for errors in parsing files with Augeas - self.check_parsing_errors("httpd.aug") - # Set Version if self.version is None: self.version = self.get_version() @@ -168,6 +163,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): raise errors.NotSupportedError( "Apache Version %s not supported.", str(self.version)) + self.parser = parser.ApacheParser( + self.aug, self.conf("server-root"), self.conf("vhost-root"), + self.conf("ctl"), self.version) + # Check for errors in parsing files with Augeas + self.check_parsing_errors("httpd.aug") + # Get all of the available vhosts self.vhosts = self.get_virtual_hosts() @@ -1277,7 +1278,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ try: - stdout, _ = le_util.run_script([self.conf("ctl"), "-v"]) + stdout, _ = le_util.run_script( + constants.os_constant("version_cmd").split(" ")) except errors.SubprocessError: raise errors.PluginError( "Unable to run %s -v" % self.conf("ctl")) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 8f3c6a3f6..410d9adb3 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -7,6 +7,7 @@ CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/sites-available", ctl="apache2ctl", + version_cmd='apache2ctl -v', enmod="a2enmod", dismod="a2dismod", le_vhost_ext="-le-ssl.conf", @@ -18,6 +19,7 @@ CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", vhost_root="/etc/httpd/conf.d", ctl="apachectl", + version_cmd="apachectl -v", enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", @@ -29,6 +31,7 @@ CLI_DEFAULTS_GENTOO = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/vhosts.d", ctl="apache2ctl", + version_cmd="/usr/sbin/apache2 -v", enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 0289c57d8..c9ab438ca 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -28,7 +28,7 @@ class ApacheParser(object): arg_var_interpreter = re.compile(r"\$\{[^ \}]*}") fnmatch_chars = set(["*", "?", "\\", "[", "]"]) - def __init__(self, aug, root, vhostroot, ctl): + def __init__(self, aug, root, vhostroot, ctl, version=(2, 4)): # Note: Order is important here. # This uses the binary, so it can be done first. @@ -36,7 +36,8 @@ class ApacheParser(object): # https://httpd.apache.org/docs/2.4/mod/core.html#ifdefine # This only handles invocation parameters and Define directives! self.variables = {} - self.update_runtime_variables(ctl) + if version >= (2, 4): + self.update_runtime_variables(ctl) self.aug = aug # Find configuration root and make sure augeas can parse it. From 03fdd03a878ce1451c6e766f32421e604ceb4f72 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Dec 2015 19:52:35 -0800 Subject: [PATCH 43/90] Experimentally try travis with the hackish-apache-test --- .travis.yml | 1 + tox.ini | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/.travis.yml b/.travis.yml index 8dde06ceb..2b37c1bef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,6 +22,7 @@ env: - TOXENV=py27 BOULDER_INTEGRATION=1 - TOXENV=lint - TOXENV=cover + - TOXENV=hackishapachetest # Only build pushes to the master branch, PRs, and branches beginning with diff --git a/tox.ini b/tox.ini index d1fafe20f..cffbc4e18 100644 --- a/tox.ini +++ b/tox.ini @@ -67,3 +67,10 @@ commands = pylint --rcfile=.pylintrc letsencrypt-nginx/letsencrypt_nginx pylint --rcfile=.pylintrc letsencrypt-compatibility-test/letsencrypt_compatibility_test pylint --rcfile=.pylintrc letshelp-letsencrypt/letshelp_letsencrypt + +[testenv:hackishapachetest] +basepython = python2.7 +setenv = + LETSENCRYPT=letsencrypt +commands = + ./tests/apache-conf-files/hackish-apache-test From 9129dcbc8797f67d85d04fa29541b5cec446e2ce Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Dec 2015 20:04:27 -0800 Subject: [PATCH 44/90] Make sure there's always a domain name to prompt a question --- tests/apache-conf-files/hackish-apache-test | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/apache-conf-files/hackish-apache-test b/tests/apache-conf-files/hackish-apache-test index b8caaadc0..27ee0fdf0 100755 --- a/tests/apache-conf-files/hackish-apache-test +++ b/tests/apache-conf-files/hackish-apache-test @@ -23,6 +23,13 @@ function Setup() { if [ "$APPEND_APACHECONF" = "" ] ; then sudo cp "$f" "$EA"/sites-available/ sudo ln -sf "$EA/sites-available/$f" "$EA/sites-enabled/$f" + sudo echo """ + + ServerName example.com + DocumentRoot /tmp/ + ErrorLog /tmp/error.log + CustomLog /tmp/requests.log combined +""" >> $EA/sites-available/throwaway-example.conf else TMP="/tmp/`basename \"$APPEND_APACHECONF\"`.$$" sudo cp -a "$APPEND_APACHECONF" "$TMP" @@ -33,6 +40,7 @@ function Setup() { function Cleanup() { if [ "$APPEND_APACHECONF" = "" ] ; then sudo rm /etc/apache2/sites-{enabled,available}/"$f" + sudo rm $EA/sites-available/throwaway-example.conf else sudo mv "$TMP" "$APPEND_APACHECONF" fi From d777e7fabad3f379c266382a36f970a13e85cf12 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 17 Dec 2015 15:45:47 -0800 Subject: [PATCH 45/90] This sort of works in tox; travis is unlikely due to sudo --- .travis.yml | 1 + .../letsencrypt_apache/tests}/apache-conf-files/NEEDED.txt | 0 .../tests}/apache-conf-files/failing/ipv6-1143.conf | 0 .../tests}/apache-conf-files/failing/ipv6-1143b.conf | 0 .../apache-conf-files/failing/missing-double-quote-1724.conf | 0 .../tests}/apache-conf-files/failing/multivhost-1093.conf | 0 .../tests}/apache-conf-files/failing/multivhost-1093b.conf | 0 .../tests}/apache-conf-files/hackish-apache-test | 2 +- .../tests}/apache-conf-files/passing/1626-1531.conf | 0 .../tests}/apache-conf-files/passing/README.modules | 0 .../tests}/apache-conf-files/passing/anarcat-1531.conf | 0 .../passing/drupal-errordocument-arg-1724.conf | 0 .../tests}/apache-conf-files/passing/drupal-htaccess-1531.conf | 0 .../tests}/apache-conf-files/passing/example-1755.conf | 0 .../tests}/apache-conf-files/passing/example-ssl.conf | 0 .../tests}/apache-conf-files/passing/example.conf | 0 .../apache-conf-files/passing/finalize-1243.apache2.conf.txt | 0 .../tests}/apache-conf-files/passing/finalize-1243.conf | 0 .../tests}/apache-conf-files/passing/missing-quote-1724.conf | 0 .../tests}/apache-conf-files/passing/modmacro-1385.conf | 0 .../tests}/apache-conf-files/passing/owncloud-1264.conf | 0 .../tests}/apache-conf-files/passing/roundcube-1222.conf | 0 .../tests}/apache-conf-files/passing/semacode-1598.conf | 0 .../apache-conf-files/passing/sslrequire-wordlist-1827.htaccess | 0 .../apache-conf-files/passing/two-blocks-one-line-1693.conf | 0 tox.ini | 2 +- 26 files changed, 3 insertions(+), 2 deletions(-) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/NEEDED.txt (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/failing/ipv6-1143.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/failing/ipv6-1143b.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/failing/missing-double-quote-1724.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/failing/multivhost-1093.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/failing/multivhost-1093b.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/hackish-apache-test (97%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/1626-1531.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/README.modules (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/anarcat-1531.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/drupal-errordocument-arg-1724.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/drupal-htaccess-1531.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/example-1755.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/example-ssl.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/example.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/finalize-1243.apache2.conf.txt (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/finalize-1243.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/missing-quote-1724.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/modmacro-1385.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/owncloud-1264.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/roundcube-1222.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/semacode-1598.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/sslrequire-wordlist-1827.htaccess (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/two-blocks-one-line-1693.conf (100%) diff --git a/.travis.yml b/.travis.yml index 2b37c1bef..dfbc09e2e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ language: python services: - rabbitmq - mariadb + - apache2 # 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 diff --git a/tests/apache-conf-files/NEEDED.txt b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/NEEDED.txt similarity index 100% rename from tests/apache-conf-files/NEEDED.txt rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/NEEDED.txt diff --git a/tests/apache-conf-files/failing/ipv6-1143.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/ipv6-1143.conf similarity index 100% rename from tests/apache-conf-files/failing/ipv6-1143.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/ipv6-1143.conf diff --git a/tests/apache-conf-files/failing/ipv6-1143b.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/ipv6-1143b.conf similarity index 100% rename from tests/apache-conf-files/failing/ipv6-1143b.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/ipv6-1143b.conf diff --git a/tests/apache-conf-files/failing/missing-double-quote-1724.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/missing-double-quote-1724.conf similarity index 100% rename from tests/apache-conf-files/failing/missing-double-quote-1724.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/missing-double-quote-1724.conf diff --git a/tests/apache-conf-files/failing/multivhost-1093.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/multivhost-1093.conf similarity index 100% rename from tests/apache-conf-files/failing/multivhost-1093.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/multivhost-1093.conf diff --git a/tests/apache-conf-files/failing/multivhost-1093b.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/multivhost-1093b.conf similarity index 100% rename from tests/apache-conf-files/failing/multivhost-1093b.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/failing/multivhost-1093b.conf diff --git a/tests/apache-conf-files/hackish-apache-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test similarity index 97% rename from tests/apache-conf-files/hackish-apache-test rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test index 27ee0fdf0..9e828bf2d 100755 --- a/tests/apache-conf-files/hackish-apache-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test @@ -7,7 +7,7 @@ # assess, but it should be automated export EA=/etc/apache2/ TESTDIR="`dirname $0`" -LEROOT="`realpath \"$TESTDIR/../../\"`" +LEROOT="`realpath \"$TESTDIR/../../../../\"`" cd $TESTDIR/passing LETSENCRYPT="${LETSENCRYPT:-$LEROOT/venv/bin/letsencrypt}" diff --git a/tests/apache-conf-files/passing/1626-1531.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/1626-1531.conf similarity index 100% rename from tests/apache-conf-files/passing/1626-1531.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/1626-1531.conf diff --git a/tests/apache-conf-files/passing/README.modules b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/README.modules similarity index 100% rename from tests/apache-conf-files/passing/README.modules rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/README.modules diff --git a/tests/apache-conf-files/passing/anarcat-1531.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/anarcat-1531.conf similarity index 100% rename from tests/apache-conf-files/passing/anarcat-1531.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/anarcat-1531.conf diff --git a/tests/apache-conf-files/passing/drupal-errordocument-arg-1724.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/drupal-errordocument-arg-1724.conf similarity index 100% rename from tests/apache-conf-files/passing/drupal-errordocument-arg-1724.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/drupal-errordocument-arg-1724.conf diff --git a/tests/apache-conf-files/passing/drupal-htaccess-1531.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/drupal-htaccess-1531.conf similarity index 100% rename from tests/apache-conf-files/passing/drupal-htaccess-1531.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/drupal-htaccess-1531.conf diff --git a/tests/apache-conf-files/passing/example-1755.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/example-1755.conf similarity index 100% rename from tests/apache-conf-files/passing/example-1755.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/example-1755.conf diff --git a/tests/apache-conf-files/passing/example-ssl.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/example-ssl.conf similarity index 100% rename from tests/apache-conf-files/passing/example-ssl.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/example-ssl.conf diff --git a/tests/apache-conf-files/passing/example.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/example.conf similarity index 100% rename from tests/apache-conf-files/passing/example.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/example.conf diff --git a/tests/apache-conf-files/passing/finalize-1243.apache2.conf.txt b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/finalize-1243.apache2.conf.txt similarity index 100% rename from tests/apache-conf-files/passing/finalize-1243.apache2.conf.txt rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/finalize-1243.apache2.conf.txt diff --git a/tests/apache-conf-files/passing/finalize-1243.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/finalize-1243.conf similarity index 100% rename from tests/apache-conf-files/passing/finalize-1243.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/finalize-1243.conf diff --git a/tests/apache-conf-files/passing/missing-quote-1724.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/missing-quote-1724.conf similarity index 100% rename from tests/apache-conf-files/passing/missing-quote-1724.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/missing-quote-1724.conf diff --git a/tests/apache-conf-files/passing/modmacro-1385.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/modmacro-1385.conf similarity index 100% rename from tests/apache-conf-files/passing/modmacro-1385.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/modmacro-1385.conf diff --git a/tests/apache-conf-files/passing/owncloud-1264.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/owncloud-1264.conf similarity index 100% rename from tests/apache-conf-files/passing/owncloud-1264.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/owncloud-1264.conf diff --git a/tests/apache-conf-files/passing/roundcube-1222.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/roundcube-1222.conf similarity index 100% rename from tests/apache-conf-files/passing/roundcube-1222.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/roundcube-1222.conf diff --git a/tests/apache-conf-files/passing/semacode-1598.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/semacode-1598.conf similarity index 100% rename from tests/apache-conf-files/passing/semacode-1598.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/semacode-1598.conf diff --git a/tests/apache-conf-files/passing/sslrequire-wordlist-1827.htaccess b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/sslrequire-wordlist-1827.htaccess similarity index 100% rename from tests/apache-conf-files/passing/sslrequire-wordlist-1827.htaccess rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/sslrequire-wordlist-1827.htaccess diff --git a/tests/apache-conf-files/passing/two-blocks-one-line-1693.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/two-blocks-one-line-1693.conf similarity index 100% rename from tests/apache-conf-files/passing/two-blocks-one-line-1693.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/two-blocks-one-line-1693.conf diff --git a/tox.ini b/tox.ini index cffbc4e18..1d2f12e6c 100644 --- a/tox.ini +++ b/tox.ini @@ -73,4 +73,4 @@ basepython = python2.7 setenv = LETSENCRYPT=letsencrypt commands = - ./tests/apache-conf-files/hackish-apache-test + sudo ./letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test From a819ee146d6369ab08fe022ae3f7481e50d32ed9 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 17 Dec 2015 18:03:34 -0800 Subject: [PATCH 46/90] Experimentally try sudo in travis This may decontainerise us, so we might not want to merge even if it works... --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index dfbc09e2e..a0ca6576f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,7 +35,7 @@ branches: - /^test-.*$/ # container-based infrastructure -sudo: false +sudo: true addons: # make sure simplehttp simple verification works (custom /etc/hosts) From 7a16e2e2489bceb6cb88e21e0fa0d75900572250 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 18 Dec 2015 00:17:22 -0800 Subject: [PATCH 47/90] Wrangle things to actually run in travis --- .travis.yml | 11 +++++++---- .../tests/apache-conf-files/hackish-apache-test | 1 + tox.ini | 5 +++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index a0ca6576f..b1fe7f55d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,11 +19,11 @@ env: - GOPATH=/tmp/go - PATH=$GOPATH/bin:$PATH matrix: - - TOXENV=py26 BOULDER_INTEGRATION=1 - - TOXENV=py27 BOULDER_INTEGRATION=1 - - TOXENV=lint - - TOXENV=cover - TOXENV=hackishapachetest +# - TOXENV=py26 BOULDER_INTEGRATION=1 +# - TOXENV=py27 BOULDER_INTEGRATION=1 +# - TOXENV=lint +# - TOXENV=cover # Only build pushes to the master branch, PRs, and branches beginning with @@ -60,6 +60,9 @@ addons: - openssl # For Boulder integration testing - rsyslog + # for hackishapachetest + - realpath + - apache2 install: "travis_retry pip install tox coveralls" script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/travis-integration.sh)' diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test index 9e828bf2d..664423d7a 100755 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test @@ -51,6 +51,7 @@ trap CleanupExit INT for f in *.conf ; do echo -n testing "$f"... Setup + echo running from $PWD RESULT=`echo c | sudo "$LETSENCRYPT" --staging --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` if echo $RESULT | grep -Eq \("Please specify --domains"\|"mod_macro is not yet"\) ; then echo passed diff --git a/tox.ini b/tox.ini index 1d2f12e6c..1a637777d 100644 --- a/tox.ini +++ b/tox.ini @@ -69,8 +69,9 @@ commands = pylint --rcfile=.pylintrc letshelp-letsencrypt/letshelp_letsencrypt [testenv:hackishapachetest] -basepython = python2.7 +#basepython = python2.7 setenv = - LETSENCRYPT=letsencrypt + LETSENCRYPT=/home/travis/build/letsencrypt/letsencrypt/.tox/hackishapachetest/bin/letsencrypt commands = + pip install -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx -e letsencrypt-compatibility-test -e letshelp-letsencrypt sudo ./letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test From 8d71b2d6c38aa062ef20a4cb10d7e9fb4cc345c3 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 18 Dec 2015 08:48:49 -0800 Subject: [PATCH 48/90] Install Apache modules in travis --- .travis.yml | 2 ++ .../tests/apache-conf-files/hackish-apache-test | 13 +++++++++++-- tox.ini | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index b1fe7f55d..6b0af69c3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -63,6 +63,8 @@ addons: # for hackishapachetest - realpath - apache2 + - libapache2-mod-wsgi + - sudo install: "travis_retry pip install tox coveralls" script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/travis-integration.sh)' diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test index 664423d7a..cf06b48af 100755 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test @@ -46,13 +46,22 @@ function Cleanup() { fi } +# if our environment asks us to enable modules, do our best! +if [ "$1" = --debian-modules ] ; then + sudo apt-get install -y libapache2-mod-wsgi + + for mod in ssl rewrite macro wsgi deflate ; do + sudo a2enmod $mod + done +fi + + FAILS=0 trap CleanupExit INT for f in *.conf ; do echo -n testing "$f"... Setup - echo running from $PWD - RESULT=`echo c | sudo "$LETSENCRYPT" --staging --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` + RESULT=`echo c | sudo "$LETSENCRYPT" -vvvv --debug --staging --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` if echo $RESULT | grep -Eq \("Please specify --domains"\|"mod_macro is not yet"\) ; then echo passed else diff --git a/tox.ini b/tox.ini index 1a637777d..abb934055 100644 --- a/tox.ini +++ b/tox.ini @@ -74,4 +74,4 @@ setenv = LETSENCRYPT=/home/travis/build/letsencrypt/letsencrypt/.tox/hackishapachetest/bin/letsencrypt commands = pip install -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx -e letsencrypt-compatibility-test -e letshelp-letsencrypt - sudo ./letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test + sudo ./letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test --debian-modules From a0e902d405e8cd4bd3b584847ec8636affc818e0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 18 Dec 2015 11:43:20 -0800 Subject: [PATCH 49/90] More module deps! --- .../tests/apache-conf-files/hackish-apache-test | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test index cf06b48af..bfe71cc51 100755 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test @@ -48,9 +48,9 @@ function Cleanup() { # if our environment asks us to enable modules, do our best! if [ "$1" = --debian-modules ] ; then - sudo apt-get install -y libapache2-mod-wsgi + sudo apt-get install -y libapache2-mod-{wsgi,macro} - for mod in ssl rewrite macro wsgi deflate ; do + for mod in ssl rewrite macro wsgi deflate userdir version ; do sudo a2enmod $mod done fi From 05e210d42ad5fe38a4b1b66a659e6dfe3d77e736 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 18 Dec 2015 11:44:52 -0800 Subject: [PATCH 50/90] Also add dependencies to travis.yml --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 6b0af69c3..ab1931be1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -64,6 +64,7 @@ addons: - realpath - apache2 - libapache2-mod-wsgi + - libapache2-mod-macro - sudo install: "travis_retry pip install tox coveralls" From 47f7e70b764c4b5c760256659260059afee14df2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 21 Dec 2015 13:49:46 -0800 Subject: [PATCH 51/90] This is a more correct test --- .../tests/apache-conf-files/hackish-apache-test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test index bfe71cc51..c661e6435 100755 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test @@ -62,7 +62,7 @@ for f in *.conf ; do echo -n testing "$f"... Setup RESULT=`echo c | sudo "$LETSENCRYPT" -vvvv --debug --staging --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1` - if echo $RESULT | grep -Eq \("Please specify --domains"\|"mod_macro is not yet"\) ; then + if echo $RESULT | grep -Eq \("Which names would you like"\|"mod_macro is not yet"\) ; then echo passed else echo failed From 7b05d573f2d08eff5f62490935ca69429b93b67f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 22 Dec 2015 10:32:18 +0200 Subject: [PATCH 52/90] Abstracted the -D DUMP_RUN_CFG command to use os specific one, defined in constants --- letsencrypt-apache/letsencrypt_apache/constants.py | 5 ++++- letsencrypt-apache/letsencrypt_apache/parser.py | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 712e7f240..a859f57d4 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -7,7 +7,8 @@ CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/sites-available", ctl="apache2ctl", - version_cmd='apache2ctl -v', + version_cmd="apache2ctl -v", + define_cmd="apache2ctl -t -D DUMP_RUN_CFG", enmod="a2enmod", dismod="a2dismod", le_vhost_ext="-le-ssl.conf", @@ -20,6 +21,7 @@ CLI_DEFAULTS_CENTOS = dict( vhost_root="/etc/httpd/conf.d", ctl="apachectl", version_cmd="apachectl -v", + define_cmd="apachectl -t -D DUMP_RUN_CFG", enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", @@ -32,6 +34,7 @@ CLI_DEFAULTS_GENTOO = dict( vhost_root="/etc/apache2/vhosts.d", ctl="apache2ctl", version_cmd="/usr/sbin/apache2 -v", + define_cmd="/usr/sbin/apache2 -t -D DUMP_RUN_CFG", enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index c9ab438ca..19eb566c4 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -8,6 +8,7 @@ import subprocess from letsencrypt import errors +from letsencrypt_apache import constants logger = logging.getLogger(__name__) @@ -108,7 +109,7 @@ class ApacheParser(object): for match in matches: if match.count("=") > 1: logger.error("Unexpected number of equal signs in " - "apache2ctl -D DUMP_RUN_CFG") + "runtime config dump.") raise errors.PluginError( "Error parsing Apache runtime variables") parts = match.partition("=") @@ -124,7 +125,7 @@ class ApacheParser(object): """ try: proc = subprocess.Popen( - [ctl, "-t", "-D", "DUMP_RUN_CFG"], + constants.os_constant("define_cmd").split(" "), stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout, stderr = proc.communicate() From e41ddd2cc7210563506bb5107841cfb76c27c6e2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 22 Dec 2015 15:50:48 -0800 Subject: [PATCH 53/90] Rename hackishapachetest -> apacheconftest Reenable other travis tests as well as this one --- .travis.yml | 10 +++++----- .../{hackish-apache-test => apache-conf-test} | 0 tox.ini | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) rename letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/{hackish-apache-test => apache-conf-test} (100%) diff --git a/.travis.yml b/.travis.yml index ab1931be1..9efb95165 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,11 +19,11 @@ env: - GOPATH=/tmp/go - PATH=$GOPATH/bin:$PATH matrix: - - TOXENV=hackishapachetest -# - TOXENV=py26 BOULDER_INTEGRATION=1 -# - TOXENV=py27 BOULDER_INTEGRATION=1 -# - TOXENV=lint -# - TOXENV=cover + - TOXENV=py26 BOULDER_INTEGRATION=1 + - TOXENV=py27 BOULDER_INTEGRATION=1 + - TOXENV=lint + - TOXENV=cover + - TOXENV=apacheconftest # Only build pushes to the master branch, PRs, and branches beginning with diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test diff --git a/tox.ini b/tox.ini index abb934055..aac7c15eb 100644 --- a/tox.ini +++ b/tox.ini @@ -68,10 +68,10 @@ commands = pylint --rcfile=.pylintrc letsencrypt-compatibility-test/letsencrypt_compatibility_test pylint --rcfile=.pylintrc letshelp-letsencrypt/letshelp_letsencrypt -[testenv:hackishapachetest] +[testenv:apacheconftest] #basepython = python2.7 setenv = - LETSENCRYPT=/home/travis/build/letsencrypt/letsencrypt/.tox/hackishapachetest/bin/letsencrypt + LETSENCRYPT=/home/travis/build/letsencrypt/letsencrypt/.tox/apacheconftest/bin/letsencrypt commands = pip install -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx -e letsencrypt-compatibility-test -e letshelp-letsencrypt - sudo ./letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/hackish-apache-test --debian-modules + sudo ./letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test --debian-modules From ef1973ae2888232a5b19d2c5dae4783506ccf83c Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 22 Dec 2015 15:58:09 -0800 Subject: [PATCH 54/90] Move recently included tests to the new apache-conf-test location --- .../tests}/apache-conf-files/passing/graphite-quote-1934.conf | 0 .../tests}/apache-conf-files/passing/rewrite-quote-1960.conf | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/graphite-quote-1934.conf (100%) rename {tests => letsencrypt-apache/letsencrypt_apache/tests}/apache-conf-files/passing/rewrite-quote-1960.conf (100%) diff --git a/tests/apache-conf-files/passing/graphite-quote-1934.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/graphite-quote-1934.conf similarity index 100% rename from tests/apache-conf-files/passing/graphite-quote-1934.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/graphite-quote-1934.conf diff --git a/tests/apache-conf-files/passing/rewrite-quote-1960.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/rewrite-quote-1960.conf similarity index 100% rename from tests/apache-conf-files/passing/rewrite-quote-1960.conf rename to letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/rewrite-quote-1960.conf From d9ea151fbb2c54e5dc390b23590857de7f02cf73 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 22 Dec 2015 16:04:30 -0800 Subject: [PATCH 55/90] Tweak the graphite config file so that apacheconftest passes again (At least on debian and other systems that have a www-data user...) --- .../tests/apache-conf-files/passing/graphite-quote-1934.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/graphite-quote-1934.conf b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/graphite-quote-1934.conf index 2a8734b43..f257dd9a8 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/graphite-quote-1934.conf +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/graphite-quote-1934.conf @@ -1,6 +1,6 @@ - WSGIDaemonProcess _graphite processes=5 threads=5 display-name='%{GROUP}' inactivity-timeout=120 user=_graphite group=_graphite + WSGIDaemonProcess _graphite processes=5 threads=5 display-name='%{GROUP}' inactivity-timeout=120 user=www-data group=www-data WSGIProcessGroup _graphite WSGIImportScript /usr/share/graphite-web/graphite.wsgi process-group=_graphite application-group=%{GLOBAL} WSGIScriptAlias / /usr/share/graphite-web/graphite.wsgi From 23e8d6c641dcb2d108583232e437f58d9cb1c6f4 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 22 Dec 2015 16:57:08 -0800 Subject: [PATCH 56/90] When testing apache2, don't use letsencrypt-auto --- scripts/test_apache2.sh | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/scripts/test_apache2.sh b/scripts/test_apache2.sh index a048a1ad0..6c8dec365 100755 --- a/scripts/test_apache2.sh +++ b/scripts/test_apache2.sh @@ -36,7 +36,20 @@ fi # run letsencrypt-apache2 via letsencrypt-auto cd letsencrypt -./letsencrypt-auto -v --debug --text --agree-dev-preview --agree-tos \ + +export SUDO=sudo +if [ -f /etc/debian_version ] ; then + echo "Bootstrapping dependencies for Debian-based OSes..." + $SUDO bootstrap/_deb_common.sh +elif [ -f /etc/redhat-release ] ; then + echo "Bootstrapping dependencies for RedHat-based OSes..." + $SUDO bootstrap/_rpm_common.sh +else + echo "Dont have bootstrapping for this OS!" + exit 1 +fi + +venv/bin/letsencrypt -v --debug --text --agree-dev-preview --agree-tos \ --renew-by-default --redirect --register-unsafely-without-email \ --domain $PUBLIC_HOSTNAME --server $BOULDER_URL if [ $? -ne 0 ] ; then From 237472c361a49caf951de730c85612d62a76fd1b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 22 Dec 2015 17:09:45 -0800 Subject: [PATCH 57/90] Do things in the correct and new-fashioned way --- scripts/test_apache2.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/test_apache2.sh b/scripts/test_apache2.sh index 6c8dec365..7507bb47b 100755 --- a/scripts/test_apache2.sh +++ b/scripts/test_apache2.sh @@ -58,7 +58,7 @@ fi if [ "$OS_TYPE" = "ubuntu" ] ; then export LETSENCRYPT="$HOME/.local/share/letsencrypt/bin/letsencrypt" - tests/apache-conf-files/hackish-apache-test --debian-modules + venv/bin/tox apacheconftest else echo Not running hackish apache tests on $OS_TYPE fi @@ -69,5 +69,5 @@ fi # return error if any of the subtests failed if [ "$FAIL" = 1 ] ; then - return 1 + exit 1 fi From 068504ddbf2d4943d4254e8bf0a8904cd347f7cd Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 22 Dec 2015 17:19:01 -0800 Subject: [PATCH 58/90] Correct comment --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 9efb95165..4e0849e3b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -60,7 +60,7 @@ addons: - openssl # For Boulder integration testing - rsyslog - # for hackishapachetest + # for apacheconftest - realpath - apache2 - libapache2-mod-wsgi From 91f53dc8dbf971bbc8624a5ae0677d164aeaa0b8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 22 Dec 2015 18:57:37 -0800 Subject: [PATCH 59/90] Fix various bugs --- multitester.py | 2 +- scripts/test_apache2.sh | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/multitester.py b/multitester.py index fb29ab9eb..a9b766913 100644 --- a/multitester.py +++ b/multitester.py @@ -75,7 +75,7 @@ parser.add_argument('--saveinstances', action='store_true', help="don't kill EC2 instances after run, useful for debugging") parser.add_argument('--alt_pip', - default='https://certainly.isnot.org/pip/', + default='', help="server from which to pull candidate release packages") cl_args = parser.parse_args() diff --git a/scripts/test_apache2.sh b/scripts/test_apache2.sh index 7507bb47b..583f1f911 100755 --- a/scripts/test_apache2.sh +++ b/scripts/test_apache2.sh @@ -49,7 +49,8 @@ else exit 1 fi -venv/bin/letsencrypt -v --debug --text --agree-dev-preview --agree-tos \ +bootstrap/dev/venv.sh +sudo venv/bin/letsencrypt -v --debug --text --agree-dev-preview --agree-tos \ --renew-by-default --redirect --register-unsafely-without-email \ --domain $PUBLIC_HOSTNAME --server $BOULDER_URL if [ $? -ne 0 ] ; then @@ -58,7 +59,7 @@ fi if [ "$OS_TYPE" = "ubuntu" ] ; then export LETSENCRYPT="$HOME/.local/share/letsencrypt/bin/letsencrypt" - venv/bin/tox apacheconftest + venv/bin/tox -e apacheconftest else echo Not running hackish apache tests on $OS_TYPE fi From 2366b29755159195ed941c880d54bd6711079158 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 23 Dec 2015 20:07:15 +0200 Subject: [PATCH 60/90] Fix the flow let #1961 and #1811 coexist --- .../letsencrypt_apache/configurator.py | 20 +++++++++++++++---- .../letsencrypt_apache/parser.py | 7 +++---- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 221baea2b..90f60da15 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -556,10 +556,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :param str port: Port to listen on """ - if "ssl_module" not in self.parser.modules: - self.enable_mod("ssl", temp=temp) - if self.version >= (2, 4) and "socache_shmcb_module" not in self.parser.modules: - self.enable_mod("socache_shmcb", temp=temp) + + self.prepare_https_modules(temp) # Check for Listen # Note: This could be made to also look for ip:443 combo listens = [self.parser.get_arg(x).split()[0] for x in self.parser.find_dir("Listen")] @@ -600,6 +598,20 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): ip, port, self.parser.loc["listen"]) listens.append("%s:%s" % (ip, port)) + def prepare_https_modules(self, temp): + """Helper method for prepare_server_https, taking care of enabling + needed modules + + :param boolean temp: If the change is temporary + """ + + if self.conf("handle-modules"): + if "ssl_module" not in self.parser.modules: + self.enable_mod("ssl", temp=temp) + if self.version >= (2, 4) and ("socache_shmcb_module" not in + self.parser.modules): + self.enable_mod("socache_shmcb", temp=temp) + def make_addrs_sni_ready(self, addrs): """Checks to see if the server is ready for SNI challenges. diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index bd21b3a5b..b1d604631 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -36,8 +36,8 @@ class ApacheParser(object): # https://httpd.apache.org/docs/2.4/mod/core.html#ifdefine # This only handles invocation parameters and Define directives! self.variables = {} - self.unparsable = False - self.update_runtime_variables(ctl) + if version >= (2, 4): + self.update_runtime_variables(ctl) self.aug = aug # Find configuration root and make sure augeas can parse it. @@ -63,7 +63,7 @@ class ApacheParser(object): self._parse_file(self.vhostroot + "/*.conf") #check to see if there were unparsed define statements - if self.unparsable: + if version < (2, 4): if self.find_dir("Define", exclude=False): raise errors.PluginError("Error parsing runtime variables") @@ -108,7 +108,6 @@ class ApacheParser(object): try: matches.remove("DUMP_RUN_CFG") except ValueError: - self.unparsable = True return for match in matches: From c29c6c96ae0c2ede9b29469203ed27b6d8518187 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 23 Dec 2015 20:08:44 +0200 Subject: [PATCH 61/90] Fix tests to complete the merge --- .../tests/configurator_test.py | 6 +++--- .../letsencrypt_apache/tests/parser_test.py | 17 +++++------------ 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 4e0eb6b86..218c085f9 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -27,7 +27,7 @@ class TwoVhost80Test(util.ApacheTest): super(TwoVhost80Test, self).setUp() self.config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir) + self.config_path, self.vhost_path, self.config_dir, self.work_dir) self.config = self.mock_deploy_cert(self.config) self.vh_truth = util.get_vh_truth( self.temp_dir, "debian_apache_2_4/two_vhost_80") @@ -311,7 +311,7 @@ class TwoVhost80Test(util.ApacheTest): def test_deploy_cert_newssl_no_fullchain(self): self.config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir, version=(2, 4, 16)) + self.config_path, self.vhost_path, self.config_dir, self.work_dir, version=(2, 4, 16)) self.config = self.mock_deploy_cert(self.config) self.config.parser.modules.add("ssl_module") @@ -325,7 +325,7 @@ class TwoVhost80Test(util.ApacheTest): def test_deploy_cert_old_apache_no_chain(self): self.config = util.get_apache_configurator( - self.config_path, self.config_dir, self.work_dir, version=(2, 4, 7)) + self.config_path, self.vhost_path, self.config_dir, self.work_dir, version=(2, 4, 7)) self.config = self.mock_deploy_cert(self.config) self.config.parser.modules.add("ssl_module") diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index ffbae107d..023b3990a 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -152,7 +152,6 @@ class BasicParserTest(util.ParserTest): def test_update_runtime_vars_bad_output(self, mock_cfg): mock_cfg.return_value = "Define: TLS=443=24" self.parser.update_runtime_variables("ctl") - self.assertTrue(self.parser.unparsable) mock_cfg.return_value = "Define: DUMP_RUN_CFG\nDefine: TLS=443=24" self.assertRaises( @@ -188,17 +187,11 @@ class ParserInitTest(util.ApacheTest): @mock.patch("letsencrypt_apache.parser.ApacheParser._get_runtime_cfg") def test_unparsable(self, mock_cfg): from letsencrypt_apache.parser import ApacheParser - def unparsable_true(self, arg): - """a helper to set the self unparsabale to true""" - print "side effect has passed in arg: %s", arg - self.unparsable = True - with mock.patch.object(ApacheParser, 'update_runtime_variables', autospec=True) as urv: - urv.side_effect = unparsable_true - mock_cfg.return_value = ('Define: TEST') - self.assertRaises( - errors.PluginError, - ApacheParser, self.aug, os.path.relpath(self.config_path), "ctl") - self.assertEquals(1, 1) + mock_cfg.return_value = ('Define: TEST') + self.assertRaises( + errors.PluginError, + ApacheParser, self.aug, os.path.relpath(self.config_path), + "/dummy/vhostpath", "ctl", version=(2, 2, 22)) def test_root_normalized(self): from letsencrypt_apache.parser import ApacheParser From c9b9b0edda27c3be3173fdcda0e98bcbd4995b3c Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 23 Dec 2015 10:31:31 -0800 Subject: [PATCH 62/90] add debug statement --- letsencrypt-apache/letsencrypt_apache/configurator.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index f72492ac2..1d86da066 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1271,6 +1271,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ self.config_test() + logger.debug(self.aug.view_config_changes(self)) self._reload() def _reload(self): From 70cc516ed817c02cc9fe4eef64d06acfd6735861 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Dec 2015 10:38:57 -0800 Subject: [PATCH 63/90] Avoid scrollback for investigating logs --- multitester.py | 1 + 1 file changed, 1 insertion(+) diff --git a/multitester.py b/multitester.py index a9b766913..5aca79b7a 100644 --- a/multitester.py +++ b/multitester.py @@ -478,6 +478,7 @@ for outq in outputs: results_file.close() if not cl_args.saveinstances: + print('Logs in ', LOGDIR) print('Terminating EC2 Instances and Cleaning Dangling EBS Volumes') boulder_server.terminate() terminate_and_clean(instances) From 6db40626196c790af20e5834f5b85432dd358cb8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Dec 2015 10:45:08 -0800 Subject: [PATCH 64/90] Split module installation into substeps - This may prevent failures if one thing is uninstallable --- .../tests/apache-conf-files/apache-conf-test | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test index c661e6435..38d5f05c7 100755 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test @@ -48,7 +48,8 @@ function Cleanup() { # if our environment asks us to enable modules, do our best! if [ "$1" = --debian-modules ] ; then - sudo apt-get install -y libapache2-mod-{wsgi,macro} + sudo apt-get install -y libapache2-mod-wsgi + sudo apt-get install -y libapache2-mod-macro for mod in ssl rewrite macro wsgi deflate userdir version ; do sudo a2enmod $mod From 494e6e77110c6af028df31ad270bfa70d1658ad2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Dec 2015 10:53:13 -0800 Subject: [PATCH 65/90] Remove TODO that has been done --- .../letsencrypt_apache/tests/apache-conf-files/apache-conf-test | 2 -- 1 file changed, 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test index 38d5f05c7..ec0041534 100755 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test @@ -3,8 +3,6 @@ # A hackish script to see if the client is behaving as expected # with each of the "passing" conf files. -# TODO presently this requires interaction and human judgement to -# assess, but it should be automated export EA=/etc/apache2/ TESTDIR="`dirname $0`" LEROOT="`realpath \"$TESTDIR/../../../../\"`" From 3dc3df4b345ee45d3b5d28a71fb1379d2e064446 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Dec 2015 10:58:28 -0800 Subject: [PATCH 66/90] Document the inclusion of apacheconftest in tox --- docs/contributing.rst | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index c71aefeec..6c70830b8 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -65,8 +65,14 @@ Testing The following tools are there to help you: -- ``tox`` starts a full set of tests. Please make sure you run it - before submitting a new pull request. +- ``tox`` starts a full set of tests. Please note that it includes + apacheconftest, which uses the system's Apache install to test config file + parsing, so it should only be run on systems that have an + experimental, non-production Apache2 install on them. ``tox -e + apacheconftest`` can be used to run those specific Apache conf tests. + +- ``tox -e py27``, ``tox -e py26`` etc, run unit tests for specific Python + versions. - ``tox -e cover`` checks the test coverage only. Calling the ``./tox.cover.sh`` script directly (or even ``./tox.cover.sh $pkg1 From 8b50274d8821becfab40b53bb379a7f350ea757b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Dec 2015 11:33:39 -0800 Subject: [PATCH 67/90] --hsts should not use includeSubDomains Fixes #1728 --- letsencrypt-apache/letsencrypt_apache/constants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index eb004b975..4944ded1f 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -33,7 +33,7 @@ REWRITE_HTTPS_ARGS_WITH_END = [ https vhost""" HSTS_ARGS = ["always", "set", "Strict-Transport-Security", - "\"max-age=31536000; includeSubDomains\""] + "\"max-age=31536000\""] """Apache header arguments for HSTS""" UIR_ARGS = ["always", "set", "Content-Security-Policy", From 263f6d64292181de38ee8e45918fb71de358977b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Dec 2015 12:26:22 -0800 Subject: [PATCH 68/90] We don't want to hardcode a letsencrypt-auto venv anymore --- scripts/test_apache2.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/test_apache2.sh b/scripts/test_apache2.sh index 583f1f911..3f646aef0 100755 --- a/scripts/test_apache2.sh +++ b/scripts/test_apache2.sh @@ -58,7 +58,6 @@ if [ $? -ne 0 ] ; then fi if [ "$OS_TYPE" = "ubuntu" ] ; then - export LETSENCRYPT="$HOME/.local/share/letsencrypt/bin/letsencrypt" venv/bin/tox -e apacheconftest else echo Not running hackish apache tests on $OS_TYPE From 8f844928b769fff68a8e2ea873c276a53831f657 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Dec 2015 14:42:26 -0800 Subject: [PATCH 69/90] Make sure we install realpath (some systems don't have it :/) --- scripts/test_apache2.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/test_apache2.sh b/scripts/test_apache2.sh index 3f646aef0..4032e2195 100755 --- a/scripts/test_apache2.sh +++ b/scripts/test_apache2.sh @@ -8,6 +8,7 @@ then CONFFILE=/etc/apache2/sites-available/000-default.conf sudo apt-get update sudo apt-get -y --no-upgrade install apache2 #curl + sudo apt-get -y install realpath # needed for test-apache-conf # For apache 2.4, set up ServerName sudo sed -i '/ServerName/ s/#ServerName/ServerName/' $CONFFILE sudo sed -i '/ServerName/ s/www.example.com/'$PUBLIC_HOSTNAME'/' $CONFFILE From 49f36f8071b7804b235e7b80be3bd7b8a9d2754f Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 23 Dec 2015 14:57:14 -0800 Subject: [PATCH 70/90] also debug the written conf file --- letsencrypt-apache/letsencrypt_apache/tls_sni_01.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py index 4284e240c..def3b18a6 100644 --- a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py +++ b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py @@ -104,8 +104,9 @@ class ApacheTlsSni01(common.TLSSNI01): self.configurator.reverter.register_file_creation( True, self.challenge_conf) - with open(self.challenge_conf, "w") as new_conf: + with open(self.challenge_conf, "rw") as new_conf: new_conf.write(config_text) + logger.debug(new_conf.read()) return addrs From ce9e3c1f94213d1c2158664436505e22e732e216 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Dec 2015 15:00:07 -0800 Subject: [PATCH 71/90] Some OSes don't enable the mime module by default? --- .../letsencrypt_apache/tests/apache-conf-files/apache-conf-test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test index ec0041534..4e0443bb7 100755 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test @@ -49,7 +49,7 @@ if [ "$1" = --debian-modules ] ; then sudo apt-get install -y libapache2-mod-wsgi sudo apt-get install -y libapache2-mod-macro - for mod in ssl rewrite macro wsgi deflate userdir version ; do + for mod in ssl rewrite macro wsgi deflate userdir version mime ; do sudo a2enmod $mod done fi From 6a026597f4e7961bbe8557f17c823a39cd718d00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20HUBSCHER?= Date: Thu, 24 Dec 2015 00:30:51 +0100 Subject: [PATCH 72/90] =?UTF-8?q?Move=20validator=20to=20compatibility-tes?= =?UTF-8?q?t=20=E2=80=94=20Refs=20#1997?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../test_driver.py | 3 +- .../validator.py | 0 .../validator_test.py | 35 ++++++++++--------- letsencrypt-compatibility-test/setup.py | 6 ++++ setup.py | 6 ---- 5 files changed, 27 insertions(+), 23 deletions(-) rename {letsencrypt => letsencrypt-compatibility-test/letsencrypt_compatibility_test}/validator.py (100%) rename {letsencrypt/tests => letsencrypt-compatibility-test/letsencrypt_compatibility_test}/validator_test.py (77%) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/test_driver.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/test_driver.py index 5765003b9..ee679bdb7 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/test_driver.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/test_driver.py @@ -15,11 +15,12 @@ from acme import crypto_util from acme import messages from letsencrypt import achallenges from letsencrypt import errors as le_errors -from letsencrypt import validator from letsencrypt.tests import acme_util from letsencrypt_compatibility_test import errors from letsencrypt_compatibility_test import util +from letsencrypt_compatibility_test import validator + from letsencrypt_compatibility_test.configurators.apache import apache24 diff --git a/letsencrypt/validator.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/validator.py similarity index 100% rename from letsencrypt/validator.py rename to letsencrypt-compatibility-test/letsencrypt_compatibility_test/validator.py diff --git a/letsencrypt/tests/validator_test.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/validator_test.py similarity index 77% rename from letsencrypt/tests/validator_test.py rename to letsencrypt-compatibility-test/letsencrypt_compatibility_test/validator_test.py index c7416dc46..3a3bbc4b2 100644 --- a/letsencrypt/tests/validator_test.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/validator_test.py @@ -1,4 +1,4 @@ -"""Tests for letsencrypt.validator.""" +"""Tests for letsencrypt_compatibility_test.validator.""" import requests import unittest @@ -6,28 +6,31 @@ import mock import OpenSSL from acme import errors as acme_errors -from letsencrypt import validator +from letsencrypt_compatibility_test import validator class ValidatorTest(unittest.TestCase): def setUp(self): self.validator = validator.Validator() - @mock.patch("letsencrypt.validator.crypto_util.probe_sni") + @mock.patch( + "letsencrypt_compatibility_test.validator.crypto_util.probe_sni") def test_certificate_success(self, mock_probe_sni): cert = OpenSSL.crypto.X509() mock_probe_sni.return_value = cert self.assertTrue(self.validator.certificate( cert, "test.com", "127.0.0.1")) - @mock.patch("letsencrypt.validator.crypto_util.probe_sni") + @mock.patch( + "letsencrypt_compatibility_test.validator.crypto_util.probe_sni") def test_certificate_error(self, mock_probe_sni): cert = OpenSSL.crypto.X509() mock_probe_sni.side_effect = [acme_errors.Error] self.assertFalse(self.validator.certificate( cert, "test.com", "127.0.0.1")) - @mock.patch("letsencrypt.validator.crypto_util.probe_sni") + @mock.patch( + "letsencrypt_compatibility_test.validator.crypto_util.probe_sni") def test_certificate_failure(self, mock_probe_sni): cert = OpenSSL.crypto.X509() cert.set_serial_number(1337) @@ -35,67 +38,67 @@ class ValidatorTest(unittest.TestCase): self.assertFalse(self.validator.certificate( cert, "test.com", "127.0.0.1")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_succesful_redirect(self, mock_get_request): mock_get_request.return_value = create_response( 301, {"location": "https://test.com"}) self.assertTrue(self.validator.redirect("test.com")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_redirect_with_headers(self, mock_get_request): mock_get_request.return_value = create_response( 301, {"location": "https://test.com"}) self.assertTrue(self.validator.redirect( "test.com", headers={"Host": "test.com"})) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_redirect_missing_location(self, mock_get_request): mock_get_request.return_value = create_response(301) self.assertFalse(self.validator.redirect("test.com")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_redirect_wrong_status_code(self, mock_get_request): mock_get_request.return_value = create_response( 201, {"location": "https://test.com"}) self.assertFalse(self.validator.redirect("test.com")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_redirect_wrong_redirect_code(self, mock_get_request): mock_get_request.return_value = create_response( 303, {"location": "https://test.com"}) self.assertFalse(self.validator.redirect("test.com")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_hsts_empty(self, mock_get_request): mock_get_request.return_value = create_response( headers={"strict-transport-security": ""}) self.assertFalse(self.validator.hsts("test.com")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_hsts_malformed(self, mock_get_request): mock_get_request.return_value = create_response( headers={"strict-transport-security": "sdfal"}) self.assertFalse(self.validator.hsts("test.com")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_hsts_bad_max_age(self, mock_get_request): mock_get_request.return_value = create_response( headers={"strict-transport-security": "max-age=not-an-int"}) self.assertFalse(self.validator.hsts("test.com")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_hsts_expire(self, mock_get_request): mock_get_request.return_value = create_response( headers={"strict-transport-security": "max-age=3600"}) self.assertFalse(self.validator.hsts("test.com")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_hsts(self, mock_get_request): mock_get_request.return_value = create_response( headers={"strict-transport-security": "max-age=31536000"}) self.assertTrue(self.validator.hsts("test.com")) - @mock.patch("letsencrypt.validator.requests.get") + @mock.patch("letsencrypt_compatibility_test.validator.requests.get") def test_hsts_include_subdomains(self, mock_get_request): mock_get_request.return_value = create_response( headers={"strict-transport-security": diff --git a/letsencrypt-compatibility-test/setup.py b/letsencrypt-compatibility-test/setup.py index eb7e23036..1ff9e7649 100644 --- a/letsencrypt-compatibility-test/setup.py +++ b/letsencrypt-compatibility-test/setup.py @@ -10,6 +10,7 @@ install_requires = [ 'letsencrypt=={0}'.format(version), 'letsencrypt-apache=={0}'.format(version), 'docker-py', + 'requests', 'zope.interface', ] @@ -18,6 +19,11 @@ if sys.version_info < (2, 7): else: install_requires.append('mock') +if sys.version_info < (2, 7, 9): + # For secure SSL connexion with Python 2.7 (InsecurePlatformWarning) + install_requires.append('ndg-httpsclient') + install_requires.append('pyasn1') + docs_extras = [ 'repoze.sphinx.autointerface', 'Sphinx>=1.0', # autodoc_member_order = 'bysource', autodoc_default_flags diff --git a/setup.py b/setup.py index ae36777ef..f95f672ff 100644 --- a/setup.py +++ b/setup.py @@ -40,7 +40,6 @@ install_requires = [ 'pyrfc3339', 'python2-pythondialog>=3.2.2rc1', # Debian squeeze support, cf. #280 'pytz', - 'requests', 'setuptools', # pkg_resources 'six', 'zope.component', @@ -61,11 +60,6 @@ else: 'mock', ]) -if sys.version_info < (2, 7, 9): - # For secure SSL connexion with Python 2.7 (InsecurePlatformWarning) - install_requires.append('ndg-httpsclient') - install_requires.append('pyasn1') - dev_extras = [ # Pin astroid==1.3.5, pylint==1.4.2 as a workaround for #289 'astroid==1.3.5', From ea2c86b9265bc05a750fcbfe999ed2f9ea918878 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 23 Dec 2015 16:08:33 -0800 Subject: [PATCH 73/90] fixed linting and added logger --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- letsencrypt-apache/letsencrypt_apache/tls_sni_01.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 1d86da066..1baa06128 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1271,7 +1271,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ self.config_test() - logger.debug(self.aug.view_config_changes(self)) + logger.debug(self.reverter.view_config_changes()) self._reload() def _reload(self): diff --git a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py index def3b18a6..a770804d1 100644 --- a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py +++ b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py @@ -1,12 +1,14 @@ """A class that performs TLS-SNI-01 challenges for Apache""" import os +import logging from letsencrypt.plugins import common from letsencrypt_apache import obj from letsencrypt_apache import parser +logger = logging.getLogger(__name__) class ApacheTlsSni01(common.TLSSNI01): """Class that performs TLS-SNI-01 challenges within the Apache configurator @@ -104,9 +106,9 @@ class ApacheTlsSni01(common.TLSSNI01): self.configurator.reverter.register_file_creation( True, self.challenge_conf) - with open(self.challenge_conf, "rw") as new_conf: + logger.debug("writing a config file with text: %s", config_text) + with open(self.challenge_conf, "w") as new_conf: new_conf.write(config_text) - logger.debug(new_conf.read()) return addrs From c728219bc970b4a5f738c22ee2092f6d12cc9548 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 25 Dec 2015 10:18:24 +0200 Subject: [PATCH 74/90] Changed define and version commands from string to list to avoid unneeded parsing later on --- .../letsencrypt_apache/configurator.py | 2 +- letsencrypt-apache/letsencrypt_apache/constants.py | 12 ++++++------ letsencrypt-apache/letsencrypt_apache/parser.py | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 90f60da15..6c6685257 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1343,7 +1343,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ try: stdout, _ = le_util.run_script( - constants.os_constant("version_cmd").split(" ")) + constants.os_constant("version_cmd")) except errors.SubprocessError: raise errors.PluginError( "Unable to run %s -v" % self.conf("ctl")) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index a859f57d4..6b248b6ad 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -7,8 +7,8 @@ CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/sites-available", ctl="apache2ctl", - version_cmd="apache2ctl -v", - define_cmd="apache2ctl -t -D DUMP_RUN_CFG", + version_cmd=['apache2ctl', '-v'], + define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'], enmod="a2enmod", dismod="a2dismod", le_vhost_ext="-le-ssl.conf", @@ -20,8 +20,8 @@ CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", vhost_root="/etc/httpd/conf.d", ctl="apachectl", - version_cmd="apachectl -v", - define_cmd="apachectl -t -D DUMP_RUN_CFG", + version_cmd=['apachectl', '-v'], + define_cmd=['apachectl', '-t', '-D', 'DUMP_RUN_CFG'], enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", @@ -33,8 +33,8 @@ CLI_DEFAULTS_GENTOO = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/vhosts.d", ctl="apache2ctl", - version_cmd="/usr/sbin/apache2 -v", - define_cmd="/usr/sbin/apache2 -t -D DUMP_RUN_CFG", + version_cmd=['/usr/sbin/apache2', '-v'], + define_cmd=['/usr/sbin/apache2', '-t', '-D', 'DUMP_RUN_CFG'], enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index b1d604631..12704c859 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -129,7 +129,7 @@ class ApacheParser(object): """ try: proc = subprocess.Popen( - constants.os_constant("define_cmd").split(" "), + constants.os_constant("define_cmd"), stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout, stderr = proc.communicate() From d6dcfa7b7fa4db97aacd0d3da5f271baa4a5b7e8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 25 Dec 2015 13:18:19 -0800 Subject: [PATCH 75/90] Revert "Issue 2002" --- letsencrypt-apache/letsencrypt_apache/configurator.py | 1 - letsencrypt-apache/letsencrypt_apache/tls_sni_01.py | 3 --- 2 files changed, 4 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 32da8b95d..39b8f2426 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1272,7 +1272,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ self.config_test() - logger.debug(self.reverter.view_config_changes()) self._reload() def _reload(self): diff --git a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py index a770804d1..4284e240c 100644 --- a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py +++ b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py @@ -1,14 +1,12 @@ """A class that performs TLS-SNI-01 challenges for Apache""" import os -import logging from letsencrypt.plugins import common from letsencrypt_apache import obj from letsencrypt_apache import parser -logger = logging.getLogger(__name__) class ApacheTlsSni01(common.TLSSNI01): """Class that performs TLS-SNI-01 challenges within the Apache configurator @@ -106,7 +104,6 @@ class ApacheTlsSni01(common.TLSSNI01): self.configurator.reverter.register_file_creation( True, self.challenge_conf) - logger.debug("writing a config file with text: %s", config_text) with open(self.challenge_conf, "w") as new_conf: new_conf.write(config_text) From f20cd73e8dc1e16ecff9d1161ec73076c05f836d Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 26 Dec 2015 20:13:45 -0800 Subject: [PATCH 76/90] For now, disable apacheconftest in travis :( --- .travis.yml | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4e0849e3b..a5d6d8a85 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,8 @@ language: python services: - rabbitmq - mariadb - - apache2 + # apacheconftest + #- apache2 # 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 @@ -23,7 +24,9 @@ env: - TOXENV=py27 BOULDER_INTEGRATION=1 - TOXENV=lint - TOXENV=cover - - TOXENV=apacheconftest +# Disabled for now due to requiring sudo -> causing more boulder integration +# DNS timeouts :( +# - TOXENV=apacheconftest # Only build pushes to the master branch, PRs, and branches beginning with @@ -35,7 +38,7 @@ branches: - /^test-.*$/ # container-based infrastructure -sudo: true +sudo: false addons: # make sure simplehttp simple verification works (custom /etc/hosts) @@ -61,11 +64,11 @@ addons: # For Boulder integration testing - rsyslog # for apacheconftest - - realpath - - apache2 - - libapache2-mod-wsgi - - libapache2-mod-macro - - sudo + #- realpath + #- apache2 + #- libapache2-mod-wsgi + #- libapache2-mod-macro + #- sudo install: "travis_retry pip install tox coveralls" script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/travis-integration.sh)' From 83812dc16a081ddc43d8f4204d61f0b7e73e8768 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 28 Dec 2015 12:56:44 +0200 Subject: [PATCH 77/90] Abstract config file matching to os based constants --- letsencrypt-apache/letsencrypt_apache/constants.py | 3 +++ letsencrypt-apache/letsencrypt_apache/parser.py | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index f8712c247..800959463 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -6,6 +6,7 @@ from letsencrypt import le_util CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/sites-available", + vhost_files="*", ctl="apache2ctl", version_cmd=['apache2ctl', '-v'], define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'], @@ -19,6 +20,7 @@ CLI_DEFAULTS_DEBIAN = dict( CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", vhost_root="/etc/httpd/conf.d", + vhost_files="*.conf", ctl="apachectl", version_cmd=['apachectl', '-v'], define_cmd=['apachectl', '-t', '-D', 'DUMP_RUN_CFG'], @@ -32,6 +34,7 @@ CLI_DEFAULTS_CENTOS = dict( CLI_DEFAULTS_GENTOO = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/vhosts.d", + vhost_files="*.conf", ctl="apache2ctl", version_cmd=['/usr/sbin/apache2', '-v'], define_cmd=['/usr/sbin/apache2', '-t', '-D', 'DUMP_RUN_CFG'], diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 12704c859..5f84e9f52 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -60,9 +60,10 @@ class ApacheParser(object): self.loc.update(self._set_locations()) # Must also attempt to parse virtual host root - self._parse_file(self.vhostroot + "/*.conf") + self._parse_file(self.vhostroot + "/" + + constants.os_constant("vhost_files")) - #check to see if there were unparsed define statements + # check to see if there were unparsed define statements if version < (2, 4): if self.find_dir("Define", exclude=False): raise errors.PluginError("Error parsing runtime variables") From e3358bb15346cbcedd229c37d072c05f2198cf4a Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 28 Dec 2015 13:47:14 +0200 Subject: [PATCH 78/90] Abstract the remaining commands to configurable ones --- .../letsencrypt_apache/configurator.py | 22 ++++++++----------- .../letsencrypt_apache/constants.py | 9 +++++--- .../letsencrypt_apache/parser.py | 16 ++++++++------ 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 6c6685257..5e2156edc 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -86,10 +86,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): @classmethod def add_parser_arguments(cls, add): - add("ctl", default=constants.os_constant("ctl"), - help="Path to the 'apache2ctl' binary, used for 'configtest', " - "retrieving the Apache2 version number, and initialization " - "parameters.") add("enmod", default=constants.os_constant("enmod"), help="Path to the Apache 'a2enmod' binary.") add("dismod", default=constants.os_constant("dismod"), @@ -148,10 +144,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # Verify Apache is installed - for exe in (self.conf("ctl"), self.conf("enmod"), self.conf("dismod")): - if exe is not None: - if not le_util.exe_exists(exe): - raise errors.NoInstallationError + for exe in constants.os_constant("restart_cmd")[0]: + if not le_util.exe_exists(exe): + raise errors.NoInstallationError # Make sure configuration is valid self.config_test() @@ -165,7 +160,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.parser = parser.ApacheParser( self.aug, self.conf("server-root"), self.conf("vhost-root"), - self.conf("ctl"), self.version) + self.version) # Check for errors in parsing files with Augeas self.check_parsing_errors("httpd.aug") @@ -1277,7 +1272,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Modules can enable additional config files. Variables may be defined # within these new configuration sections. # Reload is not necessary as DUMP_RUN_CFG uses latest config. - self.parser.update_runtime_variables(self.conf("ctl")) + self.parser.update_runtime_variables() def _add_parser_mod(self, mod_name): """Shortcut for updating parser modules.""" @@ -1315,7 +1310,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ try: - le_util.run_script([self.conf("ctl"), "graceful"]) + le_util.run_script(constants.os_constant("restart_cmd")) except errors.SubprocessError as err: raise errors.MisconfigurationError(str(err)) @@ -1326,7 +1321,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ try: - le_util.run_script([self.conf("ctl"), "configtest"]) + le_util.run_script(constants.os_constant("conftest_cmd")) except errors.SubprocessError as err: raise errors.MisconfigurationError(str(err)) @@ -1346,7 +1341,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): constants.os_constant("version_cmd")) except errors.SubprocessError: raise errors.PluginError( - "Unable to run %s -v" % self.conf("ctl")) + "Unable to run %s -v" % + constants.os_constant("version_cmd")) regex = re.compile(r"Apache/([0-9\.]*)", re.IGNORECASE) matches = regex.findall(stdout) diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 800959463..8ac88b197 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -7,9 +7,10 @@ CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/sites-available", vhost_files="*", - ctl="apache2ctl", version_cmd=['apache2ctl', '-v'], define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'], + restart_cmd=['apache2ctl', 'graceful'], + conftest_cmd=['apache2ctl', 'configtest'], enmod="a2enmod", dismod="a2dismod", le_vhost_ext="-le-ssl.conf", @@ -21,9 +22,10 @@ CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", vhost_root="/etc/httpd/conf.d", vhost_files="*.conf", - ctl="apachectl", version_cmd=['apachectl', '-v'], define_cmd=['apachectl', '-t', '-D', 'DUMP_RUN_CFG'], + restart_cmd=['apachectl', 'graceful'], + conftest_cmd=['apachectl', 'configtest'], enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", @@ -35,9 +37,10 @@ CLI_DEFAULTS_GENTOO = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/vhosts.d", vhost_files="*.conf", - ctl="apache2ctl", version_cmd=['/usr/sbin/apache2', '-v'], define_cmd=['/usr/sbin/apache2', '-t', '-D', 'DUMP_RUN_CFG'], + restart_cmd=['apache2ctl', 'graceful'], + conftest_cmd=['apache2ctl', 'configtest'], enmod=None, dismod=None, le_vhost_ext="-le-ssl.conf", diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 5f84e9f52..593c807cc 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -28,7 +28,7 @@ class ApacheParser(object): arg_var_interpreter = re.compile(r"\$\{[^ \}]*}") fnmatch_chars = set(["*", "?", "\\", "[", "]"]) - def __init__(self, aug, root, vhostroot, ctl, version=(2, 4)): + def __init__(self, aug, root, vhostroot, version=(2, 4)): # Note: Order is important here. # This uses the binary, so it can be done first. @@ -37,7 +37,7 @@ class ApacheParser(object): # This only handles invocation parameters and Define directives! self.variables = {} if version >= (2, 4): - self.update_runtime_variables(ctl) + self.update_runtime_variables() self.aug = aug # Find configuration root and make sure augeas can parse it. @@ -92,7 +92,7 @@ class ApacheParser(object): self.modules.add( os.path.basename(self.get_arg(match_filename))[:-2] + "c") - def update_runtime_variables(self, ctl): + def update_runtime_variables(self): """" .. note:: Compile time variables (apache2ctl -V) are not used within the @@ -102,7 +102,7 @@ class ApacheParser(object): .. todo:: Create separate compile time variables... simply for arg_get() """ - stdout = self._get_runtime_cfg(ctl) + stdout = self._get_runtime_cfg() variables = dict() matches = re.compile(r"Define: ([^ \n]*)").findall(stdout) @@ -122,7 +122,7 @@ class ApacheParser(object): self.variables = variables - def _get_runtime_cfg(self, ctl): # pylint: disable=no-self-use + def _get_runtime_cfg(self): # pylint: disable=no-self-use """Get runtime configuration info. :returns: stdout from DUMP_RUN_CFG @@ -137,9 +137,11 @@ class ApacheParser(object): except (OSError, ValueError): logger.error( - "Error accessing %s for runtime parameters!%s", ctl, os.linesep) + "Error running command %s for runtime parameters!%s", + constants.os_constant("define_cmd"), os.linesep) raise errors.MisconfigurationError( - "Error accessing loaded Apache parameters: %s", ctl) + "Error accessing loaded Apache parameters: %s", + constants.os_constant("define_cmd")) # Small errors that do not impede if proc.returncode != 0: logger.warn("Error in checking parameter list: %s", stderr) From ab069741f2c4cf32b911180e1a166c809a144c0a Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 28 Dec 2015 14:03:50 +0200 Subject: [PATCH 79/90] Fix tests --- .../tests/constants_test.py | 9 +++++--- .../letsencrypt_apache/tests/parser_test.py | 22 ++++++++++--------- .../letsencrypt_apache/tests/util.py | 2 +- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py b/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py index 63eb5c783..289b61bb1 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/constants_test.py @@ -11,14 +11,17 @@ class ConstantsTest(unittest.TestCase): @mock.patch("letsencrypt.le_util.get_os_info") def test_get_debian_value(self, os_info): os_info.return_value = ('Debian', '', '') - self.assertEqual(constants.os_constant("ctl"), "apache2ctl") + self.assertEqual(constants.os_constant("vhost_root"), + "/etc/apache2/sites-available") @mock.patch("letsencrypt.le_util.get_os_info") def test_get_centos_value(self, os_info): os_info.return_value = ('CentOS Linux', '', '') - self.assertEqual(constants.os_constant("ctl"), "apachectl") + self.assertEqual(constants.os_constant("vhost_root"), + "/etc/httpd/conf.d") @mock.patch("letsencrypt.le_util.get_os_info") def test_get_default_value(self, os_info): os_info.return_value = ('Nonexistent Linux', '', '') - self.assertEqual(constants.os_constant("ctl"), "apache2ctl") + self.assertEqual(constants.os_constant("vhost_root"), + "/etc/apache2/sites-available") diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index 023b3990a..b871f89b7 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -145,24 +145,26 @@ class BasicParserTest(util.ParserTest): expected_vars = {"TEST": "", "U_MICH": "", "TLS": "443", "example_path": "Documents/path"} - self.parser.update_runtime_variables("ctl") + self.parser.update_runtime_variables() self.assertEqual(self.parser.variables, expected_vars) @mock.patch("letsencrypt_apache.parser.ApacheParser._get_runtime_cfg") def test_update_runtime_vars_bad_output(self, mock_cfg): mock_cfg.return_value = "Define: TLS=443=24" - self.parser.update_runtime_variables("ctl") + self.parser.update_runtime_variables() mock_cfg.return_value = "Define: DUMP_RUN_CFG\nDefine: TLS=443=24" self.assertRaises( - errors.PluginError, self.parser.update_runtime_variables, "ctl") + errors.PluginError, self.parser.update_runtime_variables) + @mock.patch("letsencrypt_apache.constants.os_constant") @mock.patch("letsencrypt_apache.parser.subprocess.Popen") - def test_update_runtime_vars_bad_ctl(self, mock_popen): + def test_update_runtime_vars_bad_ctl(self, mock_popen, mock_const): mock_popen.side_effect = OSError + mock_const.return_value = "nonexistent" self.assertRaises( errors.MisconfigurationError, - self.parser.update_runtime_variables, "ctl") + self.parser.update_runtime_variables) @mock.patch("letsencrypt_apache.parser.subprocess.Popen") def test_update_runtime_vars_bad_exit(self, mock_popen): @@ -170,7 +172,7 @@ class BasicParserTest(util.ParserTest): mock_popen.returncode = -1 self.assertRaises( errors.MisconfigurationError, - self.parser.update_runtime_variables, "ctl") + self.parser.update_runtime_variables) class ParserInitTest(util.ApacheTest): @@ -191,7 +193,7 @@ class ParserInitTest(util.ApacheTest): self.assertRaises( errors.PluginError, ApacheParser, self.aug, os.path.relpath(self.config_path), - "/dummy/vhostpath", "ctl", version=(2, 2, 22)) + "/dummy/vhostpath", version=(2, 2, 22)) def test_root_normalized(self): from letsencrypt_apache.parser import ApacheParser @@ -203,7 +205,7 @@ class ParserInitTest(util.ApacheTest): "debian_apache_2_4/////two_vhost_80/../two_vhost_80/apache2") parser = ApacheParser(self.aug, path, - "/dummy/vhostpath", "dummy_ctl") + "/dummy/vhostpath") self.assertEqual(parser.root, self.config_path) @@ -213,7 +215,7 @@ class ParserInitTest(util.ApacheTest): "update_runtime_variables"): parser = ApacheParser( self.aug, os.path.relpath(self.config_path), - "/dummy/vhostpath", "dummy_ctl") + "/dummy/vhostpath") self.assertEqual(parser.root, self.config_path) @@ -223,7 +225,7 @@ class ParserInitTest(util.ApacheTest): "update_runtime_variables"): parser = ApacheParser( self.aug, self.config_path + os.path.sep, - "/dummy/vhostpath", "dummy_ctl") + "/dummy/vhostpath") self.assertEqual(parser.root, self.config_path) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index 95c95e6a9..798d4814b 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -58,7 +58,7 @@ class ParserTest(ApacheTest): # pytlint: disable=too-few-public-methods with mock.patch("letsencrypt_apache.parser.ApacheParser." "update_runtime_variables"): self.parser = ApacheParser( - self.aug, self.config_path, self.vhost_path, "dummy_ctl_path") + self.aug, self.config_path, self.vhost_path) def get_apache_configurator( From 3fadfb5444b38e65fb60507c2592eaeac5cdde6f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 28 Dec 2015 15:56:24 +0200 Subject: [PATCH 80/90] Fixed the find exe condition --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 5e2156edc..836d77135 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -144,9 +144,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # Verify Apache is installed - for exe in constants.os_constant("restart_cmd")[0]: - if not le_util.exe_exists(exe): - raise errors.NoInstallationError + if not le_util.exe_exists(constants.os_constant("restart_cmd")[0]): + raise errors.NoInstallationError # Make sure configuration is valid self.config_test() From fd4f6fb2eef3fd24d427836023918103bac08ada Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 29 Dec 2015 08:47:14 +0000 Subject: [PATCH 81/90] Use GH pages for IETF spec repo link --- acme/acme/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acme/acme/__init__.py b/acme/acme/__init__.py index 0f5f0e4bd..e8a0b16a8 100644 --- a/acme/acme/__init__.py +++ b/acme/acme/__init__.py @@ -3,10 +3,10 @@ This module is an implementation of the `ACME protocol`_. Latest supported version: `draft-ietf-acme-01`_. -.. _`ACME protocol`: https://github.com/ietf-wg-acme/acme/ + +.. _`ACME protocol`: https://ietf-wg-acme.github.io/acme .. _`draft-ietf-acme-01`: https://github.com/ietf-wg-acme/acme/tree/draft-ietf-acme-acme-01 - """ From 7788799a9bdb6a67f25833ccba031799b7b6429a Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 29 Dec 2015 08:55:13 +0000 Subject: [PATCH 82/90] Staging URI in dev-cli.ini example --- examples/cli.ini | 3 --- examples/dev-cli.ini | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/cli.ini b/examples/cli.ini index 6b6b05d7d..f0c993c57 100644 --- a/examples/cli.ini +++ b/examples/cli.ini @@ -5,9 +5,6 @@ # Use a 4096 bit RSA key instead of 2048 rsa-key-size = 4096 -# Always use the staging/testing server -server = https://acme-staging.api.letsencrypt.org/directory - # Uncomment and update to register with the specified e-mail address # email = foo@example.com diff --git a/examples/dev-cli.ini b/examples/dev-cli.ini index be703814a..c02038ca1 100644 --- a/examples/dev-cli.ini +++ b/examples/dev-cli.ini @@ -1,3 +1,6 @@ +# Always use the staging/testing server - avoids rate limiting +server = https://acme-staging.api.letsencrypt.org/directory + # This is an example configuration file for developers config-dir = /tmp/le/conf work-dir = /tmp/le/conf From 8f984bd24f2779490bf526d47f7fbe14633fcfb1 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 1 Jan 2016 16:35:57 -0800 Subject: [PATCH 83/90] Better Nginx error handling. Raise MisconfigurationError on restart failure, so we don't attempt to continue with an authorization we know will fail. Log at debug level the config files that are about to be written out, so it's easier to debug restart failures. Fix https://github.com/letsencrypt/letsencrypt/issues/942: Error out if adding a conflicting directive. Remove unnecessarily-inserted access_log and error_log directives. These were added to make integration testing easier but are no longer needed. Incidentally this makes the plugin work with some configs where it wouldn't have worked previously. Change the semantics of add_server_directives with replace=True so only the first instance of a given directive is replaced, not all of them. This works fine with the one place in the code that calls add_server_directives with replace=True, because all of the involved directives aren't allowed to be duplicated in a given block. Make add_http_directives do inserts into the structure itself, since its needs were significantly different than the more general add_server_directives. This also allows us to narrow the scope of the `block.insert(0, directive)` hack that we inserted to work around https://trac.nginx.org/nginx/ticket/810, since it's only necessary for http blocks. --- .../letsencrypt_nginx/configurator.py | 24 ++---- letsencrypt-nginx/letsencrypt_nginx/parser.py | 81 +++++++++++++------ .../tests/configurator_test.py | 63 ++++++++------- .../letsencrypt_nginx/tests/parser_test.py | 29 ++++--- .../tests/boulder-integration.conf.sh | 6 +- 5 files changed, 115 insertions(+), 88 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index aaaf43c5f..89b1145e7 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -311,17 +311,11 @@ class NginxConfigurator(common.Plugin): """ snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() ssl_block = [['listen', '{0} ssl'.format(self.config.tls_sni_01_port)], - # access and error logs necessary for integration - # testing (non-root) - ['access_log', os.path.join( - self.config.work_dir, 'access.log')], - ['error_log', os.path.join( - self.config.work_dir, 'error.log')], ['ssl_certificate', snakeoil_cert], ['ssl_certificate_key', snakeoil_key], ['include', self.parser.loc["ssl_options"]]] self.parser.add_server_directives( - vhost.filep, vhost.names, ssl_block) + vhost.filep, vhost.names, ssl_block, replace=False) vhost.ssl = True vhost.raw.extend(ssl_block) vhost.addrs.add(obj.Addr( @@ -384,7 +378,7 @@ class NginxConfigurator(common.Plugin): [['return', '301 https://$host$request_uri']] ]] self.parser.add_server_directives( - vhost.filep, vhost.names, redirect_block) + vhost.filep, vhost.names, redirect_block, replace=False) logger.info("Redirecting all traffic to ssl in %s", vhost.filep) ###################################### @@ -393,11 +387,10 @@ class NginxConfigurator(common.Plugin): def restart(self): """Restarts nginx server. - :returns: Success - :rtype: bool + :raises .errors.MisconfigurationError: If either the reload fails. """ - return nginx_restart(self.conf('ctl'), self.nginx_conf) + nginx_restart(self.conf('ctl'), self.nginx_conf) def config_test(self): # pylint: disable=no-self-use """Check the configuration of Nginx for errors. @@ -631,19 +624,16 @@ def nginx_restart(nginx_ctl, nginx_conf="/etc/nginx.conf"): if nginx_proc.returncode != 0: # Enter recovery routine... - logger.error("Nginx Restart Failed!\n%s\n%s", stdout, stderr) - return False + raise errors.MisconfigurationError( + "nginx restart failed:\n%s\n%s" % (stdout, stderr)) except (OSError, ValueError): - logger.fatal("Nginx Restart Failed - Please Check the Configuration") - sys.exit(1) + raise errors.MisconfigurationError("nginx restart failed") # Nginx can take a moment to recognize a newly added TLS SNI servername, so sleep # for a second. TODO: Check for expected servername and loop until it # appears or return an error if looping too long. time.sleep(1) - return True - def temp_install(options_ssl): """Temporary install for convenience.""" diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index 14db2f8b7..1d424f834 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -213,6 +213,7 @@ class NginxParser(object): if ext: filename = filename + os.path.extsep + ext try: + logger.debug('Dumping to %s:\n%s', filename, nginxparser.dumps(tree)) with open(filename, 'w') as _file: nginxparser.dump(tree, _file) except IOError: @@ -252,7 +253,7 @@ class NginxParser(object): return server_names == names def add_server_directives(self, filename, names, directives, - replace=False): + replace): """Add or replace directives in the first server block with names. ..note :: If replace is True, this raises a misconfiguration error @@ -269,20 +270,27 @@ class NginxParser(object): :param bool replace: Whether to only replace existing directives """ - _do_for_subarray(self.parsed[filename], - lambda x: self._has_server_names(x, names), - lambda x: _add_directives(x, directives, replace)) + try: + _do_for_subarray(self.parsed[filename], + lambda x: self._has_server_names(x, names), + lambda x: _add_directives(x, directives, replace)) + except errors.MisconfigurationError as err: + raise errors.MisconfigurationError("Problem in %s: %s" % (filename, err.message)) def add_http_directives(self, filename, directives): """Adds directives to the first encountered HTTP block in filename. + We insert new directives at the top of the block to work around + https://trac.nginx.org/nginx/ticket/810: If the first server block + doesn't enable OCSP stapling, stapling is broken for all blocks. + :param str filename: The absolute filename of the config file :param list directives: The directives to add """ _do_for_subarray(self.parsed[filename], lambda x: x[0] == ['http'], - lambda x: _add_directives(x[1], [directives], False)) + lambda x: x[1].insert(0, directives)) def get_all_certs_keys(self): """Gets all certs and keys in the nginx config. @@ -467,9 +475,14 @@ def _parse_server(server): return parsed_server -def _add_directives(block, directives, replace=False): - """Adds or replaces directives in a block. If the directive doesn't exist in - the entry already, raises a misconfiguration error. +def _add_directives(block, directives, replace): + """Adds or replaces directives in a config block. + + When replace=False, it's an error to try and add a directive that already + exists in the config block with a conflicting value. + + When replace=True, a directive with the same name MUST already exist in the + config block, and the first instance will be replaced. ..todo :: Find directives that are in included files. @@ -478,21 +491,39 @@ def _add_directives(block, directives, replace=False): """ for directive in directives: - if not replace: - # We insert new directives at the top of the block, mostly - # to work around https://trac.nginx.org/nginx/ticket/810 - # Only add directive if its not already in the block - if directive not in block: - block.insert(0, directive) - else: - changed = False - if len(directive) == 0: - continue - for index, line in enumerate(block): - if len(line) > 0 and line[0] == directive[0]: - block[index] = directive - changed = True - if not changed: + _add_directive(block, directive, replace) + +repeatable_directives = set(['server_name', 'listen', 'include']) + +def _add_directive(block, directive, replace): + """Adds or replaces a single directive in a config block. + + See _add_directives for more documentation. + + """ + location = -1 + # Find the index of a config line where the name of the directive matches + # the name of the directive we want to add. + for index, line in enumerate(block): + if len(line) > 0 and line[0] == directive[0]: + location = index + break + if replace: + if location == -1: + raise errors.MisconfigurationError( + 'expected directive for %s in the Nginx ' + 'config but did not find it.' % directive[0]) + block[location] = directive + else: + # Append directive. Fail if the name is not a repeatable directive name, + # and there is already a copy of that directive with a different value + # in the config file. + if location != -1 and directive[0].__str__() not in repeatable_directives: + if block[location][1] == directive[1]: + pass + else: raise errors.MisconfigurationError( - 'Let\'s Encrypt expected directive for %s in the Nginx ' - 'config but did not find it.' % directive[0]) + 'tried to insert directive "%s" but found conflicting "%s".' % ( + directive, block[location])) + else: + block.append(directive) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 56ad5110c..f6a4f7eee 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -65,16 +65,19 @@ class NginxConfiguratorTest(util.NginxTest): filep = self.config.parser.abs_path('sites-enabled/example.com') self.config.parser.add_server_directives( filep, set(['.example.com', 'example.*']), - [['listen', '5001 ssl']]) + [['listen', '5001 ssl']], + replace=False) self.config.save() # pylint: disable=protected-access parsed = self.config.parser._parse_files(filep, override=True) - self.assertEqual([[['server'], [['listen', '5001 ssl'], + self.assertEqual([[['server'], [ ['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], ['server_name', '.example.com'], - ['server_name', 'example.*']]]], + ['server_name', 'example.*'], + ['listen', '5001 ssl'] + ]]], parsed[0]) def test_choose_vhost(self): @@ -154,38 +157,36 @@ class NginxConfiguratorTest(util.NginxTest): parsed_server_conf = util.filter_comments(self.config.parser.parsed[server_conf]) parsed_nginx_conf = util.filter_comments(self.config.parser.parsed[nginx_conf]) - access_log = os.path.join(self.work_dir, "access.log") - error_log = os.path.join(self.work_dir, "error.log") self.assertEqual([[['server'], - [['include', self.config.parser.loc["ssl_options"]], - ['ssl_certificate_key', 'example/key.pem'], - ['ssl_certificate', 'example/fullchain.pem'], - ['error_log', error_log], - ['access_log', access_log], - - ['listen', '5001 ssl'], + [ ['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], ['server_name', '.example.com'], - ['server_name', 'example.*']]]], + ['server_name', 'example.*'], + + ['listen', '5001 ssl'], + ['ssl_certificate', 'example/fullchain.pem'], + ['ssl_certificate_key', 'example/key.pem'], + ['include', self.config.parser.loc["ssl_options"]] + ]]], parsed_example_conf) self.assertEqual([['server_name', 'somename alias another.alias']], parsed_server_conf) - self.assertTrue(util.contains_at_depth(parsed_nginx_conf, - [['server'], - [['include', self.config.parser.loc["ssl_options"]], - ['ssl_certificate_key', '/etc/nginx/key.pem'], - ['ssl_certificate', '/etc/nginx/fullchain.pem'], - ['error_log', error_log], - ['access_log', access_log], - ['listen', '5001 ssl'], - ['listen', '8000'], - ['listen', 'somename:8080'], - ['include', 'server.conf'], - [['location', '/'], - [['root', 'html'], - ['index', 'index.html index.htm']]]]], - 2)) + self.assertTrue(util.contains_at_depth( + parsed_nginx_conf, + [['server'], + [ + ['listen', '8000'], + ['listen', 'somename:8080'], + ['include', 'server.conf'], + [['location', '/'], + [['root', 'html'], + ['index', 'index.html index.htm']]], + ['listen', '5001 ssl'], + ['ssl_certificate', '/etc/nginx/fullchain.pem'], + ['ssl_certificate_key', '/etc/nginx/key.pem'], + ['include', self.config.parser.loc["ssl_options"]]]], + 2)) def test_get_all_certs_keys(self): nginx_conf = self.config.parser.abs_path('nginx.conf') @@ -297,19 +298,19 @@ class NginxConfiguratorTest(util.NginxTest): mocked = mock_popen() mocked.communicate.return_value = ('', '') mocked.returncode = 0 - self.assertTrue(self.config.restart()) + self.config.restart() @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen") def test_nginx_restart_fail(self, mock_popen): mocked = mock_popen() mocked.communicate.return_value = ('', '') mocked.returncode = 1 - self.assertFalse(self.config.restart()) + self.assertRaises(errors.MisconfigurationError, self.config.restart) @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen") def test_no_nginx_start(self, mock_popen): mock_popen.side_effect = OSError("Can't find program") - self.assertRaises(SystemExit, self.config.restart) + self.assertRaises(errors.MisconfigurationError, self.config.restart) @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen") def test_config_test(self, mock_popen): diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index 2d6156429..6559a5df6 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -127,7 +127,8 @@ class NginxParserTest(util.NginxTest): set(['localhost', r'~^(www\.)?(example|bar)\.']), [['foo', 'bar'], ['ssl_certificate', - '/etc/ssl/cert.pem']]) + '/etc/ssl/cert.pem']], + replace=False) ssl_re = re.compile(r'\n\s+ssl_certificate /etc/ssl/cert.pem') dump = nginxparser.dumps(nparser.parsed[nparser.abs_path('nginx.conf')]) self.assertEqual(1, len(re.findall(ssl_re, dump))) @@ -136,12 +137,15 @@ class NginxParserTest(util.NginxTest): names = set(['alias', 'another.alias', 'somename']) nparser.add_server_directives(server_conf, names, [['foo', 'bar'], ['ssl_certificate', - '/etc/ssl/cert2.pem']]) - nparser.add_server_directives(server_conf, names, [['foo', 'bar']]) + '/etc/ssl/cert2.pem']], + replace=False) + nparser.add_server_directives(server_conf, names, [['foo', 'bar']], + replace=False) self.assertEqual(nparser.parsed[server_conf], - [['ssl_certificate', '/etc/ssl/cert2.pem'], + [['server_name', 'somename alias another.alias'], ['foo', 'bar'], - ['server_name', 'somename alias another.alias']]) + ['ssl_certificate', '/etc/ssl/cert2.pem'] + ]) def test_add_http_directives(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) @@ -165,17 +169,19 @@ class NginxParserTest(util.NginxTest): target = set(['.example.com', 'example.*']) filep = nparser.abs_path('sites-enabled/example.com') nparser.add_server_directives( - filep, target, [['server_name', 'foo bar']], True) + filep, target, [['server_name', 'foobar.com']], replace=True) self.assertEqual( nparser.parsed[filep], [[['server'], [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], - ['server_name', 'foo bar'], - ['server_name', 'foo bar']]]]) + ['server_name', 'foobar.com'], + ['server_name', 'example.*'], + ]]]) self.assertRaises(errors.MisconfigurationError, nparser.add_server_directives, - filep, set(['foo', 'bar']), - [['ssl_certificate', 'cert.pem']], True) + filep, set(['foobar.com', 'example.*']), + [['ssl_certificate', 'cert.pem']], + replace=True) def test_get_best_match(self): target_name = 'www.eff.org' @@ -217,7 +223,8 @@ class NginxParserTest(util.NginxTest): set(['.example.com', 'example.*']), [['ssl_certificate', 'foo.pem'], ['ssl_certificate_key', 'bar.key'], - ['listen', '443 ssl']]) + ['listen', '443 ssl']], + replace=False) c_k = nparser.get_all_certs_keys() self.assertEqual(set([('foo.pem', 'bar.key', filep)]), c_k) diff --git a/letsencrypt-nginx/tests/boulder-integration.conf.sh b/letsencrypt-nginx/tests/boulder-integration.conf.sh index 12610d895..d77669a76 100755 --- a/letsencrypt-nginx/tests/boulder-integration.conf.sh +++ b/letsencrypt-nginx/tests/boulder-integration.conf.sh @@ -20,13 +20,14 @@ events { } http { - # Set an array of temp and cache file options that will otherwise default to + # Set an array of temp, cache and log file options that will otherwise default to # restricted locations accessible only to root. client_body_temp_path $root/client_body; fastcgi_temp_path $root/fastcgi_temp; proxy_temp_path $root/proxy_temp; #scgi_temp_path $root/scgi_temp; #uwsgi_temp_path $root/uwsgi_temp; + access_log $root/error.log; # This should be turned off in a Virtualbox VM, as it can cause some # interesting issues with data corruption in delivered files. @@ -54,9 +55,6 @@ http { root $root/webroot; - access_log $root/access.log; - error_log $root/error.log; - location / { # First attempt to serve request as file, then as directory, then fall # back to index.html. From dc3a2da9b11b26be54b035ac1520128e8efbfe22 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sun, 3 Jan 2016 10:49:50 -0500 Subject: [PATCH 84/90] Fixed a typo in a comment --- acme/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/setup.py b/acme/setup.py index ba2c88394..7314152cd 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -31,7 +31,7 @@ else: install_requires.append('mock') if sys.version_info < (2, 7, 9): - # For secure SSL connexion with Python 2.7 (InsecurePlatformWarning) + # For secure SSL connection with Python 2.7 (InsecurePlatformWarning) install_requires.append('ndg-httpsclient') install_requires.append('pyasn1') From 0454031cce4b88fef44e3e129e879a35b49c2314 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sun, 3 Jan 2016 14:37:08 -0500 Subject: [PATCH 85/90] Fixed a pair of typos in docstrings --- acme/acme/jose/json_util.py | 2 +- acme/acme/jose/util.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acme/acme/jose/json_util.py b/acme/acme/jose/json_util.py index 7b95e3fce..977a06622 100644 --- a/acme/acme/jose/json_util.py +++ b/acme/acme/jose/json_util.py @@ -226,7 +226,7 @@ class JSONObjectWithFields(util.ImmutableMap, interfaces.JSONDeSerializable): :param str name: Name of the field to be encoded. - :raises erors.SerializationError: if field cannot be serialized + :raises errors.SerializationError: if field cannot be serialized :raises errors.Error: if field could not be found """ diff --git a/acme/acme/jose/util.py b/acme/acme/jose/util.py index ab3606efc..600077b20 100644 --- a/acme/acme/jose/util.py +++ b/acme/acme/jose/util.py @@ -130,7 +130,7 @@ class ImmutableMap(collections.Mapping, collections.Hashable): """Immutable key to value mapping with attribute access.""" __slots__ = () - """Must be overriden in subclasses.""" + """Must be overridden in subclasses.""" def __init__(self, **kwargs): if set(kwargs) != set(self.__slots__): From e722a381978afe3faaeb265eefbfc45b9a83e35f Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 4 Jan 2016 11:18:13 -0800 Subject: [PATCH 86/90] Clarify parser check for duplicate values. --- letsencrypt-nginx/letsencrypt_nginx/parser.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index 1d424f834..c60d0102a 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -518,8 +518,12 @@ def _add_directive(block, directive, replace): # Append directive. Fail if the name is not a repeatable directive name, # and there is already a copy of that directive with a different value # in the config file. - if location != -1 and directive[0].__str__() not in repeatable_directives: - if block[location][1] == directive[1]: + directive_name = directive[0] + directive_value = directive[1] + if location != -1 and directive_name.__str__() not in repeatable_directives: + if block[location][1] == directive_value: + # There's a conflict, but the existing value matches the one we + # want to insert, so it's fine. pass else: raise errors.MisconfigurationError( From 4d3d6ff03157fe9ec77ba60e2fd59cca78ee8ce9 Mon Sep 17 00:00:00 2001 From: watercrossing Date: Tue, 5 Jan 2016 16:51:34 +0000 Subject: [PATCH 87/90] Fix for listen bug --- .../letsencrypt_apache/configurator.py | 2 ++ .../tests/configurator_test.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 836d77135..ff12055ac 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -558,6 +558,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # In case no Listens are set (which really is a broken apache config) if not listens: listens = ["80"] + if port in listens: + return for listen in listens: # For any listen statement, check if the machine also listens on Port 443. # If not, add such a listen statement. diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 218c085f9..9838b4f52 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -461,6 +461,25 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(mock_add_dir.call_args_list[1][0][2], ["[::1]:8080", "https"]) self.assertEqual(mock_add_dir.call_args_list[2][0][2], ["1.1.1.1:8080", "https"]) + def test_prepare_server_https_mixed_listen(self): + + mock_find = mock.Mock() + mock_find.return_value = ["test1", "test2"] + mock_get = mock.Mock() + mock_get.side_effect = ["1.2.3.4:8080", "443"] + mock_add_dir = mock.Mock() + mock_enable = mock.Mock() + + self.config.parser.find_dir = mock_find + self.config.parser.get_arg = mock_get + self.config.parser.add_dir_to_ifmodssl = mock_add_dir + self.config.enable_mod = mock_enable + + # Test Listen statements with specific ip listeed + self.config.prepare_server_https("443") + # Should only be 2 here, as the third interface already listens to the correct port + self.assertEqual(mock_add_dir.call_count, 0) + def test_make_vhost_ssl(self): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) From cf74446b58fa1229c6b2ad6bf5465f3b9ebe41ef Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Mon, 4 Jan 2016 23:04:18 -0500 Subject: [PATCH 88/90] Add test for redirect enhancement --- .../letsencrypt_nginx/tests/configurator_test.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 56ad5110c..5e237b04a 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -330,6 +330,17 @@ class NginxConfiguratorTest(util.NginxTest): OpenSSL.crypto.load_privatekey( OpenSSL.crypto.FILETYPE_PEM, key_file.read()) + def test_redirect_enhance(self): + expected = [ + ['if', '($scheme != "https")'], + [['return', '301 https://$host$request_uri']] + ] + + example_conf = self.config.parser.abs_path('sites-enabled/example.com') + self.config.enhance("www.example.com", "redirect") + + generated_conf = self.config.parser.parsed[example_conf] + self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) if __name__ == "__main__": unittest.main() # pragma: no cover From adcb7934aed5780ba96e9f5c861c467bff8f616d Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Mon, 4 Jan 2016 22:48:01 -0500 Subject: [PATCH 89/90] Improve choose_vhost() test by verifying the output file --- .../tests/configurator_test.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 5e237b04a..9a962a55f 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -91,12 +91,26 @@ class NginxConfiguratorTest(util.NginxTest): 'test.www.example.com': foo_conf, 'abc.www.foo.com': foo_conf, 'www.bar.co.uk': localhost_conf} + + conf_path = {'localhost': "etc_nginx/nginx.conf", + 'alias': "etc_nginx/nginx.conf", + 'example.com': "etc_nginx/sites-enabled/example.com", + 'example.com.uk.test': "etc_nginx/sites-enabled/example.com", + 'www.example.com': "etc_nginx/sites-enabled/example.com", + 'test.www.example.com': "etc_nginx/foo.conf", + 'abc.www.foo.com': "etc_nginx/foo.conf", + 'www.bar.co.uk': "etc_nginx/nginx.conf"} + bad_results = ['www.foo.com', 'example', 't.www.bar.co', '69.255.225.155'] for name in results: - self.assertEqual(results[name], - self.config.choose_vhost(name).names) + vhost = self.config.choose_vhost(name) + path = os.path.relpath(vhost.filep, self.temp_dir) + + self.assertEqual(results[name], vhost.names) + self.assertEqual(conf_path[name], path) + for name in bad_results: self.assertEqual(set([name]), self.config.choose_vhost(name).names) From 20829e05ed6a665a224fdef43e4f91cd5c002199 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Tue, 5 Jan 2016 15:15:29 -0500 Subject: [PATCH 90/90] Add missing test condition for prepare() --- .../tests/configurator_test.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 9a962a55f..3ad0b834f 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -40,6 +40,23 @@ class NginxConfiguratorTest(util.NginxTest): self.assertEquals((1, 6, 2), self.config.version) self.assertEquals(5, len(self.config.parser.parsed)) + @mock.patch("letsencrypt_nginx.configurator.le_util.exe_exists") + @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen") + def test_prepare_initializes_version(self, mock_popen, mock_exe_exists): + mock_popen().communicate.return_value = ( + "", "\n".join(["nginx version: nginx/1.6.2", + "built by clang 6.0 (clang-600.0.56)" + " (based on LLVM 3.5svn)", + "TLS SNI support enabled", + "configure arguments: --prefix=/usr/local/Cellar/" + "nginx/1.6.2 --with-http_ssl_module"])) + + mock_exe_exists.return_value = True + + self.config.version = None + self.config.prepare() + self.assertEquals((1, 6, 2), self.config.version) + @mock.patch("letsencrypt_nginx.configurator.socket.gethostbyaddr") def test_get_all_names(self, mock_gethostbyaddr): mock_gethostbyaddr.return_value = ('155.225.50.69.nephoscale.net', [], [])