From 0a60122cf65935bd41652aba67d8b75c51e287a5 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 24 Jul 2015 20:27:16 -0700 Subject: [PATCH 1/9] Started submission script --- letshelp/letshelp_letsencrypt_apache.py | 196 ++++++++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100755 letshelp/letshelp_letsencrypt_apache.py diff --git a/letshelp/letshelp_letsencrypt_apache.py b/letshelp/letshelp_letsencrypt_apache.py new file mode 100755 index 000000000..06955ac6c --- /dev/null +++ b/letshelp/letshelp_letsencrypt_apache.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python +"""Let's Encrypt Apache configuration submission script""" +import argparse +import os +import subprocess +import sys +import tempfile + + +DESCRIPTION = """ +Let's Help is a simple script you can run to help out the Let's Encrypt +project. Since Let's Encrypt will support automatically configuring HTTPS on +many servers, we want to test this functionality on as many configurations as +possible. This script will create a sanitized copy of your Apache +configuration, notifying you of the files that have been selected. If (and only +if) you approve this selection, these files will be sent to the Let's Encrypt +developers. Of course, your submission will be encrypted. + +""" + +NO_APACHECTL = """ +Unable to find `apachectl` which is required for this script to work. If it is +installed, please run this script again with the --apache-ctl command line +argument and path to the binary. + +""" + + +def copy_config(server_root, temp_dir): + """Safely copies server_root to temp_dir and returns copied files""" + copied_files, copied_dirs = list(), list() + dir_len = len(os.path.dirname(server_root)) + + for config_path, config_dirs, config_files in os.walk(server_root): + relative_path = config_path if not dir_len else config_path[dir_len+1:] + temp_path = os.path.join(temp_dir, relative_path) + os.mkdir(temp_path) + + copied_all = True + copied_files_in_current_dir = list() + for config_file in config_files: + config_file_path = os.path.join(config_path, config_file) + temp_file_path = os.path.join(temp_path, config_file) + if os.path.islink(config_file_path): + os.symlink(os.readlink(config_file_path), temp_file_path) + elif _safe_config_file(config_file_path): + _copy_file_without_comments(config_file_path, temp_file_path) + copied_files_in_current_dir.append(config_file_path) + else: + copied_all = False + + # If copied all files in leaf directory + if copied_all and not config_dirs: + copied_dirs.append(config_path) + else: + copied_files += copied_files_in_current_dir + + return copied_files, copied_dirs + + +def _copy_file_without_comments(source, destination): + """Copies source to destination, removing comments""" + with open(source, "r") as infile: + with open(destination, "w") as outfile: + for line in infile: + if not (line.isspace() or line.lstrip().startswith("#")): + outfile.write(line) + + +def _safe_config_file(config_file): + """Returns True if config_file can be safely copied""" + if "ASCII" in subprocess.check_output(["file", config_file]): + if not config_file.endswith(".pem"): + with open(config_file) as f: + for line in f: + if line.startswith("-----BEGIN"): + return False + return True + + return False + + +def setup_tempdir(args): + """Creates a temporary directory and necessary files for config""" + tempdir = tempfile.mkdtemp() + + with open(os.path.join(tempdir, "config_file"), "w") as f: + f.write(args.config_file + "\n") + + with open(os.path.join(tempdir, "version"), "w") as f: + f.write(subprocess.check_output([args.apache_ctl, "-v"])) + + with open(os.path.join(tempdir, "modules"), "w") as f: + f.write(subprocess.check_output( + [args.apache_ctl, "-d", args.server_root, + "-f", args.config_file, "-M"] + )) + + with open(os.path.join(tempdir, "vhosts"), "w") as f: + f.write(subprocess.check_output( + [args.apache_ctl, "-d", args.server_root, "-f", + args.config_file, "-t", "-D", "DUMP_VHOSTS"] + )) + + return tempdir + + +def verify_config(args): + """Verifies server_root and config_file specify a valid config""" + try: + subprocess.check_call( + [args.apache_ctl, "-d", args.server_root, + "-f", args.config_file, "-t"] + ) + except OSError: + sys.exit(NO_APACHECTL) + except subprocess.CalledProcessError: + sys.exit("Syntax check from apachectl failed") + + +def locate_config(apache_ctl): + """Uses the apachectl binary to find configuration files""" + try: + output = subprocess.check_output([apache_ctl, "-V"]) + except OSError: + sys.exit(NO_APACHECTL) + + for line in output.splitlines(): + if "HTTPD_ROOT" in line: + server_root = line[line.find("\"")+1:-1] + elif "SERVER_CONFIG_FILE" in line: + config_file = line[line.find("\"")+1:-1] + + if not (server_root and config_file): + sys.exit( + "Unable to locate Apache configuration. Please run this script " + "again and specify --server-root and --config-file." + ) + + return server_root, config_file + + +def get_args(): + """Parses command line arguments""" + parser = argparse.ArgumentParser(description=DESCRIPTION) + parser.add_argument( + "-c", "--apache-ctl", default="apachectl", + help="path to the `apachectl` binary" + ) + parser.add_argument( + "-d", "--server-root", + help="location of the root directory of your Apache configuration" + ) + parser.add_argument( + "-f", "--config-file", + help="location of your main Apache configuration file" + ) + args = parser.parse_args() + + # args.server_root XOR args.config_file + if bool(args.server_root) != bool(args.config_file): + sys.exit( + "If either --server-root and --config-file are specified, they " + "both must be included." + ) + elif args.server_root and args.config_file: + if args.config_file.startswith(args.server_root): + args.config_file = args.config_file[len(args.server_root):] + else: + sys.exit( + "This script expects the Apache configuration file to be " + "inside the server root." + ) + + return args + + +def main(): + """Main script execution""" + args = get_args() + if not args.server_root: + args.server_root, args.config_file = locate_config(args.apache_ctl) + + verify_config(args) + tempdir = setup_tempdir(args) + files, dirs = copy_config(args.server_root, tempdir) + print "Copied these files:" + for f in files: + print f + print "Copied all files in these directories:" + for d in dirs: + print d + + +if __name__ == "__main__": + main() From 12b997078780bef28f17cb7b6d080844311ca383 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 27 Jul 2015 19:26:28 -0700 Subject: [PATCH 2/9] Finished script besides commit and started unit tests --- .../letshelp_letsencrypt/__init__.py | 1 + .../letshelp_letsencrypt_apache.py | 302 ++++++++++++++++++ .../letshelp_letsencrypt_apache_test.py | 162 ++++++++++ .../testdata/conf-available/charset.conf | 8 + .../testdata/conf-enabled/charset.conf | 1 + .../letshelp_letsencrypt/testdata/passwd | 1 + letshelp-letsencrypt/setup.py | 11 + letshelp/letshelp_letsencrypt_apache.py | 196 ------------ tox.cover.sh | 3 +- tox.ini | 8 +- 10 files changed, 493 insertions(+), 200 deletions(-) create mode 100644 letshelp-letsencrypt/letshelp_letsencrypt/__init__.py create mode 100755 letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py create mode 100644 letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py create mode 100644 letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-available/charset.conf create mode 120000 letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-enabled/charset.conf create mode 100644 letshelp-letsencrypt/letshelp_letsencrypt/testdata/passwd create mode 100644 letshelp-letsencrypt/setup.py delete mode 100755 letshelp/letshelp_letsencrypt_apache.py diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/__init__.py b/letshelp-letsencrypt/letshelp_letsencrypt/__init__.py new file mode 100644 index 000000000..6882a19d4 --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/__init__.py @@ -0,0 +1 @@ +"""Tools for submitting server configurations""" diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py new file mode 100755 index 000000000..5c3e5b831 --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py @@ -0,0 +1,302 @@ +#!/usr/bin/env python +"""Let's Encrypt Apache configuration submission script""" +import argparse +import os +import subprocess +import sys +import tarfile +import tempfile +import textwrap + + +_DESCRIPTION = """ +Let's Help is a simple script you can run to help out the Let's Encrypt +project. Since Let's Encrypt will support automatically configuring HTTPS on +many servers, we want to test this functionality on as many configurations as +possible. This script will create a sanitized copy of your Apache +configuration, notifying you of the files that have been selected. If (and only +if) you approve this selection, these files will be sent to the Let's Encrypt +developers. + +""" + + +_NO_APACHECTL = """ +Unable to find `apachectl` which is required for this script to work. If it is +installed, please run this script again with the --apache-ctl command line +argument and the path to the binary. + +""" + + +def make_and_verify_selection(server_root, temp_dir): + """Copies server_root to temp_dir and verifies selection with the user + + :param str server_root: Path to the Apache server root + :param str temp_dir: Path to the temporary directory to copy files to + + """ + copied_files, copied_dirs = copy_config(server_root, temp_dir) + + print textwrap.fill("A secure copy of the files that have been selected " + "for submission has been created under {0}. All " + "comments have been removed and the files are only " + "accessible by the current user. A list of the files " + "that have been included is shown below. Please make " + "sure that this selection does not contain private " + "keys, passwords, or any other sensitive " + "information.".format(temp_dir)) + print "\nFiles:" + for copied_file in copied_files: + print copied_file + print "Directories (including all contained files):" + for copied_dir in copied_dirs: + print copied_dir + + sys.stdout.write("\nIs it safe to submit these files? ") + while True: + ans = raw_input("(Y)es/(N)o: ") + if ans.startswith("Y") or ans.startswith("y"): + return + elif ans.startswith("N") or ans.startswith("n"): + sys.exit("Your files were not submitted") + + +def copy_config(server_root, temp_dir): + """Safely copies server_root to temp_dir and returns copied files + + :param str server_root: Absolute path to the Apache server root + :param str temp_dir: Path to the temporary directory to copy files to + + :returns: List of copied files and a list of leaf directories where + all contained files were copied + :rtype: `tuple` of `list` of `str` + + """ + copied_files, copied_dirs = list(), list() + dir_len = len(os.path.dirname(server_root)) + + for config_path, config_dirs, config_files in os.walk(server_root): + temp_path = os.path.join(temp_dir, config_path[dir_len+1:]) + os.mkdir(temp_path) + + copied_all = True + copied_files_in_current_dir = list() + for config_file in config_files: + config_file_path = os.path.join(config_path, config_file) + temp_file_path = os.path.join(temp_path, config_file) + if os.path.islink(config_file_path): + os.symlink(os.readlink(config_file_path), temp_file_path) + elif safe_config_file(config_file_path): + copy_file_without_comments(config_file_path, temp_file_path) + copied_files_in_current_dir.append(config_file_path) + else: + copied_all = False + + # If copied all files in leaf directory + if copied_all and not config_dirs: + copied_dirs.append(config_path) + else: + copied_files += copied_files_in_current_dir + + return copied_files, copied_dirs + + +def copy_file_without_comments(source, destination): + """Copies source to destination, removing comments + + :param str source: Path to the file to be copied + :param str destination: Path where source should be copied to + + """ + with open(source, "r") as infile: + with open(destination, "w") as outfile: + for line in infile: + if not (line.isspace() or line.lstrip().startswith("#")): + outfile.write(line) + + +def safe_config_file(config_file): + """Returns True if config_file can be safely copied + + :param str config_file: Path to an Apache configuration file + + :returns: True if config_file can be safely copied + :rtype: bool + + """ + file_output = subprocess.check_output( + ["file", config_file], close_fds=True, stderr=open(os.devnull, "w") + ) + + if "ASCII" in file_output: + possible_password_file = empty_or_all_comments = True + with open(config_file) as config_fd: + for line in config_fd: + if not (line.isspace() or line.lstrip().startswith("#")): + empty_or_all_comments = False + if line.startswith("-----BEGIN"): + return False + elif not ":" in line: + possible_password_file = False + # If file isn't empty or commented out and could be a password file, + # don't include it in selection. It is safe to include the file if + # it consists solely of comments because comments are removed before + # submission. + return empty_or_all_comments or not possible_password_file + + return False + + +def setup_tempdir(args): + """Creates a temporary directory and necessary files for config + + :param argparse.Namespace args: Parsed command line arguments + + :returns: Path to temporary directory + :rtype: str + + """ + tempdir = tempfile.mkdtemp() + + with open(os.path.join(tempdir, "config_file"), "w") as config_fd: + config_fd.write(args.config_file + "\n") + + with open(os.path.join(tempdir, "version"), "w") as version_fd: + version_fd.write(subprocess.check_output( + [args.apache_ctl, "-v"], close_fds=True, + stderr=open(os.devnull, "w") + )) + + with open(os.path.join(tempdir, "modules"), "w") as modules_fd: + modules_fd.write(subprocess.check_output( + [args.apache_ctl, "-d", args.server_root, + "-f", args.config_file, "-M"], close_fds=True, + stderr=open(os.devnull, "w") + )) + + with open(os.path.join(tempdir, "vhosts"), "w") as vhosts_fd: + vhosts_fd.write(subprocess.check_output( + [args.apache_ctl, "-d", args.server_root, "-f", + args.config_file, "-t", "-D", "DUMP_VHOSTS"], close_fds=True, + stderr=open(os.devnull, "w") + )) + + return tempdir + + +def verify_config(args): + """Verifies server_root and config_file specify a valid config + + :param argparse.Namespace args: Parsed command line arguments + + """ + try: + subprocess.check_call( + [args.apache_ctl, "-d", args.server_root, + "-f", args.config_file, "-t"], close_fds=True, + stdout=open(os.devnull, "w"), stderr=open(os.devnull, "w") + ) + except OSError: + sys.exit(_NO_APACHECTL) + except subprocess.CalledProcessError: + sys.exit("Syntax check from apachectl failed") + + +def locate_config(apache_ctl): + """Uses the apachectl binary to find configuration files + + :param str apache_ctl: Path to `apachectl` binary + + + :returns: Path to Apache server root and main configuration file + :rtype: `tuple` of `str` + + """ + try: + output = subprocess.check_output( + [apache_ctl, "-V"], close_fds=True, stderr=open(os.devnull, "w") + ) + except OSError: + sys.exit(_NO_APACHECTL) + + server_root = config_file = "" + for line in output.splitlines(): + # Relevant output lines are of the form: -D DIRECTIVE="VALUE" + if "HTTPD_ROOT" in line: + server_root = line[line.find("\"")+1:-1] + elif "SERVER_CONFIG_FILE" in line: + config_file = line[line.find("\"")+1:-1] + + if not (server_root and config_file): + sys.exit( + "Unable to locate Apache configuration. Please run this script " + "again and specify --server-root and --config-file" + ) + + return server_root, config_file + + +def get_args(): + """Parses command line arguments + + :returns: Parsed command line options + :rtype: argparse.Namespace + + """ + parser = argparse.ArgumentParser(description=_DESCRIPTION) + parser.add_argument( + "-c", "--apache-ctl", default="apachectl", + help="path to the `apachectl` binary" + ) + parser.add_argument( + "-d", "--server-root", + help="location of the root directory of your Apache configuration" + ) + parser.add_argument( + "-f", "--config-file", + help="location of your main Apache configuration file relative to the " + "server root" + ) + args = parser.parse_args() + + # args.server_root XOR args.config_file + if bool(args.server_root) != bool(args.config_file): + sys.exit( + "If either --server-root and --config-file are specified, they " + "both must be included" + ) + elif args.server_root and args.config_file: + args.server_root = os.path.abspath(args.server_root) + args.config_file = os.path.abspath(args.config_file) + + if args.config_file.startswith(args.server_root): + args.config_file = args.config_file[len(args.server_root)+1:] + else: + sys.exit( + "This script expects the Apache configuration file to be " + "inside the server root" + ) + + return args + + +def main(): + """Main script execution""" + args = get_args() + if not args.server_root: + args.server_root, args.config_file = locate_config(args.apache_ctl) + + verify_config(args) + tempdir = setup_tempdir(args) + make_and_verify_selection(args.server_root, tempdir) + + tarpath = os.path.join(tempdir, "config.tar.gz") + with tarfile.open(tarpath, mode="w:gz") as tar: + tar.add(tempdir, arcname=".") + + # Submit tarpath + + +if __name__ == "__main__": + main() diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py new file mode 100644 index 000000000..3d36e93d9 --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py @@ -0,0 +1,162 @@ +"""Tests for letshelp.letshelp_letsencrypt_apache.py""" +import argparse +import functools +import os +import pkg_resources +import tempfile +import unittest + +import mock + +import letshelp_letsencrypt.letshelp_letsencrypt_apache as letshelp_le_apache + + +_PASSWD_FILE = pkg_resources.resource_filename(__name__, "testdata/passwd") +_CONF_FILE = pkg_resources.resource_filename( + __name__, "testdata/conf-available/charset.conf") + + +_MODULE_NAME = "letshelp_letsencrypt.letshelp_letsencrypt_apache" + + +_COMPILE_SETTINGS = """Server version: Apache/2.4.10 (Debian) +Server built: Mar 15 2015 09:51:43 +Server's Module Magic Number: 20120211:37 +Server loaded: APR 1.5.1, APR-UTIL 1.5.4 +Compiled using: APR 1.5.1, APR-UTIL 1.5.4 +Architecture: 64-bit +Server MPM: event + threaded: yes (fixed thread count) + forked: yes (variable process count) +Server compiled with.... + -D APR_HAS_SENDFILE + -D APR_HAS_MMAP + -D APR_HAVE_IPV6 (IPv4-mapped addresses enabled) + -D APR_USE_SYSVSEM_SERIALIZE + -D APR_USE_PTHREAD_SERIALIZE + -D SINGLE_LISTEN_UNSERIALIZED_ACCEPT + -D APR_HAS_OTHER_CHILD + -D AP_HAVE_RELIABLE_PIPED_LOGS + -D DYNAMIC_MODULE_LIMIT=256 + -D HTTPD_ROOT="/etc/apache2" + -D SUEXEC_BIN="/usr/lib/apache2/suexec" + -D DEFAULT_PIDLOG="/var/run/apache2.pid" + -D DEFAULT_SCOREBOARD="logs/apache_runtime_status" + -D DEFAULT_ERRORLOG="logs/error_log" + -D AP_TYPES_CONFIG_FILE="mime.types" + -D SERVER_CONFIG_FILE="apache2.conf" + +""" + + +class LetsHelpApacheTest(unittest.TestCase): + @mock.patch(_MODULE_NAME + ".copy_config") + def test_make_and_verify_selection(self, mock_copy_config): + mock_copy_config.return_value = ["apache2.conf"], ["apache2"] + + with mock.patch("__builtin__.raw_input") as mock_input: + with mock.patch(_MODULE_NAME + ".sys.stdout"): + mock_input.side_effect = ["Yes", "No"] + letshelp_le_apache.make_and_verify_selection("root", "temp") + self.assertRaises( + SystemExit, letshelp_le_apache.make_and_verify_selection, + "server_root", "temp_dir") + + def test_copy_config(self): + tempdir = tempfile.mkdtemp() + server_root = pkg_resources.resource_filename(__name__, "testdata") + letshelp_le_apache.copy_config(server_root, tempdir) + + temp_testdata = os.path.join(tempdir, "testdata") + self.assertFalse(os.path.exists(os.path.join(temp_testdata, "passwd"))) + self.assertTrue(os.path.exists(os.path.join( + temp_testdata, "conf-available", "charset.conf"))) + self.assertTrue(os.path.exists(os.path.join( + temp_testdata, "conf-enabled", "charset.conf"))) + + def test_copy_file_without_comments(self): + dest = tempfile.mkstemp()[1] + letshelp_le_apache.copy_file_without_comments(_PASSWD_FILE, dest) + + with open(_PASSWD_FILE) as original: + with open(dest) as copy: + for original_line, copied_line in zip(original, copy): + self.assertEqual(original_line, copied_line) + + @mock.patch(_MODULE_NAME + ".subprocess.check_output") + def test_safe_config_file(self, mock_check_output): + mock_check_output.return_value = "PEM RSA private key" + self.assertFalse(letshelp_le_apache.safe_config_file("filename")) + + mock_check_output.return_value = "ASCII text" + self.assertFalse(letshelp_le_apache.safe_config_file(_PASSWD_FILE)) + self.assertTrue(letshelp_le_apache.safe_config_file(_CONF_FILE)) + + @mock.patch(_MODULE_NAME + ".subprocess.check_output") + def test_tempdir(self, mock_check_output): + mock_check_output.side_effect = ["version", "modules", "vhosts"] + args = argparse.Namespace() + args.apache_ctl = "apache_ctl" + args.config_file = "config_file" + args.server_root = "server_root" + + tempdir = letshelp_le_apache.setup_tempdir(args) + + with open(os.path.join(tempdir, "config_file")) as config_fd: + self.assertEqual(config_fd.read(), args.config_file + "\n") + + with open(os.path.join(tempdir, "version")) as version_fd: + self.assertEqual(version_fd.read(), "version") + + with open(os.path.join(tempdir, "modules")) as modules_fd: + self.assertEqual(modules_fd.read(), "modules") + + with open(os.path.join(tempdir, "vhosts")) as vhosts_fd: + self.assertEqual(vhosts_fd.read(), "vhosts") + + @mock.patch(_MODULE_NAME + ".subprocess.check_output") + def test_locate_config(self, mock_check_output): + mock_check_output.side_effect = [OSError, "bad_output", + _COMPILE_SETTINGS,] + + self.assertRaises( + SystemExit, letshelp_le_apache.locate_config, "ctl") + self.assertRaises( + SystemExit, letshelp_le_apache.locate_config, "ctl") + server_root, config_file = letshelp_le_apache.locate_config("ctl") + self.assertEqual(server_root, "/etc/apache2") + self.assertEqual(config_file, "apache2.conf") + + @mock.patch(_MODULE_NAME + ".argparse") + def test_get_args(self, mock_argparse): + argv = ["-d", "/etc/apache2"] + mock_argparse.ArgumentParser.return_value = _create_mock_parser(argv) + self.assertRaises(SystemExit, letshelp_le_apache.get_args) + + server_root = "/etc/apache2" + config_file = server_root + "/apache2.conf" + argv = ["-d", server_root, "-f", config_file] + mock_argparse.ArgumentParser.return_value = _create_mock_parser(argv) + args = letshelp_le_apache.get_args() + self.assertEqual(args.apache_ctl, "apachectl") + self.assertEqual(args.server_root, server_root) + self.assertEqual(args.config_file, os.path.basename(config_file)) + + server_root = "/etc/apache2" + config_file = "/etc/httpd/httpd.conf" + argv = ["-d", server_root, "-f", config_file] + mock_argparse.ArgumentParser.return_value = _create_mock_parser(argv) + self.assertRaises(SystemExit, letshelp_le_apache.get_args) + + +def _create_mock_parser(argv): + parser = argparse.ArgumentParser() + mock_parser = mock.MagicMock() + mock_parser.add_argument = parser.add_argument + mock_parser.parse_args = functools.partial(parser.parse_args, argv) + + return mock_parser + + +if __name__ == "__main__": + unittest.main() # pragma: no cover diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-available/charset.conf b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-available/charset.conf new file mode 100644 index 000000000..8b0f41594 --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-available/charset.conf @@ -0,0 +1,8 @@ +# Read the documentation before enabling AddDefaultCharset. +# In general, it is only a good idea if you know that all your files +# have this encoding. It will override any encoding given in the files +# in meta http-equiv or xml encoding tags. + +#AddDefaultCharset UTF-8 + +# vim: syntax=apache ts=4 sw=4 sts=4 sr noet diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-enabled/charset.conf b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-enabled/charset.conf new file mode 120000 index 000000000..4a6ca0846 --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-enabled/charset.conf @@ -0,0 +1 @@ +../conf-available/charset.conf \ No newline at end of file diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/passwd b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/passwd new file mode 100644 index 000000000..3559c1d1f --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/passwd @@ -0,0 +1 @@ +johntheripper:$apr1$fIGE9.JL$jTCwNWZy9Ak/yvOLuOyzQ1 diff --git a/letshelp-letsencrypt/setup.py b/letshelp-letsencrypt/setup.py new file mode 100644 index 000000000..58b28bc7e --- /dev/null +++ b/letshelp-letsencrypt/setup.py @@ -0,0 +1,11 @@ +from setuptools import setup +from setuptools import find_packages + + +install_requires = ["mock<1.1.0",] + +setup( + name="letshelp-letsencrypt", + packages=find_packages(), + install_requires=install_requires, +) diff --git a/letshelp/letshelp_letsencrypt_apache.py b/letshelp/letshelp_letsencrypt_apache.py deleted file mode 100755 index 06955ac6c..000000000 --- a/letshelp/letshelp_letsencrypt_apache.py +++ /dev/null @@ -1,196 +0,0 @@ -#!/usr/bin/env python -"""Let's Encrypt Apache configuration submission script""" -import argparse -import os -import subprocess -import sys -import tempfile - - -DESCRIPTION = """ -Let's Help is a simple script you can run to help out the Let's Encrypt -project. Since Let's Encrypt will support automatically configuring HTTPS on -many servers, we want to test this functionality on as many configurations as -possible. This script will create a sanitized copy of your Apache -configuration, notifying you of the files that have been selected. If (and only -if) you approve this selection, these files will be sent to the Let's Encrypt -developers. Of course, your submission will be encrypted. - -""" - -NO_APACHECTL = """ -Unable to find `apachectl` which is required for this script to work. If it is -installed, please run this script again with the --apache-ctl command line -argument and path to the binary. - -""" - - -def copy_config(server_root, temp_dir): - """Safely copies server_root to temp_dir and returns copied files""" - copied_files, copied_dirs = list(), list() - dir_len = len(os.path.dirname(server_root)) - - for config_path, config_dirs, config_files in os.walk(server_root): - relative_path = config_path if not dir_len else config_path[dir_len+1:] - temp_path = os.path.join(temp_dir, relative_path) - os.mkdir(temp_path) - - copied_all = True - copied_files_in_current_dir = list() - for config_file in config_files: - config_file_path = os.path.join(config_path, config_file) - temp_file_path = os.path.join(temp_path, config_file) - if os.path.islink(config_file_path): - os.symlink(os.readlink(config_file_path), temp_file_path) - elif _safe_config_file(config_file_path): - _copy_file_without_comments(config_file_path, temp_file_path) - copied_files_in_current_dir.append(config_file_path) - else: - copied_all = False - - # If copied all files in leaf directory - if copied_all and not config_dirs: - copied_dirs.append(config_path) - else: - copied_files += copied_files_in_current_dir - - return copied_files, copied_dirs - - -def _copy_file_without_comments(source, destination): - """Copies source to destination, removing comments""" - with open(source, "r") as infile: - with open(destination, "w") as outfile: - for line in infile: - if not (line.isspace() or line.lstrip().startswith("#")): - outfile.write(line) - - -def _safe_config_file(config_file): - """Returns True if config_file can be safely copied""" - if "ASCII" in subprocess.check_output(["file", config_file]): - if not config_file.endswith(".pem"): - with open(config_file) as f: - for line in f: - if line.startswith("-----BEGIN"): - return False - return True - - return False - - -def setup_tempdir(args): - """Creates a temporary directory and necessary files for config""" - tempdir = tempfile.mkdtemp() - - with open(os.path.join(tempdir, "config_file"), "w") as f: - f.write(args.config_file + "\n") - - with open(os.path.join(tempdir, "version"), "w") as f: - f.write(subprocess.check_output([args.apache_ctl, "-v"])) - - with open(os.path.join(tempdir, "modules"), "w") as f: - f.write(subprocess.check_output( - [args.apache_ctl, "-d", args.server_root, - "-f", args.config_file, "-M"] - )) - - with open(os.path.join(tempdir, "vhosts"), "w") as f: - f.write(subprocess.check_output( - [args.apache_ctl, "-d", args.server_root, "-f", - args.config_file, "-t", "-D", "DUMP_VHOSTS"] - )) - - return tempdir - - -def verify_config(args): - """Verifies server_root and config_file specify a valid config""" - try: - subprocess.check_call( - [args.apache_ctl, "-d", args.server_root, - "-f", args.config_file, "-t"] - ) - except OSError: - sys.exit(NO_APACHECTL) - except subprocess.CalledProcessError: - sys.exit("Syntax check from apachectl failed") - - -def locate_config(apache_ctl): - """Uses the apachectl binary to find configuration files""" - try: - output = subprocess.check_output([apache_ctl, "-V"]) - except OSError: - sys.exit(NO_APACHECTL) - - for line in output.splitlines(): - if "HTTPD_ROOT" in line: - server_root = line[line.find("\"")+1:-1] - elif "SERVER_CONFIG_FILE" in line: - config_file = line[line.find("\"")+1:-1] - - if not (server_root and config_file): - sys.exit( - "Unable to locate Apache configuration. Please run this script " - "again and specify --server-root and --config-file." - ) - - return server_root, config_file - - -def get_args(): - """Parses command line arguments""" - parser = argparse.ArgumentParser(description=DESCRIPTION) - parser.add_argument( - "-c", "--apache-ctl", default="apachectl", - help="path to the `apachectl` binary" - ) - parser.add_argument( - "-d", "--server-root", - help="location of the root directory of your Apache configuration" - ) - parser.add_argument( - "-f", "--config-file", - help="location of your main Apache configuration file" - ) - args = parser.parse_args() - - # args.server_root XOR args.config_file - if bool(args.server_root) != bool(args.config_file): - sys.exit( - "If either --server-root and --config-file are specified, they " - "both must be included." - ) - elif args.server_root and args.config_file: - if args.config_file.startswith(args.server_root): - args.config_file = args.config_file[len(args.server_root):] - else: - sys.exit( - "This script expects the Apache configuration file to be " - "inside the server root." - ) - - return args - - -def main(): - """Main script execution""" - args = get_args() - if not args.server_root: - args.server_root, args.config_file = locate_config(args.apache_ctl) - - verify_config(args) - tempdir = setup_tempdir(args) - files, dirs = copy_config(args.server_root, tempdir) - print "Copied these files:" - for f in files: - print f - print "Copied all files in these directories:" - for d in dirs: - print d - - -if __name__ == "__main__": - main() diff --git a/tox.cover.sh b/tox.cover.sh index f1d882cee..1e8f2d5f7 100755 --- a/tox.cover.sh +++ b/tox.cover.sh @@ -23,4 +23,5 @@ rm -f .coverage # --cover-erase is off, make sure stats are correct cover letsencrypt 97 && \ cover acme 100 && \ cover letsencrypt_apache 78 && \ - cover letsencrypt_nginx 96 + cover letsencrypt_nginx 96 && \ + cover letshelp_letsencrypt 100 diff --git a/tox.ini b/tox.ini index 1921fdd9c..f1174c384 100644 --- a/tox.ini +++ b/tox.ini @@ -10,12 +10,13 @@ envlist = py26,py27,py33,py34,cover,lint [testenv] commands = - pip install -r requirements.txt -e acme -e .[testing] -e letsencrypt-apache -e letsencrypt-nginx + pip install -r requirements.txt -e acme -e .[testing] -e letsencrypt-apache -e letsencrypt-nginx -e letshelp-letsencrypt # -q does not suppress errors python setup.py test -q python setup.py test -q -s acme python setup.py test -q -s letsencrypt_apache python setup.py test -q -s letsencrypt_nginx + python setup.py test -q -s letshelp_letsencrypt setenv = PYTHONPATH = {toxinidir} @@ -35,7 +36,7 @@ commands = [testenv:cover] basepython = python2.7 commands = - pip install -r requirements.txt -e acme -e .[testing] -e letsencrypt-apache -e letsencrypt-nginx + pip install -r requirements.txt -e acme -e .[testing] -e letsencrypt-apache -e letsencrypt-nginx -e letshelp-letsencrypt ./tox.cover.sh [testenv:lint] @@ -45,8 +46,9 @@ basepython = python2.7 # duplicate code checking; if one of the commands fails, others will # continue, but tox return code will reflect previous error commands = - pip install -r requirements.txt -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx + pip install -r requirements.txt -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx -e letshelp-letsencrypt pylint --rcfile=.pylintrc letsencrypt pylint --rcfile=.pylintrc acme/acme pylint --rcfile=.pylintrc letsencrypt-apache/letsencrypt_apache pylint --rcfile=.pylintrc letsencrypt-nginx/letsencrypt_nginx + pylint --rcfile=.pylintrc letshelp-letsencrypt/letshelp_letsencrypt From 6d9c62d586b24b6c0b83a12b614e982f64566e4d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 28 Jul 2015 13:39:24 -0700 Subject: [PATCH 3/9] Finished testing and cleanup --- .../letshelp_letsencrypt_apache.py | 2 +- .../letshelp_letsencrypt_apache_test.py | 91 ++++++++++++++++--- .../testdata/conf-available/charset.conf | 8 -- .../testdata/conf-enabled/charset.conf | 1 - .../letshelp_letsencrypt/testdata/key | 6 ++ .../testdata/mods-available/ssl.load | 2 + .../testdata/mods-enabled/ssl.load | 1 + 7 files changed, 88 insertions(+), 23 deletions(-) delete mode 100644 letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-available/charset.conf delete mode 120000 letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-enabled/charset.conf create mode 100644 letshelp-letsencrypt/letshelp_letsencrypt/testdata/key create mode 100644 letshelp-letsencrypt/letshelp_letsencrypt/testdata/mods-available/ssl.load create mode 120000 letshelp-letsencrypt/letshelp_letsencrypt/testdata/mods-enabled/ssl.load diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py index 5c3e5b831..49f8021e2 100755 --- a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py @@ -299,4 +299,4 @@ def main(): if __name__ == "__main__": - main() + main() # pragma: no cover diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py index 3d36e93d9..835938dbe 100644 --- a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py @@ -3,6 +3,8 @@ import argparse import functools import os import pkg_resources +import subprocess +import tarfile import tempfile import unittest @@ -11,10 +13,14 @@ import mock import letshelp_letsencrypt.letshelp_letsencrypt_apache as letshelp_le_apache -_PASSWD_FILE = pkg_resources.resource_filename(__name__, "testdata/passwd") -_CONF_FILE = pkg_resources.resource_filename( - __name__, "testdata/conf-available/charset.conf") - +_PARTIAL_CONF_PATH = os.path.join("mods-available", "ssl.load") +_PARTIAL_LINK_PATH = os.path.join("mods-enabled", "ssl.load") +_CONFIG_FILE = pkg_resources.resource_filename( + __name__, os.path.join("testdata", _PARTIAL_CONF_PATH)) +_PASSWD_FILE = pkg_resources.resource_filename( + __name__, os.path.join("testdata", "passwd")) +_KEY_FILE = pkg_resources.resource_filename( + __name__, os.path.join("testdata", "key")) _MODULE_NAME = "letshelp_letsencrypt.letshelp_letsencrypt_apache" @@ -52,7 +58,7 @@ Server compiled with.... class LetsHelpApacheTest(unittest.TestCase): @mock.patch(_MODULE_NAME + ".copy_config") def test_make_and_verify_selection(self, mock_copy_config): - mock_copy_config.return_value = ["apache2.conf"], ["apache2"] + mock_copy_config.return_value = (["apache2.conf"], ["apache2"]) with mock.patch("__builtin__.raw_input") as mock_input: with mock.patch(_MODULE_NAME + ".sys.stdout"): @@ -68,11 +74,14 @@ class LetsHelpApacheTest(unittest.TestCase): letshelp_le_apache.copy_config(server_root, tempdir) temp_testdata = os.path.join(tempdir, "testdata") - self.assertFalse(os.path.exists(os.path.join(temp_testdata, "passwd"))) + self.assertFalse(os.path.exists(os.path.join( + temp_testdata, os.path.basename(_PASSWD_FILE)))) + self.assertFalse(os.path.exists(os.path.join( + temp_testdata, os.path.basename(_KEY_FILE)))) self.assertTrue(os.path.exists(os.path.join( - temp_testdata, "conf-available", "charset.conf"))) + temp_testdata, _PARTIAL_CONF_PATH))) self.assertTrue(os.path.exists(os.path.join( - temp_testdata, "conf-enabled", "charset.conf"))) + temp_testdata, _PARTIAL_LINK_PATH))) def test_copy_file_without_comments(self): dest = tempfile.mkstemp()[1] @@ -90,15 +99,13 @@ class LetsHelpApacheTest(unittest.TestCase): mock_check_output.return_value = "ASCII text" self.assertFalse(letshelp_le_apache.safe_config_file(_PASSWD_FILE)) - self.assertTrue(letshelp_le_apache.safe_config_file(_CONF_FILE)) + self.assertFalse(letshelp_le_apache.safe_config_file(_KEY_FILE)) + self.assertTrue(letshelp_le_apache.safe_config_file(_CONFIG_FILE)) @mock.patch(_MODULE_NAME + ".subprocess.check_output") def test_tempdir(self, mock_check_output): mock_check_output.side_effect = ["version", "modules", "vhosts"] - args = argparse.Namespace() - args.apache_ctl = "apache_ctl" - args.config_file = "config_file" - args.server_root = "server_root" + args = _get_args() tempdir = letshelp_le_apache.setup_tempdir(args) @@ -114,6 +121,16 @@ class LetsHelpApacheTest(unittest.TestCase): with open(os.path.join(tempdir, "vhosts")) as vhosts_fd: self.assertEqual(vhosts_fd.read(), "vhosts") + @mock.patch(_MODULE_NAME + ".subprocess.check_call") + def test_verify_config(self, mock_check_call): + args = _get_args() + mock_check_call.side_effect = [ + None, OSError, subprocess.CalledProcessError(1, "apachectl")] + + letshelp_le_apache.verify_config(args) + self.assertRaises(SystemExit, letshelp_le_apache.verify_config, args) + self.assertRaises(SystemExit, letshelp_le_apache.verify_config, args) + @mock.patch(_MODULE_NAME + ".subprocess.check_output") def test_locate_config(self, mock_check_output): mock_check_output.side_effect = [OSError, "bad_output", @@ -148,6 +165,45 @@ class LetsHelpApacheTest(unittest.TestCase): mock_argparse.ArgumentParser.return_value = _create_mock_parser(argv) self.assertRaises(SystemExit, letshelp_le_apache.get_args) + def test_main_with_args(self): + with mock.patch(_MODULE_NAME + ".get_args"): + self._test_main_common() + + def test_main_without_args(self): + with mock.patch(_MODULE_NAME + ".get_args") as get_args: + args = _get_args() + server_root, config_file = args.server_root, args.config_file + args.server_root = args.config_file = None + get_args.return_value = args + with mock.patch(_MODULE_NAME + ".locate_config") as locate: + locate.return_value = (server_root, config_file) + self._test_main_common() + + def _test_main_common(self): + with mock.patch(_MODULE_NAME + ".verify_config"): + with mock.patch(_MODULE_NAME + ".setup_tempdir") as mock_setup: + tempdir_path = tempfile.mkdtemp() + mock_setup.return_value = tempdir_path + with mock.patch(_MODULE_NAME + ".make_and_verify_selection"): + testdir_basename = "test" + os.mkdir(os.path.join(tempdir_path, testdir_basename)) + + letshelp_le_apache.main() + + tar = tarfile.open(os.path.join( + tempdir_path, "config.tar.gz")) + + tempdir = tar.next() + self.assertTrue(tempdir.isdir()) + self.assertEqual(tempdir.name, ".") + + testdir = tar.next() + self.assertTrue(testdir.isdir()) + testdir_path = os.path.join(".", testdir_basename) + self.assertEqual(testdir.name, testdir_path) + + self.assertEqual(tar.next(), None) + def _create_mock_parser(argv): parser = argparse.ArgumentParser() @@ -158,5 +214,14 @@ def _create_mock_parser(argv): return mock_parser +def _get_args(): + args = argparse.Namespace() + args.apache_ctl = "apache_ctl" + args.config_file = "config_file" + args.server_root = "server_root" + + return args + + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-available/charset.conf b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-available/charset.conf deleted file mode 100644 index 8b0f41594..000000000 --- a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-available/charset.conf +++ /dev/null @@ -1,8 +0,0 @@ -# Read the documentation before enabling AddDefaultCharset. -# In general, it is only a good idea if you know that all your files -# have this encoding. It will override any encoding given in the files -# in meta http-equiv or xml encoding tags. - -#AddDefaultCharset UTF-8 - -# vim: syntax=apache ts=4 sw=4 sts=4 sr noet diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-enabled/charset.conf b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-enabled/charset.conf deleted file mode 120000 index 4a6ca0846..000000000 --- a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/conf-enabled/charset.conf +++ /dev/null @@ -1 +0,0 @@ -../conf-available/charset.conf \ No newline at end of file diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/key b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/key new file mode 100644 index 000000000..659274d1d --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/key @@ -0,0 +1,6 @@ +-----BEGIN RSA PRIVATE KEY----- +MIGrAgEAAiEAm2Fylv+Uz7trgTW8EBHP3FQSMeZs2GNQ6VRo1sIVJEkCAwEAAQIh +AJT0BA/xD01dFCAXzSNyj9nfSZa3NpqzJZZn/eOm7vghAhEAzUVNZn4lLLBD1R6N +E8TKNQIRAMHHyn3O5JeY36lwKwkUlEUCEAliRauN0L0+QZuYjfJ9aJECEGx4dru3 +rTPCyighdqWNlHUCEQCiLjlwSRtWgmMBudCkVjzt +-----END RSA PRIVATE KEY----- diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/mods-available/ssl.load b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/mods-available/ssl.load new file mode 100644 index 000000000..3d2336ae0 --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/mods-available/ssl.load @@ -0,0 +1,2 @@ +# Depends: setenvif mime socache_shmcb +LoadModule ssl_module /usr/lib/apache2/modules/mod_ssl.so diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/mods-enabled/ssl.load b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/mods-enabled/ssl.load new file mode 120000 index 000000000..9d7972384 --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/mods-enabled/ssl.load @@ -0,0 +1 @@ +../mods-available/ssl.load \ No newline at end of file From 88e472eb0886bc0a9b8ab91d9f0d3a2a93273abc Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 28 Jul 2015 17:12:01 -0700 Subject: [PATCH 4/9] Fixed py26 support, cleanup, and added filename check --- letshelp-letsencrypt/MANIFEST.in | 1 + .../letshelp_letsencrypt_apache.py | 100 ++++++++---------- .../letshelp_letsencrypt_apache_test.py | 33 +++--- .../testdata/super_secret_file.txt | 1 + .../testdata/{key => uncommonly_named_k3y} | 0 .../{passwd => uncommonly_named_p4sswd} | 0 6 files changed, 69 insertions(+), 66 deletions(-) create mode 100644 letshelp-letsencrypt/MANIFEST.in create mode 100644 letshelp-letsencrypt/letshelp_letsencrypt/testdata/super_secret_file.txt rename letshelp-letsencrypt/letshelp_letsencrypt/testdata/{key => uncommonly_named_k3y} (100%) rename letshelp-letsencrypt/letshelp_letsencrypt/testdata/{passwd => uncommonly_named_p4sswd} (100%) diff --git a/letshelp-letsencrypt/MANIFEST.in b/letshelp-letsencrypt/MANIFEST.in new file mode 100644 index 000000000..61a3d3150 --- /dev/null +++ b/letshelp-letsencrypt/MANIFEST.in @@ -0,0 +1 @@ +recursive-include letshelp-letsencrypt/testdata * diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py index 49f8021e2..2d0514a62 100755 --- a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py @@ -29,6 +29,11 @@ argument and the path to the binary. """ +# Keywords likely to be found in filenames of sensitive files +_SENSITIVE_KEYWORDS = ["private", "secret", "cert", "crt", "key", "pem", "der", + "rsa", "dsa", "pass", "pw"] + + def make_and_verify_selection(server_root, temp_dir): """Copies server_root to temp_dir and verifies selection with the user @@ -55,10 +60,10 @@ def make_and_verify_selection(server_root, temp_dir): sys.stdout.write("\nIs it safe to submit these files? ") while True: - ans = raw_input("(Y)es/(N)o: ") - if ans.startswith("Y") or ans.startswith("y"): + ans = raw_input("(Y)es/(N)o: ").lower() + if ans.startswith("y"): return - elif ans.startswith("N") or ans.startswith("n"): + elif ans.startswith("n"): sys.exit("Your files were not submitted") @@ -125,9 +130,13 @@ def safe_config_file(config_file): :rtype: bool """ - file_output = subprocess.check_output( - ["file", config_file], close_fds=True, stderr=open(os.devnull, "w") - ) + for keyword in _SENSITIVE_KEYWORDS: + if keyword in config_file: + return False + + proc = subprocess.Popen(["file", config_file], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + file_output, _ = proc.communicate() if "ASCII" in file_output: possible_password_file = empty_or_all_comments = True @@ -162,25 +171,22 @@ def setup_tempdir(args): with open(os.path.join(tempdir, "config_file"), "w") as config_fd: config_fd.write(args.config_file + "\n") + proc = subprocess.Popen([args.apache_ctl, "-v"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) with open(os.path.join(tempdir, "version"), "w") as version_fd: - version_fd.write(subprocess.check_output( - [args.apache_ctl, "-v"], close_fds=True, - stderr=open(os.devnull, "w") - )) + version_fd.write(proc.communicate()[0]) + proc = subprocess.Popen([args.apache_ctl, "-d", args.server_root, "-f", + args.config_file, "-M"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) with open(os.path.join(tempdir, "modules"), "w") as modules_fd: - modules_fd.write(subprocess.check_output( - [args.apache_ctl, "-d", args.server_root, - "-f", args.config_file, "-M"], close_fds=True, - stderr=open(os.devnull, "w") - )) + modules_fd.write(proc.communicate()[0]) + proc = subprocess.Popen([args.apache_ctl, "-d", args.server_root, "-f", + args.config_file, "-t", "-D", "DUMP_VHOSTS"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) with open(os.path.join(tempdir, "vhosts"), "w") as vhosts_fd: - vhosts_fd.write(subprocess.check_output( - [args.apache_ctl, "-d", args.server_root, "-f", - args.config_file, "-t", "-D", "DUMP_VHOSTS"], close_fds=True, - stderr=open(os.devnull, "w") - )) + vhosts_fd.write(proc.communicate()[0]) return tempdir @@ -192,11 +198,10 @@ def verify_config(args): """ try: - subprocess.check_call( - [args.apache_ctl, "-d", args.server_root, - "-f", args.config_file, "-t"], close_fds=True, - stdout=open(os.devnull, "w"), stderr=open(os.devnull, "w") - ) + with open(os.devnull, "w") as devnull: + subprocess.check_call([args.apache_ctl, "-d", args.server_root, + "-f", args.config_file, "-t"], + stdout=devnull, stderr=subprocess.STDOUT) except OSError: sys.exit(_NO_APACHECTL) except subprocess.CalledProcessError: @@ -214,9 +219,9 @@ def locate_config(apache_ctl): """ try: - output = subprocess.check_output( - [apache_ctl, "-V"], close_fds=True, stderr=open(os.devnull, "w") - ) + proc = subprocess.Popen([apache_ctl, "-V"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + output, _ = proc.communicate() except OSError: sys.exit(_NO_APACHECTL) @@ -229,10 +234,8 @@ def locate_config(apache_ctl): config_file = line[line.find("\"")+1:-1] if not (server_root and config_file): - sys.exit( - "Unable to locate Apache configuration. Please run this script " - "again and specify --server-root and --config-file" - ) + sys.exit("Unable to locate Apache configuration. Please run this " + "script again and specify --server-root and --config-file") return server_root, config_file @@ -245,27 +248,20 @@ def get_args(): """ parser = argparse.ArgumentParser(description=_DESCRIPTION) - parser.add_argument( - "-c", "--apache-ctl", default="apachectl", - help="path to the `apachectl` binary" - ) - parser.add_argument( - "-d", "--server-root", - help="location of the root directory of your Apache configuration" - ) - parser.add_argument( - "-f", "--config-file", - help="location of your main Apache configuration file relative to the " - "server root" - ) + parser.add_argument("-c", "--apache-ctl", default="apachectl", + help="path to the `apachectl` binary") + parser.add_argument("-d", "--server-root", + help=("location of the root directory of your Apache " + "configuration")) + parser.add_argument("-f", "--config-file", + help=("location of your main Apache configuration " + "file relative to the server root")) args = parser.parse_args() # args.server_root XOR args.config_file if bool(args.server_root) != bool(args.config_file): - sys.exit( - "If either --server-root and --config-file are specified, they " - "both must be included" - ) + sys.exit("If either --server-root and --config-file are specified, " + "they both must be included") elif args.server_root and args.config_file: args.server_root = os.path.abspath(args.server_root) args.config_file = os.path.abspath(args.config_file) @@ -273,10 +269,8 @@ def get_args(): if args.config_file.startswith(args.server_root): args.config_file = args.config_file[len(args.server_root)+1:] else: - sys.exit( - "This script expects the Apache configuration file to be " - "inside the server root" - ) + sys.exit("This script expects the Apache configuration file to be " + "inside the server root") return args diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py index 835938dbe..7ae394de7 100644 --- a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py @@ -18,9 +18,12 @@ _PARTIAL_LINK_PATH = os.path.join("mods-enabled", "ssl.load") _CONFIG_FILE = pkg_resources.resource_filename( __name__, os.path.join("testdata", _PARTIAL_CONF_PATH)) _PASSWD_FILE = pkg_resources.resource_filename( - __name__, os.path.join("testdata", "passwd")) + __name__, os.path.join("testdata", "uncommonly_named_p4sswd")) _KEY_FILE = pkg_resources.resource_filename( - __name__, os.path.join("testdata", "key")) + __name__, os.path.join("testdata", "uncommonly_named_k3y")) +_SECRET_FILE = pkg_resources.resource_filename( + __name__, os.path.join("testdata", "super_secret_file.txt")) + _MODULE_NAME = "letshelp_letsencrypt.letshelp_letsencrypt_apache" @@ -78,6 +81,8 @@ class LetsHelpApacheTest(unittest.TestCase): temp_testdata, os.path.basename(_PASSWD_FILE)))) self.assertFalse(os.path.exists(os.path.join( temp_testdata, os.path.basename(_KEY_FILE)))) + self.assertFalse(os.path.exists(os.path.join( + temp_testdata, os.path.basename(_SECRET_FILE)))) self.assertTrue(os.path.exists(os.path.join( temp_testdata, _PARTIAL_CONF_PATH))) self.assertTrue(os.path.exists(os.path.join( @@ -92,19 +97,21 @@ class LetsHelpApacheTest(unittest.TestCase): for original_line, copied_line in zip(original, copy): self.assertEqual(original_line, copied_line) - @mock.patch(_MODULE_NAME + ".subprocess.check_output") - def test_safe_config_file(self, mock_check_output): - mock_check_output.return_value = "PEM RSA private key" + @mock.patch(_MODULE_NAME + ".subprocess.Popen") + def test_safe_config_file(self, mock_popen): + mock_popen().communicate.return_value = ("PEM RSA private key", None) self.assertFalse(letshelp_le_apache.safe_config_file("filename")) - mock_check_output.return_value = "ASCII text" + mock_popen().communicate.return_value = ("ASCII text", None) self.assertFalse(letshelp_le_apache.safe_config_file(_PASSWD_FILE)) self.assertFalse(letshelp_le_apache.safe_config_file(_KEY_FILE)) + self.assertFalse(letshelp_le_apache.safe_config_file(_SECRET_FILE)) self.assertTrue(letshelp_le_apache.safe_config_file(_CONFIG_FILE)) - @mock.patch(_MODULE_NAME + ".subprocess.check_output") - def test_tempdir(self, mock_check_output): - mock_check_output.side_effect = ["version", "modules", "vhosts"] + @mock.patch(_MODULE_NAME + ".subprocess.Popen") + def test_tempdir(self, mock_popen): + mock_popen().communicate.side_effect = [ + ("version", None), ("modules", None), ("vhosts", None)] args = _get_args() tempdir = letshelp_le_apache.setup_tempdir(args) @@ -131,10 +138,10 @@ class LetsHelpApacheTest(unittest.TestCase): self.assertRaises(SystemExit, letshelp_le_apache.verify_config, args) self.assertRaises(SystemExit, letshelp_le_apache.verify_config, args) - @mock.patch(_MODULE_NAME + ".subprocess.check_output") - def test_locate_config(self, mock_check_output): - mock_check_output.side_effect = [OSError, "bad_output", - _COMPILE_SETTINGS,] + @mock.patch(_MODULE_NAME + ".subprocess.Popen") + def test_locate_config(self, mock_popen): + mock_popen().communicate.side_effect = [ + OSError, ("bad_output", None), (_COMPILE_SETTINGS, None),] self.assertRaises( SystemExit, letshelp_le_apache.locate_config, "ctl") diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/super_secret_file.txt b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/super_secret_file.txt new file mode 100644 index 000000000..9f592eb7d --- /dev/null +++ b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/super_secret_file.txt @@ -0,0 +1 @@ +hunter2 diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/key b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/uncommonly_named_k3y similarity index 100% rename from letshelp-letsencrypt/letshelp_letsencrypt/testdata/key rename to letshelp-letsencrypt/letshelp_letsencrypt/testdata/uncommonly_named_k3y diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/testdata/passwd b/letshelp-letsencrypt/letshelp_letsencrypt/testdata/uncommonly_named_p4sswd similarity index 100% rename from letshelp-letsencrypt/letshelp_letsencrypt/testdata/passwd rename to letshelp-letsencrypt/letshelp_letsencrypt/testdata/uncommonly_named_p4sswd From 50ce91b76944315469710bc2105ac112c6f2c07f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 28 Jul 2015 17:23:03 -0700 Subject: [PATCH 5/9] Wrapped tarfile for py26 --- .../letshelp_letsencrypt/letshelp_letsencrypt_apache.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py index 2d0514a62..c6b1fbfaf 100755 --- a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py @@ -1,6 +1,7 @@ #!/usr/bin/env python """Let's Encrypt Apache configuration submission script""" import argparse +import contextlib import os import subprocess import sys @@ -286,7 +287,8 @@ def main(): make_and_verify_selection(args.server_root, tempdir) tarpath = os.path.join(tempdir, "config.tar.gz") - with tarfile.open(tarpath, mode="w:gz") as tar: + # contextlib.closing used for py26 support + with contextlib.closing(tarfile.open(tarpath, mode="w:gz")) as tar: tar.add(tempdir, arcname=".") # Submit tarpath From 6bbc41274838c421f054f42092d7ba81433a2ac8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 29 Jul 2015 09:40:49 -0700 Subject: [PATCH 6/9] Incorporated Schoen's feedback and really fixed py26 support?... --- .../letshelp_letsencrypt_apache.py | 15 ++++++++++----- .../letshelp_letsencrypt_apache_test.py | 4 ++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py index c6b1fbfaf..94ba979d9 100755 --- a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py @@ -1,8 +1,11 @@ #!/usr/bin/env python """Let's Encrypt Apache configuration submission script""" import argparse +import atexit import contextlib import os +import re +import shutil import subprocess import sys import tarfile @@ -31,8 +34,9 @@ argument and the path to the binary. # Keywords likely to be found in filenames of sensitive files -_SENSITIVE_KEYWORDS = ["private", "secret", "cert", "crt", "key", "pem", "der", - "rsa", "dsa", "pass", "pw"] +_SENSITIVE_FILENAME_REGEX = re.compile(r"^(?!.*proxy_fdpass).*pass.*$|private|" + r"secret|cert|crt|key|\.pem|\.der|rsa|" + r"dsa|pw") def make_and_verify_selection(server_root, temp_dir): @@ -131,9 +135,9 @@ def safe_config_file(config_file): :rtype: bool """ - for keyword in _SENSITIVE_KEYWORDS: - if keyword in config_file: - return False + config_file_lower = config_file.lower() + if _SENSITIVE_FILENAME_REGEX.search(config_file_lower): + return False proc = subprocess.Popen(["file", config_file], stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -284,6 +288,7 @@ def main(): verify_config(args) tempdir = setup_tempdir(args) + atexit.register(lambda: shutil.rmtree(tempdir)) make_and_verify_selection(args.server_root, tempdir) tarpath = os.path.join(tempdir, "config.tar.gz") diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py index 7ae394de7..c514118fb 100644 --- a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py @@ -206,8 +206,8 @@ class LetsHelpApacheTest(unittest.TestCase): testdir = tar.next() self.assertTrue(testdir.isdir()) - testdir_path = os.path.join(".", testdir_basename) - self.assertEqual(testdir.name, testdir_path) + self.assertEqual(os.path.basename(testdir.name), + testdir_basename) self.assertEqual(tar.next(), None) From 6d6c6f284badd953e462da439e737d6c6baa9911 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 29 Jul 2015 11:47:56 -0700 Subject: [PATCH 7/9] Incorporated Kuba's feedback --- ...tshelp_letsencrypt_apache.py => apache.py} | 24 +++++++++---------- ...sencrypt_apache_test.py => apache_test.py} | 4 ++-- letshelp-letsencrypt/setup.py | 13 +++++++++- 3 files changed, 26 insertions(+), 15 deletions(-) rename letshelp-letsencrypt/letshelp_letsencrypt/{letshelp_letsencrypt_apache.py => apache.py} (95%) rename letshelp-letsencrypt/letshelp_letsencrypt/{letshelp_letsencrypt_apache_test.py => apache_test.py} (98%) diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py b/letshelp-letsencrypt/letshelp_letsencrypt/apache.py similarity index 95% rename from letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py rename to letshelp-letsencrypt/letshelp_letsencrypt/apache.py index 94ba979d9..046542641 100755 --- a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/apache.py @@ -83,7 +83,7 @@ def copy_config(server_root, temp_dir): :rtype: `tuple` of `list` of `str` """ - copied_files, copied_dirs = list(), list() + copied_files, copied_dirs = [], [] dir_len = len(os.path.dirname(server_root)) for config_path, config_dirs, config_files in os.walk(server_root): @@ -91,7 +91,7 @@ def copy_config(server_root, temp_dir): os.mkdir(temp_path) copied_all = True - copied_files_in_current_dir = list() + copied_files_in_current_dir = [] for config_file in config_files: config_file_path = os.path.join(config_path, config_file) temp_file_path = os.path.join(temp_path, config_file) @@ -202,15 +202,15 @@ def verify_config(args): :param argparse.Namespace args: Parsed command line arguments """ - try: - with open(os.devnull, "w") as devnull: + with open(os.devnull, "w") as devnull: + try: subprocess.check_call([args.apache_ctl, "-d", args.server_root, "-f", args.config_file, "-t"], stdout=devnull, stderr=subprocess.STDOUT) - except OSError: - sys.exit(_NO_APACHECTL) - except subprocess.CalledProcessError: - sys.exit("Syntax check from apachectl failed") + except OSError: + sys.exit(_NO_APACHECTL) + except subprocess.CalledProcessError: + sys.exit("Syntax check from apachectl failed") def locate_config(apache_ctl): @@ -234,9 +234,9 @@ def locate_config(apache_ctl): for line in output.splitlines(): # Relevant output lines are of the form: -D DIRECTIVE="VALUE" if "HTTPD_ROOT" in line: - server_root = line[line.find("\"")+1:-1] + server_root = line[line.find('"')+1:-1] elif "SERVER_CONFIG_FILE" in line: - config_file = line[line.find("\"")+1:-1] + config_file = line[line.find('"')+1:-1] if not (server_root and config_file): sys.exit("Unable to locate Apache configuration. Please run this " @@ -283,7 +283,7 @@ def get_args(): def main(): """Main script execution""" args = get_args() - if not args.server_root: + if args.server_root is None: args.server_root, args.config_file = locate_config(args.apache_ctl) verify_config(args) @@ -296,7 +296,7 @@ def main(): with contextlib.closing(tarfile.open(tarpath, mode="w:gz")) as tar: tar.add(tempdir, arcname=".") - # Submit tarpath + # TODO: Submit tarpath if __name__ == "__main__": diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py b/letshelp-letsencrypt/letshelp_letsencrypt/apache_test.py similarity index 98% rename from letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py rename to letshelp-letsencrypt/letshelp_letsencrypt/apache_test.py index c514118fb..e1012797a 100644 --- a/letshelp-letsencrypt/letshelp_letsencrypt/letshelp_letsencrypt_apache_test.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/apache_test.py @@ -10,7 +10,7 @@ import unittest import mock -import letshelp_letsencrypt.letshelp_letsencrypt_apache as letshelp_le_apache +import letshelp_letsencrypt.apache as letshelp_le_apache _PARTIAL_CONF_PATH = os.path.join("mods-available", "ssl.load") @@ -25,7 +25,7 @@ _SECRET_FILE = pkg_resources.resource_filename( __name__, os.path.join("testdata", "super_secret_file.txt")) -_MODULE_NAME = "letshelp_letsencrypt.letshelp_letsencrypt_apache" +_MODULE_NAME = "letshelp_letsencrypt.apache" _COMPILE_SETTINGS = """Server version: Apache/2.4.10 (Debian) diff --git a/letshelp-letsencrypt/setup.py b/letshelp-letsencrypt/setup.py index 58b28bc7e..95c3e5c74 100644 --- a/letshelp-letsencrypt/setup.py +++ b/letshelp-letsencrypt/setup.py @@ -1,11 +1,22 @@ +import sys + from setuptools import setup from setuptools import find_packages -install_requires = ["mock<1.1.0",] +install_requires = [] +if sys.version_info < (2, 7): + install_requires.append("mock<1.1.0") +else: + install_requires.append("mock") setup( name="letshelp-letsencrypt", packages=find_packages(), install_requires=install_requires, + entry_points={ + 'console_scripts': [ + "letshelp-letsencrypt-apache = letshelp_letsencrypt.apache:main", + ], + }, ) From 46d8b163be193af4a60c15697f17eedcde575f4d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 30 Jul 2015 07:25:41 -0700 Subject: [PATCH 8/9] Added include_package_data=True --- letshelp-letsencrypt/setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letshelp-letsencrypt/setup.py b/letshelp-letsencrypt/setup.py index 95c3e5c74..6b89a6d09 100644 --- a/letshelp-letsencrypt/setup.py +++ b/letshelp-letsencrypt/setup.py @@ -19,4 +19,5 @@ setup( "letshelp-letsencrypt-apache = letshelp_letsencrypt.apache:main", ], }, + include_package_data=True, ) From 2605d5297c519eb7a9de0c7566cf0d6f120f0482 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 4 Aug 2015 12:24:50 -0700 Subject: [PATCH 9/9] Added check for pkcs format --- letshelp-letsencrypt/letshelp_letsencrypt/apache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/apache.py b/letshelp-letsencrypt/letshelp_letsencrypt/apache.py index 046542641..3b3ab31e7 100755 --- a/letshelp-letsencrypt/letshelp_letsencrypt/apache.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/apache.py @@ -35,8 +35,8 @@ argument and the path to the binary. # Keywords likely to be found in filenames of sensitive files _SENSITIVE_FILENAME_REGEX = re.compile(r"^(?!.*proxy_fdpass).*pass.*$|private|" - r"secret|cert|crt|key|\.pem|\.der|rsa|" - r"dsa|pw") + r"secret|cert|crt|key|rsa|dsa|pw|\.pem|" + r"\.der|\.p12|\.pfx|\.p7b") def make_and_verify_selection(server_root, temp_dir):