From 1d45b466a3fac24c6587406a131bad666e46ab3c Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 2 Feb 2015 21:11:59 +0000 Subject: [PATCH] IConfig.apache_server_root without trailing slash --- letsencrypt/client/apache/configurator.py | 13 +++---- letsencrypt/client/apache/dvsni.py | 7 ++-- letsencrypt/client/apache/parser.py | 25 ++++++++----- .../client/tests/apache/parser_test.py | 37 ++++++++++++++----- letsencrypt/client/tests/apache/util.py | 3 +- letsencrypt/scripts/main.py | 19 +++++----- 6 files changed, 62 insertions(+), 42 deletions(-) diff --git a/letsencrypt/client/apache/configurator.py b/letsencrypt/client/apache/configurator.py index 6b7e03f24..bafe94f54 100644 --- a/letsencrypt/client/apache/configurator.py +++ b/letsencrypt/client/apache/configurator.py @@ -317,7 +317,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # Search sites-available, httpd.conf for possible virtual hosts paths = self.aug.match( - ("/files%ssites-available//*[label()=~regexp('%s')]" % + ("/files%s/sites-available//*[label()=~regexp('%s')]" % (self.parser.root, parser.case_i('VirtualHost')))) vhs = [] @@ -682,8 +682,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if ssl_vhost.names[0] < (255-23): redirect_filename = "le-redirect-%s.conf" % ssl_vhost.names[0] - redirect_filepath = ("%ssites-available/%s" % - (self.parser.root, redirect_filename)) + redirect_filepath = os.path.join( + self.parser.root, 'sites-available', redirect_filename) # Register the new file that will be created # Note: always register the creation before writing to ensure file will @@ -697,8 +697,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.aug.load() # Make a new vhost data structure and add it to the lists - new_fp = self.parser.root + "sites-available/" + redirect_filename - new_vhost = self._create_vhost(parser.get_aug_path(new_fp)) + new_vhost = self._create_vhost(parser.get_aug_path(redirect_filepath)) self.vhosts.append(new_vhost) # Finally create documentation for the change @@ -829,7 +828,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :rtype: bool """ - enabled_dir = os.path.join(self.parser.root, "sites-enabled/") + enabled_dir = os.path.join(self.parser.root, "sites-enabled") for entry in os.listdir(enabled_dir): if os.path.realpath(os.path.join(enabled_dir, entry)) == avail_fp: return True @@ -854,7 +853,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return True if "/sites-available/" in vhost.filep: - enabled_path = ("%ssites-enabled/%s" % + enabled_path = ("%s/sites-enabled/%s" % (self.parser.root, os.path.basename(vhost.filep))) self.reverter.register_file_creation(False, enabled_path) os.symlink(vhost.filep, enabled_path) diff --git a/letsencrypt/client/apache/dvsni.py b/letsencrypt/client/apache/dvsni.py index 79ea5f5c4..f9efdf559 100644 --- a/letsencrypt/client/apache/dvsni.py +++ b/letsencrypt/client/apache/dvsni.py @@ -167,6 +167,8 @@ class ApacheDvsni(object): """ ips = " ".join(str(i) for i in ip_addrs) + document_root = os.path.join( + self.configurator.config.config_dir, "dvsni_page/") return ("\n" "ServerName " + nonce + constants.DVSNI_DOMAIN_SUFFIX + "\n" "UseCanonicalName on\n" @@ -178,8 +180,7 @@ class ApacheDvsni(object): "SSLCertificateFile " + self.get_cert_file(nonce) + "\n" "SSLCertificateKeyFile " + dvsni_key_file + "\n" "\n" - "DocumentRoot " + - self.configurator.config.config_dir + "dvsni_page/\n" + "DocumentRoot " + document_root + "\n" "\n\n") def get_cert_file(self, nonce): @@ -191,4 +192,4 @@ class ApacheDvsni(object): :rtype: str """ - return self.configurator.config.work_dir + nonce + ".crt" + return os.path.join(self.configurator.config.work_dir, nonce + ".crt") diff --git a/letsencrypt/client/apache/parser.py b/letsencrypt/client/apache/parser.py index 8bc636dd1..0a5eff97c 100644 --- a/letsencrypt/client/apache/parser.py +++ b/letsencrypt/client/apache/parser.py @@ -6,18 +6,23 @@ from letsencrypt.client import errors class ApacheParser(object): - """Class handles the fine details of parsing the Apache Configuration.""" + """Class handles the fine details of parsing the Apache Configuration. + + :ivar str root: Normalized abosulte path to the server root + directory. Without trailing slash. + + """ def __init__(self, aug, root, ssl_options): # Find configuration root and make sure augeas can parse it. self.aug = aug - self.root = root + self.root = os.path.abspath(root) self.loc = self._set_locations(ssl_options) self._parse_file(self.loc["root"]) # 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/*")) + self._parse_file(os.path.join(self.root, "sites-available") + "/*") # This problem has been fixed in Augeas 1.0 self.standardize_excl() @@ -190,7 +195,7 @@ class ApacheParser(object): arg = cur_dir + arg # conf/ is a special variable for ServerRoot in Apache elif arg.startswith("conf/"): - arg = self.root + arg[5:] + arg = self.root + arg[4:] # TODO: Test if Apache allows ../ or ~/ for Includes # Attempts to add a transform to the file if one does not already exist @@ -301,12 +306,12 @@ class ApacheParser(object): excl = ["*.augnew", "*.augsave", "*.dpkg-dist", "*.dpkg-bak", "*.dpkg-new", "*.dpkg-old", "*.rpmsave", "*.rpmnew", "*~", - self.root + "*.augsave", - self.root + "*~", - self.root + "*/*augsave", - self.root + "*/*~", - self.root + "*/*/*.augsave", - self.root + "*/*/*~"] + self.root + "/*.augsave", + self.root + "/*~", + self.root + "/*/*augsave", + self.root + "/*/*~", + self.root + "/*/*/*.augsave", + self.root + "/*/*/*~"] for i in range(len(excl)): self.aug.set("/augeas/load/Httpd/excl[%d]" % (i+1), excl[i]) diff --git a/letsencrypt/client/tests/apache/parser_test.py b/letsencrypt/client/tests/apache/parser_test.py index 453952a19..fe9e96ed5 100644 --- a/letsencrypt/client/tests/apache/parser_test.py +++ b/letsencrypt/client/tests/apache/parser_test.py @@ -10,7 +10,6 @@ import zope.component from letsencrypt.client import display from letsencrypt.client import errors -from letsencrypt.client.apache import parser from letsencrypt.client.tests.apache import util @@ -23,15 +22,32 @@ class ApacheParserTest(util.ApacheTest): zope.component.provideUtility(display.FileDisplay(sys.stdout)) - self.parser = parser.ApacheParser( - augeas.Augeas(flags=augeas.Augeas.NONE), - self.config_path, self.ssl_options) + from letsencrypt.client.apache.parser import ApacheParser + self.aug = augeas.Augeas(flags=augeas.Augeas.NONE) + self.parser = ApacheParser(self.aug, self.config_path, self.ssl_options) def tearDown(self): shutil.rmtree(self.temp_dir) shutil.rmtree(self.config_dir) shutil.rmtree(self.work_dir) + def test_root_normalized(self): + from letsencrypt.client.apache.parser import ApacheParser + path = os.path.join(self.temp_dir, "debian_apache_2_4/////" + "two_vhost_80/../two_vhost_80/apache2") + parser = ApacheParser(self.aug, path, None) + self.assertEqual(parser.root, self.config_path) + + def test_root_absolute(self): + from letsencrypt.client.apache.parser import ApacheParser + parser = ApacheParser(self.aug, os.path.relpath(self.config_path), None) + self.assertEqual(parser.root, self.config_path) + + def test_root_no_trailing_slash(self): + from letsencrypt.client.apache.parser import ApacheParser + parser = ApacheParser(self.aug, self.config_path + os.path.sep, None) + self.assertEqual(parser.root, self.config_path) + def test_parse_file(self): """Test parse_file. @@ -51,10 +67,10 @@ class ApacheParserTest(util.ApacheTest): self.assertTrue(matches) def test_find_dir(self): - test = self.parser.find_dir(parser.case_i("Listen"), "443") + from letsencrypt.client.apache.parser import case_i + test = self.parser.find_dir(case_i("Listen"), "443") # This will only look in enabled hosts - test2 = self.parser.find_dir( - parser.case_i("documentroot")) + test2 = self.parser.find_dir(case_i("documentroot")) self.assertEqual(len(test), 2) self.assertEqual(len(test2), 3) @@ -76,8 +92,9 @@ class ApacheParserTest(util.ApacheTest): Path must be valid before attempting to add to augeas """ + from letsencrypt.client.apache.parser import get_aug_path self.parser.add_dir_to_ifmodssl( - parser.get_aug_path(self.parser.loc["default"]), + get_aug_path(self.parser.loc["default"]), "FakeDirective", "123") matches = self.parser.find_dir("FakeDirective", "123") @@ -86,8 +103,8 @@ class ApacheParserTest(util.ApacheTest): self.assertTrue("IfModule" in matches[0]) def test_get_aug_path(self): - self.assertEqual( - "/files/etc/apache", parser.get_aug_path("/etc/apache")) + from letsencrypt.client.apache.parser import get_aug_path + self.assertEqual("/files/etc/apache", get_aug_path("/etc/apache")) def test_set_locations(self): with mock.patch("letsencrypt.client.apache.parser." diff --git a/letsencrypt/client/tests/apache/util.py b/letsencrypt/client/tests/apache/util.py index df981aede..78566e1e4 100644 --- a/letsencrypt/client/tests/apache/util.py +++ b/letsencrypt/client/tests/apache/util.py @@ -22,9 +22,8 @@ class ApacheTest(unittest.TestCase): # pylint: disable=too-few-public-methods self.ssl_options = setup_apache_ssl_options(self.config_dir) - # Final slash is currently important self.config_path = os.path.join( - self.temp_dir, "debian_apache_2_4/two_vhost_80/apache2/") + self.temp_dir, "debian_apache_2_4/two_vhost_80/apache2") self.rsa256_file = pkg_resources.resource_filename( "letsencrypt.client.tests", 'testdata/rsa256_key.pem') diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index b92298a9a..a6a12f725 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -54,26 +54,25 @@ def create_parser(): help="Use the text output instead of the curses UI.") add("--test", action="store_true", help="Run in test mode.") - # TODO: trailing slashes might be important! check and remove - add("--config-dir", default="/etc/letsencrypt/", + add("--config-dir", default="/etc/letsencrypt", help="Configuration directory.") - add("--work-dir", default="/var/lib/letsencrypt/", + add("--work-dir", default="/var/lib/letsencrypt", help="Working directory.") - add("--backup-dir", default="/var/lib/letsencrypt/backups/", + add("--backup-dir", default="/var/lib/letsencrypt/backups", help="Configuration backups directory.") add("--temp-checkpoint-dir", - default="/var/lib/letsencrypt/temp_checkpoint/", + default="/var/lib/letsencrypt/temp_checkpoint", help="Temporary checkpoint directory.") add("--in-progress-dir", - default="/var/lib/letsencrypt/backups/IN_PROGRESS/", + default="/var/lib/letsencrypt/backups/IN_PROGRESS", help="Directory used before a permanent checkpoint is finalized") - add("--cert-key-backup", default="/var/lib/letsencrypt/keys-certs/", + add("--cert-key-backup", default="/var/lib/letsencrypt/keys-certs", help="Directory where all certificates and keys are stored. " "Used for easy revocation.") - add("--rev-tokens-dir", default="/var/lib/letsencrypt/revocation_tokens/", + add("--rev-tokens-dir", default="/var/lib/letsencrypt/revocation_tokens", help="Directory where all revocation tokens are saved.") - add("--key-dir", default="/etc/letsencrypt/keys/", help="Keys storage.") - add("--cert-dir", default="/etc/letsencrypt/certs/", + add("--key-dir", default="/etc/letsencrypt/keys", help="Keys storage.") + add("--cert-dir", default="/etc/letsencrypt/certs", help="Certificates storage.") add("--le-vhost-ext", default="-le-ssl.conf",