diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 34eca4aa8..d36b897aa 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -302,9 +302,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.aug.set(path["cert_key"][-1], key_path) # Enable the new vhost if needed - if self.conf("handle-sites"): - if not vhost.enabled: - self.enable_site(vhost) + if not vhost.enabled: + self.enable_site(vhost) # Save notes about the transaction that took place self.save_notes += ("Changed vhost at %s with addresses of %s\n" @@ -594,28 +593,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if "/macro/" in path.lower(): macro = True - vhost_enabled = self.parsed_in_original(filename) + vhost_enabled = self.parser.parsed_in_original(filename) vhost = obj.VirtualHost(filename, path, addrs, is_ssl, vhost_enabled, modmacro=macro) self._add_servernames(vhost) return vhost - def add_include(self, main_config, inc_path): - """Add Include for a new configuration file if one does not exist - - :param str main_config: file path to main Apache config file - :param str inc_path: path of file to include - - """ - if len(self.parser.find_dir( - parser.case_i("Include"), inc_path)) == 0: - logger.debug("Adding Include %s to %s", - inc_path, parser.get_aug_path(main_config)) - self.parser.add_dir( - parser.get_aug_path(main_config), - "Include", inc_path) - def get_virtual_hosts(self): """Returns list of virtual hosts found in the Apache configuration. @@ -890,9 +874,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): raise errors.PluginError( "Could not reverse map the HTTPS VirtualHost to the original") - if not self.parsed_in_original(ssl_fp): - # Add direct include to root conf - self.add_include(self.parser.loc["default"], ssl_fp) # Update Addresses self._update_ssl_vhosts_addrs(vh_p) @@ -918,7 +899,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # This is for compliance with versions of Apache < 2.4 self._add_name_vhost_if_necessary(ssl_vhost) - return ssl_vhost def _get_new_vh_path(self, orig_matches, new_matches): @@ -1025,7 +1005,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): new_file.write("\n") # Add new file to augeas paths if we're supposed to handle # activation (it's not included as default) - if not self.parsed_in_current(ssl_fp): + if not self.parser.parsed_in_current(ssl_fp): self.parser.parse_file(ssl_fp) except IOError: logger.fatal("Error writing/reading to file in make_vhost_ssl") @@ -1680,7 +1660,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): redirect_file.write(text) # Add new include to configuration if it doesn't exist yet - if not self.parsed_in_current(redirect_filepath): + if not self.parser.parsed_in_current(redirect_filepath): self.parser.parse_file(redirect_filepath) logger.info("Created redirect file: %s", redirect_filename) @@ -1722,38 +1702,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return redirects - def parsed_in_current(self, filep): - """Checks if the file path is parsed by current Augeas parser config - ie. returns True if the file is found on a path that's found in live - Augeas configuration. - - :param str filep: Path to match - - :returns: True if file is parsed in existing configuration tree - :rtype: bool - """ - return self._parsed_by_parser_paths(filep, self.parser.parser_paths) - - def parsed_in_original(self, filep): - """Checks if the file path is parsed by existing Apache config. - ie. returns True if the file is found on a path that matches Include or - IncludeOptional statement in the Apache configuration. - - :param str filep: Path to match - - :returns: True if file is parsed in existing configuration tree - :rtype: bool - """ - return self._parsed_by_parser_paths(filep, self.parser.existing_paths) - - def _parsed_by_parser_paths(self, filep, paths): - """Helper function that searches through provided paths and returns - True if file path is found in the set""" - for directory in paths.keys(): - for filename in paths[directory]: - if fnmatch.fnmatch(filep, os.path.join(directory, filename)): - return True - return False def enable_site(self, vhost): """Enables an available site, Apache reload required. @@ -1776,6 +1724,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if vhost.enabled: return + # Handle non-debian systems + if not self.conf("handle-sites"): + if not self.parser.parsed_in_original(vhost.filep): + # Add direct include to root conf + self.parser.add_include(self.parser.loc["default"], vhost.filep) + return + enabled_path = ("%s/sites-enabled/%s" % (self.parser.root, os.path.basename(vhost.filep))) self.reverter.register_file_creation(False, enabled_path) diff --git a/certbot-apache/certbot_apache/parser.py b/certbot-apache/certbot_apache/parser.py index a42b72f05..db690f547 100644 --- a/certbot-apache/certbot_apache/parser.py +++ b/certbot-apache/certbot_apache/parser.py @@ -79,6 +79,30 @@ class ApacheParser(object): if self.find_dir("Define", exclude=False): raise errors.PluginError("Error parsing runtime variables") + def add_include(self, main_config, inc_path): + """Add Include for a new configuration file if one does not exist + + :param str main_config: file path to main Apache config file + :param str inc_path: path of file to include + + """ + if len(self.find_dir(case_i("Include"), inc_path)) == 0: + logger.debug("Adding Include %s to %s", + inc_path, get_aug_path(main_config)) + self.add_dir( + get_aug_path(main_config), + "Include", inc_path) + + # Add new path to parser paths + new_dir = os.path.dirname(inc_path) + new_file = os.path.basename(inc_path) + if new_dir in self.existing_paths.keys(): + # Add to existing path + self.existing_paths[new_dir].append(new_file) + else: + # Create a new path + self.existing_paths[new_dir] = [new_file] + def init_modules(self): """Iterates on the configuration until no new modules are loaded. @@ -506,6 +530,39 @@ class ApacheParser(object): self._add_httpd_transform(filepath) self.aug.load() + def parsed_in_current(self, filep): + """Checks if the file path is parsed by current Augeas parser config + ie. returns True if the file is found on a path that's found in live + Augeas configuration. + + :param str filep: Path to match + + :returns: True if file is parsed in existing configuration tree + :rtype: bool + """ + return self._parsed_by_parser_paths(filep, self.parser_paths) + + def parsed_in_original(self, filep): + """Checks if the file path is parsed by existing Apache config. + ie. returns True if the file is found on a path that matches Include or + IncludeOptional statement in the Apache configuration. + + :param str filep: Path to match + + :returns: True if file is parsed in existing configuration tree + :rtype: bool + """ + return self._parsed_by_parser_paths(filep, self.existing_paths) + + def _parsed_by_parser_paths(self, filep, paths): + """Helper function that searches through provided paths and returns + True if file path is found in the set""" + for directory in paths.keys(): + for filename in paths[directory]: + if fnmatch.fnmatch(filep, os.path.join(directory, filename)): + return True + return False + def _check_path_actions(self, filepath): """Determine actions to take with a new augeas path diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index ed7d3192f..dbb84b790 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -356,6 +356,30 @@ class MultipleVhostsTest(util.ApacheTest): self.config.enable_site, obj.VirtualHost("asdf", "afsaf", set(), False, False)) + def test_enable_site_nondebian(self): + mock_c = "certbot_apache.configurator.ApacheConfigurator.conf" + def conf_side_effect(arg): + """ Mock function for ApacheConfigurator.conf """ + confvars = {"handle-sites": False} + if arg in confvars: + return confvars[arg] + inc_path = "/path/to/whereever" + vhost = self.vh_truth[0] + with mock.patch(mock_c) as mock_conf: + mock_conf.side_effect = conf_side_effect + vhost.enabled = False + vhost.filep = inc_path + self.assertFalse(self.config.parser.find_dir("Include", inc_path)) + self.assertFalse( + os.path.dirname(inc_path) in self.config.parser.existing_paths) + self.config.enable_site(vhost) + self.assertTrue(self.config.parser.find_dir("Include", inc_path)) + self.assertTrue( + os.path.dirname(inc_path) in self.config.parser.existing_paths) + self.assertTrue( + os.path.basename(inc_path) in self.config.parser.existing_paths[ + os.path.dirname(inc_path)]) + def test_deploy_cert_enable_new_vhost(self): # Create ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) @@ -443,6 +467,39 @@ class MultipleVhostsTest(util.ApacheTest): "random.demo", "example/cert.pem", "example/key.pem")) + def test_deploy_cert_not_parsed_path(self): + # Make sure that we add include to root config for vhosts when + # handle-sites is false + self.config.parser.modules.add("ssl_module") + self.config.parser.modules.add("mod_ssl.c") + tmp_path = os.path.realpath(tempfile.mkdtemp("vhostroot")) + os.chmod(tmp_path, 0o755) + mock_p = "certbot_apache.configurator.ApacheConfigurator._get_ssl_vhost_path" + mock_a = "certbot_apache.parser.ApacheParser.add_include" + mock_c = "certbot_apache.configurator.ApacheConfigurator.conf" + orig_conf = self.config.conf + def conf_side_effect(arg): + """ Mock function for ApacheConfigurator.conf """ + confvars = {"handle-sites": False} + if arg in confvars: + return confvars[arg] + else: + return orig_conf("arg") + + with mock.patch(mock_c) as mock_conf: + mock_conf.side_effect = conf_side_effect + with mock.patch(mock_p) as mock_path: + mock_path.return_value = os.path.join(tmp_path, "whatever.conf") + with mock.patch(mock_a) as mock_add: + self.config.deploy_cert( + "encryption-example.demo", + "example/cert.pem", "example/key.pem", + "example/cert_chain.pem") + # Test that we actually called add_include + self.assertTrue(mock_add.called) + shutil.rmtree(tmp_path) + + def test_deploy_cert(self): self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") @@ -689,19 +746,6 @@ class MultipleVhostsTest(util.ApacheTest): os.path.dirname(os.path.realpath( self.vh_truth[1].filep))) - def test_make_vhost_ssl_nonparsed_path(self): - tmp_path = os.path.realpath(tempfile.mkdtemp("vhostroot")) - os.chmod(tmp_path, 0o755) - mock_p = "certbot_apache.configurator.ApacheConfigurator._get_ssl_vhost_path" - mock_a = "certbot_apache.configurator.ApacheConfigurator.add_include" - - with mock.patch(mock_p) as mock_path: - mock_path.return_value = os.path.join(tmp_path, "whatever.conf") - with mock.patch(mock_a) as mock_add: - self.config.make_vhost_ssl(self.vh_truth[1]) - self.assertTrue(mock_add.called) - shutil.rmtree(tmp_path) - def test_make_vhost_ssl(self): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) @@ -1444,6 +1488,7 @@ class MultiVhostsTest(util.ApacheTest): self.assertTrue(ssl_vhost.ssl) self.assertFalse(ssl_vhost.enabled) + self.assertEqual(self.config.is_name_vhost(self.vh_truth[1]), self.config.is_name_vhost(ssl_vhost)) diff --git a/certbot-apache/certbot_apache/tls_sni_01.py b/certbot-apache/certbot_apache/tls_sni_01.py index 01d360725..5ce96ac5f 100644 --- a/certbot-apache/certbot_apache/tls_sni_01.py +++ b/certbot-apache/certbot_apache/tls_sni_01.py @@ -104,8 +104,8 @@ class ApacheTlsSni01(common.TLSSNI01): config_text += "\n" - self.configurator.add_include(self.configurator.parser.loc["default"], - self.challenge_conf) + self.configurator.parser.add_include( + self.configurator.parser.loc["default"], self.challenge_conf) self.configurator.reverter.register_file_creation( True, self.challenge_conf)