Fixed py26 support, cleanup, and added filename check

letshelp-letsencrypt/MANIFEST.in

@@ -0,0 +1 @@
+recursive-include letshelp-letsencrypt/testdata *

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: ")
+        ans = raw_input("(Y)es/(N)o: ").lower()
-        if ans.startswith("Y") or ans.startswith("y"):
+        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(
+    for keyword in _SENSITIVE_KEYWORDS:
-        ["file", config_file], close_fds=True, stderr=open(os.devnull, "w")
+        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(
+        version_fd.write(proc.communicate()[0])
-            [args.apache_ctl, "-v"], close_fds=True,
-            stderr=open(os.devnull, "w")
-        ))
 
+    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(
+        modules_fd.write(proc.communicate()[0])
-            [args.apache_ctl, "-d", args.server_root,
-             "-f", args.config_file, "-M"], close_fds=True,
-            stderr=open(os.devnull, "w")
-        ))
 
+    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(
+        vhosts_fd.write(proc.communicate()[0])
-            [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
 
@@ -192,11 +198,10 @@ def verify_config(args):
 
     """
     try:
-        subprocess.check_call(
+        with open(os.devnull, "w") as devnull:
-            [args.apache_ctl, "-d", args.server_root,
+            subprocess.check_call([args.apache_ctl, "-d", args.server_root,
-             "-f", args.config_file, "-t"], close_fds=True,
+                                   "-f", args.config_file, "-t"],
-            stdout=open(os.devnull, "w"), stderr=open(os.devnull, "w")
+                                  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(
+        proc = subprocess.Popen([apache_ctl, "-V"],
-            [apache_ctl, "-V"], close_fds=True, stderr=open(os.devnull, "w")
+                                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(
+        sys.exit("Unable to locate Apache configuration. Please run this "
-            "Unable to locate Apache configuration. Please run this script "
+                 "script again and specify --server-root and --config-file")
-            "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(
+    parser.add_argument("-c", "--apache-ctl", default="apachectl",
-        "-c", "--apache-ctl", default="apachectl",
+                        help="path to the `apachectl` binary")
-        help="path to the `apachectl` binary"
+    parser.add_argument("-d", "--server-root",
-    )
+                        help=("location of the root directory of your Apache "
-    parser.add_argument(
+                              "configuration"))
-        "-d", "--server-root",
+    parser.add_argument("-f", "--config-file",
-        help="location of the root directory of your Apache configuration"
+                        help=("location of your main Apache configuration "
-    )
+                              "file relative to the server root"))
-    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(
+        sys.exit("If either --server-root and --config-file are specified, "
-            "If either --server-root and --config-file are specified, they "
+                 "they both must be included")
-            "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(
+            sys.exit("This script expects the Apache configuration file to be "
-                "This script expects the Apache configuration file to be "
+                     "inside the server root")
-                "inside the server root"
-            )
 
     return args
 

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")
+    @mock.patch(_MODULE_NAME + ".subprocess.Popen")
-    def test_safe_config_file(self, mock_check_output):
+    def test_safe_config_file(self, mock_popen):
-        mock_check_output.return_value = "PEM RSA private key"
+        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")
+    @mock.patch(_MODULE_NAME + ".subprocess.Popen")
-    def test_tempdir(self, mock_check_output):
+    def test_tempdir(self, mock_popen):
-        mock_check_output.side_effect = ["version", "modules", "vhosts"]
+        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")
+    @mock.patch(_MODULE_NAME + ".subprocess.Popen")
-    def test_locate_config(self, mock_check_output):
+    def test_locate_config(self, mock_popen):
-        mock_check_output.side_effect = [OSError, "bad_output",
+        mock_popen().communicate.side_effect = [
-                                         _COMPILE_SETTINGS,]
+            OSError, ("bad_output", None), (_COMPILE_SETTINGS, None),]
 
         self.assertRaises(
             SystemExit, letshelp_le_apache.locate_config, "ctl")

letshelp-letsencrypt/letshelp_letsencrypt/testdata/super_secret_file.txt

@@ -0,0 +1 @@
+hunter2