From 5b4bf427b81af5094abdb8dd6123df750425274e Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 3 Dec 2015 12:22:28 +0200 Subject: [PATCH 01/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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 9e8f3cc644e7deb388361dc1f3e7430880a0609a Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 21 Dec 2015 22:52:32 +0200 Subject: [PATCH 35/40] 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 36/40] 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 7b05d573f2d08eff5f62490935ca69429b93b67f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 22 Dec 2015 10:32:18 +0200 Subject: [PATCH 37/40] 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 2366b29755159195ed941c880d54bd6711079158 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 23 Dec 2015 20:07:15 +0200 Subject: [PATCH 38/40] 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 39/40] 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 c728219bc970b4a5f738c22ee2092f6d12cc9548 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 25 Dec 2015 10:18:24 +0200 Subject: [PATCH 40/40] 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()