Merge branch 'master' into save-more-hooks

.travis.yml

@@ -17,12 +17,7 @@ env:
 matrix:
   include:
     - python: "2.7"
-      env: TOXENV=cover BOULDER_INTEGRATION=1
-      sudo: required
-      after_failure:
-        - sudo cat /var/log/mysql/error.log
-        - ps aux | grep mysql
-      services: docker
+      env: TOXENV=cover
     - python: "2.7"
       env: TOXENV=lint
     - python: "2.7"
@@ -46,6 +41,13 @@ matrix:
         - sudo cat /var/log/mysql/error.log
         - ps aux | grep mysql
       services: docker
+    - python: "2.7"
+      env: TOXENV=py27_install BOULDER_INTEGRATION=1
+      sudo: required
+      after_failure:
+        - sudo cat /var/log/mysql/error.log
+        - ps aux | grep mysql
+      services: docker
     - sudo: required
       env: TOXENV=apache_compat
       services: docker

certbot-apache/certbot_apache/configurator.py

@@ -1,6 +1,7 @@
 """Apache Configuration based off of Augeas Configurator."""
 # pylint: disable=too-many-lines
 import filecmp
+import fnmatch
 import logging
 import os
 import re
@@ -362,18 +363,24 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         return vhost
 
     def included_in_wildcard(self, names, target_name):
-        """Helper function to see if alias is covered by wildcard"""
-        target_name = target_name.split(".")[::-1]
-        wildcards = [domain.split(".")[1:] for domain in
-                     names if domain.startswith("*")]
-        for wildcard in wildcards:
-            if len(wildcard) > len(target_name):
-                continue
-            for idx, segment in enumerate(wildcard[::-1]):
-                if segment != target_name[idx]:
-                    break
-            else:
-                # https://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops
+        """Is target_name covered by a wildcard?
+
+        :param names: server aliases
+        :type names: `collections.Iterable` of `str`
+        :param str target_name: name to compare with wildcards
+
+        :returns: True if target_name is covered by a wildcard,
+            otherwise, False
+        :rtype: bool
+
+        """
+        # use lowercase strings because fnmatch can be case sensitive
+        target_name = target_name.lower()
+        for name in names:
+            name = name.lower()
+            # fnmatch treats "[seq]" specially and [ or ] characters aren't
+            # valid in Apache but Apache doesn't error out if they are present
+            if "[" not in name and fnmatch.fnmatch(target_name, name):
                 return True
         return False
 
@@ -463,7 +470,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             zope.component.getUtility(interfaces.IDisplay).notification(
                 "Apache mod_macro seems to be in use in file(s):\n{0}"
                 "\n\nUnfortunately mod_macro is not yet supported".format(
-                    "\n  ".join(vhost_macro)))
+                    "\n  ".join(vhost_macro)), force_interactive=True)
 
         return all_names
 
@@ -1012,6 +1019,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             self.parser.find_dir("ServerAlias", target_name,
                                  start=vh_path, exclude=False)):
             return
+        if self._has_matching_wildcard(vh_path, target_name):
+            return
         if not self.parser.find_dir("ServerName", None,
                                     start=vh_path, exclude=False):
             self.parser.add_dir(vh_path, "ServerName", target_name)
@@ -1019,6 +1028,22 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             self.parser.add_dir(vh_path, "ServerAlias", target_name)
         self._add_servernames(vhost)
 
+    def _has_matching_wildcard(self, vh_path, target_name):
+        """Is target_name already included in a wildcard in the vhost?
+
+        :param str vh_path: Augeas path to the vhost
+        :param str target_name: name to compare with wildcards
+
+        :returns: True if there is a wildcard covering target_name in
+            the vhost in vhost_path, otherwise, False
+        :rtype: bool
+
+        """
+        matches = self.parser.find_dir(
+            "ServerAlias", start=vh_path, exclude=False)
+        aliases = (self.aug.get(match) for match in matches)
+        return self.included_in_wildcard(aliases, target_name)
+
     def _add_name_vhost_if_necessary(self, vhost):
         """Add NameVirtualHost Directives if necessary for new vhost.
 

certbot-apache/certbot_apache/display_ops.py

@@ -85,7 +85,8 @@ def _vhost_menu(domain, vhosts):
             "or Address of {0}.{1}Which virtual host would you "
             "like to choose?\n(note: conf files with multiple "
             "vhosts are not yet supported)".format(domain, os.linesep),
-            choices, help_label="More Info", ok_label="Select")
+            choices, help_label="More Info",
+            ok_label="Select", force_interactive=True)
     except errors.MissingCommandlineFlag:
         msg = ("Encountered vhost ambiguity but unable to ask for user guidance in "
                "non-interactive mode. Currently Certbot needs each vhost to be "
@@ -100,4 +101,5 @@ def _vhost_menu(domain, vhosts):
 def _more_info_vhost(vhost):
     zope.component.getUtility(interfaces.IDisplay).notification(
         "Virtual Host Information:{0}{1}{0}{2}".format(
-            os.linesep, "-" * (display_util.WIDTH - 4), str(vhost)))
+            os.linesep, "-" * (display_util.WIDTH - 4), str(vhost)),
+        force_interactive=True)

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -220,10 +220,6 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertRaises(
             errors.PluginError, self.config.choose_vhost, "none.com")
 
-    def test_choosevhost_select_vhost_with_wildcard(self):
-        chosen_vhost = self.config.choose_vhost("blue.purple.com", temp=True)
-        self.assertEqual(self.vh_truth[6], chosen_vhost)
-
     def test_findbest_continues_on_short_domain(self):
         # pylint: disable=protected-access
         chosen_vhost = self.config._find_best_vhost("purple.com")
@@ -1255,8 +1251,6 @@ class AugeasVhostsTest(util.ApacheTest):
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir)
-        self.vh_truth = util.get_vh_truth(
-            self.temp_dir, "debian_apache_2_4/augeas_vhosts")
 
     def tearDown(self):
         shutil.rmtree(self.temp_dir)
@@ -1281,5 +1275,41 @@ class AugeasVhostsTest(util.ApacheTest):
         vhs = self.config.get_virtual_hosts()
         self.assertEqual([], vhs)
 
+    def test_choose_vhost_with_matching_wildcard(self):
+        names = (
+            "an.example.net", "another.example.net", "an.other.example.net")
+        for name in names:
+            self.assertFalse(name in self.config.choose_vhost(name).aliases)
+
+    def test_choose_vhost_without_matching_wildcard(self):
+        mock_path = "certbot_apache.display_ops.select_vhost"
+        with mock.patch(mock_path, lambda _, vhosts: vhosts[0]):
+            for name in ("a.example.net", "other.example.net"):
+                self.assertTrue(name in self.config.choose_vhost(name).aliases)
+
+    def test_choose_vhost_wildcard_not_found(self):
+        mock_path = "certbot_apache.display_ops.select_vhost"
+        names = (
+            "abc.example.net", "not.there.tld", "aa.wildcard.tld"
+        )
+        with mock.patch(mock_path) as mock_select:
+            mock_select.return_value = self.config.vhosts[0]
+            for name in names:
+                orig_cc = mock_select.call_count
+                self.config.choose_vhost(name)
+                self.assertEqual(mock_select.call_count - orig_cc, 1)
+
+    def test_choose_vhost_wildcard_found(self):
+        mock_path = "certbot_apache.display_ops.select_vhost"
+        names = (
+            "ab.example.net", "a.wildcard.tld", "yetanother.example.net"
+        )
+        with mock.patch(mock_path) as mock_select:
+            mock_select.return_value = self.config.vhosts[0]
+            for name in names:
+                self.config.choose_vhost(name)
+                self.assertEqual(mock_select.call_count, 0)
+
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot-apache/certbot_apache/tests/display_ops_test.py

@@ -17,7 +17,8 @@ class SelectVhostTest(unittest.TestCase):
     """Tests for certbot_apache.display_ops.select_vhost."""
 
     def setUp(self):
-        zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
+        zope.component.provideUtility(display_util.FileDisplay(sys.stdout,
+                                                               False))
         self.base_dir = "/example_path"
         self.vhosts = util.get_vh_truth(
             self.base_dir, "debian_apache_2_4/multiple_vhosts")

certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/augeas_vhosts/apache2/sites-available/another_wildcard.conf

@@ -0,0 +1,11 @@
+<VirtualHost *:80>
+	ServerName wildcard.tld
+	ServerAlias ?.wildcard.tld
+	ServerAdmin webmaster@localhost
+	DocumentRoot /var/www/html
+
+	ErrorLog ${APACHE_LOG_DIR}/error.log
+	CustomLog ${APACHE_LOG_DIR}/access.log combined
+</VirtualHost>
+
+# vim: syntax=apache ts=4 sw=4 sts=4 sr noet

certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/augeas_vhosts/apache2/sites-available/wildcard.conf

@@ -0,0 +1,11 @@
+<VirtualHost *:80>
+	ServerName example.net
+	ServerAlias ??.example.net *.other.example.net *another.example.net
+	ServerAdmin webmaster@localhost
+	DocumentRoot /var/www/html
+
+	ErrorLog ${APACHE_LOG_DIR}/error.log
+	CustomLog ${APACHE_LOG_DIR}/access.log combined
+</VirtualHost>
+
+# vim: syntax=apache ts=4 sw=4 sts=4 sr noet

certbot-apache/certbot_apache/tests/util.py

@@ -64,7 +64,8 @@ class ParserTest(ApacheTest):  # pytlint: disable=too-few-public-methods
               vhost_root="debian_apache_2_4/multiple_vhosts/apache2/sites-available"):
         super(ParserTest, self).setUp(test_dir, config_root, vhost_root)
 
-        zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
+        zope.component.provideUtility(display_util.FileDisplay(sys.stdout,
+                                                               False))
 
         from certbot_apache.parser import ApacheParser
         self.aug = augeas.Augeas(

certbot-nginx/certbot_nginx/obj.py

@@ -29,10 +29,14 @@ class Addr(common.Addr):
     :param bool default: Whether the directive includes 'default_server'
 
     """
+    UNSPECIFIED_IPV4_ADDRESSES = ('', '*', '0.0.0.0')
+    CANONICAL_UNSPECIFIED_ADDRESS = UNSPECIFIED_IPV4_ADDRESSES[0]
+
     def __init__(self, host, port, ssl, default):
         super(Addr, self).__init__((host, port))
         self.ssl = ssl
         self.default = default
+        self.unspecified_address = host in self.UNSPECIFIED_IPV4_ADDRESSES
 
     @classmethod
     def fromstring(cls, str_addr):
@@ -96,6 +100,13 @@ class Addr(common.Addr):
     def super_eq(self, other):
         """Check ip/port equality, with IPv6 support.
         """
+        # If both addresses got an unspecified address, then make sure the
+        # host representation in each match when doing the comparison.
+        if self.unspecified_address and other.unspecified_address:
+            return common.Addr((self.CANONICAL_UNSPECIFIED_ADDRESS,
+                                self.tup[1]), self.ipv6) == \
+                   common.Addr((other.CANONICAL_UNSPECIFIED_ADDRESS,
+                                other.tup[1]), other.ipv6)
         # Nginx plugin currently doesn't support IPv6 but this will
         # future-proof it
         return super(Addr, self).__eq__(other)

certbot-nginx/certbot_nginx/tests/obj_test.py

@@ -1,5 +1,6 @@
 """Test the helper objects in certbot_nginx.obj."""
 import unittest
+import itertools
 
 
 class AddrTest(unittest.TestCase):
@@ -72,6 +73,24 @@ class AddrTest(unittest.TestCase):
         self.assertNotEqual(self.addr1, self.addr2)
         self.assertFalse(self.addr1 == 3333)
 
+    def test_equivalent_any_addresses(self):
+        from certbot_nginx.obj import Addr
+        any_addresses = ("0.0.0.0:80 default_server ssl",
+                         "80 default_server ssl",
+                         "*:80 default_server ssl")
+        for first, second in itertools.combinations(any_addresses, 2):
+            self.assertEqual(Addr.fromstring(first), Addr.fromstring(second))
+
+        # Also, make sure ports are checked.
+        self.assertNotEqual(Addr.fromstring(any_addresses[0]),
+                            Addr.fromstring("0.0.0.0:443 default_server ssl"))
+
+        # And they aren't equivalent to a specified address.
+        for any_address in any_addresses:
+            self.assertNotEqual(
+                Addr.fromstring("192.168.1.2:80 default_server ssl"),
+                Addr.fromstring(any_address))
+
     def test_set_inclusion(self):
         from certbot_nginx.obj import Addr
         set_a = set([self.addr1, self.addr2])

certbot/cert_manager.py

@@ -6,10 +6,9 @@ import pytz
 import traceback
 import zope.component
 
-from certbot import configuration
 from certbot import errors
 from certbot import interfaces
-from certbot import renewal
+from certbot import ocsp
 from certbot import storage
 from certbot import util
 
@@ -17,6 +16,10 @@ from certbot.display import util as display_util
 
 logger = logging.getLogger(__name__)
 
+###################
+# Commands
+###################
+
 def update_live_symlinks(config):
     """Update the certificate file family symlinks to use archive_dir.
 
@@ -26,41 +29,28 @@ def update_live_symlinks(config):
     .. note:: This assumes that the installation is using a Reverter object.
 
     :param config: Configuration.
-    :type config: :class:`certbot.interfaces.IConfig`
+    :type config: :class:`certbot.configuration.NamespaceConfig`
 
     """
-    renewer_config = configuration.RenewerConfiguration(config)
-    for renewal_file in renewal.renewal_conf_files(renewer_config):
-        storage.RenewableCert(renewal_file,
-            configuration.RenewerConfiguration(renewer_config),
-            update_symlinks=True)
+    for renewal_file in storage.renewal_conf_files(config):
+        storage.RenewableCert(renewal_file, config, update_symlinks=True)
 
 def rename_lineage(config):
     """Rename the specified lineage to the new name.
 
     :param config: Configuration.
-    :type config: :class:`certbot.interfaces.IConfig`
+    :type config: :class:`certbot.configuration.NamespaceConfig`
 
     """
     disp = zope.component.getUtility(interfaces.IDisplay)
-    renewer_config = configuration.RenewerConfiguration(config)
 
-    certname = config.certname
-    if not certname:
-        filenames = renewal.renewal_conf_files(renewer_config)
-        choices = [storage.lineagename_for_filename(name) for name in filenames]
-        if not choices:
-            raise errors.Error("No existing certificates found.")
-        code, index = disp.menu("Which certificate would you like to rename?",
-            choices, ok_label="Select", flag="--cert-name")
-        if code != display_util.OK or not index in range(0, len(choices)):
-            raise errors.Error("User ended interaction.")
-        certname = choices[index]
+    certname = _get_certname(config, "rename")
 
     new_certname = config.new_certname
     if not new_certname:
-        code, new_certname = disp.input("Enter the new name for certificate {0}"
-            .format(certname), flag="--updated-cert-name")
+        code, new_certname = disp.input(
+            "Enter the new name for certificate {0}".format(certname),
+            flag="--updated-cert-name", force_interactive=True)
         if code != display_util.OK or not new_certname:
             raise errors.Error("User ended interaction.")
 
@@ -68,77 +58,21 @@ def rename_lineage(config):
     if not lineage:
         raise errors.ConfigurationError("No existing certificate with name "
             "{0} found.".format(certname))
-    storage.rename_renewal_config(certname, new_certname, renewer_config)
+    storage.rename_renewal_config(certname, new_certname, config)
     disp.notification("Successfully renamed {0} to {1}."
         .format(certname, new_certname), pause=False)
 
-def _report_lines(msgs):
-    """Format a results report for a category of single-line renewal outcomes"""
-    return "  " + "\n  ".join(str(msg) for msg in msgs)
-
-def _report_human_readable(parsed_certs):
-    """Format a results report for a parsed cert"""
-    certinfo = []
-    for cert in parsed_certs:
-        now = pytz.UTC.fromutc(datetime.datetime.utcnow())
-        if cert.is_test_cert:
-            expiration_text = "INVALID: TEST CERT"
-        elif cert.target_expiry <= now:
-            expiration_text = "INVALID: EXPIRED"
-        else:
-            diff = cert.target_expiry - now
-            if diff.days == 1:
-                expiration_text = "VALID: 1 day"
-            elif diff.days < 1:
-                expiration_text = "VALID: {0} hour(s)".format(diff.seconds // 3600)
-            else:
-                expiration_text = "VALID: {0} days".format(diff.days)
-        valid_string = "{0} ({1})".format(cert.target_expiry, expiration_text)
-        certinfo.append("  Certificate Name: {0}\n"
-                        "    Domains: {1}\n"
-                        "    Expiry Date: {2}\n"
-                        "    Certificate Path: {3}\n"
-                        "    Private Key Path: {4}".format(
-                            cert.lineagename,
-                            " ".join(cert.names()),
-                            valid_string,
-                            cert.fullchain,
-                            cert.privkey))
-    return "\n".join(certinfo)
-
-def _describe_certs(parsed_certs, parse_failures):
-    """Print information about the certs we know about"""
-    out = []
-
-    notify = out.append
-
-    if not parsed_certs and not parse_failures:
-        notify("No certs found.")
-    else:
-        if parsed_certs:
-            notify("Found the following certs:")
-            notify(_report_human_readable(parsed_certs))
-        if parse_failures:
-            notify("\nThe following renewal configuration files "
-               "were invalid:")
-            notify(_report_lines(parse_failures))
-
-    disp = zope.component.getUtility(interfaces.IDisplay)
-    disp.notification("\n".join(out), pause=False, wrap=False)
-
 def certificates(config):
     """Display information about certs configured with Certbot
 
     :param config: Configuration.
-    :type config: :class:`certbot.interfaces.IConfig`
+    :type config: :class:`certbot.configuration.NamespaceConfig`
     """
-    renewer_config = configuration.RenewerConfiguration(config)
     parsed_certs = []
     parse_failures = []
-    for renewal_file in renewal.renewal_conf_files(renewer_config):
+    for renewal_file in storage.renewal_conf_files(config):
         try:
-            renewal_candidate = storage.RenewableCert(renewal_file,
-                configuration.RenewerConfiguration(config))
+            renewal_candidate = storage.RenewableCert(renewal_file, config)
             parsed_certs.append(renewal_candidate)
         except Exception as e:  # pylint: disable=broad-except
             logger.warning("Renewal configuration file %s produced an "
@@ -147,29 +81,19 @@ def certificates(config):
             parse_failures.append(renewal_file)
 
     # Describe all the certs
-    _describe_certs(parsed_certs, parse_failures)
+    _describe_certs(config, parsed_certs, parse_failures)
 
-def _search_lineages(config, func, initial_rv):
-    """Iterate func over unbroken lineages, allowing custom return conditions.
+def delete(config):
+    """Delete Certbot files associated with a certificate lineage."""
+    certname = _get_certname(config, "delete")
+    storage.delete_files(config, certname)
+    disp = zope.component.getUtility(interfaces.IDisplay)
+    disp.notification("Deleted all files relating to certificate {0}."
+        .format(certname), pause=False)
 
-    Allows flexible customization of return values, including multiple
-    return values and complex checks.
-    """
-    cli_config = configuration.RenewerConfiguration(config)
-    configs_dir = cli_config.renewal_configs_dir
-    # Verify the directory is there
-    util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid())
-
-    rv = initial_rv
-    for renewal_file in renewal.renewal_conf_files(cli_config):
-        try:
-            candidate_lineage = storage.RenewableCert(renewal_file, cli_config)
-        except (errors.CertStorageError, IOError):
-            logger.debug("Renewal conf file %s is broken. Skipping.", renewal_file)
-            logger.debug("Traceback was:\n%s", traceback.format_exc())
-            continue
-        rv = func(candidate_lineage, rv)
-    return rv
+###################
+# Public Helpers
+###################
 
 def lineage_for_certname(config, certname):
     """Find a lineage object with name certname."""
@@ -215,3 +139,114 @@ def find_duplicative_certs(config, domains):
         return (identical_names_cert, subset_names_cert)
 
     return _search_lineages(config, update_certs_for_domain_matches, (None, None))
+
+
+###################
+# Private Helpers
+###################
+
+def _get_certname(config, verb):
+    """Get certname from flag, interactively, or error out.
+    """
+    certname = config.certname
+    if not certname:
+        disp = zope.component.getUtility(interfaces.IDisplay)
+        filenames = storage.renewal_conf_files(config)
+        choices = [storage.lineagename_for_filename(name) for name in filenames]
+        if not choices:
+            raise errors.Error("No existing certificates found.")
+        code, index = disp.menu("Which certificate would you like to {0}?".format(verb),
+            choices, ok_label="Select", flag="--cert-name")
+        if code != display_util.OK or not index in range(0, len(choices)):
+            raise errors.Error("User ended interaction.")
+        certname = choices[index]
+    return certname
+
+def _report_lines(msgs):
+    """Format a results report for a category of single-line renewal outcomes"""
+    return "  " + "\n  ".join(str(msg) for msg in msgs)
+
+def _report_human_readable(config, parsed_certs):
+    """Format a results report for a parsed cert"""
+    certinfo = []
+    checker = ocsp.RevocationChecker()
+    for cert in parsed_certs:
+        if config.certname and cert.lineagename != config.certname:
+            continue
+        if config.domains and not set(config.domains).issubset(cert.names()):
+            continue
+        now = pytz.UTC.fromutc(datetime.datetime.utcnow())
+
+        reasons = []
+        if cert.is_test_cert:
+            reasons.append('TEST_CERT')
+        if cert.target_expiry <= now:
+            reasons.append('EXPIRED')
+        if checker.ocsp_revoked(cert.cert, cert.chain):
+            reasons.append('REVOKED')
+
+        if reasons:
+            status = "INVALID: " + ", ".join(reasons)
+        else:
+            diff = cert.target_expiry - now
+            if diff.days == 1:
+                status = "VALID: 1 day"
+            elif diff.days < 1:
+                status = "VALID: {0} hour(s)".format(diff.seconds // 3600)
+            else:
+                status = "VALID: {0} days".format(diff.days)
+
+        valid_string = "{0} ({1})".format(cert.target_expiry, status)
+        certinfo.append("  Certificate Name: {0}\n"
+                        "    Domains: {1}\n"
+                        "    Expiry Date: {2}\n"
+                        "    Certificate Path: {3}\n"
+                        "    Private Key Path: {4}".format(
+                            cert.lineagename,
+                            " ".join(cert.names()),
+                            valid_string,
+                            cert.fullchain,
+                            cert.privkey))
+    return "\n".join(certinfo)
+
+def _describe_certs(config, parsed_certs, parse_failures):
+    """Print information about the certs we know about"""
+    out = []
+
+    notify = out.append
+
+    if not parsed_certs and not parse_failures:
+        notify("No certs found.")
+    else:
+        if parsed_certs:
+            match = "matching " if config.certname or config.domains else ""
+            notify("Found the following {0}certs:".format(match))
+            notify(_report_human_readable(config, parsed_certs))
+        if parse_failures:
+            notify("\nThe following renewal configuration files "
+               "were invalid:")
+            notify(_report_lines(parse_failures))
+
+    disp = zope.component.getUtility(interfaces.IDisplay)
+    disp.notification("\n".join(out), pause=False, wrap=False)
+
+def _search_lineages(cli_config, func, initial_rv):
+    """Iterate func over unbroken lineages, allowing custom return conditions.
+
+    Allows flexible customization of return values, including multiple
+    return values and complex checks.
+    """
+    configs_dir = cli_config.renewal_configs_dir
+    # Verify the directory is there
+    util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid())
+
+    rv = initial_rv
+    for renewal_file in storage.renewal_conf_files(cli_config):
+        try:
+            candidate_lineage = storage.RenewableCert(renewal_file, cli_config)
+        except (errors.CertStorageError, IOError):
+            logger.debug("Renewal conf file %s is broken. Skipping.", renewal_file)
+            logger.debug("Traceback was:\n%s", traceback.format_exc())
+            continue
+        rv = func(candidate_lineage, rv)
+    return rv

certbot/cli.py

@@ -50,51 +50,56 @@ cli_command = LEAUTO if fragment in sys.argv[0] else "certbot"
 # to replace as much of it as we can...
 
 # This is the stub to include in help generated by argparse
-
 SHORT_USAGE = """
-  {0} [SUBCOMMAND] [options] [-d domain] [-d domain] ...
+  {0} [SUBCOMMAND] [options] [-d DOMAIN] [-d DOMAIN] ...
 
 Certbot can obtain and install HTTPS/TLS/SSL certificates.  By default,
 it will attempt to use a webserver both for obtaining and installing the
-cert. Major SUBCOMMANDS are:
+cert. """.format(cli_command)
 
-  (default) run        Obtain & install a cert in your current webserver
-  certonly             Obtain cert, but do not install it (aka "auth")
-  install              Install a previously obtained cert in a server
-  renew                Renew previously obtained certs that are near expiry
-  revoke               Revoke a previously obtained certificate
-  register             Perform tasks related to registering with the CA
-  rollback             Rollback server configuration changes made during install
-  config_changes       Show changes made to server config during installation
-  update_symlinks      Update cert symlinks based on renewal config file
-  rename               Update a certificate's name
-  plugins              Display information about installed plugins
-  certificates         Display information about certs configured with Certbot
+# This section is used for --help and --help all ; it needs information
+# about installed plugins to be fully formatted
+COMMAND_OVERVIEW = """The most common SUBCOMMANDS and flags are:
 
-""".format(cli_command)
-
-# This is the short help for certbot --help, where we disable argparse
-# altogether
-USAGE = SHORT_USAGE + """Choice of server plugins for obtaining and installing cert:
+obtain, install, and renew certificates:
+    (default) run   Obtain & install a cert in your current webserver
+    certonly        Obtain or renew a cert, but do not install it
+    renew           Renew all previously obtained certs that are near expiry
+   -d DOMAINS       Comma-separated list of domains to obtain a cert for
 
   %s
   --standalone      Run a standalone webserver for authentication
   %s
   --webroot         Place files in a server's webroot folder for authentication
-  --script          User provided shell scripts for authentication
+  --manual          Obtain certs interactively, or using shell script hooks
 
-OR use different plugins to obtain (authenticate) the cert and then install it:
+   -n               Run non-interactively
+  --test-cert       Obtain a test cert from a staging server
+  --dry-run         Test "renew" or "certonly" without saving any certs to disk
 
-  --authenticator standalone --installer apache
+manage certificates:
+    certificates    Display information about certs you have from Certbot
+    revoke          Revoke a certificate (supply --cert-path)
+    rename          Rename a certificate
+    delete          Delete a certificate
 
+manage your account with Let's Encrypt:
+    register        Create a Let's Encrypt ACME account
+  --agree-tos       Agree to the ACME server's Subscriber Agreement
+   -m EMAIL         Email address for important account notifications
+"""
+
+# This is the short help for certbot --help, where we disable argparse
+# altogether
+HELP_USAGE = """
 More detailed help:
 
-  -h, --help [topic]    print this message, or detailed help on a topic;
-                        the available topics are:
+  -h, --help [TOPIC]    print this message, or detailed help on a topic;
+                        the available TOPICS are:
 
-   all, automation, paths, security, testing, or any of the subcommands or
-   plugins (certonly, renew, install, register, nginx, apache, standalone,
-   webroot, script, etc.)
+   all, automation, commands, paths, security, testing, or any of the
+   subcommands or plugins (certonly, renew, install, register, nginx,
+   apache, standalone, webroot, etc.)
 """
 
 
@@ -140,19 +145,6 @@ def report_config_interaction(modified, modifiers):
         VAR_MODIFIERS.setdefault(var, set()).update(modifiers)
 
 
-def usage_strings(plugins):
-    """Make usage strings late so that plugins can be initialised late"""
-    if "nginx" in plugins:
-        nginx_doc = "--nginx           Use the Nginx plugin for authentication & installation"
-    else:
-        nginx_doc = "(nginx support is experimental, buggy, and not installed by default)"
-    if "apache" in plugins:
-        apache_doc = "--apache          Use the Apache plugin for authentication & installation"
-    else:
-        apache_doc = "(the apache plugin is not installed)"
-    return USAGE % (apache_doc, nginx_doc), SHORT_USAGE
-
-
 def possible_deprecation_warning(config):
     "A deprecation warning for users with the old, not-self-upgrading letsencrypt-auto."
     if cli_command != LEAUTO:
@@ -308,6 +300,104 @@ class HelpfulArgumentGroup(object):
         """Add a new command line argument to the argument group."""
         self._parser.add(self._topic, *args, **kwargs)
 
+class CustomHelpFormatter(argparse.HelpFormatter):
+    """This is a clone of ArgumentDefaultsHelpFormatter, with bugfixes.
+
+    In particular we fix https://bugs.python.org/issue28742
+    """
+
+    def _get_help_string(self, action):
+        helpstr = action.help
+        if '%(default)' not in action.help and '(default:' not in action.help:
+            if action.default != argparse.SUPPRESS:
+                defaulting_nargs = [argparse.OPTIONAL, argparse.ZERO_OR_MORE]
+                if action.option_strings or action.nargs in defaulting_nargs:
+                    helpstr += ' (default: %(default)s)'
+        return helpstr
+
+# The attributes here are:
+# short: a string that will be displayed by "certbot -h commands"
+# opts:  a string that heads the section of flags with which this command is documented,
+#        both for "cerbot -h SUBCOMMAND" and "certbot -h all"
+# usage: an optional string that overrides the header of "certbot -h SUBCOMMAND"
+VERB_HELP = [
+    ("run (default)", {
+        "short": "Obtain/renew a certificate, and install it",
+        "opts": "Options for obtaining & installing certs",
+        "usage": SHORT_USAGE.replace("[SUBCOMMAND]", ""),
+        "realname": "run"
+    }),
+    ("certonly", {
+        "short": "Obtain or renew a certificate, but do not install it",
+        "opts": "Options for modifying how a cert is obtained",
+        "usage": ("\n\n  certbot certonly [options] [-d DOMAIN] [-d DOMAIN] ...\n\n"
+                  "This command obtains a TLS/SSL certificate without installing it anywhere.")
+    }),
+    ("renew", {
+        "short": "Renew all certificates (or one specifed with --cert-name)",
+        "opts": ("The 'renew' subcommand will attempt to renew all"
+                 " certificates (or more precisely, certificate lineages) you have"
+                 " previously obtained if they are close to expiry, and print a"
+                 " summary of the results. By default, 'renew' will reuse the options"
+                 " used to create obtain or most recently successfully renew each"
+                 " certificate lineage. You can try it with `--dry-run` first. For"
+                 " more fine-grained control, you can renew individual lineages with"
+                 " the `certonly` subcommand. Hooks are available to run commands"
+                 " before and after renewal; see"
+                 " https://certbot.eff.org/docs/using.html#renewal for more"
+                 " information on these."),
+        "usage": "\n\n  certbot renew [--cert-name NAME] [options]\n\n"
+    }),
+    ("certificates", {
+        "short": "List certificates managed by Certbot",
+        "opts": "List certificates managed by Certbot",
+        "usage": ("\n\n  certbot certificates [options] ...\n\n"
+                  "Print information about the status of certificates managed by Certbot.")
+    }),
+    ("delete", {
+        "short": "Clean up all files related to a certificate",
+        "opts": "Options for deleting a certificate"
+    }),
+    ("revoke", {
+        "short": "Revoke a certificate specified with --cert-path",
+        "opts": "Options for revocation of certs",
+        "usage": "\n\n  certbot revoke --cert-path /path/to/fullchain.pem [options]\n\n"
+    }),
+    ("rename", {
+        "short": "Change a certificate's name (for management purposes)",
+        "opts": "Options for changing certificate names"
+    }),
+    ("register", {
+        "short": "Register for account with Let's Encrypt / other ACME server",
+        "opts": "Options for account registration & modification"
+    }),
+    ("install", {
+        "short": "Install an arbitrary cert in a server",
+        "opts": "Options for modifying how a cert is deployed"
+    }),
+    ("config_changes", {
+        "short": "Show changes that Certbot has made to server configurations",
+        "opts": "Options for controlling which changes are displayed"
+    }),
+    ("rollback", {
+        "short": "Roll back server conf changes made during cert installation",
+        "opts": "Options for rolling back server configuration changes"
+    }),
+    ("plugins", {
+        "short": "List plugins that are installed and available on your system",
+        "opts": 'Options for for the "plugins" subcommand'
+    }),
+    ("update_symlinks", {
+        "short": "Recreate symlinks in your /live/ directory",
+        "opts": ("Recreates cert and key symlinks in {0}, if you changed them by hand "
+                 "or edited a renewal configuration file".format(
+                  os.path.join(flag_default("config_dir"), "live")))
+    }),
+
+]
+# VERB_HELP is a list in order to preserve order, but a dict is sometimes useful
+VERB_HELP_MAP = dict(VERB_HELP)
+
 
 class HelpfulArgumentParser(object):
     """Argparse Wrapper.
@@ -318,6 +408,7 @@ class HelpfulArgumentParser(object):
 
     """
 
+
     def __init__(self, args, plugins, detect_defaults=False):
         from certbot import main
         self.VERBS = {"auth": main.obtain_cert, "certonly": main.obtain_cert,
@@ -326,26 +417,17 @@ class HelpfulArgumentParser(object):
                       "register": main.register, "renew": main.renew,
                       "revoke": main.revoke, "rollback": main.rollback,
                       "everything": main.run, "update_symlinks": main.update_symlinks,
-                      "certificates": main.certificates, "rename": main.rename}
+                      "certificates": main.certificates, "rename": main.rename,
+                      "delete": main.delete}
 
         # List of topics for which additional help can be provided
         HELP_TOPICS = ["all", "security", "paths", "automation", "testing"] + list(self.VERBS)
+        HELP_TOPICS += self.COMMANDS_TOPICS + ["manage"]
 
         plugin_names = list(plugins)
         self.help_topics = HELP_TOPICS + plugin_names + [None]
-        usage, short_usage = usage_strings(plugins)
-        self.parser = configargparse.ArgParser(
-            prog="certbot",
-            usage=short_usage,
-            formatter_class=argparse.ArgumentDefaultsHelpFormatter,
-            args_for_setting_config_path=["-c", "--config"],
-            default_config_files=flag_default("config_files"))
-
-        # This is the only way to turn off overly verbose config flag documentation
-        self.parser._add_config_file_help = False  # pylint: disable=protected-access
 
         self.detect_defaults = detect_defaults
-
         self.args = args
         self.determine_verb()
         help1 = self.prescan_for_flag("-h", self.help_topics)
@@ -354,13 +436,72 @@ class HelpfulArgumentParser(object):
             self.help_arg = help1 or help2
         else:
             self.help_arg = help1 if isinstance(help1, str) else help2
-        if self.help_arg is True:
-            # just --help with no topic; avoid argparse altogether
-            print(usage)
-            sys.exit(0)
+
+        short_usage = self._usage_string(plugins, self.help_arg)
+
         self.visible_topics = self.determine_help_topics(self.help_arg)
         self.groups = {}       # elements are added by .add_group()
-        self.defaults = {}  # elements are added by .parse_args()
+        self.defaults = {}     # elements are added by .parse_args()
+
+        self.parser = configargparse.ArgParser(
+            prog="certbot",
+            usage=short_usage,
+            formatter_class=CustomHelpFormatter,
+            args_for_setting_config_path=["-c", "--config"],
+            default_config_files=flag_default("config_files"),
+            config_arg_help_message="path to config file (default: {0})".format(
+                " and ".join(flag_default("config_files"))))
+
+        # This is the only way to turn off overly verbose config flag documentation
+        self.parser._add_config_file_help = False  # pylint: disable=protected-access
+
+    # Help that are synonyms for --help subcommands
+    COMMANDS_TOPICS = ["command", "commands", "subcommand", "subcommands", "verbs"]
+    def _list_subcommands(self):
+        longest = max(len(v) for v in VERB_HELP_MAP.keys())
+
+        text = "The full list of available SUBCOMMANDS is:\n\n"
+        for verb, props in sorted(VERB_HELP):
+            doc = props.get("short", "")
+            text += '{0:<{length}}     {1}\n'.format(verb, doc, length=longest)
+
+        text += "\nYou can get more help on a specific subcommand with --help SUBCOMMAND\n"
+        return text
+
+    def _usage_string(self, plugins, help_arg):
+        """Make usage strings late so that plugins can be initialised late
+
+        :param plugins: all discovered plugins
+        :param help_arg: False for none; True for --help; "TOPIC" for --help TOPIC
+        :rtype: str
+        :returns: a short usage string for the top of --help TOPIC)
+        """
+        if "nginx" in plugins:
+            nginx_doc = "--nginx           Use the Nginx plugin for authentication & installation"
+        else:
+            nginx_doc = "(the certbot nginx plugin is not installed)"
+        if "apache" in plugins:
+            apache_doc = "--apache          Use the Apache plugin for authentication & installation"
+        else:
+            apache_doc = "(the cerbot apache plugin is not installed)"
+
+        usage = SHORT_USAGE
+        if help_arg == True:
+            print(usage + COMMAND_OVERVIEW % (apache_doc, nginx_doc) + HELP_USAGE)
+            sys.exit(0)
+        elif help_arg in self.COMMANDS_TOPICS:
+            print(usage + self._list_subcommands())
+            sys.exit(0)
+        elif help_arg == "all":
+            # if we're doing --help all, the OVERVIEW is part of the SHORT_USAGE at
+            # the top; if we're doing --help someothertopic, it's OT so it's not
+            usage += COMMAND_OVERVIEW % (apache_doc, nginx_doc)
+        else:
+            custom = VERB_HELP_MAP.get(help_arg, {}).get("usage", None)
+            usage = custom if custom else usage
+
+        return usage
+
 
     def parse_args(self):
         """Parses command line arguments and returns the result.
@@ -382,8 +523,17 @@ class HelpfulArgumentParser(object):
         # Do any post-parsing homework here
 
         if self.verb == "renew":
+            if parsed_args.force_interactive:
+                raise errors.Error(
+                    "{0} cannot be used with renew".format(
+                        constants.FORCE_INTERACTIVE_FLAG))
             parsed_args.noninteractive_mode = True
 
+        if parsed_args.force_interactive and parsed_args.noninteractive_mode:
+            raise errors.Error(
+                "Flag for non-interactive mode and {0} conflict".format(
+                    constants.FORCE_INTERACTIVE_FLAG))
+
         if parsed_args.staging or parsed_args.dry_run:
             self.set_test_server(parsed_args)
 
@@ -562,7 +712,7 @@ class HelpfulArgumentParser(object):
         util.add_deprecated_argument(
             self.parser.add_argument, argument_name, num_args)
 
-    def add_group(self, topic, **kwargs):
+    def add_group(self, topic, verbs=(), **kwargs):
         """Create a new argument group.
 
         This method must be called once for every topic, however, calls
@@ -570,6 +720,8 @@ class HelpfulArgumentParser(object):
         clarity.
 
         :param str topic: Name of the new argument group.
+        :param str verbs: List of subcommands that should be documented as part of
+                          this help group / topic
 
         :returns: The new argument group.
         :rtype: `HelpfulArgumentGroup`
@@ -577,6 +729,9 @@ class HelpfulArgumentParser(object):
         """
         if self.visible_topics[topic]:
             self.groups[topic] = self.parser.add_argument_group(topic, **kwargs)
+            if self.help_arg:
+                for v in verbs:
+                    self.groups[topic].add_argument(v, help=VERB_HELP_MAP[v]["short"])
 
         return HelpfulArgumentGroup(self, topic)
 
@@ -617,32 +772,17 @@ class HelpfulArgumentParser(object):
 def _add_all_groups(helpful):
     helpful.add_group("automation", description="Arguments for automating execution & other tweaks")
     helpful.add_group("security", description="Security parameters & server settings")
-    helpful.add_group(
-        "testing", description="The following flags are meant for "
-        "testing purposes only! Do NOT change them, unless you "
-        "really know what you're doing!")
-    # VERBS
-    helpful.add_group(
-        "renew", description="The 'renew' subcommand will attempt to renew all"
-        " certificates (or more precisely, certificate lineages) you have"
-        " previously obtained if they are close to expiry, and print a"
-        " summary of the results. By default, 'renew' will reuse the options"
-        " used to create obtain or most recently successfully renew each"
-        " certificate lineage. You can try it with `--dry-run` first. For"
-        " more fine-grained control, you can renew individual lineages with"
-        " the `certonly` subcommand. Hooks are available to run commands"
-        " before and after renewal; see"
-        " https://certbot.eff.org/docs/using.html#renewal for more"
-        " information on these.")
-
-    helpful.add_group("certonly", description="Options for modifying how a cert is obtained")
-    helpful.add_group("install", description="Options for modifying how a cert is deployed")
-    helpful.add_group("revoke", description="Options for revocation of certs")
-    helpful.add_group("rollback", description="Options for reverting config changes")
-    helpful.add_group("plugins", description='Options for the "plugins" subcommand')
-    helpful.add_group("config_changes",
-                      description="Options for showing a history of config changes")
+    helpful.add_group("testing",
+        description="The following flags are meant for testing and integration purposes only.")
     helpful.add_group("paths", description="Arguments changing execution paths & servers")
+    helpful.add_group("manage",
+        description="Various subcommands and flags are available for managing your certificates:",
+        verbs=["certificates", "delete", "renew", "revoke", "rename"])
+
+    # VERBS
+    for verb, docs in VERB_HELP:
+        name = docs.get("realname", verb)
+        helpful.add_group(name, description=docs["opts"])
 
 
 def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: disable=too-many-statements
@@ -671,28 +811,33 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         None, "-t", "--text", dest="text_mode", action="store_true",
         help=argparse.SUPPRESS)
     helpful.add(
-        [None, "automation"], "-n", "--non-interactive", "--noninteractive",
+        [None, "automation", "run", "certonly"], "-n", "--non-interactive", "--noninteractive",
         dest="noninteractive_mode", action="store_true",
         help="Run without ever asking for user input. This may require "
               "additional command line flags; the client will try to explain "
               "which ones are required if it finds one missing")
     helpful.add(
-        [None, "run", "certonly"],
+        [None, "register", "run", "certonly"],
+        constants.FORCE_INTERACTIVE_FLAG, action="store_true",
+        help="Force Certbot to be interactive even if it detects it's not "
+             "being run in a terminal. This flag cannot be used with the "
+             "renew subcommand.")
+    helpful.add(
+        [None, "run", "certonly", "certificates"],
         "-d", "--domains", "--domain", dest="domains",
         metavar="DOMAIN", action=_DomainsAction, default=[],
         help="Domain names to apply. For multiple domains you can use "
              "multiple -d flags or enter a comma separated list of domains "
-             "as a parameter.")
+             "as a parameter. (default: Ask)")
     helpful.add(
-        [None, "run", "certonly"],
+        [None, "run", "certonly", "manage", "rename", "delete", "certificates"],
         "--cert-name", dest="certname",
         metavar="CERTNAME", default=None,
         help="Certificate name to apply. Only one certificate name can be used "
-             "per Certbot run. To see certificate names, run 'certbot certificates'."
-             "If there is no existing certificate with this name and "
-             "domains are requested, create a new certificate with this name.")
+             "per Certbot run. To see certificate names, run 'certbot certificates'. "
+             "When creating a new certificate, specifies the new certificate's name.")
     helpful.add(
-        "rename",
+        ["rename", "manage"],
         "--updated-cert-name", dest="new_certname",
         metavar="NEW_CERTNAME", default=None,
         help="New name for the certificate. Must be a valid filename.")
@@ -724,18 +869,18 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         help="With the register verb, indicates that details associated "
              "with an existing registration, such as the e-mail address, "
              "should be updated, rather than registering a new account.")
-    helpful.add(None, "-m", "--email", help=config_help("email"))
+    helpful.add(["register", "automation"], "-m", "--email", help=config_help("email"))
     helpful.add(
-        ["automation", "renew", "certonly", "run"],
+        ["automation", "certonly", "run"],
         "--keep-until-expiring", "--keep", "--reinstall",
         dest="reinstall", action="store_true",
         help="If the requested cert matches an existing cert, always keep the "
              "existing one until it is due for renewal (for the "
-             "'run' subcommand this means reinstall the existing cert)")
+             "'run' subcommand this means reinstall the existing cert). (default: Ask)")
     helpful.add(
         "automation", "--expand", action="store_true",
         help="If an existing cert covers some subset of the requested names, "
-             "always expand and replace it with the additional names.")
+             "always expand and replace it with the additional names. (default: Ask)")
     helpful.add(
         "automation", "--version", action="version",
         version="%(prog)s {0}".format(certbot.__version__),
@@ -764,7 +909,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "at this system. This option cannot be used with --csr.")
     helpful.add(
         "automation", "--agree-tos", dest="tos", action="store_true",
-        help="Agree to the ACME Subscriber Agreement")
+        help="Agree to the ACME Subscriber Agreement (default: Ask)")
     helpful.add(
         "automation", "--account", metavar="ACCOUNT_ID",
         help="Account ID to use")
@@ -778,15 +923,17 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
     helpful.add(
         "automation", "--no-self-upgrade", action="store_true",
         help="(certbot-auto only) prevent the certbot-auto script from"
-             " upgrading itself to newer released versions")
+             " upgrading itself to newer released versions (default: Upgrade"
+             " automatically)")
     helpful.add(
-        ["automation", "renew", "certonly"],
+        ["automation", "renew", "certonly", "run"],
         "-q", "--quiet", dest="quiet", action="store_true",
         help="Silence all output except errors. Useful for automation via cron."
              " Implies --non-interactive.")
     # overwrites server, handled in HelpfulArgumentParser.parse_args()
-    helpful.add("testing", "--test-cert", "--staging", action='store_true', dest='staging',
-        help='Use the staging server to obtain test (invalid) certs; equivalent'
+    helpful.add(["testing", "revoke", "run"], "--test-cert", "--staging",
+        action='store_true', dest='staging',
+        help='Use the staging server to obtain or revoke test (invalid) certs; equivalent'
              ' to --server ' + constants.STAGING_URI)
     helpful.add(
         "testing", "--debug", action="store_true",
@@ -797,11 +944,11 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         help=config_help("no_verify_ssl"),
         default=flag_default("no_verify_ssl"))
     helpful.add(
-        ["certonly", "renew", "run"], "--tls-sni-01-port", type=int,
+        ["testing", "standalone", "apache", "nginx"], "--tls-sni-01-port", type=int,
         default=flag_default("tls_sni_01_port"),
         help=config_help("tls_sni_01_port"))
     helpful.add(
-        ["certonly", "renew", "run", "manual"], "--http-01-port", type=int,
+        ["testing", "standalone", "manual"], "--http-01-port", type=int,
         dest="http01_port",
         default=flag_default("http01_port"), help=config_help("http01_port"))
     helpful.add(
@@ -817,11 +964,11 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
     helpful.add(
         "security", "--redirect", action="store_true",
         help="Automatically redirect all HTTP traffic to HTTPS for the newly "
-             "authenticated vhost.", dest="redirect", default=None)
+             "authenticated vhost. (default: Ask)", dest="redirect", default=None)
     helpful.add(
         "security", "--no-redirect", action="store_false",
         help="Do not automatically redirect all HTTP traffic to HTTPS for the newly "
-             "authenticated vhost.", dest="redirect", default=None)
+             "authenticated vhost. (default: Ask)", dest="redirect", default=None)
     helpful.add(
         "security", "--hsts", action="store_true",
         help="Add the Strict-Transport-Security header to every HTTP response."
@@ -829,8 +976,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              " Defends against SSL Stripping.", dest="hsts", default=False)
     helpful.add(
         "security", "--no-hsts", action="store_false",
-        help="Do not automatically add the Strict-Transport-Security header"
-             " to every HTTP response.", dest="hsts", default=False)
+        help=argparse.SUPPRESS, dest="hsts", default=False)
     helpful.add(
         "security", "--uir", action="store_true",
         help="Add the \"Content-Security-Policy: upgrade-insecure-requests\""
@@ -838,9 +984,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              " https:// for every http:// resource.", dest="uir", default=None)
     helpful.add(
         "security", "--no-uir", action="store_false",
-        help="Do not automatically set the \"Content-Security-Policy:"
-        " upgrade-insecure-requests\" header to every HTTP response.",
-        dest="uir", default=None)
+        help=argparse.SUPPRESS, dest="uir", default=None)
     helpful.add(
         "security", "--staple-ocsp", action="store_true",
         help="Enables OCSP Stapling. A valid OCSP response is stapled to"
@@ -848,14 +992,13 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         dest="staple", default=None)
     helpful.add(
         "security", "--no-staple-ocsp", action="store_false",
-        help="Do not automatically enable OCSP Stapling.",
-        dest="staple", default=None)
+        help=argparse.SUPPRESS, dest="staple", default=None)
     helpful.add(
         "security", "--strict-permissions", action="store_true",
         help="Require that all configuration files are owned by the current "
              "user; only needed if your config is somewhere unsafe like /tmp/")
     helpful.add(
-        ["manual", "standalone", "certonly", "renew", "run"],
+        ["manual", "standalone", "certonly", "renew"],
         "--preferred-challenges", dest="pref_challs",
         action=_PrefChallAction, default=[],
         help='A sorted, comma delimited list of the preferred challenge to '
@@ -897,7 +1040,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         " see if the programs being run are in the $PATH, so that mistakes can"
         " be caught early, even when the hooks aren't being run just yet. The"
         " validation is rather simplistic and fails if you use more advanced"
-        " shell constructs, so you can use this switch to disable it.")
+        " shell constructs, so you can use this switch to disable it."
+        " (default: False)")
 
     helpful.add_deprecated_argument("--agree-dev-preview", 0)
     helpful.add_deprecated_argument("--dialog", 0)
@@ -917,12 +1061,15 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
 def _create_subparsers(helpful):
     helpful.add("config_changes", "--num", type=int,
                 help="How many past revisions you want to be displayed")
+
+    from certbot.client import sample_user_agent # avoid import loops
     helpful.add(
         None, "--user-agent", default=None,
         help="Set a custom user agent string for the client. User agent strings allow "
              "the CA to collect high level statistics about success rates by OS and "
              "plugin. If you wish to hide your server OS version from the Let's "
-             'Encrypt server, set this to "".')
+             'Encrypt server, set this to "". '
+             '(default: {0})'.format(sample_user_agent()))
     helpful.add("certonly",
                 "--csr", type=read_file,
                 help="Path to a Certificate Signing Request (CSR) in DER or PEM format."
@@ -949,10 +1096,8 @@ def _paths_parser(helpful):
     if verb == "help":
         verb = helpful.help_arg
 
-    cph = "Path to where cert is saved (with auth --csr), installed from or revoked."
-    section = "paths"
-    if verb in ("install", "revoke", "certonly"):
-        section = verb
+    cph = "Path to where cert is saved (with auth --csr), installed from, or revoked."
+    section = ["paths", "install", "revoke", "certonly", "manage"]
     if verb == "certonly":
         add(section, "--cert-path", type=os.path.abspath,
             default=flag_default("auth_cert_path"), help=cph)
@@ -974,7 +1119,7 @@ def _paths_parser(helpful):
     default_cp = None
     if verb == "certonly":
         default_cp = flag_default("auth_chain_path")
-    add("paths", "--fullchain-path", default=default_cp, type=os.path.abspath,
+    add(["install", "paths"], "--fullchain-path", default=default_cp, type=os.path.abspath,
         help="Accompanying path to a full certificate chain (cert plus chain).")
     add("paths", "--chain-path", default=default_cp, type=os.path.abspath,
         help="Accompanying path to a certificate chain.")
@@ -997,24 +1142,20 @@ def _plugins_parsing(helpful, plugins):
         "a particular plugin by setting options provided below. Running "
         "--help <plugin_name> will list flags specific to that plugin.")
 
-    helpful.add(
-        "plugins", "-a", "--authenticator", help="Authenticator plugin name.")
-    helpful.add(
-        "plugins", "-i", "--installer", help="Installer plugin name (also used to find domains).")
-    helpful.add(
-        "plugins", "--configurator", help="Name of the plugin that is "
-        "both an authenticator and an installer. Should not be used "
-        "together with --authenticator or --installer.")
-    helpful.add(["plugins", "certonly", "run", "install"],
+    helpful.add("plugins", "--configurator",
+                help="Name of the plugin that is both an authenticator and an installer."
+                " Should not be used together with --authenticator or --installer. "
+                "(default: Ask)")
+    helpful.add("plugins", "-a", "--authenticator", help="Authenticator plugin name.")
+    helpful.add("plugins", "-i", "--installer",
+                help="Installer plugin name (also used to find domains).")
+    helpful.add(["plugins", "certonly", "run", "install", "config_changes"],
                 "--apache", action="store_true",
                 help="Obtain and install certs using Apache")
-    helpful.add(["plugins", "certonly", "run", "install"],
-                "--nginx", action="store_true",
-                help="Obtain and install certs using Nginx")
+    helpful.add(["plugins", "certonly", "run", "install", "config_changes"],
+                "--nginx", action="store_true", help="Obtain and install certs using Nginx")
     helpful.add(["plugins", "certonly"], "--standalone", action="store_true",
                 help='Obtain certs using a "standalone" webserver.')
-    helpful.add(["plugins", "certonly"], "--script", action="store_true",
-                help='Obtain certs using shell script(s)')
     helpful.add(["plugins", "certonly"], "--manual", action="store_true",
                 help='Provide laborious manual instructions for obtaining a cert')
     helpful.add(["plugins", "certonly"], "--webroot", action="store_true",

certbot/client.py

@@ -15,7 +15,6 @@ import certbot
 
 from certbot import account
 from certbot import auth_handler
-from certbot import configuration
 from certbot import constants
 from certbot import crypto_util
 from certbot import errors
@@ -38,11 +37,11 @@ def acme_from_config_key(config, key):
     "Wrangle ACME client construction"
     # TODO: Allow for other alg types besides RS256
     net = acme_client.ClientNetwork(key, verify_ssl=(not config.no_verify_ssl),
-                                    user_agent=_determine_user_agent(config))
+                                    user_agent=determine_user_agent(config))
     return acme_client.Client(config.server, key=key, net=net)
 
 
-def _determine_user_agent(config):
+def determine_user_agent(config):
     """
     Set a user_agent string in the config based on the choice of plugins.
     (this wasn't knowable at construction time)
@@ -59,6 +58,16 @@ def _determine_user_agent(config):
         ua = config.user_agent
     return ua
 
+def sample_user_agent():
+    "Document what this Certbot's user agent string will be like."
+    class DummyConfig(object):
+        "Shim for computing a sample user agent."
+        def __init__(self):
+            self.authenticator = "XXX"
+            self.installer = "YYY"
+            self.user_agent = None
+    return determine_user_agent(DummyConfig())
+
 
 def register(config, account_storage, tos_cb=None):
     """Register new account with an ACME CA.
@@ -297,7 +306,7 @@ class Client(object):
                 new_name, OpenSSL.crypto.dump_certificate(
                     OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped),
                 key.pem, crypto_util.dump_pyopenssl_chain(chain),
-                configuration.RenewerConfiguration(self.config.namespace))
+                self.config)
 
     def save_certificate(self, certr, chain_cert,
                          cert_path, chain_path, fullchain_path):

certbot/configuration.py

@@ -25,9 +25,16 @@ class NamespaceConfig(object):
       - `csr_dir`
       - `in_progress_dir`
       - `key_dir`
-      - `renewer_config_file`
       - `temp_checkpoint_dir`
 
+    And the following paths are dynamically resolved using
+    :attr:`~certbot.interfaces.IConfig.config_dir` and relative
+    paths defined in :py:mod:`certbot.constants`:
+
+      - `default_archive_dir`
+      - `live_dir`
+      - `renewal_configs_dir`
+
     :ivar namespace: Namespace typically produced by
         :meth:`argparse.ArgumentParser.parse_args`.
     :type namespace: :class:`argparse.Namespace`
@@ -85,16 +92,6 @@ class NamespaceConfig(object):
         new_ns = copy.deepcopy(self.namespace)
         return type(self)(new_ns)
 
-
-class RenewerConfiguration(object):
-    """Configuration wrapper for renewer."""
-
-    def __init__(self, namespace):
-        self.namespace = namespace
-
-    def __getattr__(self, name):
-        return getattr(self.namespace, name)
-
     @property
     def default_archive_dir(self):  # pylint: disable=missing-docstring
         return os.path.join(self.namespace.config_dir, constants.ARCHIVE_DIR)
@@ -108,11 +105,6 @@ class RenewerConfiguration(object):
         return os.path.join(
             self.namespace.config_dir, constants.RENEWAL_CONFIGS_DIR)
 
-    @property
-    def renewer_config_file(self):  # pylint: disable=missing-docstring
-        return os.path.join(
-            self.namespace.config_dir, constants.RENEWER_CONFIG_FILENAME)
-
 
 def check_config_sanity(config):
     """Validate command line options and display error message if

certbot/constants.py

@@ -93,5 +93,5 @@ TEMP_CHECKPOINT_DIR = "temp_checkpoint"
 RENEWAL_CONFIGS_DIR = "renewal"
 """Renewal configs directory, relative to `IConfig.config_dir`."""
 
-RENEWER_CONFIG_FILENAME = "renewer.conf"
-"""Renewer config file name (relative to `IConfig.config_dir`)."""
+FORCE_INTERACTIVE_FLAG = "--force-interactive"
+"""Flag to disable TTY checking in IDisplay."""

certbot/display/enhancements.py

@@ -48,7 +48,8 @@ def redirect_by_default():
 
     code, selection = util(interfaces.IDisplay).menu(
         "Please choose whether HTTPS access is required or optional.",
-        choices, default=0, cli_flag="--redirect / --no-redirect")
+        choices, default=0,
+        cli_flag="--redirect / --no-redirect", force_interactive=True)
 
     if code != display_util.OK:
         return False

certbot/display/ops.py

@@ -46,7 +46,8 @@ def get_email(invalid=False, optional=True):
     while True:
         try:
             code, email = z_util(interfaces.IDisplay).input(
-                invalid_prefix + msg if invalid else msg)
+                invalid_prefix + msg if invalid else msg,
+                force_interactive=True)
         except errors.MissingCommandlineFlag:
             msg = ("You should register before running non-interactively, "
                    "or provide --agree-tos and --email <email_address> flags.")
@@ -79,7 +80,7 @@ def choose_account(accounts):
     labels = [acc.slug for acc in accounts]
 
     code, index = z_util(interfaces.IDisplay).menu(
-        "Please choose an account", labels)
+        "Please choose an account", labels, force_interactive=True)
     if code == display_util.OK:
         return accounts[index]
     else:
@@ -157,7 +158,7 @@ def _filter_names(names):
 
     code, names = z_util(interfaces.IDisplay).checklist(
         "Which names would you like to activate HTTPS for?",
-        tags=sorted_names, cli_flag="--domains")
+        tags=sorted_names, cli_flag="--domains", force_interactive=True)
     return code, [str(s) for s in names]
 
 
@@ -173,7 +174,7 @@ def _choose_names_manually(prompt_prefix=""):
     code, input_ = z_util(interfaces.IDisplay).input(
         prompt_prefix +
         "Please enter in your domain name(s) (comma and/or space separated) ",
-        cli_flag="--domains")
+        cli_flag="--domains", force_interactive=True)
 
     if code == display_util.OK:
         invalid_domains = dict()
@@ -211,7 +212,8 @@ def _choose_names_manually(prompt_prefix=""):
 
         if retry_message:
             # We had error in input
-            retry = z_util(interfaces.IDisplay).yesno(retry_message)
+            retry = z_util(interfaces.IDisplay).yesno(retry_message,
+                                                      force_interactive=True)
             if retry:
                 return _choose_names_manually()
         else:

certbot/display/util.py

@@ -1,14 +1,18 @@
 """Certbot display."""
+import logging
 import os
 import textwrap
+import sys
 
 import six
 import zope.interface
 
+from certbot import constants
 from certbot import interfaces
 from certbot import errors
 from certbot.display import completer
 
+logger = logging.getLogger(__name__)
 
 WIDTH = 72
 
@@ -50,19 +54,25 @@ def _wrap_lines(msg):
 @zope.interface.implementer(interfaces.IDisplay)
 class FileDisplay(object):
     """File-based display."""
+    # pylint: disable=too-many-arguments
+    # see https://github.com/certbot/certbot/issues/3915
 
-    def __init__(self, outfile):
+    def __init__(self, outfile, force_interactive):
         super(FileDisplay, self).__init__()
         self.outfile = outfile
+        self.force_interactive = force_interactive
+        self.skipped_interaction = False
 
-    def notification(self, message, pause=True, wrap=True):
-        # pylint: disable=unused-argument
+    def notification(self, message, pause=True,
+                     wrap=True, force_interactive=False):
         """Displays a notification and waits for user acceptance.
 
         :param str message: Message to display
         :param bool pause: Whether or not the program should pause for the
             user's confirmation
         :param bool wrap: Whether or not the application should wrap text
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         """
         side_frame = "-" * 79
@@ -72,10 +82,14 @@ class FileDisplay(object):
             "{line}{frame}{line}{msg}{line}{frame}{line}".format(
                 line=os.linesep, frame=side_frame, msg=message))
         if pause:
-            six.moves.input("Press Enter to Continue")
+            if self._can_interact(force_interactive):
+                six.moves.input("Press Enter to Continue")
+            else:
+                logger.debug("Not pausing for user confirmation")
 
     def menu(self, message, choices, ok_label="", cancel_label="",
-             help_label="", **unused_kwargs):
+             help_label="", default=None,
+             cli_flag=None, force_interactive=False, **unused_kwargs):
         # pylint: disable=unused-argument
         """Display a menu.
 
@@ -86,7 +100,10 @@ class FileDisplay(object):
         :param choices: Menu lines, len must be > 0
         :type choices: list of tuples (tag, item) or
             list of descriptions (tags will be enumerated)
-        :param dict _kwargs: absorbs default / cli_args
+        :param default: default value to return (if one exists)
+        :param str cli_flag: option used to set this value with the CLI
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: tuple of (`code`, `index`) where
             `code` - str display exit code
@@ -95,18 +112,25 @@ class FileDisplay(object):
         :rtype: tuple
 
         """
+        if self._return_default(message, default, cli_flag, force_interactive):
+            return OK, default
+
         self._print_menu(message, choices)
 
         code, selection = self._get_valid_int_ans(len(choices))
 
         return code, selection - 1
 
-    def input(self, message, **unused_kwargs):
+    def input(self, message, default=None,
+              cli_flag=None, force_interactive=False, **unused_kwargs):
         # pylint: disable=no-self-use
         """Accept input from the user.
 
         :param str message: message to display to the user
-        :param dict _kwargs: absorbs default / cli_args
+        :param default: default value to return (if one exists)
+        :param str cli_flag: option used to set this value with the CLI
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: tuple of (`code`, `input`) where
             `code` - str display exit code
@@ -114,6 +138,9 @@ class FileDisplay(object):
         :rtype: tuple
 
         """
+        if self._return_default(message, default, cli_flag, force_interactive):
+            return OK, default
+
         ans = six.moves.input(
             textwrap.fill(
                 "%s (Enter 'c' to cancel): " % message,
@@ -126,7 +153,8 @@ class FileDisplay(object):
         else:
             return OK, ans
 
-    def yesno(self, message, yes_label="Yes", no_label="No", **unused_kwargs):
+    def yesno(self, message, yes_label="Yes", no_label="No", default=None,
+              cli_flag=None, force_interactive=False, **unused_kwargs):
         """Query the user with a yes/no question.
 
         Yes and No label must begin with different letters, and must contain at
@@ -135,12 +163,18 @@ class FileDisplay(object):
         :param str message: question for the user
         :param str yes_label: Label of the "Yes" parameter
         :param str no_label: Label of the "No" parameter
-        :param dict _kwargs: absorbs default / cli_args
+        :param default: default value to return (if one exists)
+        :param str cli_flag: option used to set this value with the CLI
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: True for "Yes", False for "No"
         :rtype: bool
 
         """
+        if self._return_default(message, default, cli_flag, force_interactive):
+            return default
+
         side_frame = ("-" * 79) + os.linesep
 
         message = _wrap_lines(message)
@@ -162,14 +196,18 @@ class FileDisplay(object):
                     ans.startswith(no_label[0].upper())):
                 return False
 
-    def checklist(self, message, tags, default_status=True, **unused_kwargs):
+    def checklist(self, message, tags, default_status=True, default=None,
+                  cli_flag=None, force_interactive=False, **unused_kwargs):
         # pylint: disable=unused-argument
         """Display a checklist.
 
         :param str message: Message to display to user
         :param list tags: `str` tags to select, len(tags) > 0
         :param bool default_status: Not used for FileDisplay
-        :param dict _kwargs: absorbs default / cli_args
+        :param default: default value to return (if one exists)
+        :param str cli_flag: option used to set this value with the CLI
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: tuple of (`code`, `tags`) where
             `code` - str display exit code
@@ -177,12 +215,16 @@ class FileDisplay(object):
         :rtype: tuple
 
         """
+        if self._return_default(message, default, cli_flag, force_interactive):
+            return OK, default
+
         while True:
             self._print_menu(message, tags)
 
             code, ans = self.input("Select the appropriate numbers separated "
                                    "by commas and/or spaces, or leave input "
-                                   "blank to select all options shown")
+                                   "blank to select all options shown",
+                                   force_interactive=True)
 
             if code == OK:
                 if len(ans.strip()) == 0:
@@ -197,10 +239,65 @@ class FileDisplay(object):
             else:
                 return code, []
 
-    def directory_select(self, message, **unused_kwargs):
+    def _return_default(self, prompt, default, cli_flag, force_interactive):
+        """Should we return the default instead of prompting the user?
+
+        :param str prompt: prompt for the user
+        :param default: default answer to prompt
+        :param str cli_flag: command line option for setting an answer
+            to this question
+        :param bool force_interactive: if interactivity is forced by the
+            IDisplay call
+
+        :returns: True if we should return the default without prompting
+        :rtype: bool
+
+        """
+        msg = "Invalid IDisplay call for this prompt:\n{0}".format(prompt)
+        if cli_flag:
+            msg += ("\nYou can set an answer to "
+                    "this prompt with the {0} flag".format(cli_flag))
+        assert default is not None or force_interactive, msg
+
+        if self._can_interact(force_interactive):
+            return False
+        else:
+            logger.debug(
+                "Falling back to default %s for the prompt:\n%s",
+                default, prompt)
+            return True
+
+    def _can_interact(self, force_interactive):
+        """Can we safely interact with the user?
+
+        :param bool force_interactive: if interactivity is forced by the
+            IDisplay call
+
+        :returns: True if the display can interact with the user
+        :rtype: bool
+
+        """
+        if (self.force_interactive or force_interactive or
+                sys.stdin.isatty() and self.outfile.isatty()):
+            return True
+        elif not self.skipped_interaction:
+            logger.warning(
+                "Skipped user interaction because Certbot doesn't appear to "
+                "be running in a terminal. You should probably include "
+                "--non-interactive or %s on the command line.",
+                constants.FORCE_INTERACTIVE_FLAG)
+            self.skipped_interaction = True
+        return False
+
+    def directory_select(self, message, default=None, cli_flag=None,
+                         force_interactive=False, **unused_kwargs):
         """Display a directory selection screen.
 
         :param str message: prompt to give the user
+        :param default: default value to return (if one exists)
+        :param str cli_flag: option used to set this value with the CLI
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: tuple of the form (`code`, `string`) where
             `code` - display exit code
@@ -208,7 +305,7 @@ class FileDisplay(object):
 
         """
         with completer.Completer():
-            return self.input(message)
+            return self.input(message, default, cli_flag, force_interactive)
 
     def _scrub_checklist_input(self, indices, tags):
         # pylint: disable=no-self-use
@@ -310,7 +407,7 @@ class FileDisplay(object):
 class NoninteractiveDisplay(object):
     """An iDisplay implementation that never asks for interactive user input"""
 
-    def __init__(self, outfile):
+    def __init__(self, outfile, *unused_args, **unused_kwargs):
         super(NoninteractiveDisplay, self).__init__()
         self.outfile = outfile
 
@@ -324,7 +421,7 @@ class NoninteractiveDisplay(object):
             msg += "\n\n(You can set this with the {0} flag)".format(cli_flag)
         raise errors.MissingCommandlineFlag(msg)
 
-    def notification(self, message, pause=False, wrap=True):
+    def notification(self, message, pause=False, wrap=True, **unused_kwargs):
         # pylint: disable=unused-argument
         """Displays a notification without waiting for user acceptance.
 
@@ -341,7 +438,7 @@ class NoninteractiveDisplay(object):
                 line=os.linesep, frame=side_frame, msg=message))
 
     def menu(self, message, choices, ok_label=None, cancel_label=None,
-             help_label=None, default=None, cli_flag=None):
+             help_label=None, default=None, cli_flag=None, **unused_kwargs):
         # pylint: disable=unused-argument,too-many-arguments
         """Avoid displaying a menu.
 
@@ -364,7 +461,7 @@ class NoninteractiveDisplay(object):
 
         return OK, default
 
-    def input(self, message, default=None, cli_flag=None):
+    def input(self, message, default=None, cli_flag=None, **unused_kwargs):
         """Accept input from the user.
 
         :param str message: message to display to the user
@@ -381,7 +478,8 @@ class NoninteractiveDisplay(object):
         else:
             return OK, default
 
-    def yesno(self, message, yes_label=None, no_label=None, default=None, cli_flag=None):
+    def yesno(self, message, yes_label=None, no_label=None,
+              default=None, cli_flag=None, **unused_kwargs):
         # pylint: disable=unused-argument
         """Decide Yes or No, without asking anybody
 
@@ -398,8 +496,8 @@ class NoninteractiveDisplay(object):
         else:
             return default
 
-    def checklist(self, message, tags, default=None, cli_flag=None, **kwargs):
-        # pylint: disable=unused-argument
+    def checklist(self, message, tags, default=None,
+                  cli_flag=None, **unused_kwargs):
         """Display a checklist.
 
         :param str message: Message to display to user
@@ -417,7 +515,8 @@ class NoninteractiveDisplay(object):
         else:
             return OK, default
 
-    def directory_select(self, message, default=None, cli_flag=None):
+    def directory_select(self, message, default=None,
+                         cli_flag=None, **unused_kwargs):
         """Simulate prompting the user for a directory.
 
         This function returns default if it is not ``None``, otherwise,

certbot/hooks.py

@@ -108,7 +108,10 @@ def execute(shell_cmd):
 
     :returns: `tuple` (`str` stderr, `str` stdout)"""
 
-    cmd = Popen(shell_cmd, shell=True, stdout=PIPE, stderr=PIPE)
+    # universal_newlines causes Popen.communicate()
+    # to return str objects instead of bytes in Python 3
+    cmd = Popen(shell_cmd, shell=True, stdout=PIPE,
+                stderr=PIPE, universal_newlines=True)
     out, err = cmd.communicate()
     if cmd.returncode != 0:
         logger.error('Hook command "%s" returned error code %d',

certbot/interfaces.py

@@ -138,15 +138,15 @@ class IAuthenticator(IPlugin):
     """
 
     def get_chall_pref(domain):
-        """Return list of challenge preferences.
+        """Return `collections.Iterable` of challenge preferences.
 
         :param str domain: Domain for which challenge preferences are sought.
 
-        :returns: List of challenge types (subclasses of
+        :returns: `collections.Iterable` of challenge types (subclasses of
             :class:`acme.challenges.Challenge`) with the most
             preferred challenges first. If a type is not specified, it means the
             Authenticator cannot perform the challenge.
-        :rtype: list
+        :rtype: `collections.Iterable`
 
         """
 
@@ -158,7 +158,7 @@ class IAuthenticator(IPlugin):
             instances, such that it contains types found within
             :func:`get_chall_pref` only.
 
-        :returns: List of ACME
+        :returns: `collections.Iterable` of ACME
             :class:`~acme.challenges.ChallengeResponse` instances
             or if the :class:`~acme.challenges.Challenge` cannot
             be fulfilled then:
@@ -168,7 +168,7 @@ class IAuthenticator(IPlugin):
             ``False``
               Authenticator will never be able to perform (error).
 
-        :rtype: :class:`list` of
+        :rtype: :class:`collections.Iterable` of
             :class:`acme.challenges.ChallengeResponse`,
             where responses are required to be returned in
             the same order as corresponding input challenges
@@ -201,7 +201,7 @@ class IConfig(zope.interface.Interface):
     """
     server = zope.interface.Attribute("ACME Directory Resource URI.")
     email = zope.interface.Attribute(
-        "Email used for registration and recovery contact.")
+        "Email used for registration and recovery contact. (default: Ask)")
     rsa_key_size = zope.interface.Attribute("Size of the RSA key.")
     must_staple = zope.interface.Attribute(
         "Adds the OCSP Must Staple extension to the certificate. "
@@ -223,9 +223,6 @@ class IConfig(zope.interface.Interface):
     temp_checkpoint_dir = zope.interface.Attribute(
         "Temporary checkpoint directory.")
 
-    renewer_config_file = zope.interface.Attribute(
-        "Location of renewal configuration file.")
-
     no_verify_ssl = zope.interface.Attribute(
         "Disable verification of the ACME server's certificate.")
     tls_sni_01_port = zope.interface.Attribute(
@@ -257,7 +254,7 @@ class IInstaller(IPlugin):
     def get_all_names():
         """Returns all names that may be authenticated.
 
-        :rtype: `list` of `str`
+        :rtype: `collections.Iterable` of `str`
 
         """
 
@@ -292,11 +289,11 @@ class IInstaller(IPlugin):
         """
 
     def supported_enhancements():
-        """Returns a list of supported enhancements.
+        """Returns a `collections.Iterable` of supported enhancements.
 
         :returns: supported enhancements which should be a subset of
             :const:`~certbot.constants.ENHANCEMENTS`
-        :rtype: :class:`list` of :class:`str`
+        :rtype: :class:`collections.Iterable` of :class:`str`
 
         """
 
@@ -364,21 +361,29 @@ class IInstaller(IPlugin):
 
 class IDisplay(zope.interface.Interface):
     """Generic display."""
+    # pylint: disable=too-many-arguments
+    # see https://github.com/certbot/certbot/issues/3915
 
-    def notification(message, pause, wrap=True):
+    def notification(message, pause, wrap=True, force_interactive=False):
         """Displays a string message
 
         :param str message: Message to display
         :param bool pause: Whether or not the application should pause for
             confirmation (if available)
         :param bool wrap: Whether or not the application should wrap text
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         """
 
-    def menu(message, choices, ok_label="OK",      # pylint: disable=too-many-arguments
-             cancel_label="Cancel", help_label="", default=None, cli_flag=None):
+    def menu(message, choices, ok_label="OK",
+             cancel_label="Cancel", help_label="",
+             default=None, cli_flag=None, force_interactive=False):
         """Displays a generic menu.
 
+        When not setting force_interactive=True, you must provide a
+        default value.
+
         :param str message: message to display
 
         :param choices: choices
@@ -389,6 +394,8 @@ class IDisplay(zope.interface.Interface):
         :param str help_label: label for Help button
         :param int default: default (non-interactive) choice from the menu
         :param str cli_flag: to automate choice from the menu, eg "--keep"
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: tuple of (`code`, `index`) where
             `code` - str display exit code
@@ -399,10 +406,16 @@ class IDisplay(zope.interface.Interface):
 
         """
 
-    def input(message, default=None, cli_args=None):
+    def input(message, default=None, cli_args=None, force_interactive=False):
         """Accept input from the user.
 
+        When not setting force_interactive=True, you must provide a
+        default value.
+
         :param str message: message to display to the user
+        :param str default: default (non-interactive) response to prompt
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: tuple of (`code`, `input`) where
             `code` - str display exit code
@@ -415,14 +428,19 @@ class IDisplay(zope.interface.Interface):
         """
 
     def yesno(message, yes_label="Yes", no_label="No", default=None,
-              cli_args=None):
+              cli_args=None, force_interactive=False):
         """Query the user with a yes/no question.
 
         Yes and No label must begin with different letters.
 
+        When not setting force_interactive=True, you must provide a
+        default value.
+
         :param str message: question for the user
         :param str default: default (non-interactive) choice from the menu
         :param str cli_flag: to automate choice from the menu, eg "--redirect / --no-redirect"
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: True for "Yes", False for "No"
         :rtype: bool
@@ -432,14 +450,20 @@ class IDisplay(zope.interface.Interface):
 
         """
 
-    def checklist(message, tags, default_state, default=None, cli_args=None):
+    def checklist(message, tags, default_state,
+                  default=None, cli_args=None, force_interactive=False):
         """Allow for multiple selections from a menu.
 
+        When not setting force_interactive=True, you must provide a
+        default value.
+
         :param str message: message to display to the user
         :param list tags: where each is of type :class:`str` len(tags) > 0
         :param bool default_status: If True, items are in a selected state by default.
         :param str default: default (non-interactive) state of the checklist
         :param str cli_flag: to automate choice from the menu, eg "--domains"
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: tuple of the form (code, list_tags) where
             `code` - int display exit code
@@ -451,15 +475,21 @@ class IDisplay(zope.interface.Interface):
 
         """
 
-    def directory_select(self, message, default=None, cli_flag=None):
+    def directory_select(self, message, default=None,
+                         cli_flag=None, force_interactive=False):
         """Display a directory selection screen.
 
+        When not setting force_interactive=True, you must provide a
+        default value.
+
         :param str message: prompt to give the user
         :param default: the default value to return, if one exists, when
             using the NoninteractiveDisplay
         :param str cli_flag: option used to set this value with the CLI,
             if one exists, to be included in error messages given by
             NoninteractiveDisplay
+        :param bool force_interactive: True if it's safe to prompt the user
+            because it won't cause any workflow regressions
 
         :returns: tuple of the form (`code`, `string`) where
             `code` - int display exit code

certbot/main.py

@@ -139,7 +139,8 @@ def _handle_subset_cert_request(config, domains, cert):
              br=os.linesep)
     if config.expand or config.renew_by_default or zope.component.getUtility(
             interfaces.IDisplay).yesno(question, "Expand", "Cancel",
-                                       cli_flag="--expand"):
+                                       cli_flag="--expand",
+                                       force_interactive=True):
         return "renew", cert
     else:
         reporter_util = zope.component.getUtility(interfaces.IReporter)
@@ -188,7 +189,8 @@ def _handle_identical_cert_request(config, lineage):
                "Renew & replace the cert (limit ~5 per 7 days)"]
 
     display = zope.component.getUtility(interfaces.IDisplay)
-    response = display.menu(question, choices, "OK", "Cancel", default=0)
+    response = display.menu(question, choices, "OK", "Cancel",
+                            default=0, force_interactive=True)
     if response[0] == display_util.CANCEL:
         # TODO: Add notification related to command-line options for
         #       skipping the menu for this case.
@@ -282,17 +284,26 @@ def _find_domains_or_certname(config, installer):
     """Retrieve domains and certname from config or user input.
     """
     domains = None
+    certname = config.certname
+    # first, try to get domains from the config
     if config.domains:
         domains = config.domains
-    elif not config.certname:
+    # if we can't do that but we have a certname, get the domains
+    # with that certname
+    elif certname:
+        domains = cert_manager.domains_for_certname(config, certname)
+
+    # that certname might not have existed, or there was a problem.
+    # try to get domains from the user.
+    if not domains:
         domains = display_ops.choose_names(installer)
 
-    if not domains and not config.certname:
+    if not domains and not certname:
         raise errors.Error("Please specify --domains, or --installer that "
                            "will help in domain names autodiscovery, or "
                            "--cert-name for an existing certificate name.")
 
-    return domains, config.certname
+    return domains, certname
 
 
 def _report_new_cert(config, cert_path, fullchain_path):
@@ -365,7 +376,8 @@ def _determine_account(config):
                        "server at {1}".format(
                            regr.terms_of_service, config.server))
                 obj = zope.component.getUtility(interfaces.IDisplay)
-                return obj.yesno(msg, "Agree", "Cancel", cli_flag="--agree-tos")
+                return obj.yesno(msg, "Agree", "Cancel",
+                                 cli_flag="--agree-tos", force_interactive=True)
 
             try:
                 acc, acme = client.register(
@@ -511,6 +523,14 @@ def rename(config, unused_plugins):
     """
     cert_manager.rename_lineage(config)
 
+def delete(config, unused_plugins):
+    """Delete a certificate
+
+    Use the information in the config file to delete an existing
+    lineage.
+    """
+    cert_manager.delete(config)
+
 def certificates(config, unused_plugins):
     """Display information about certs configured with Certbot
     """
@@ -712,6 +732,7 @@ def _handle_exception(exc_type, exc_value, trace, config):
                 with open(logfile, "w") as logfd:
                     traceback.print_exception(
                         exc_type, exc_value, trace, file=logfd)
+                assert "--debug" not in sys.argv  # config is None if this explodes
             except:  # pylint: disable=bare-except
                 sys.exit(tb_str)
             if "--debug" in sys.argv:
@@ -779,7 +800,8 @@ def set_displayer(config):
     elif config.noninteractive_mode:
         displayer = display_util.NoninteractiveDisplay(sys.stdout)
     else:
-        displayer = display_util.FileDisplay(sys.stdout)
+        displayer = display_util.FileDisplay(sys.stdout,
+                                             config.force_interactive)
     zope.component.provideUtility(displayer)
 
 def _post_logging_setup(config, plugins, cli_args):

certbot/ocsp.py

@@ -0,0 +1,109 @@
+"""Tools for checking certificate revocation."""
+import logging
+
+from subprocess import Popen, PIPE
+
+from certbot import errors
+from certbot import util
+
+logger = logging.getLogger(__name__)
+
+class RevocationChecker(object):
+    "This class figures out OCSP checking on this system, and performs it."
+
+    def __init__(self):
+        self.broken = False
+
+        if not util.exe_exists("openssl"):
+            logging.info("openssl not installed, can't check revocation")
+            self.broken = True
+            return
+
+       # New versions of openssl want -header var=val, old ones want -header var val
+        test_host_format = Popen(["openssl", "ocsp", "-header", "var", "val"],
+                                 stdout=PIPE, stderr=PIPE, universal_newlines=True)
+        _out, err = test_host_format.communicate()
+        if "Missing =" in err:
+            self.host_args = lambda host: ["Host=" + host]
+        else:
+            self.host_args = lambda host: ["Host", host]
+
+
+    def ocsp_revoked(self, cert_path, chain_path):
+        """Get revoked status for a particular cert version.
+
+        .. todo:: Make this a non-blocking call
+
+        :param str cert_path: Path to certificate
+        :param str chain_path: Path to intermediate cert
+        :rtype bool or None:
+        :returns: True if revoked; False if valid or the check failed
+
+        """
+        if self.broken:
+            return False
+
+
+        logger.debug("Querying OCSP for %s", cert_path)
+        url, host = self.determine_ocsp_server(cert_path)
+        if not host:
+            return False
+        # jdkasten thanks "Bulletproof SSL and TLS - Ivan Ristic" for documenting this!
+        cmd = ["openssl", "ocsp",
+               "-no_nonce",
+               "-issuer", chain_path,
+               "-cert", cert_path,
+               "-url", url,
+               "-CAfile", chain_path,
+               "-verify_other", chain_path,
+               "-header"] + self.host_args(host)
+        try:
+            output, err = util.run_script(cmd, log=logging.debug)
+        except errors.SubprocessError as e:
+            logger.info("OCSP check failed for %s (are we offline?)", cert_path)
+            logger.debug("Command was:\n%s\nError was:\n%s", " ".join(cmd), e)
+            return False
+
+        return _translate_ocsp_query(cert_path, output, err)
+
+
+    def determine_ocsp_server(self, cert_path):
+        """Extract the OCSP server host from a certificate.
+
+        :param str cert_path: Path to the cert we're checking OCSP for
+        :rtype tuple:
+        :returns: (OCSP server URL or None, OCSP server host or None)
+
+        """
+        try:
+            url, _err = util.run_script(
+                ["openssl", "x509", "-in", cert_path, "-noout", "-ocsp_uri"],
+                log=logging.debug)
+        except errors.SubprocessError as e:
+            logger.info("Cannot extract OCSP URI from %s", cert_path)
+            logger.debug("Error was:\n%s", e)
+            return None, None
+
+        url = url.rstrip()
+        host = url.partition("://")[2].rstrip("/")
+        if host:
+            return url, host
+        else:
+            logger.info("Cannot process OCSP host from URL (%s) in cert at %s", url, cert_path)
+            return None, None
+
+def _translate_ocsp_query(cert_path, ocsp_output, ocsp_errors):
+    """Parse openssl's weird output to work out what it means."""
+
+    if not "Response verify OK" in ocsp_errors:
+        logger.info("Revocation status for %s is unknown", cert_path)
+        logger.debug("Uncertain ouput:\n%s\nstderr:\n%s", ocsp_output, ocsp_errors)
+        return False
+    if cert_path + ": good" in ocsp_output:
+        return False
+    elif cert_path + ": revoked" in ocsp_output:
+        return True
+    else:
+        logger.warn("Unable to properly parse OCSP output: %s", ocsp_output)
+        return False
+

certbot/plugins/manual.py

@@ -1,56 +1,49 @@
-"""Manual plugin."""
+"""Manual authenticator plugin"""
 import os
-import logging
-import pipes
-import shutil
-import socket
-import subprocess
-import sys
-import tempfile
-import time
 
-import six
 import zope.component
 import zope.interface
 
 from acme import challenges
-from acme import errors as acme_errors
 
-from certbot import errors
 from certbot import interfaces
+from certbot import errors
+from certbot import hooks
 from certbot.plugins import common
 
 
-logger = logging.getLogger(__name__)
-
-
 @zope.interface.implementer(interfaces.IAuthenticator)
 @zope.interface.provider(interfaces.IPluginFactory)
 class Authenticator(common.Plugin):
-    """Manual Authenticator.
+    """Manual authenticator
 
-    This plugin requires user's manual intervention in setting up a HTTP
-    server for solving http-01 challenges and thus does not need to be
-    run as a privileged process. Alternatively shows instructions on how
-    to use Python's built-in HTTP server.
-
-    .. todo:: Support for `~.challenges.TLSSNI01`.
+    This plugin allows the user to perform the domain validation
+    challenge(s) themselves. This either be done manually by the user or
+    through shell scripts provided to Certbot.
 
     """
+
+    description = 'Manual configuration or run your own shell scripts'
     hidden = True
-
-    description = "Manually configure an HTTP server"
-
-    MESSAGE_TEMPLATE = {
-        "dns-01": """\
+    long_description = (
+        'Authenticate through manual configuration or custom shell scripts. '
+        'When using shell scripts, an authenticator script must be provided. '
+        'The environment variables available to this script are '
+        '$CERTBOT_DOMAIN which contains the domain being authenticated, '
+        '$CERTBOT_VALIDATION which is the validation string, and '
+        '$CERTBOT_TOKEN which is the filename of the resource requested when '
+        'performing an HTTP-01 challenge. An additional cleanup script can '
+        'also be provided and can use the additional variable '
+        '$CERTBOT_AUTH_OUTPUT which contains the stdout output from the auth '
+        'script.')
+    _DNS_INSTRUCTIONS = """\
 Please deploy a DNS TXT record under the name
 {domain} with the following value:
 
 {validation}
 
-Once this is deployed,
-""",
-        "http-01": """\
+Once this is deployed,"""
+    _HTTP_INSTRUCTIONS = """\
 Make sure your web server displays the following content at
 {uri} before continuing:
 
@@ -59,203 +52,114 @@ Make sure your web server displays the following content at
 If you don't have HTTP server configured, you can run the following
 command on the target server (as root):
 
-{command}
-"""}
-
-    # a disclaimer about your current IP being transmitted to Let's Encrypt's servers.
-    IP_DISCLAIMER = """\
-NOTE: The IP of this machine will be publicly logged as having requested this certificate. \
-If you're running certbot in manual mode on a machine that is not your server, \
-please ensure you're okay with that.
-
-Are you OK with your IP being logged?
-"""
-
-    # "cd /tmp/certbot" makes sure user doesn't serve /root,
-    # separate "public_html" ensures that cert.pem/key.pem are not
-    # served and makes it more obvious that Python command will serve
-    # anything recursively under the cwd
-
-    CMD_TEMPLATE = """\
-mkdir -p {root}/public_html/{achall.URI_ROOT_PATH}
-cd {root}/public_html
+mkdir -p /tmp/certbot/public_html/{achall.URI_ROOT_PATH}
+cd /tmp/certbot/public_html
 printf "%s" {validation} > {achall.URI_ROOT_PATH}/{encoded_token}
 # run only once per server:
 $(command -v python2 || command -v python2.7 || command -v python2.6) -c \\
 "import BaseHTTPServer, SimpleHTTPServer; \\
 s = BaseHTTPServer.HTTPServer(('', {port}), SimpleHTTPServer.SimpleHTTPRequestHandler); \\
 s.serve_forever()" """
-    """Command template."""
 
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
-        self._root = (tempfile.mkdtemp() if self.conf("test-mode")
-                      else "/tmp/certbot")
-        self._httpd = None
+        self.env = dict()
 
     @classmethod
     def add_parser_arguments(cls, add):
-        add("test-mode", action="store_true",
-            help="Test mode. Executes the manual command in subprocess.")
-        add("public-ip-logging-ok", action="store_true",
-            help="Automatically allows public IP logging.")
+        add('auth-hook',
+            help='Path or command to execute for the authentication script')
+        add('cleanup-hook',
+            help='Path or command to execute for the cleanup script')
+        add('public-ip-logging-ok', action='store_true',
+            help='Automatically allows public IP logging (default: Ask)')
 
-    def prepare(self):  # pylint: disable=missing-docstring,no-self-use
-        if self.config.noninteractive_mode and not self.conf("test-mode"):
-            raise errors.PluginError("Running manual mode non-interactively is not supported")
+    def prepare(self):  # pylint: disable=missing-docstring
+        if self.config.noninteractive_mode and not self.conf('auth-hook'):
+            raise errors.PluginError(
+                'An authentication script must be provided with --{0} when '
+                'using the manual plugin non-interactively.'.format(
+                    self.option_name('auth-hook')))
+        self._validate_hooks()
+
+    def _validate_hooks(self):
+        if self.config.validate_hooks:
+            for name in ('auth-hook', 'cleanup-hook'):
+                hook = self.conf(name)
+                if hook is not None:
+                    hook_prefix = self.option_name(name)[:-len('-hook')]
+                    hooks.validate_hook(hook, hook_prefix)
 
     def more_info(self):  # pylint: disable=missing-docstring,no-self-use
-        return ("This plugin requires user's manual intervention in setting "
-                "up challenges to prove control of a domain and does not need "
-                "to be run as a privileged process. When solving "
-                "http-01 challenges, the user is responsible for setting up "
-                "an HTTP server. Alternatively, instructions are shown on how "
-                "to use Python's built-in HTTP server. The user is "
-                "responsible for configuration of a domain's DNS when solving "
-                "dns-01 challenges. The type of challenges used can be "
-                "controlled through the --preferred-challenges flag.")
+        return (
+            'This plugin allows the user to customize setup for domain '
+            'validation challenges either through shell scripts provided by '
+            'the user or by performing the setup manually.')
 
     def get_chall_pref(self, domain):
         # pylint: disable=missing-docstring,no-self-use,unused-argument
         return [challenges.HTTP01, challenges.DNS01]
 
-    def perform(self, achalls):
-        # pylint: disable=missing-docstring
-        self._get_ip_logging_permission()
-        mapping = {"http-01": self._perform_http01_challenge,
-                   "dns-01": self._perform_dns01_challenge}
+    def perform(self, achalls):  # pylint: disable=missing-docstring
+        self._verify_ip_logging_ok()
+
+        if self.conf('auth-hook'):
+            perform_achall = self._perform_achall_with_script
+        else:
+            perform_achall = self._perform_achall_manually
+
         responses = []
-        # TODO: group achalls by the same socket.gethostbyname(_ex)
-        # and prompt only once per server (one "echo -n" per domain)
         for achall in achalls:
-            responses.append(mapping[achall.typ](achall))
+            perform_achall(achall)
+            responses.append(achall.response(achall.account_key))
         return responses
 
-    @classmethod
-    def _test_mode_busy_wait(cls, port):
-        while True:
-            time.sleep(1)
-            sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-            try:
-                sock.connect(("localhost", port))
-            except socket.error:  # pragma: no cover
-                pass
+    def _verify_ip_logging_ok(self):
+        if not self.conf('public-ip-logging-ok'):
+            cli_flag = '--{0}'.format(self.option_name('public-ip-logging-ok'))
+            msg = ('NOTE: The IP of this machine will be publicly logged as '
+                   "having requested this certificate. If you're running "
+                   'certbot in manual mode on a machine that is not your '
+                   "server, please ensure you're okay with that.\n\n"
+                   'Are you OK with your IP being logged?')
+            display = zope.component.getUtility(interfaces.IDisplay)
+            if display.yesno(msg, cli_flag=cli_flag, force_interactive=True):
+                setattr(self.config, self.dest('public-ip-logging-ok'), True)
             else:
-                break
-            finally:
-                sock.close()
+                raise errors.PluginError('Must agree to IP logging to proceed')
 
-    def cleanup(self, achalls):
-        # pylint: disable=missing-docstring
-        for achall in achalls:
-            if isinstance(achall.chall, challenges.HTTP01):
-                self._cleanup_http01_challenge(achall)
-
-    def _perform_http01_challenge(self, achall):
-        # same path for each challenge response would be easier for
-        # users, but will not work if multiple domains point at the
-        # same server: default command doesn't support virtual hosts
-        response, validation = achall.response_and_validation()
-
-        port = (response.port if self.config.http01_port is None
-                else int(self.config.http01_port))
-        command = self.CMD_TEMPLATE.format(
-            root=self._root, achall=achall, response=response,
-            # TODO(kuba): pipes still necessary?
-            validation=pipes.quote(validation),
-            encoded_token=achall.chall.encode("token"),
-            port=port)
-        if self.conf("test-mode"):
-            logger.debug("Test mode. Executing the manual command: %s", command)
-            # sh shipped with OS X does't support echo -n, but supports printf
-            try:
-                self._httpd = subprocess.Popen(
-                    command,
-                    # don't care about setting stdout and stderr,
-                    # we're in test mode anyway
-                    shell=True,
-                    executable=None,
-                    # "preexec_fn" is UNIX specific, but so is "command"
-                    preexec_fn=os.setsid)
-            except OSError as error:  # ValueError should not happen!
-                logger.debug(
-                    "Couldn't execute manual command: %s", error, exc_info=True)
-                return False
-            logger.debug("Manual command running as PID %s.", self._httpd.pid)
-            # give it some time to bootstrap, before we try to verify
-            # (cert generation in case of simpleHttpS might take time)
-            self._test_mode_busy_wait(port)
-
-            if self._httpd.poll() is not None:
-                raise errors.Error("Couldn't execute manual command")
+    def _perform_achall_with_script(self, achall):
+        env = dict(CERTBOT_DOMAIN=achall.domain,
+                   CERTBOT_VALIDATION=achall.validation(achall.account_key))
+        if isinstance(achall.chall, challenges.HTTP01):
+            env['CERTBOT_TOKEN'] = achall.chall.encode('token')
         else:
-            self._notify_and_wait(
-                self._get_message(achall).format(
-                    validation=validation,
-                    response=response,
-                    uri=achall.chall.uri(achall.domain),
-                    command=command))
+            os.environ.pop('CERTBOT_TOKEN', None)
+        os.environ.update(env)
+        _, out = hooks.execute(self.conf('auth-hook'))
+        env['CERTBOT_AUTH_OUTPUT'] = out.strip()
+        self.env[achall.domain] = env
 
-        if not response.simple_verify(
-                achall.chall, achall.domain,
-                achall.account_key.public_key(), self.config.http01_port):
-            logger.warning("Self-verify of challenge failed.")
-
-        return response
-
-    def _perform_dns01_challenge(self, achall):
-        response, validation = achall.response_and_validation()
-        if not self.conf("test-mode"):
-            self._notify_and_wait(
-                self._get_message(achall).format(
-                    validation=validation,
-                    domain=achall.validation_domain_name(achall.domain),
-                    response=response))
-
-        try:
-            verification_status = response.simple_verify(
-                achall.chall, achall.domain,
-                achall.account_key.public_key())
-        except acme_errors.DependencyError:
-            logger.warning("Self verification requires optional "
-                           "dependency `dnspython` to be installed.")
+    def _perform_achall_manually(self, achall):
+        validation = achall.validation(achall.account_key)
+        if isinstance(achall.chall, challenges.HTTP01):
+            msg = self._HTTP_INSTRUCTIONS.format(
+                achall=achall, encoded_token=achall.chall.encode('token'),
+                port=self.config.http01_port,
+                uri=achall.chall.uri(achall.domain), validation=validation)
         else:
-            if not verification_status:
-                logger.warning("Self-verify of challenge failed.")
+            assert isinstance(achall.chall, challenges.DNS01)
+            msg = self._DNS_INSTRUCTIONS.format(
+                domain=achall.validation_domain_name(achall.domain),
+                validation=validation)
+        display = zope.component.getUtility(interfaces.IDisplay)
+        display.notification(msg, wrap=False, force_interactive=True)
 
-        return response
-
-    def _cleanup_http01_challenge(self, achall):
-        # pylint: disable=missing-docstring,unused-argument
-        if self.conf("test-mode"):
-            assert self._httpd is not None, (
-                "cleanup() must be called after perform()")
-            if self._httpd.poll() is None:
-                logger.debug("Terminating manual command process")
-                self._httpd.terminate()
-            else:
-                logger.debug("Manual command process already terminated "
-                             "with %s code", self._httpd.returncode)
-            shutil.rmtree(self._root)
-
-    def _notify_and_wait(self, message):
-        # pylint: disable=no-self-use
-        # TODO: IDisplay wraps messages, breaking the command
-        #answer = zope.component.getUtility(interfaces.IDisplay).notification(
-        #    message=message, pause=True)
-        sys.stdout.write(message)
-        six.moves.input("Press ENTER to continue")
-
-    def _get_ip_logging_permission(self):
-        # pylint: disable=missing-docstring
-        if not (self.conf("test-mode") or self.conf("public-ip-logging-ok")):
-            if not zope.component.getUtility(interfaces.IDisplay).yesno(
-                    self.IP_DISCLAIMER, "Yes", "No",
-                    cli_flag="--manual-public-ip-logging-ok"):
-                raise errors.PluginError("Must agree to IP logging to proceed")
-            else:
-                self.config.namespace.manual_public_ip_logging_ok = True
-
-    def _get_message(self, achall):
-        # pylint: disable=missing-docstring,no-self-use,unused-argument
-        return self.MESSAGE_TEMPLATE.get(achall.chall.typ, "")
+    def cleanup(self, achalls):  # pylint: disable=missing-docstring
+        if self.conf('cleanup-hook'):
+            for achall in achalls:
+                env = self.env.pop(achall.domain)
+                if 'CERTBOT_TOKEN' not in env:
+                    os.environ.pop('CERTBOT_TOKEN', None)
+                os.environ.update(env)
+                hooks.execute(self.conf('cleanup-hook'))

certbot/plugins/manual_test.py

@@ -1,134 +1,112 @@
-"""Tests for certbot.plugins.manual."""
+"""Tests for certbot.plugins.manual"""
+import os
 import unittest
 
+import six
 import mock
 
 from acme import challenges
-from acme import errors as acme_errors
-from acme import jose
 
-from certbot import achallenges
 from certbot import errors
-
 from certbot.tests import acme_util
-from certbot.tests import util as test_util
-
-
-KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key.pem"))
 
 
 class AuthenticatorTest(unittest.TestCase):
     """Tests for certbot.plugins.manual.Authenticator."""
 
     def setUp(self):
-        from certbot.plugins.manual import Authenticator
+        self.http_achall = acme_util.HTTP01_A
+        self.dns_achall = acme_util.DNS01_A
+        self.achalls = [self.http_achall, self.dns_achall]
         self.config = mock.MagicMock(
-            http01_port=8080, manual_test_mode=False,
-            manual_public_ip_logging_ok=False, noninteractive_mode=True)
-        self.auth = Authenticator(config=self.config, name="manual")
+            http01_port=0, manual_auth_hook=None, manual_cleanup_hook=None,
+            manual_public_ip_logging_ok=False, noninteractive_mode=False,
+            validate_hooks=False)
 
-        self.http01 = achallenges.KeyAuthorizationAnnotatedChallenge(
-            challb=acme_util.HTTP01_P, domain="foo.com", account_key=KEY)
-        self.dns01 = achallenges.KeyAuthorizationAnnotatedChallenge(
-            challb=acme_util.DNS01_P, domain="foo.com", account_key=KEY)
+        from certbot.plugins.manual import Authenticator
+        self.auth = Authenticator(self.config, name='manual')
 
-        self.achalls = [self.http01, self.dns01]
-
-        config_test_mode = mock.MagicMock(
-            http01_port=8080, manual_test_mode=True, noninteractive_mode=True)
-        self.auth_test_mode = Authenticator(
-            config=config_test_mode, name="manual")
-
-    def test_prepare(self):
+    def test_prepare_no_hook_noninteractive(self):
+        self.config.noninteractive_mode = True
         self.assertRaises(errors.PluginError, self.auth.prepare)
-        self.auth_test_mode.prepare()  # error not raised
+
+    def test_prepare_bad_hook(self):
+        self.config.manual_auth_hook = os.path.abspath(os.sep)  # is / on UNIX
+        self.config.validate_hooks = True
+        self.assertRaises(errors.HookCommandNotFound, self.auth.prepare)
 
     def test_more_info(self):
-        self.assertTrue(isinstance(self.auth.more_info(), str))
+        self.assertTrue(isinstance(self.auth.more_info(), six.string_types))
 
     def test_get_chall_pref(self):
-        self.assertTrue(all(issubclass(pref, challenges.Challenge)
-                            for pref in self.auth.get_chall_pref("foo.com")))
+        self.assertEqual(self.auth.get_chall_pref('example.org'),
+                         [challenges.HTTP01, challenges.DNS01])
 
-    @mock.patch("certbot.plugins.manual.zope.component.getUtility")
-    def test_perform_empty(self, mock_interaction):
-        mock_interaction().yesno.return_value = True
-        self.assertEqual([], self.auth.perform([]))
+    @mock.patch('certbot.plugins.manual.zope.component.getUtility')
+    def test_ip_logging_not_ok(self, mock_get_utility):
+        mock_get_utility().yesno.return_value = False
+        self.assertRaises(errors.PluginError, self.auth.perform, [])
 
-    @mock.patch("certbot.plugins.manual.zope.component.getUtility")
-    @mock.patch("certbot.plugins.manual.sys.stdout")
-    @mock.patch("acme.challenges.HTTP01Response.simple_verify")
-    @mock.patch("six.moves.input")
-    def test_perform(self, mock_raw_input, mock_verify, mock_stdout, mock_interaction):
-        mock_verify.return_value = True
-        mock_interaction().yesno.return_value = True
+    @mock.patch('certbot.plugins.manual.zope.component.getUtility')
+    def test_ip_logging_ok(self, mock_get_utility):
+        mock_get_utility().yesno.return_value = True
+        self.auth.perform([])
+        self.assertTrue(self.config.manual_public_ip_logging_ok)
 
-        resp_http = self.http01.response(KEY)
-        resp_dns = self.dns01.response(KEY)
+    def test_script_perform(self):
+        self.config.manual_public_ip_logging_ok = True
+        self.config.manual_auth_hook = (
+            'echo $CERTBOT_DOMAIN; echo ${CERTBOT_TOKEN:-notoken}; '
+            'echo $CERTBOT_VALIDATION;')
+        dns_expected = '{0}\n{1}\n{2}'.format(
+            self.dns_achall.domain, 'notoken',
+            self.dns_achall.validation(self.dns_achall.account_key))
+        http_expected = '{0}\n{1}\n{2}'.format(
+            self.http_achall.domain, self.http_achall.chall.encode('token'),
+            self.http_achall.validation(self.http_achall.account_key))
 
-        self.assertEqual([resp_http, resp_dns], self.auth.perform(self.achalls))
-        self.assertEqual(2, mock_raw_input.call_count)
-        mock_verify.assert_called_with(
-            self.http01.challb.chall, "foo.com", KEY.public_key(), 8080)
+        self.assertEqual(
+            self.auth.perform(self.achalls),
+            [achall.response(achall.account_key) for achall in self.achalls])
+        self.assertEqual(
+            self.auth.env[self.dns_achall.domain]['CERTBOT_AUTH_OUTPUT'],
+            dns_expected)
+        self.assertEqual(
+            self.auth.env[self.http_achall.domain]['CERTBOT_AUTH_OUTPUT'],
+            http_expected)
 
-        message = mock_stdout.write.mock_calls[0][1][0]
-        self.assertTrue(self.http01.chall.encode("token") in message)
+    @mock.patch('certbot.plugins.manual.zope.component.getUtility')
+    def test_manual_perform(self, mock_get_utility):
+        self.config.manual_public_ip_logging_ok = True
+        self.assertEqual(
+            self.auth.perform(self.achalls),
+            [achall.response(achall.account_key) for achall in self.achalls])
+        for i, (args, kwargs) in enumerate(mock_get_utility().notification.call_args_list):
+            achall = self.achalls[i]
+            self.assertTrue(achall.validation(achall.account_key) in args[0])
+            self.assertFalse(kwargs['wrap'])
 
-        mock_verify.return_value = False
-        with mock.patch("certbot.plugins.manual.logger") as mock_logger:
-            self.auth.perform(self.achalls)
-            self.assertEqual(2, mock_logger.warning.call_count)
+    def test_cleanup(self):
+        self.config.manual_public_ip_logging_ok = True
+        self.config.manual_auth_hook = 'echo foo;'
+        self.config.manual_cleanup_hook = '# cleanup'
+        self.auth.perform(self.achalls)
 
-    @mock.patch("certbot.plugins.manual.zope.component.getUtility")
-    @mock.patch("acme.challenges.DNS01Response.simple_verify")
-    @mock.patch("six.moves.input")
-    def test_perform_missing_dependency(self, mock_raw_input, mock_verify, mock_interaction):
-        mock_interaction().yesno.return_value = True
-        mock_verify.side_effect = acme_errors.DependencyError()
+        for achall in self.achalls:
+            self.auth.cleanup([achall])
+            self.assertEqual(os.environ['CERTBOT_AUTH_OUTPUT'], 'foo')
+            self.assertEqual(os.environ['CERTBOT_DOMAIN'], achall.domain)
+            self.assertEqual(
+                os.environ['CERTBOT_VALIDATION'],
+                achall.validation(achall.account_key))
 
-        with mock.patch("certbot.plugins.manual.logger") as mock_logger:
-            self.auth.perform([self.dns01])
-            self.assertEqual(1, mock_logger.warning.call_count)
-
-        mock_raw_input.assert_called_once_with("Press ENTER to continue")
-
-    @mock.patch("certbot.plugins.manual.zope.component.getUtility")
-    @mock.patch("certbot.plugins.manual.Authenticator._notify_and_wait")
-    def test_disagree_with_ip_logging(self, mock_notify, mock_interaction):
-        mock_interaction().yesno.return_value = False
-        mock_notify.side_effect = errors.Error("Exception not raised, \
-            continued execution even after disagreeing with IP logging")
-
-        self.assertRaises(errors.PluginError, self.auth.perform, self.achalls)
-
-    @mock.patch("certbot.plugins.manual.subprocess.Popen", autospec=True)
-    def test_perform_test_command_oserror(self, mock_popen):
-        mock_popen.side_effect = OSError
-        self.assertEqual([False], self.auth_test_mode.perform([self.http01]))
-
-    @mock.patch("certbot.plugins.manual.socket.socket")
-    @mock.patch("certbot.plugins.manual.time.sleep", autospec=True)
-    @mock.patch("certbot.plugins.manual.subprocess.Popen", autospec=True)
-    def test_perform_test_command_run_failure(
-            self, mock_popen, unused_mock_sleep, unused_mock_socket):
-        mock_popen.poll.return_value = 10
-        mock_popen.return_value.pid = 1234
-        self.assertRaises(
-            errors.Error, self.auth_test_mode.perform, self.achalls)
-
-    def test_cleanup_test_mode_already_terminated(self):
-        # pylint: disable=protected-access
-        self.auth_test_mode._httpd = httpd = mock.Mock()
-        httpd.poll.return_value = 0
-        self.auth_test_mode.cleanup(self.achalls)
-
-    def test_cleanup_test_mode_kills_still_running(self):
-        # pylint: disable=protected-access
-        self.auth_test_mode._httpd = httpd = mock.Mock(pid=1234)
-        httpd.poll.return_value = None
-        self.auth_test_mode.cleanup(self.achalls)
-        httpd.terminate.assert_called_once_with()
+            if isinstance(achall.chall, challenges.HTTP01):
+                self.assertEqual(
+                    os.environ['CERTBOT_TOKEN'],
+                    achall.chall.encode('token'))
+            else:
+                self.assertFalse('CERTBOT_TOKEN' in os.environ)
 
 
-if __name__ == "__main__":
+if __name__ == '__main__':
     unittest.main()  # pragma: no cover

certbot/plugins/script.py

@@ -1,161 +0,0 @@
-"""Script-based Authenticator."""
-import logging
-import os
-import sys
-
-import zope.interface
-
-from acme import challenges
-
-from certbot import errors
-from certbot import interfaces
-from certbot import hooks
-
-from certbot.plugins import common
-
-logger = logging.getLogger(__name__)
-
-
-CHALLENGES = ["http-01", "dns-01"]
-
-
-@zope.interface.implementer(interfaces.IAuthenticator)
-@zope.interface.provider(interfaces.IPluginFactory)
-class Authenticator(common.Plugin):
-    """Script authenticator
-
-    calls user defined script to perform authentication and
-    optionally cleanup.
-
-    """
-
-    description = "Authenticate using user provided script(s)"
-
-    long_description = ("Authenticate using user provided script(s). " +
-                        "Authenticator script has the following environment " +
-                        "variables available for it: " +
-                        "CERTBOT_DOMAIN - The domain being authenticated " +
-                        "CERTBOT_VALIDATION - The validation string " +
-                        "CERTBOT_TOKEN - Resource name part of HTTP-01 " +
-                        "challenge (HTTP-01 only). " +
-                        "Cleanup script has all the above, and additional " +
-                        "var: CERTBOT_AUTH_OUTPUT - stdout output from the " +
-                        "authenticator"
-                    )
-
-    def __init__(self, *args, **kwargs):
-        super(Authenticator, self).__init__(*args, **kwargs)
-        self.cleanup_script = None
-        self.auth_script = None
-        self.challenges = []
-
-    @classmethod
-    def add_parser_arguments(cls, add):
-        add("auth", default=None, required=False,
-            help="path or command for the authentication script")
-        add("cleanup", default=None, required=False,
-            help="path or command for the cleanup script")
-
-    @property
-    def supported_challenges(self):
-        """Challenges supported by this plugin."""
-        return self.challenges
-
-    def more_info(self):  # pylint: disable=missing-docstring
-        return("This authenticator enables user to perform authentication " +
-               "using shell script(s).")
-
-    def prepare(self):
-        """Prepare script plugin, check challenge, scripts and register them"""
-        pref_challenges = self.config.pref_challs
-        for c in pref_challenges:
-            if c.typ in CHALLENGES:
-                self.challenges.append(c)
-        if not self.challenges and len(pref_challenges):
-            # Challenges requested, but not supported
-            raise errors.PluginError(
-                "Unfortunately script plugin doesn't yet support " +
-                "the requested challenges")
-
-        # Challenge not defined on cli, set default
-        if not self.challenges:
-            self.challenges.append(challenges.Challenge.TYPES["http-01"])
-
-        if not self.conf("auth"):
-            raise errors.PluginError("Parameter --script-auth is required " +
-                                     "for script plugin")
-        self._prepare_scripts()
-
-    def _prepare_scripts(self):
-        """Helper method for prepare, to take care of validating scripts"""
-        script_path = self.conf("auth")
-        cleanup_path = self.conf("cleanup")
-        if self.config.validate_hooks:
-            hooks.validate_hook(script_path, "script_auth")
-        self.auth_script = script_path
-        if cleanup_path:
-            if self.config.validate_hooks:
-                hooks.validate_hook(cleanup_path, "script_cleanup")
-            self.cleanup_script = cleanup_path
-
-    def get_chall_pref(self, domain):
-        """Return challenge(s) we're answering to """
-        # pylint: disable=unused-argument
-        return self.challenges
-
-    def perform(self, achalls):
-        """Perform the authentication per challenge"""
-        mapping = {"http-01": self._setup_env_http,
-                   "dns-01": self._setup_env_dns}
-        responses = []
-        for achall in achalls:
-            response, validation = achall.response_and_validation()
-            # Setup env vars
-            mapping[achall.typ](achall, validation)
-            output = self.execute(self.auth_script)
-            if output:
-                self._write_auth_output(output)
-            responses.append(response)
-        return responses
-
-    def _setup_env_http(self, achall, validation):
-        """Write environment variables for http challenge"""
-        ev = dict()
-        ev["CERTBOT_TOKEN"] = achall.chall.encode("token")
-        ev["CERTBOT_VALIDATION"] = validation
-        ev["CERTBOT_DOMAIN"] = achall.domain
-        os.environ.update(ev)
-
-    def _setup_env_dns(self, achall, validation):
-        """Write environment variables for dns challenge"""
-        ev = dict()
-        ev["CERTBOT_VALIDATION"] = validation
-        ev["CERTBOT_DOMAIN"] = achall.domain
-        os.environ.update(ev)
-
-    def _write_auth_output(self, out):
-        """Write output from auth script to env var for
-        cleanup to act upon"""
-        os.environ.update({"CERTBOT_AUTH_OUTPUT": out.strip()})
-
-    def _normalize_string(self, value):
-        """Return string instead of bytestring for Python3.
-        Helper function for writing env vars, as os.environ needs str"""
-
-        if isinstance(value, bytes):
-            value = value.decode(sys.getdefaultencoding())
-        return str(value)
-
-    def execute(self, shell_cmd):
-        """Run a script.
-
-        :param str shell_cmd: Command to run
-        :returns: `str` stdout output"""
-
-        _, out = hooks.execute(shell_cmd)
-        return self._normalize_string(out)
-
-    def cleanup(self, achalls):  # pylint: disable=unused-argument
-        """Run cleanup.sh """
-        if self.cleanup_script:
-            self.execute(self.cleanup_script)

certbot/plugins/script_test.py

@@ -1,170 +0,0 @@
-"""Tests for certbot.plugins.manual."""
-import os
-import tempfile
-import unittest
-
-import mock
-
-from acme import challenges
-from acme import jose
-
-from certbot import achallenges
-from certbot import errors
-
-from certbot.tests import acme_util
-from certbot.tests import util as test_util
-
-
-KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key.pem"))
-
-
-class AuthenticatorTest(unittest.TestCase):
-    """Tests for certbot.plugins.script.Authenticator."""
-
-    def setUp(self):
-        from certbot.plugins.script import Authenticator
-        self.auth_return_value = "return from auth\n"
-        self.script_nonexec = create_script(b'# empty')
-        self.script_exec = create_script_exec(b'echo "return from auth\n"')
-        self.config = mock.MagicMock(
-            script_auth=self.script_exec,
-            script_cleanup=self.script_exec,
-            pref_challs=[challenges.Challenge.TYPES["http-01"],
-                         challenges.Challenge.TYPES["dns-01"],
-                         challenges.Challenge.TYPES["tls-sni-01"]])
-
-        self.tlssni_config = mock.MagicMock(
-            script_auth=self.script_exec,
-            script_cleanup=self.script_exec,
-            pref_challs=[challenges.Challenge.TYPES["tls-sni-01"]])
-
-        self.nochall_config = mock.MagicMock(
-            script_auth=self.script_exec,
-            script_cleanup=self.script_exec,
-            )
-
-        self.default = Authenticator(config=self.config, name="script")
-        self.onlytlssni = Authenticator(config=self.tlssni_config,
-                                        name="script")
-        self.nochall = Authenticator(config=self.nochall_config,
-                                     name="script")
-
-        self.http01 = achallenges.KeyAuthorizationAnnotatedChallenge(
-            challb=acme_util.HTTP01_P, domain="foo.com", account_key=KEY)
-        self.dns01 = achallenges.KeyAuthorizationAnnotatedChallenge(
-            challb=acme_util.DNS01_P, domain="foo.com", account_key=KEY)
-
-        self.achalls = [self.http01, self.dns01]
-
-    def tearDown(self):
-        os.remove(self.script_exec)
-        os.remove(self.script_nonexec)
-
-    def test_prepare_normal(self):
-        """Test prepare with typical configuration"""
-        from certbot.plugins.script import Authenticator
-        # Erroring combinations in from of (auth_script, cleanup_script, error)
-        for v in [("/NONEXISTENT/script.sh", "/NONEXISTENT/script.sh",
-                   errors.HookCommandNotFound),
-                  (self.script_nonexec, "/NONEXISTENT/script.sh",
-                   errors.HookCommandNotFound),
-                  (self.script_exec, "/NONEXISTENT/script.sh",
-                   errors.HookCommandNotFound),
-                  ("/NONEXISTENT/script.sh", self.script_nonexec,
-                   errors.HookCommandNotFound),
-                  ("/NONEXISTENT/script.sh", self.script_exec,
-                   errors.HookCommandNotFound),
-                  (None, self.script_exec,
-                   errors.PluginError)]:
-            testconf = mock.MagicMock(
-                script_auth=v[0],
-                script_cleanup=v[1],
-                pref_challs=[challenges.Challenge.TYPES["http-01"]])
-            testauth = Authenticator(config=testconf, name="script")
-            self.assertRaises(v[2], testauth.prepare)
-
-        # This should not error
-        self.default.prepare()
-        self.assertEqual(len(self.default.challenges), 2)
-
-    def test_prepare_tlssni(self):
-        """Test for provided, but unsupported challenge type"""
-        self.assertRaises(errors.PluginError, self.onlytlssni.prepare)
-
-    def test_prepare_nochall(self):
-        """Test for default challenge"""
-        self.nochall.prepare()
-        self.assertEqual(len(self.nochall.challenges), 1)
-
-    def test_more_info(self):
-        self.assertTrue(isinstance(self.default.more_info(), str))
-
-    def test_get_chall_pref(self):
-        self.default.prepare()
-        self.assertTrue(all(issubclass(pref, challenges.Challenge)
-                            for pref in self.default.get_chall_pref(
-                                "foo.com")))
-
-    def test_get_supported_challenges(self):
-        self.default.prepare()
-        self.assertTrue(all(issubclass(sup, challenges.Challenge)
-                            for sup in self.default.supported_challenges))
-
-    def test_perform(self):
-        resp_http = self.http01.response(KEY)
-        resp_dns = self.dns01.response(KEY)
-        self.default.prepare()
-        # Check for the env vars prior to the run
-        self.assertFalse("CERTBOT_VALIDATION" in os.environ.keys())
-        self.assertFalse("CERTBOT_DOMAIN" in os.environ.keys())
-        self.assertFalse("CERTBOT_AUTH_OUTPUT" in os.environ.keys())
-
-        pref_resp = self.default.perform(self.achalls)
-        self.assertEqual([resp_http, resp_dns], pref_resp)
-        # Check for the env vars post run
-        self.assertTrue("CERTBOT_VALIDATION" in os.environ.keys())
-        self.assertTrue("CERTBOT_DOMAIN" in os.environ.keys())
-        self.assertTrue("CERTBOT_AUTH_OUTPUT" in os.environ.keys())
-        self.assertEqual(os.environ["CERTBOT_AUTH_OUTPUT"],
-                         self.auth_return_value.strip())
-
-    @mock.patch('certbot.plugins.script.Authenticator.execute')
-    def test_cleanup(self, mock_exec):
-        mock_exec.return_value = (0, None, None)
-        self.default.prepare()
-        self.default.cleanup(self.achalls)
-        self.assertEqual(mock_exec.call_count, 1)
-
-    @mock.patch('certbot.hooks.Popen')
-    def test_execute(self, mock_popen):
-        proc = mock.Mock()
-        # tuple values: stdout, stderr, errorcode, num_of_logger_calls
-        for t in [("", "", 0, 0),
-                  (self.auth_return_value, "", 0, 0),
-                  (None, "stderr_output", 0, 1),
-                  ("whatever", "stderr_output", 1, 2),
-                  (b'bytestring outval', "", 0, 0)]:
-            proc = mock.Mock()
-            attrs = {'communicate.return_value': (t[0], t[1]),
-                     'returncode': t[2]}
-            proc.configure_mock(**attrs)  # pylint: disable=star-args
-            mock_popen.return_value = proc
-            with mock.patch('certbot.hooks.logger.error') as mock_log:
-                output = self.default.execute(self.script_exec)
-                self.assertEqual(mock_log.call_count, t[3])
-                self.assertTrue(isinstance(output, str))
-
-
-def create_script(contents):
-    """ Helper to create temporary file """
-    f = tempfile.NamedTemporaryFile(delete=False, prefix='.sh')
-    f.write(contents)
-    f.close()
-    return f.name
-
-
-def create_script_exec(contents):
-    """ Helper to create temporary file with exec permissions"""
-    fname = create_script(contents)
-    os.chmod(fname, 0o700)
-    return fname

certbot/plugins/selection.py

@@ -111,7 +111,8 @@ def choose_plugin(prepared, question):
 
     while True:
         disp = z_util(interfaces.IDisplay)
-        code, index = disp.menu(question, opts, help_label="More Info")
+        code, index = disp.menu(
+            question, opts, help_label="More Info", force_interactive=True)
 
         if code == display_util.OK:
             plugin_ep = prepared[index]
@@ -127,11 +128,12 @@ def choose_plugin(prepared, question):
                 msg = "Reported Error: %s" % prepared[index].prepare()
             else:
                 msg = prepared[index].init().more_info()
-            z_util(interfaces.IDisplay).notification(msg)
+            z_util(interfaces.IDisplay).notification(msg,
+                                                     force_interactive=True)
         else:
             return None
 
-noninstaller_plugins = ["webroot", "manual", "standalone", "script"]
+noninstaller_plugins = ["webroot", "manual", "standalone"]
 
 def record_chosen_plugins(config, plugins, auth, inst):
     "Update the config entries to reflect the plugins we actually selected."
@@ -236,8 +238,6 @@ def cli_plugin_requests(config):
         req_auth = set_configurator(req_auth, "webroot")
     if config.manual:
         req_auth = set_configurator(req_auth, "manual")
-    if config.script:
-        req_auth = set_configurator(req_auth, "script")
     logger.debug("Requested authenticator %s and installer %s", req_auth, req_inst)
     return req_auth, req_inst
 

certbot/plugins/selection_test.py

@@ -110,7 +110,8 @@ class ChoosePluginTest(unittest.TestCase):
     """Tests for certbot.plugins.selection.choose_plugin."""
 
     def setUp(self):
-        zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
+        zope.component.provideUtility(display_util.FileDisplay(sys.stdout,
+                                                               False))
         self.mock_apache = mock.Mock(
             description_with_name="a", misconfigured=True)
         self.mock_stand = mock.Mock(

certbot/plugins/standalone.py

@@ -243,13 +243,13 @@ class Authenticator(common.Plugin):
                     "Could not bind TCP port {0} because you don't have "
                     "the appropriate permissions (for example, you "
                     "aren't running this program as "
-                    "root).".format(error.port))
+                    "root).".format(error.port), force_interactive=True)
             elif error.socket_error.errno == socket.errno.EADDRINUSE:
                 display.notification(
                     "Could not bind TCP port {0} because it is already in "
                     "use by another process on this system (such as a web "
                     "server). Please stop the program in question and then "
-                    "try again.".format(error.port))
+                    "try again.".format(error.port), force_interactive=True)
             else:
                 raise  # XXX: How to handle unknown errors in binding?
 

certbot/plugins/util.py

@@ -103,7 +103,7 @@ def already_listening_socket(port, renewer=False):
                 "Port {0} is already in use by another process. This will "
                 "prevent us from binding to that port. Please stop the "
                 "process that is populating the port in question and try "
-                "again. {1}".format(port, extra))
+                "again. {1}".format(port, extra), force_interactive=True)
             return True
         finally:
             testsocket.close()
@@ -151,7 +151,8 @@ def already_listening_psutil(port, renewer=False):
                 "The program {0} (process ID {1}) is already listening "
                 "on TCP port {2}. This will prevent us from binding to "
                 "that port. Please stop the {0} program temporarily "
-                "and then try again.{3}".format(name, pid, port, extra))
+                "and then try again.{3}".format(name, pid, port, extra),
+                force_interactive=True)
             return True
     except (psutil.NoSuchProcess, psutil.AccessDenied):
         # Perhaps the result of a race where the process could have

certbot/plugins/webroot.py

@@ -45,7 +45,7 @@ to serve all files under specified web root ({0})."""
                  "times to handle different domains; each domain will have "
                  "the webroot path that preceded it.  For instance: `-w "
                  "/var/www/example -d example.com -d www.example.com -w "
-                 "/var/www/thing -d thing.net -d m.thing.net`")
+                 "/var/www/thing -d thing.net -d m.thing.net` (default: Ask)")
         add("map", default={}, action=_WebrootMapAction,
             help="JSON dictionary mapping domains to webroot paths; this "
                  "implies -d for each entry. You may need to escape this from "
@@ -129,7 +129,8 @@ to serve all files under specified web root ({0})."""
                     "public_html or webroot directory. The webroot "
                     "plugin works by temporarily saving necessary "
                     "resources in the HTTP server's webroot directory "
-                    "to pass domain validation challenges.")
+                    "to pass domain validation challenges.",
+                    force_interactive=True)
             else:  # code == display_util.OK
                 return None if index == 0 else known_webroots[index - 1]
 
@@ -138,7 +139,8 @@ to serve all files under specified web root ({0})."""
 
         while True:
             code, webroot = display.directory_select(
-                "Input the webroot for {0}:".format(domain))
+                "Input the webroot for {0}:".format(domain),
+                force_interactive=True)
             if code == display_util.HELP:
                 # Displaying help is not currently implemented
                 return None

certbot/renewal.py

@@ -1,7 +1,6 @@
 """Functionality for autorenewal and associated juggling of configurations"""
 from __future__ import print_function
 import copy
-import glob
 import logging
 import os
 import traceback
@@ -11,7 +10,6 @@ import zope.component
 
 import OpenSSL
 
-from certbot import configuration
 from certbot import cli
 
 from certbot import crypto_util
@@ -35,17 +33,6 @@ STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
 INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
 
 
-def renewal_conf_files(config):
-    """Return /path/to/*.conf in the renewal conf directory"""
-    return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
-
-def renewal_file_for_certname(config, certname):
-    """Return /path/to/certname.conf in the renewal conf directory"""
-    path = os.path.join(config.renewal_configs_dir, "{0}.conf".format(certname))
-    if not os.path.exists(path):
-        raise errors.CertStorageError("No certificate found with name {0}.".format(certname))
-    return path
-
 def _reconstitute(config, full_path):
     """Try to instantiate a RenewableCert, updating config with relevant items.
 
@@ -64,8 +51,7 @@ def _reconstitute(config, full_path):
 
     """
     try:
-        renewal_candidate = storage.RenewableCert(
-            full_path, configuration.RenewerConfiguration(config))
+        renewal_candidate = storage.RenewableCert(full_path, config)
     except (errors.CertStorageError, IOError) as exc:
         logger.warning(exc)
         logger.warning("Renewal configuration file %s is broken. Skipping.", full_path)
@@ -247,9 +233,8 @@ def renew_cert(config, le_client, lineage):
         new_cert = OpenSSL.crypto.dump_certificate(
             OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped)
         new_chain = crypto_util.dump_pyopenssl_chain(new_chain)
-        renewal_conf = configuration.RenewerConfiguration(config.namespace)
         # TODO: Check return value of save_successor
-        lineage.save_successor(prior_version, new_cert, new_key.pem, new_chain, renewal_conf)
+        lineage.save_successor(prior_version, new_cert, new_key.pem, new_chain, config)
         lineage.update_all_links_to(lineage.latest_common_version())
 
     hooks.renew_hook(config, lineage.names(), lineage.live_dir)
@@ -318,12 +303,10 @@ def handle_renewal_request(config):
                            "command. The renew verb may provide other options "
                            "for selecting certificates to renew in the future.")
 
-    renewer_config = configuration.RenewerConfiguration(config)
-
     if config.certname:
-        conf_files = [renewal_file_for_certname(renewer_config, config.certname)]
+        conf_files = [storage.renewal_file_for_certname(config, config.certname)]
     else:
-        conf_files = renewal_conf_files(renewer_config)
+        conf_files = storage.renewal_conf_files(config)
 
     renew_successes = []
     renew_failures = []

certbot/reverter.py

@@ -181,7 +181,7 @@ class Reverter(object):
         if for_logging:
             return os.linesep.join(output)
         zope.component.getUtility(interfaces.IDisplay).notification(
-            os.linesep.join(output))
+            os.linesep.join(output), force_interactive=True)
 
     def add_to_temp_checkpoint(self, save_files, save_notes):
         """Add files to temporary checkpoint.

certbot/storage.py

@@ -1,5 +1,6 @@
 """Renewable certificates storage."""
 import datetime
+import glob
 import logging
 import os
 import re
@@ -7,6 +8,7 @@ import re
 import configobj
 import parsedatetime
 import pytz
+import shutil
 import six
 
 import certbot
@@ -20,9 +22,22 @@ from certbot import util
 logger = logging.getLogger(__name__)
 
 ALL_FOUR = ("cert", "privkey", "chain", "fullchain")
+README = "README"
 CURRENT_VERSION = util.get_strict_version(certbot.__version__)
 
 
+def renewal_conf_files(config):
+    """Return /path/to/*.conf in the renewal conf directory"""
+    return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
+
+def renewal_file_for_certname(config, certname):
+    """Return /path/to/certname.conf in the renewal conf directory"""
+    path = os.path.join(config.renewal_configs_dir, "{0}.conf".format(certname))
+    if not os.path.exists(path):
+        raise errors.CertStorageError("No certificate found with name {0} (expected "
+            "{1}).".format(certname, path))
+    return path
+
 def config_with_defaults(config=None):
     """Merge supplied config, if provided, on top of builtin defaults."""
     defaults_copy = configobj.ConfigObj(constants.RENEWER_DEFAULTS)
@@ -99,13 +114,11 @@ def write_renewal_config(o_filename, n_filename, archive_dir, target, relevant_d
 def rename_renewal_config(prev_name, new_name, cli_config):
     """Renames cli_config.certname's config to cli_config.new_certname.
 
-    :param .RenewerConfiguration cli_config: parsed command line
+    :param .NamespaceConfig cli_config: parsed command line
         arguments
     """
-    prev_filename = os.path.join(
-        cli_config.renewal_configs_dir, prev_name) + ".conf"
-    new_filename = os.path.join(
-        cli_config.renewal_configs_dir, new_name) + ".conf"
+    prev_filename = renewal_filename_for_lineagename(cli_config, prev_name)
+    new_filename = renewal_filename_for_lineagename(cli_config, new_name)
     if os.path.exists(new_filename):
         raise errors.ConfigurationError("The new certificate name "
             "is already in use.")
@@ -122,15 +135,14 @@ def update_configuration(lineagename, archive_dir, target, cli_config):
     :param str lineagename: Name of the lineage being modified
     :param str archive_dir: Absolute path to the archive directory
     :param dict target: Maps ALL_FOUR to their symlink paths
-    :param .RenewerConfiguration cli_config: parsed command line
+    :param .NamespaceConfig cli_config: parsed command line
         arguments
 
     :returns: Configuration object for the updated config file
     :rtype: configobj.ConfigObj
 
     """
-    config_filename = os.path.join(
-        cli_config.renewal_configs_dir, lineagename) + ".conf"
+    config_filename = renewal_filename_for_lineagename(cli_config, lineagename)
     temp_filename = config_filename + ".new"
 
     # If an existing tempfile exists, delete it
@@ -198,6 +210,98 @@ def lineagename_for_filename(config_filename):
             "renewal config file name must end in .conf")
     return os.path.basename(config_filename[:-len(".conf")])
 
+def renewal_filename_for_lineagename(config, lineagename):
+    """Returns the lineagename for a configuration filename.
+    """
+    return os.path.join(config.renewal_configs_dir, lineagename) + ".conf"
+
+def _relpath_from_file(archive_dir, from_file):
+    """Path to a directory from a file"""
+    return os.path.relpath(archive_dir, os.path.dirname(from_file))
+
+def _full_archive_path(config_obj, cli_config, lineagename):
+    """Returns the full archive path for a lineagename
+
+    Uses cli_config to determine archive path if not available from config_obj.
+
+    :param configobj.ConfigObj config_obj: Renewal conf file contents (can be None)
+    :param configuration.NamespaceConfig cli_config: Main config file
+    :param str lineagename: Certificate name
+    """
+    if config_obj and "archive_dir" in config_obj:
+        return config_obj["archive_dir"]
+    else:
+        return os.path.join(cli_config.default_archive_dir, lineagename)
+
+def _full_live_path(cli_config, lineagename):
+    """Returns the full default live path for a lineagename"""
+    return os.path.join(cli_config.live_dir, lineagename)
+
+def delete_files(config, certname):
+    """Delete all files related to the certificate.
+
+    If some files are not found, ignore them and continue.
+    """
+    renewal_filename = renewal_file_for_certname(config, certname)
+    # file exists
+    full_default_archive_dir = _full_archive_path(None, config, certname)
+    full_default_live_dir = _full_live_path(config, certname)
+    try:
+        renewal_config = configobj.ConfigObj(renewal_filename)
+    except configobj.ConfigObjError:
+        # config is corrupted
+        logger.warning("Could not parse %s. You may wish to manually "
+            "delete the contents of %s and %s.", renewal_filename,
+            full_default_live_dir, full_default_archive_dir)
+        raise errors.CertStorageError(
+            "error parsing {0}".format(renewal_filename))
+    finally:
+        # we couldn't read it, but let's at least delete it
+        # if this was going to fail, it already would have.
+        os.remove(renewal_filename)
+        logger.debug("Removed %s", renewal_filename)
+
+    # cert files and (hopefully) live directory
+    # it's not guaranteed that the files are in our default storage
+    # structure. so, first delete the cert files.
+    directory_names = set()
+    for kind in ALL_FOUR:
+        link = renewal_config.get(kind)
+        try:
+            os.remove(link)
+            logger.debug("Removed %s", link)
+        except OSError:
+            logger.debug("Unable to delete %s", link)
+        directory = os.path.dirname(link)
+        directory_names.add(directory)
+
+    # if all four were in the same directory, and the only thing left
+    # is the README file (or nothing), delete that directory.
+    # this will be wrong in very few but some cases.
+    if len(directory_names) == 1:
+        # delete the README file
+        directory = directory_names.pop()
+        readme_path = os.path.join(directory, README)
+        try:
+            os.remove(readme_path)
+            logger.debug("Removed %s", readme_path)
+        except OSError:
+            logger.debug("Unable to delete %s", readme_path)
+        # if it's now empty, delete the directory
+        try:
+            os.rmdir(directory) # only removes empty directories
+            logger.debug("Removed %s", directory)
+        except OSError:
+            logger.debug("Unable to remove %s; may not be empty.", directory)
+
+    # archive directory
+    try:
+        archive_path = _full_archive_path(renewal_config, config, certname)
+        shutil.rmtree(archive_path)
+        logger.debug("Removed %s", archive_path)
+    except OSError:
+        logger.debug("Unable to remove %s", archive_path)
+
 
 class RenewableCert(object):
     # pylint: disable=too-many-instance-attributes,too-many-public-methods
@@ -240,7 +344,7 @@ class RenewableCert(object):
 
         :param str config_filename: the path to the renewal config file
             that defines this lineage.
-        :param .RenewerConfiguration: parsed command line arguments
+        :param .NamespaceConfig: parsed command line arguments
 
         :raises .CertStorageError: if the configuration file's name didn't end
             in ".conf", or the file is missing or broken.
@@ -299,11 +403,15 @@ class RenewableCert(object):
     @property
     def archive_dir(self):
         """Returns the default or specified archive directory"""
-        if "archive_dir" in self.configuration:
-            return self.configuration["archive_dir"]
-        else:
-            return os.path.join(
-                self.cli_config.default_archive_dir, self.lineagename)
+        return _full_archive_path(self.configuration,
+            self.cli_config, self.lineagename)
+
+    def relative_archive_dir(self, from_file):
+        """Returns the default or specified archive directory as a relative path
+
+        Used for creating symbolic links.
+        """
+        return _relpath_from_file(self.archive_dir, from_file)
 
     @property
     def is_test_cert(self):
@@ -331,7 +439,8 @@ class RenewableCert(object):
         for kind in ALL_FOUR:
             link = getattr(self, kind)
             previous_link = get_link_target(link)
-            new_link = os.path.join(self.archive_dir, os.path.basename(previous_link))
+            new_link = os.path.join(self.relative_archive_dir(link),
+                os.path.basename(previous_link))
 
             os.unlink(link)
             os.symlink(new_link, link)
@@ -815,7 +924,7 @@ class RenewableCert(object):
         :param str cert: the initial certificate version in PEM format
         :param str privkey: the private key in PEM format
         :param str chain: the certificate chain in PEM format
-        :param .RenewerConfiguration cli_config: parsed command line
+        :param .NamespaceConfig cli_config: parsed command line
             arguments
 
         :returns: the newly-created RenewalCert object
@@ -831,16 +940,13 @@ class RenewableCert(object):
                 logger.debug("Creating directory %s.", i)
         config_file, config_filename = util.unique_lineage_name(
             cli_config.renewal_configs_dir, lineagename)
-        if not config_filename.endswith(".conf"):
-            raise errors.CertStorageError(
-                "renewal config file name must end in .conf")
 
         # Determine where on disk everything will go
         # lineagename will now potentially be modified based on which
         # renewal configuration file could actually be created
-        lineagename = os.path.basename(config_filename)[:-len(".conf")]
-        archive = os.path.join(cli_config.default_archive_dir, lineagename)
-        live_dir = os.path.join(cli_config.live_dir, lineagename)
+        lineagename = lineagename_for_filename(config_filename)
+        archive = _full_archive_path(None, cli_config, lineagename)
+        live_dir = _full_live_path(cli_config, lineagename)
         if os.path.exists(archive):
             raise errors.CertStorageError(
                 "archive directory exists for " + lineagename)
@@ -856,7 +962,7 @@ class RenewableCert(object):
         target = dict([(kind, os.path.join(live_dir, kind + ".pem"))
                        for kind in ALL_FOUR])
         for kind in ALL_FOUR:
-            os.symlink(os.path.join(archive, kind + "1.pem"),
+            os.symlink(os.path.join(_relpath_from_file(archive, target[kind]), kind + "1.pem"),
                        target[kind])
         with open(target["cert"], "wb") as f:
             logger.debug("Writing certificate to %s.", target["cert"])
@@ -875,7 +981,7 @@ class RenewableCert(object):
             f.write(cert + chain)
 
         # Write a README file to the live directory
-        readme_path = os.path.join(live_dir, "README")
+        readme_path = os.path.join(live_dir, README)
         with open(readme_path, "w") as f:
             logger.debug("Writing README to %s.", readme_path)
             f.write("This directory contains your keys and certificates.\n\n"
@@ -916,7 +1022,7 @@ class RenewableCert(object):
         :param str new_privkey: the new private key, in PEM format,
             or ``None``, if the private key has not changed
         :param str new_chain: the new chain, in PEM format
-        :param .RenewerConfiguration cli_config: parsed command line
+        :param .NamespaceConfig cli_config: parsed command line
             arguments
 
         :returns: the new version number that was created

certbot/tests/acme_util.py

@@ -7,9 +7,12 @@ from acme import challenges
 from acme import jose
 from acme import messages
 
+from certbot import auth_handler
+
 from certbot.tests import util
 
 
+JWK = jose.JWK.load(util.load_vector('rsa512_key.pem'))
 KEY = util.load_rsa_private_key('rsa512_key.pem')
 
 # Challenges
@@ -50,6 +53,14 @@ DNS01_P = chall_to_challb(DNS01, messages.STATUS_PENDING)
 CHALLENGES_P = [HTTP01_P, TLSSNI01_P, DNS01_P]
 
 
+# AnnotatedChallenge objects
+HTTP01_A = auth_handler.challb_to_achall(HTTP01_P, JWK, "example.com")
+TLSSNI01_A = auth_handler.challb_to_achall(TLSSNI01_P, JWK, "example.net")
+DNS01_A = auth_handler.challb_to_achall(DNS01_P, JWK, "example.org")
+
+ACHALLENGES = [HTTP01_A, TLSSNI01_A, DNS01_A]
+
+
 def gen_authzr(authz_status, domain, challs, statuses, combos=True):
     """Generate an authorization resource.
 

certbot/tests/cert_manager_test.py

@@ -1,6 +1,7 @@
 """Tests for certbot.cert_manager."""
-# pylint disable=protected-access
+# pylint: disable=protected-access
 import os
+import re
 import shutil
 import tempfile
 import unittest
@@ -25,12 +26,12 @@ class BaseCertManagerTest(unittest.TestCase):
 
         os.makedirs(os.path.join(self.tempdir, "renewal"))
 
-        self.cli_config = mock.MagicMock(
+        self.cli_config = configuration.NamespaceConfig(mock.MagicMock(
             config_dir=self.tempdir,
             work_dir=self.tempdir,
             logs_dir=self.tempdir,
             quiet=False,
-        )
+        ))
 
         self.domains = {
             "example.org": None,
@@ -47,7 +48,7 @@ class BaseCertManagerTest(unittest.TestCase):
         junk.close()
 
     def _set_up_config(self, domain, custom_archive):
-        # TODO: maybe provide RenewerConfiguration.make_dirs?
+        # TODO: maybe provide NamespaceConfig.make_dirs?
         # TODO: main() should create those dirs, c.f. #902
         os.makedirs(os.path.join(self.tempdir, "live", domain))
         config = configobj.ConfigObj()
@@ -99,10 +100,31 @@ class UpdateLiveSymlinksTest(BaseCertManagerTest):
         cert_manager.update_live_symlinks(self.cli_config)
 
         # check that symlinks go where they should
-        for domain in self.domains:
-            for kind in ALL_FOUR:
-                self.assertEqual(os.readlink(self.configs[domain][kind]),
-                    archive_paths[domain][kind])
+        prev_dir = os.getcwd()
+        try:
+            for domain in self.domains:
+                for kind in ALL_FOUR:
+                    os.chdir(os.path.dirname(self.configs[domain][kind]))
+                    self.assertEqual(
+                        os.path.realpath(os.readlink(self.configs[domain][kind])),
+                        os.path.realpath(archive_paths[domain][kind]))
+        finally:
+            os.chdir(prev_dir)
+
+
+class DeleteTest(storage_test.BaseRenewableCertTest):
+    """Tests for certbot.cert_manager.delete
+    """
+    @mock.patch('zope.component.getUtility')
+    @mock.patch('certbot.cert_manager.lineage_for_certname')
+    @mock.patch('certbot.storage.delete_files')
+    def test_delete(self, mock_delete_files, mock_lineage_for_certname, unused_get_utility):
+        """Test delete"""
+        mock_lineage_for_certname.return_value = self.test_rc
+        self.cli_config.certname = "example.org"
+        from certbot import cert_manager
+        cert_manager.delete(self.cli_config)
+        self.assertTrue(mock_delete_files.called)
 
 
 class CertificatesTest(BaseCertManagerTest):
@@ -145,12 +167,12 @@ class CertificatesTest(BaseCertManagerTest):
     def test_certificates_no_files(self, mock_utility, mock_logger):
         tempdir = tempfile.mkdtemp()
 
-        cli_config = mock.MagicMock(
+        cli_config = configuration.NamespaceConfig(mock.MagicMock(
                 config_dir=tempdir,
                 work_dir=tempdir,
                 logs_dir=tempdir,
                 quiet=False,
-        )
+        ))
 
         os.makedirs(os.path.join(tempdir, "renewal"))
         self._certificates(cli_config)
@@ -158,7 +180,9 @@ class CertificatesTest(BaseCertManagerTest):
         self.assertTrue(mock_utility.called)
         shutil.rmtree(tempdir)
 
-    def test_report_human_readable(self):
+    @mock.patch('certbot.cert_manager.ocsp.RevocationChecker.ocsp_revoked')
+    def test_report_human_readable(self, mock_revoked):
+        mock_revoked.return_value = None
         from certbot import cert_manager
         import datetime, pytz
         expiry = pytz.UTC.fromutc(datetime.datetime.utcnow())
@@ -168,118 +192,140 @@ class CertificatesTest(BaseCertManagerTest):
         cert.names.return_value = ["nameone", "nametwo"]
         cert.is_test_cert = False
         parsed_certs = [cert]
+
         # pylint: disable=protected-access
-        out = cert_manager._report_human_readable(parsed_certs)
+        get_report = lambda: cert_manager._report_human_readable(mock_config, parsed_certs)
+
+        mock_config = mock.MagicMock(certname=None, lineagename=None)
+        # pylint: disable=protected-access
+        out = get_report()
         self.assertTrue("INVALID: EXPIRED" in out)
 
         cert.target_expiry += datetime.timedelta(hours=2)
         # pylint: disable=protected-access
-        out = cert_manager._report_human_readable(parsed_certs)
+        out = get_report()
         self.assertTrue('1 hour(s)' in out)
         self.assertTrue('VALID' in out and not 'INVALID' in out)
 
         cert.target_expiry += datetime.timedelta(days=1)
         # pylint: disable=protected-access
-        out = cert_manager._report_human_readable(parsed_certs)
+        out = get_report()
         self.assertTrue('1 day' in out)
         self.assertFalse('under' in out)
         self.assertTrue('VALID' in out and not 'INVALID' in out)
 
         cert.target_expiry += datetime.timedelta(days=2)
         # pylint: disable=protected-access
-        out = cert_manager._report_human_readable(parsed_certs)
+        out = get_report()
         self.assertTrue('3 days' in out)
         self.assertTrue('VALID' in out and not 'INVALID' in out)
 
         cert.is_test_cert = True
-        out = cert_manager._report_human_readable(parsed_certs)
-        self.assertTrue('INVALID: TEST CERT' in out)
+        mock_revoked.return_value = True
+        out = get_report()
+        self.assertTrue('INVALID: TEST_CERT, REVOKED' in out)
+
+        cert = mock.MagicMock(lineagename="indescribable")
+        cert.target_expiry = expiry
+        cert.names.return_value = ["nameone", "thrice.named"]
+        cert.is_test_cert = True
+        parsed_certs.append(cert)
+
+        out = get_report()
+        self.assertEqual(len(re.findall("INVALID:", out)), 2)
+        mock_config.domains = ["thrice.named"]
+        out = get_report()
+        self.assertEqual(len(re.findall("INVALID:", out)), 1)
+        mock_config.domains = ["nameone"]
+        out = get_report()
+        self.assertEqual(len(re.findall("INVALID:", out)), 2)
+        mock_config.certname = "indescribable"
+        out = get_report()
+        self.assertEqual(len(re.findall("INVALID:", out)), 1)
+        mock_config.certname = "horror"
+        out = get_report()
+        self.assertEqual(len(re.findall("INVALID:", out)), 0)
 
 
-class SearchLineagesTest(unittest.TestCase):
+class SearchLineagesTest(BaseCertManagerTest):
     """Tests for certbot.cert_manager._search_lineages."""
 
-    @mock.patch('certbot.configuration.RenewerConfiguration')
     @mock.patch('certbot.util.make_or_verify_dir')
-    @mock.patch('certbot.renewal.renewal_conf_files')
+    @mock.patch('certbot.storage.renewal_conf_files')
     @mock.patch('certbot.storage.RenewableCert')
     def test_cert_storage_error(self, mock_renewable_cert, mock_renewal_conf_files,
-        mock_make_or_verify_dir, mock_renewer_config):
+        mock_make_or_verify_dir):
         mock_renewal_conf_files.return_value = ["badfile"]
         mock_renewable_cert.side_effect = errors.CertStorageError
         from certbot import cert_manager
         # pylint: disable=protected-access
-        self.assertEqual(cert_manager._search_lineages(None, lambda x: x, "check"), "check")
+        self.assertEqual(cert_manager._search_lineages(self.cli_config, lambda x: x, "check"),
+            "check")
         self.assertTrue(mock_make_or_verify_dir.called)
-        self.assertTrue(mock_renewer_config)
 
 
-class LineageForCertnameTest(unittest.TestCase):
+class LineageForCertnameTest(BaseCertManagerTest):
     """Tests for certbot.cert_manager.lineage_for_certname"""
 
-    @mock.patch('certbot.configuration.RenewerConfiguration')
     @mock.patch('certbot.util.make_or_verify_dir')
-    @mock.patch('certbot.renewal.renewal_conf_files')
+    @mock.patch('certbot.storage.renewal_conf_files')
     @mock.patch('certbot.storage.RenewableCert')
     def test_found_match(self, mock_renewable_cert, mock_renewal_conf_files,
-        mock_make_or_verify_dir, mock_renewer_config):
+        mock_make_or_verify_dir):
         mock_renewal_conf_files.return_value = ["somefile.conf"]
         mock_match = mock.Mock(lineagename="example.com")
         mock_renewable_cert.return_value = mock_match
         from certbot import cert_manager
-        self.assertEqual(cert_manager.lineage_for_certname(None, "example.com"), mock_match)
+        self.assertEqual(cert_manager.lineage_for_certname(self.cli_config, "example.com"),
+            mock_match)
         self.assertTrue(mock_make_or_verify_dir.called)
-        self.assertTrue(mock_renewer_config)
 
-    @mock.patch('certbot.configuration.RenewerConfiguration')
     @mock.patch('certbot.util.make_or_verify_dir')
-    @mock.patch('certbot.renewal.renewal_conf_files')
+    @mock.patch('certbot.storage.renewal_conf_files')
     @mock.patch('certbot.storage.RenewableCert')
     def test_no_match(self, mock_renewable_cert, mock_renewal_conf_files,
-        mock_make_or_verify_dir, mock_renewer_config):
+        mock_make_or_verify_dir):
         mock_renewal_conf_files.return_value = ["somefile.conf"]
         mock_match = mock.Mock(lineagename="other.com")
         mock_renewable_cert.return_value = mock_match
         from certbot import cert_manager
-        self.assertEqual(cert_manager.lineage_for_certname(None, "example.com"), None)
+        self.assertEqual(cert_manager.lineage_for_certname(self.cli_config, "example.com"),
+            None)
         self.assertTrue(mock_make_or_verify_dir.called)
-        self.assertTrue(mock_renewer_config)
 
 
-class DomainsForCertnameTest(unittest.TestCase):
+class DomainsForCertnameTest(BaseCertManagerTest):
     """Tests for certbot.cert_manager.domains_for_certname"""
 
-    @mock.patch('certbot.configuration.RenewerConfiguration')
     @mock.patch('certbot.util.make_or_verify_dir')
-    @mock.patch('certbot.renewal.renewal_conf_files')
+    @mock.patch('certbot.storage.renewal_conf_files')
     @mock.patch('certbot.storage.RenewableCert')
     def test_found_match(self, mock_renewable_cert, mock_renewal_conf_files,
-        mock_make_or_verify_dir, mock_renewer_config):
+        mock_make_or_verify_dir):
         mock_renewal_conf_files.return_value = ["somefile.conf"]
         mock_match = mock.Mock(lineagename="example.com")
         domains = ["example.com", "example.org"]
         mock_match.names.return_value = domains
         mock_renewable_cert.return_value = mock_match
         from certbot import cert_manager
-        self.assertEqual(cert_manager.domains_for_certname(None, "example.com"), domains)
+        self.assertEqual(cert_manager.domains_for_certname(self.cli_config, "example.com"),
+            domains)
         self.assertTrue(mock_make_or_verify_dir.called)
-        self.assertTrue(mock_renewer_config)
 
-    @mock.patch('certbot.configuration.RenewerConfiguration')
     @mock.patch('certbot.util.make_or_verify_dir')
-    @mock.patch('certbot.renewal.renewal_conf_files')
+    @mock.patch('certbot.storage.renewal_conf_files')
     @mock.patch('certbot.storage.RenewableCert')
     def test_no_match(self, mock_renewable_cert, mock_renewal_conf_files,
-        mock_make_or_verify_dir, mock_renewer_config):
+        mock_make_or_verify_dir):
         mock_renewal_conf_files.return_value = ["somefile.conf"]
         mock_match = mock.Mock(lineagename="example.com")
         domains = ["example.com", "example.org"]
         mock_match.names.return_value = domains
         mock_renewable_cert.return_value = mock_match
         from certbot import cert_manager
-        self.assertEqual(cert_manager.domains_for_certname(None, "other.com"), None)
+        self.assertEqual(cert_manager.domains_for_certname(self.cli_config, "other.com"),
+            None)
         self.assertTrue(mock_make_or_verify_dir.called)
-        self.assertTrue(mock_renewer_config)
 
 
 class RenameLineageTest(BaseCertManagerTest):
@@ -287,7 +333,7 @@ class RenameLineageTest(BaseCertManagerTest):
 
     def setUp(self):
         super(RenameLineageTest, self).setUp()
-        self.mock_config = configuration.RenewerConfiguration(
+        self.mock_config = configuration.NamespaceConfig(
             namespace=mock.MagicMock(
                 config_dir=self.tempdir,
                 work_dir=self.tempdir,
@@ -301,7 +347,7 @@ class RenameLineageTest(BaseCertManagerTest):
         from certbot import cert_manager
         return cert_manager.rename_lineage(*args, **kwargs)
 
-    @mock.patch('certbot.renewal.renewal_conf_files')
+    @mock.patch('certbot.storage.renewal_conf_files')
     @mock.patch('certbot.main.zope.component.getUtility')
     def test_no_certname(self, mock_get_utility, mock_renewal_conf_files):
         mock_config = mock.Mock(certname=None, new_certname="two")
@@ -377,7 +423,7 @@ class RenameLineageTest(BaseCertManagerTest):
         mock_config.new_certname = "example.org"
         self.assertRaises(errors.ConfigurationError, self._call, mock_config)
 
-        mock_config.new_certname = "one/two"
+        mock_config.new_certname = "one{0}two".format(os.path.sep)
         self.assertRaises(errors.ConfigurationError, self._call, mock_config)
 
 

certbot/tests/cli_test.py

@@ -81,15 +81,17 @@ class ParseTest(unittest.TestCase):
         out = self._help_output(['--help', 'all'])
         self.assertTrue("--configurator" in out)
         self.assertTrue("how a cert is deployed" in out)
-        self.assertTrue("--manual-test-mode" in out)
+        self.assertTrue("--webroot-path" in out)
         self.assertTrue("--text" not in out)
         self.assertTrue("--dialog" not in out)
+        self.assertTrue("%s" not in out)
+        self.assertTrue("{0}" not in out)
 
         out = self._help_output(['-h', 'nginx'])
         if "nginx" in self.plugins:
             # may be false while building distributions without plugins
             self.assertTrue("--nginx-ctl" in out)
-        self.assertTrue("--manual-test-mode" not in out)
+        self.assertTrue("--webroot-path" not in out)
         self.assertTrue("--checkpoints" not in out)
 
         out = self._help_output(['-h'])
@@ -97,10 +99,10 @@ class ParseTest(unittest.TestCase):
         if "nginx" in self.plugins:
             self.assertTrue("Use the Nginx plugin" in out)
         else:
-            self.assertTrue("(nginx support is experimental" in out)
+            self.assertTrue("(the certbot nginx plugin is not" in out)
 
         out = self._help_output(['--help', 'plugins'])
-        self.assertTrue("--manual-test-mode" not in out)
+        self.assertTrue("--webroot-path" not in out)
         self.assertTrue("--prepare" in out)
         self.assertTrue('"plugins" subcommand' in out)
 
@@ -125,8 +127,10 @@ class ParseTest(unittest.TestCase):
         self.assertTrue("--key-path" not in out)
 
         out = self._help_output(['-h'])
-
-        self.assertTrue(cli.usage_strings(self.plugins)[0] in out)
+        self.assertTrue(cli.SHORT_USAGE in out)
+        self.assertTrue(cli.COMMAND_OVERVIEW[:100] in out)
+        self.assertTrue("%s" not in out)
+        self.assertTrue("{0}" not in out)
 
     def test_parse_domains(self):
         short_args = ['-d', 'example.com']
@@ -258,6 +262,12 @@ class ParseTest(unittest.TestCase):
         self.assertFalse(cli.option_was_set(
             config_dir_option, cli.flag_default(config_dir_option)))
 
+    def test_force_interactive(self):
+        self.assertRaises(
+            errors.Error, self.parse, "renew --force-interactive".split())
+        self.assertRaises(
+            errors.Error, self.parse, "-n --force-interactive".split())
+
 
 class DefaultTest(unittest.TestCase):
     """Tests for certbot.cli._Default."""
@@ -295,22 +305,22 @@ class SetByCliTest(unittest.TestCase):
 
     def test_report_config_interaction_str(self):
         cli.report_config_interaction('manual_public_ip_logging_ok',
-                                      'manual_test_mode')
-        cli.report_config_interaction('manual_test_mode', 'manual')
+                                      'manual_auth_hook')
+        cli.report_config_interaction('manual_auth_hook', 'manual')
 
         self._test_report_config_interaction_common()
 
     def test_report_config_interaction_iterable(self):
         cli.report_config_interaction(('manual_public_ip_logging_ok',),
-                                      ('manual_test_mode',))
-        cli.report_config_interaction(('manual_test_mode',), ('manual',))
+                                      ('manual_auth_hook',))
+        cli.report_config_interaction(('manual_auth_hook',), ('manual',))
 
         self._test_report_config_interaction_common()
 
     def _test_report_config_interaction_common(self):
         """Tests implied interaction between manual flags.
 
-        --manual implies --manual-test-mode which implies
+        --manual implies --manual-auth-hook which implies
         --manual-public-ip-logging-ok. These interactions don't actually
         exist in the client, but are used here for testing purposes.
 
@@ -318,13 +328,13 @@ class SetByCliTest(unittest.TestCase):
 
         args = ['--manual']
         verb = 'renew'
-        for v in ('manual', 'manual_test_mode', 'manual_public_ip_logging_ok'):
+        for v in ('manual', 'manual_auth_hook', 'manual_public_ip_logging_ok'):
             self.assertTrue(_call_set_by_cli(v, args, verb))
 
         cli.set_by_cli.detector = None
 
-        args = ['--manual-test-mode']
-        for v in ('manual_test_mode', 'manual_public_ip_logging_ok'):
+        args = ['--manual-auth-hook', 'command']
+        for v in ('manual_auth_hook', 'manual_public_ip_logging_ok'):
             self.assertTrue(_call_set_by_cli(v, args, verb))
 
         self.assertFalse(_call_set_by_cli('manual', args, verb))

certbot/tests/configuration_test.py

@@ -88,31 +88,19 @@ class NamespaceConfigTest(unittest.TestCase):
         self.assertTrue(os.path.isabs(config.key_dir))
         self.assertTrue(os.path.isabs(config.temp_checkpoint_dir))
 
-
-class RenewerConfigurationTest(unittest.TestCase):
-    """Test for certbot.configuration.RenewerConfiguration."""
-
-    def setUp(self):
-        self.namespace = mock.MagicMock(config_dir='/tmp/config')
-        from certbot.configuration import RenewerConfiguration
-        self.config = RenewerConfiguration(self.namespace)
-
     @mock.patch('certbot.configuration.constants')
-    def test_dynamic_dirs(self, constants):
+    def test_renewal_dynamic_dirs(self, constants):
         constants.ARCHIVE_DIR = 'a'
         constants.LIVE_DIR = 'l'
         constants.RENEWAL_CONFIGS_DIR = 'renewal_configs'
-        constants.RENEWER_CONFIG_FILENAME = 'r.conf'
 
         self.assertEqual(self.config.default_archive_dir, '/tmp/config/a')
         self.assertEqual(self.config.live_dir, '/tmp/config/l')
         self.assertEqual(
             self.config.renewal_configs_dir, '/tmp/config/renewal_configs')
-        self.assertEqual(self.config.renewer_config_file, '/tmp/config/r.conf')
 
-    def test_absolute_paths(self):
+    def test_renewal_absolute_paths(self):
         from certbot.configuration import NamespaceConfig
-        from certbot.configuration import RenewerConfiguration
 
         config_base = "foo"
         work_base = "bar"
@@ -125,12 +113,11 @@ class RenewerConfigurationTest(unittest.TestCase):
         mock_namespace.config_dir = config_base
         mock_namespace.work_dir = work_base
         mock_namespace.logs_dir = logs_base
-        config = RenewerConfiguration(NamespaceConfig(mock_namespace))
+        config = NamespaceConfig(mock_namespace)
 
         self.assertTrue(os.path.isabs(config.default_archive_dir))
         self.assertTrue(os.path.isabs(config.live_dir))
         self.assertTrue(os.path.isabs(config.renewal_configs_dir))
-        self.assertTrue(os.path.isabs(config.renewer_config_file))
 
 
 if __name__ == '__main__':

certbot/tests/display/ops_test.py

@@ -84,7 +84,8 @@ class GetEmailTest(unittest.TestCase):
 class ChooseAccountTest(unittest.TestCase):
     """Tests for certbot.display.ops.choose_account."""
     def setUp(self):
-        zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
+        zope.component.provideUtility(display_util.FileDisplay(sys.stdout,
+                                                               False))
 
         self.accounts_dir = tempfile.mkdtemp("accounts")
         self.account_keys_dir = os.path.join(self.accounts_dir, "keys")
@@ -127,7 +128,8 @@ class ChooseAccountTest(unittest.TestCase):
 class GenSSLLabURLs(unittest.TestCase):
     """Loose test of _gen_ssl_lab_urls. URL can change easily in the future."""
     def setUp(self):
-        zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
+        zope.component.provideUtility(display_util.FileDisplay(sys.stdout,
+                                                               False))
 
     @classmethod
     def _call(cls, domains):
@@ -146,7 +148,8 @@ class GenSSLLabURLs(unittest.TestCase):
 class GenHttpsNamesTest(unittest.TestCase):
     """Test _gen_https_names."""
     def setUp(self):
-        zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
+        zope.component.provideUtility(display_util.FileDisplay(sys.stdout,
+                                                               False))
 
     @classmethod
     def _call(cls, domains):
@@ -193,7 +196,8 @@ class GenHttpsNamesTest(unittest.TestCase):
 class ChooseNamesTest(unittest.TestCase):
     """Test choose names."""
     def setUp(self):
-        zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
+        zope.component.provideUtility(display_util.FileDisplay(sys.stdout,
+                                                               False))
         self.mock_install = mock.MagicMock()
 
     @classmethod

certbot/tests/display/util_test.py

@@ -1,10 +1,12 @@
 """Test :mod:`certbot.display.util`."""
+import inspect
 import os
 import unittest
 
 import mock
 
-import certbot.errors as errors
+from certbot import errors
+from certbot import interfaces
 
 from certbot.display import util as display_util
 
@@ -20,10 +22,11 @@ class FileOutputDisplayTest(unittest.TestCase):
     functions look to a user, uncomment the test_visual function.
 
     """
+    # pylint:disable=too-many-public-methods
     def setUp(self):
         super(FileOutputDisplayTest, self).setUp()
         self.mock_stdout = mock.MagicMock()
-        self.displayer = display_util.FileDisplay(self.mock_stdout)
+        self.displayer = display_util.FileDisplay(self.mock_stdout, False)
 
     def test_notification_no_pause(self):
         self.displayer.notification("message", False)
@@ -33,79 +36,128 @@ class FileOutputDisplayTest(unittest.TestCase):
 
     def test_notification_pause(self):
         with mock.patch("six.moves.input", return_value="enter"):
-            self.displayer.notification("message")
+            self.displayer.notification("message", force_interactive=True)
 
         self.assertTrue("message" in self.mock_stdout.write.call_args[0][0])
 
+    def test_notification_noninteractive(self):
+        self._force_noninteractive(self.displayer.notification, "message")
+        string = self.mock_stdout.write.call_args[0][0]
+        self.assertTrue("message" in string)
+
+    def test_notification_noninteractive2(self):
+        # The main purpose of this test is to make sure we only call
+        # logger.warning once which _force_noninteractive checks internally
+        self._force_noninteractive(self.displayer.notification, "message")
+        string = self.mock_stdout.write.call_args[0][0]
+        self.assertTrue("message" in string)
+
+        self.assertTrue(self.displayer.skipped_interaction)
+
+        self._force_noninteractive(self.displayer.notification, "message2")
+        string = self.mock_stdout.write.call_args[0][0]
+        self.assertTrue("message2" in string)
+
     @mock.patch("certbot.display.util."
                 "FileDisplay._get_valid_int_ans")
     def test_menu(self, mock_ans):
         mock_ans.return_value = (display_util.OK, 1)
-        ret = self.displayer.menu("message", CHOICES)
+        ret = self.displayer.menu("message", CHOICES, force_interactive=True)
         self.assertEqual(ret, (display_util.OK, 0))
 
+    def test_menu_noninteractive(self):
+        default = 0
+        result = self._force_noninteractive(
+            self.displayer.menu, "msg", CHOICES, default=default)
+        self.assertEqual(result, (display_util.OK, default))
+
     def test_input_cancel(self):
         with mock.patch("six.moves.input", return_value="c"):
-            code, _ = self.displayer.input("message")
+            code, _ = self.displayer.input("message", force_interactive=True)
 
         self.assertTrue(code, display_util.CANCEL)
 
     def test_input_normal(self):
         with mock.patch("six.moves.input", return_value="domain.com"):
-            code, input_ = self.displayer.input("message")
+            code, input_ = self.displayer.input("message", force_interactive=True)
 
         self.assertEqual(code, display_util.OK)
         self.assertEqual(input_, "domain.com")
 
+    def test_input_noninteractive(self):
+        default = "foo"
+        code, input_ = self._force_noninteractive(
+            self.displayer.input, "message", default=default)
+
+        self.assertEqual(code, display_util.OK)
+        self.assertEqual(input_, default)
+
+    def test_input_assertion_fail(self):
+        self.assertRaises(AssertionError, self._force_noninteractive,
+                          self.displayer.input, "message", cli_flag="--flag")
+
     def test_yesno(self):
         with mock.patch("six.moves.input", return_value="Yes"):
-            self.assertTrue(self.displayer.yesno("message"))
+            self.assertTrue(self.displayer.yesno(
+                "message", force_interactive=True))
         with mock.patch("six.moves.input", return_value="y"):
-            self.assertTrue(self.displayer.yesno("message"))
+            self.assertTrue(self.displayer.yesno(
+                "message", force_interactive=True))
         with mock.patch("six.moves.input", side_effect=["maybe", "y"]):
-            self.assertTrue(self.displayer.yesno("message"))
+            self.assertTrue(self.displayer.yesno(
+                "message", force_interactive=True))
         with mock.patch("six.moves.input", return_value="No"):
-            self.assertFalse(self.displayer.yesno("message"))
+            self.assertFalse(self.displayer.yesno(
+                "message", force_interactive=True))
         with mock.patch("six.moves.input", side_effect=["cancel", "n"]):
-            self.assertFalse(self.displayer.yesno("message"))
+            self.assertFalse(self.displayer.yesno(
+                "message", force_interactive=True))
 
         with mock.patch("six.moves.input", return_value="a"):
-            self.assertTrue(self.displayer.yesno("msg", yes_label="Agree"))
+            self.assertTrue(self.displayer.yesno(
+                "msg", yes_label="Agree", force_interactive=True))
 
-    @mock.patch("certbot.display.util.FileDisplay.input")
+    def test_yesno_noninteractive(self):
+        self.assertTrue(self._force_noninteractive(
+            self.displayer.yesno, "message", default=True))
+
+    @mock.patch("certbot.display.util.six.moves.input")
     def test_checklist_valid(self, mock_input):
-        mock_input.return_value = (display_util.OK, "2 1")
-        code, tag_list = self.displayer.checklist("msg", TAGS)
+        mock_input.return_value = "2 1"
+        code, tag_list = self.displayer.checklist(
+            "msg", TAGS, force_interactive=True)
         self.assertEqual(
             (code, set(tag_list)), (display_util.OK, set(["tag1", "tag2"])))
 
-    @mock.patch("certbot.display.util.FileDisplay.input")
+    @mock.patch("certbot.display.util.six.moves.input")
     def test_checklist_empty(self, mock_input):
-        mock_input.return_value = (display_util.OK, "")
-        code, tag_list = self.displayer.checklist("msg", TAGS)
+        mock_input.return_value = ""
+        code, tag_list = self.displayer.checklist("msg", TAGS, force_interactive=True)
         self.assertEqual(
             (code, set(tag_list)), (display_util.OK, set(["tag1", "tag2", "tag3"])))
 
-    @mock.patch("certbot.display.util.FileDisplay.input")
+    @mock.patch("certbot.display.util.six.moves.input")
     def test_checklist_miss_valid(self, mock_input):
-        mock_input.side_effect = [
-            (display_util.OK, "10"),
-            (display_util.OK, "tag1 please"),
-            (display_util.OK, "1")
-        ]
+        mock_input.side_effect = ["10", "tag1 please", "1"]
 
-        ret = self.displayer.checklist("msg", TAGS)
+        ret = self.displayer.checklist("msg", TAGS, force_interactive=True)
         self.assertEqual(ret, (display_util.OK, ["tag1"]))
 
-    @mock.patch("certbot.display.util.FileDisplay.input")
+    @mock.patch("certbot.display.util.six.moves.input")
     def test_checklist_miss_quit(self, mock_input):
-        mock_input.side_effect = [
-            (display_util.OK, "10"),
-            (display_util.CANCEL, "1")
-        ]
-        ret = self.displayer.checklist("msg", TAGS)
+        mock_input.side_effect = ["10", "c"]
+
+        ret = self.displayer.checklist("msg", TAGS, force_interactive=True)
         self.assertEqual(ret, (display_util.CANCEL, []))
 
+    def test_checklist_noninteractive(self):
+        default = TAGS
+        code, input_ = self._force_noninteractive(
+            self.displayer.checklist, "msg", TAGS, default=default)
+
+        self.assertEqual(code, display_util.OK)
+        self.assertEqual(input_, default)
+
     def test_scrub_checklist_input_valid(self):
         # pylint: disable=protected-access
         indices = [
@@ -123,14 +175,38 @@ class FileOutputDisplayTest(unittest.TestCase):
                 self.displayer._scrub_checklist_input(list_, TAGS))
             self.assertEqual(set_tags, exp[i])
 
-    @mock.patch("certbot.display.util.FileDisplay.input")
+    @mock.patch("certbot.display.util.six.moves.input")
     def test_directory_select(self, mock_input):
-        message = "msg"
-        result = (display_util.OK, "/var/www/html",)
-        mock_input.return_value = result
+        # pylint: disable=star-args
+        args = ["msg", "/var/www/html", "--flag", True]
+        user_input = "/var/www/html"
+        mock_input.return_value = user_input
 
-        self.assertEqual(self.displayer.directory_select(message), result)
-        mock_input.assert_called_once_with(message)
+        returned = self.displayer.directory_select(*args)
+        self.assertEqual(returned, (display_util.OK, user_input))
+
+    def test_directory_select_noninteractive(self):
+        default = "/var/www/html"
+        code, input_ = self._force_noninteractive(
+            self.displayer.directory_select, "msg", default=default)
+
+        self.assertEqual(code, display_util.OK)
+        self.assertEqual(input_, default)
+
+    def _force_noninteractive(self, func, *args, **kwargs):
+        skipped_interaction = self.displayer.skipped_interaction
+
+        with mock.patch("certbot.display.util.sys.stdin") as mock_stdin:
+            mock_stdin.isatty.return_value = False
+            with mock.patch("certbot.display.util.logger") as mock_logger:
+                result = func(*args, **kwargs)
+
+        if skipped_interaction:
+            self.assertFalse(mock_logger.warning.called)
+        else:
+            self.assertEqual(mock_logger.warning.call_count, 1)
+
+        return result
 
     def test_scrub_checklist_input_invalid(self):
         # pylint: disable=protected-access
@@ -185,6 +261,13 @@ class FileOutputDisplayTest(unittest.TestCase):
                     self.displayer._get_valid_int_ans(3),
                     (display_util.CANCEL, -1))
 
+    def test_methods_take_force_interactive(self):
+        # Every IDisplay method implemented by FileDisplay must take
+        # force_interactive to prevent workflow regressions.
+        for name in interfaces.IDisplay.names():  # pylint: disable=no-member
+            arg_spec = inspect.getargspec(getattr(self.displayer, name))
+            self.assertTrue("force_interactive" in arg_spec.args)
+
 
 class NoninteractiveDisplayTest(unittest.TestCase):
     """Test non-interactive display.
@@ -235,6 +318,16 @@ class NoninteractiveDisplayTest(unittest.TestCase):
         self.assertRaises(
             errors.MissingCommandlineFlag, self.displayer.directory_select, "msg")
 
+    def test_methods_take_kwargs(self):
+        # Every IDisplay method implemented by NoninteractiveDisplay
+        # should take **kwargs because every method of FileDisplay must
+        # take force_interactive which doesn't apply to
+        # NoninteractiveDisplay.
+        for name in interfaces.IDisplay.names():  # pylint: disable=no-member
+            method = getattr(self.displayer, name)
+            # asserts method accepts arbitrary keyword arguments
+            self.assertFalse(inspect.getargspec(method).keywords is None)
+
 
 class SeparateListInputTest(unittest.TestCase):
     """Test Module functions."""

certbot/tests/main_test.py

@@ -159,10 +159,12 @@ class ObtainCertTest(unittest.TestCase):
         self.assertRaises(errors.ConfigurationError, self._call,
             ('certonly --webroot -d example.com -d test.com --cert-name example.com').split())
 
+    @mock.patch('certbot.cert_manager.domains_for_certname')
+    @mock.patch('certbot.display.ops.choose_names')
     @mock.patch('certbot.cert_manager.lineage_for_certname')
     @mock.patch('certbot.main._report_new_cert')
     def test_find_lineage_for_domains_new_certname(self, mock_report_cert,
-        mock_lineage):
+        mock_lineage, mock_choose_names, mock_domains_for_certname):
         mock_lineage.return_value = None
 
         # no lineage with this name but we specified domains so create a new cert
@@ -172,8 +174,10 @@ class ObtainCertTest(unittest.TestCase):
         self.assertTrue(mock_report_cert.call_count == 1)
 
         # no lineage with this name and we didn't give domains
-        self.assertRaises(errors.ConfigurationError, self._call,
-            ('certonly --webroot --cert-name example.com').split())
+        mock_choose_names.return_value = ["somename"]
+        mock_domains_for_certname.return_value = None
+        self._call(('certonly --webroot --cert-name example.com').split())
+        self.assertTrue(mock_choose_names.called)
 
 class FindDomainsOrCertnameTest(unittest.TestCase):
     """Tests for certbot.main._find_domains_or_certname."""
@@ -193,6 +197,14 @@ class FindDomainsOrCertnameTest(unittest.TestCase):
         # pylint: disable=protected-access
         self.assertRaises(errors.Error, main._find_domains_or_certname, mock_config, None)
 
+    @mock.patch('certbot.cert_manager.domains_for_certname')
+    def test_grab_domains(self, mock_domains):
+        mock_config = mock.Mock(domains=None, certname="one.com")
+        mock_domains.return_value = ["one.com", "two.com"]
+        # pylint: disable=protected-access
+        self.assertEqual(main._find_domains_or_certname(mock_config, None),
+            (["one.com", "two.com"], "one.com"))
+
 
 class RevokeTest(unittest.TestCase):
     """Tests for certbot.main.revoke."""
@@ -567,6 +579,11 @@ class MainTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self._call_no_clientmock(['certificates'])
         self.assertEqual(1, mock_cert_manager.call_count)
 
+    @mock.patch('certbot.cert_manager.delete')
+    def test_delete(self, mock_cert_manager):
+        self._call_no_clientmock(['delete'])
+        self.assertEqual(1, mock_cert_manager.call_count)
+
     def test_plugins(self):
         flags = ['--init', '--prepare', '--authenticators', '--installers']
         for args in itertools.chain(
@@ -856,8 +873,7 @@ class MainTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         rc_path = test_util.make_lineage(self, 'sample-renewal-ancient.conf')
         args = mock.MagicMock(account=None, email=None, webroot_path=None)
         config = configuration.NamespaceConfig(args)
-        lineage = storage.RenewableCert(rc_path,
-            configuration.RenewerConfiguration(config))
+        lineage = storage.RenewableCert(rc_path, config)
         renewalparams = lineage.configuration["renewalparams"]
         # pylint: disable=protected-access
         renewal._restore_webroot_config(config, renewalparams)

certbot/tests/ocsp_test.py

@@ -0,0 +1,137 @@
+"""Tests for ocsp.py"""
+# pylint: disable=protected-access
+
+import unittest
+
+import mock
+
+from certbot import errors
+
+out = """Missing = in header key=value
+ocsp: Use -help for summary.
+"""
+
+class OCSPTest(unittest.TestCase):
+
+    _multiprocess_can_split_ = True
+
+    def setUp(self):
+        from certbot import ocsp
+        with mock.patch('certbot.ocsp.Popen') as mock_popen:
+            with mock.patch('certbot.util.exe_exists') as mock_exists:
+                mock_communicate = mock.MagicMock()
+                mock_communicate.communicate.return_value = (None, out)
+                mock_popen.return_value = mock_communicate
+                mock_exists.return_value = True
+                self.checker = ocsp.RevocationChecker()
+
+    def tearDown(self):
+        pass
+
+    @mock.patch('certbot.ocsp.logging.info')
+    @mock.patch('certbot.ocsp.Popen')
+    @mock.patch('certbot.util.exe_exists')
+    def test_init(self, mock_exists, mock_popen, mock_log):
+        mock_communicate = mock.MagicMock()
+        mock_communicate.communicate.return_value = (None, out)
+        mock_popen.return_value = mock_communicate
+        mock_exists.return_value = True
+
+        from certbot import ocsp
+        checker = ocsp.RevocationChecker()
+        self.assertEqual(mock_popen.call_count, 1)
+        self.assertEqual(checker.host_args("x"), ["Host=x"])
+
+        mock_communicate.communicate.return_value = (None, out.partition("\n")[2])
+        checker = ocsp.RevocationChecker()
+        self.assertEqual(checker.host_args("x"), ["Host", "x"])
+        self.assertEqual(checker.broken, False)
+
+        mock_exists.return_value = False
+        mock_popen.call_count = 0
+        checker = ocsp.RevocationChecker()
+        self.assertEqual(mock_popen.call_count, 0)
+        self.assertEqual(mock_log.call_count, 1)
+        self.assertEqual(checker.broken, True)
+
+    @mock.patch('certbot.ocsp.RevocationChecker.determine_ocsp_server')
+    @mock.patch('certbot.util.run_script')
+    def test_ocsp_revoked(self, mock_run, mock_determine):
+        self.checker.broken = True
+        mock_determine.return_value = ("", "")
+        self.assertEqual(self.checker.ocsp_revoked("x", "y"), False)
+
+        self.checker.broken = False
+        mock_run.return_value = tuple(openssl_happy[1:])
+        self.assertEqual(self.checker.ocsp_revoked("x", "y"), False)
+        self.assertEqual(mock_run.call_count, 0)
+
+        mock_determine.return_value = ("http://x.co", "x.co")
+        self.assertEqual(self.checker.ocsp_revoked("blah.pem", "chain.pem"), False)
+        mock_run.side_effect = errors.SubprocessError("Unable to load certificate launcher")
+        self.assertEqual(self.checker.ocsp_revoked("x", "y"), False)
+        self.assertEqual(mock_run.call_count, 2)
+
+
+    @mock.patch('certbot.ocsp.logger.debug')
+    @mock.patch('certbot.ocsp.logger.info')
+    @mock.patch('certbot.util.run_script')
+    def test_determine_ocsp_server(self, mock_run, mock_info, mock_debug):
+        uri = "http://ocsp.stg-int-x1.letsencrypt.org/"
+        host = "ocsp.stg-int-x1.letsencrypt.org"
+        mock_run.return_value = uri, ""
+        self.assertEqual(self.checker.determine_ocsp_server("beep"), (uri, host))
+        mock_run.return_value = "ftp:/" + host + "/", ""
+        self.assertEqual(self.checker.determine_ocsp_server("beep"), (None, None))
+        self.assertEqual(mock_info.call_count, 1)
+
+        c = "confusion"
+        mock_run.side_effect = errors.SubprocessError(c)
+        self.assertEqual(self.checker.determine_ocsp_server("beep"), (None, None))
+        self.assertTrue(c in repr(mock_debug.call_args[0][1]))
+
+    @mock.patch('certbot.ocsp.logger')
+    @mock.patch('certbot.util.run_script')
+    def test_translate_ocsp(self, mock_run, mock_log):
+        # pylint: disable=protected-access,star-args
+        mock_run.return_value = openssl_confused
+        from certbot import ocsp
+        self.assertEqual(ocsp._translate_ocsp_query(*openssl_happy), False)
+        self.assertEqual(ocsp._translate_ocsp_query(*openssl_confused), False)
+        self.assertEqual(mock_log.debug.call_count, 1)
+        self.assertEqual(mock_log.warn.call_count, 0)
+        self.assertEqual(ocsp._translate_ocsp_query(*openssl_broken), False)
+        self.assertEqual(mock_log.warn.call_count, 1)
+        self.assertEqual(ocsp._translate_ocsp_query(*openssl_revoked), True)
+
+
+# pylint: disable=line-too-long
+openssl_confused = ("", """
+/etc/letsencrypt/live/example.org/cert.pem: good
+	This Update: Dec 17 00:00:00 2016 GMT
+	Next Update: Dec 24 00:00:00 2016 GMT
+""",
+"""
+Response Verify Failure
+139903674214048:error:27069065:OCSP routines:OCSP_basic_verify:certificate verify error:ocsp_vfy.c:138:Verify error:unable to get local issuer certificate
+""")
+
+openssl_happy = ("blah.pem", """
+blah.pem: good
+	This Update: Dec 20 18:00:00 2016 GMT
+	Next Update: Dec 27 18:00:00 2016 GMT
+""",
+"Response verify OK")
+
+openssl_revoked = ("blah.pem", """
+blah.pem: revoked
+	This Update: Dec 20 01:00:00 2016 GMT
+	Next Update: Dec 27 01:00:00 2016 GMT
+	Revocation Time: Dec 20 01:46:34 2016 GMT
+""",
+"""Response verify OK""")
+
+openssl_broken = ("", "tentacles", "Response verify OK")
+
+if __name__ == '__main__':
+    unittest.main()  # pragma: no cover

certbot/tests/renewal_test.py

@@ -21,8 +21,7 @@ class RenewalTest(unittest.TestCase):
         rc_path = util.make_lineage(self, 'sample-renewal-ancient.conf')
         args = mock.MagicMock(account=None, email=None, webroot_path=None)
         config = configuration.NamespaceConfig(args)
-        lineage = storage.RenewableCert(
-                rc_path, configuration.RenewerConfiguration(config))
+        lineage = storage.RenewableCert(rc_path, config)
         renewalparams = lineage.configuration["renewalparams"]
         # pylint: disable=protected-access
         from certbot import renewal

certbot/tests/storage_test.py

@@ -49,7 +49,7 @@ class BaseRenewableCertTest(unittest.TestCase):
         from certbot import storage
         self.tempdir = tempfile.mkdtemp()
 
-        self.cli_config = configuration.RenewerConfiguration(
+        self.cli_config = configuration.NamespaceConfig(
             namespace=mock.MagicMock(
                 config_dir=self.tempdir,
                 work_dir=self.tempdir,
@@ -57,16 +57,22 @@ class BaseRenewableCertTest(unittest.TestCase):
             )
         )
 
-        # TODO: maybe provide RenewerConfiguration.make_dirs?
+        # TODO: maybe provide NamespaceConfig.make_dirs?
         # TODO: main() should create those dirs, c.f. #902
         os.makedirs(os.path.join(self.tempdir, "live", "example.org"))
-        os.makedirs(os.path.join(self.tempdir, "archive", "example.org"))
+        archive_path = os.path.join(self.tempdir, "archive", "example.org")
+        os.makedirs(archive_path)
         os.makedirs(os.path.join(self.tempdir, "renewal"))
 
         config = configobj.ConfigObj()
         for kind in ALL_FOUR:
-            config[kind] = os.path.join(self.tempdir, "live", "example.org",
+            kind_path = os.path.join(self.tempdir, "live", "example.org",
                                         kind + ".pem")
+            config[kind] = kind_path
+        with open(os.path.join(self.tempdir, "live", "example.org",
+                                        "README"), 'a'):
+            pass
+        config["archive"] = archive_path
         config.filename = os.path.join(self.tempdir, "renewal",
                                        "example.org.conf")
         config.write()
@@ -770,5 +776,95 @@ class RenewableCertTests(BaseRenewableCertTest):
         storage.RenewableCert(self.config.filename, self.cli_config,
             update_symlinks=True)
 
+class DeleteFilesTest(BaseRenewableCertTest):
+    """Tests for certbot.storage.delete_files"""
+    def setUp(self):
+        super(DeleteFilesTest, self).setUp()
+        for kind in ALL_FOUR:
+            kind_path = os.path.join(self.tempdir, "live", "example.org",
+                                        kind + ".pem")
+            with open(kind_path, 'a'):
+                pass
+        self.config.write()
+        self.assertTrue(os.path.exists(os.path.join(
+            self.cli_config.renewal_configs_dir, "example.org.conf")))
+        self.assertTrue(os.path.exists(os.path.join(
+            self.cli_config.live_dir, "example.org")))
+        self.assertTrue(os.path.exists(os.path.join(
+            self.tempdir, "archive", "example.org")))
+
+    def _call(self):
+        from certbot import storage
+        with mock.patch("certbot.storage.logger"):
+            storage.delete_files(self.cli_config, "example.org")
+
+    def test_delete_all_files(self):
+        self._call()
+
+        self.assertFalse(os.path.exists(os.path.join(
+            self.cli_config.renewal_configs_dir, "example.org.conf")))
+        self.assertFalse(os.path.exists(os.path.join(
+            self.cli_config.live_dir, "example.org")))
+        self.assertFalse(os.path.exists(os.path.join(
+            self.tempdir, "archive", "example.org")))
+
+    def test_bad_renewal_config(self):
+        with open(self.config.filename, 'a') as config_file:
+            config_file.write("asdfasfasdfasdf")
+
+        self.assertRaises(errors.CertStorageError, self._call)
+        self.assertTrue(os.path.exists(os.path.join(
+            self.cli_config.live_dir, "example.org")))
+        self.assertFalse(os.path.exists(os.path.join(
+            self.cli_config.renewal_configs_dir, "example.org.conf")))
+
+    def test_no_renewal_config(self):
+        os.remove(self.config.filename)
+        self.assertRaises(errors.CertStorageError, self._call)
+        self.assertTrue(os.path.exists(os.path.join(
+            self.cli_config.live_dir, "example.org")))
+        self.assertFalse(os.path.exists(self.config.filename))
+
+    def test_no_cert_file(self):
+        os.remove(os.path.join(
+            self.cli_config.live_dir, "example.org", "cert.pem"))
+        self._call()
+        self.assertFalse(os.path.exists(self.config.filename))
+        self.assertFalse(os.path.exists(os.path.join(
+            self.cli_config.live_dir, "example.org")))
+        self.assertFalse(os.path.exists(os.path.join(
+            self.tempdir, "archive", "example.org")))
+
+    def test_no_readme_file(self):
+        os.remove(os.path.join(
+            self.cli_config.live_dir, "example.org", "README"))
+        self._call()
+        self.assertFalse(os.path.exists(self.config.filename))
+        self.assertFalse(os.path.exists(os.path.join(
+            self.cli_config.live_dir, "example.org")))
+        self.assertFalse(os.path.exists(os.path.join(
+            self.tempdir, "archive", "example.org")))
+
+    def test_livedir_not_empty(self):
+        with open(os.path.join(
+            self.cli_config.live_dir, "example.org", "other_file"), 'a'):
+            pass
+        self._call()
+        self.assertFalse(os.path.exists(self.config.filename))
+        self.assertTrue(os.path.exists(os.path.join(
+            self.cli_config.live_dir, "example.org")))
+        self.assertFalse(os.path.exists(os.path.join(
+            self.tempdir, "archive", "example.org")))
+
+    def test_no_archive(self):
+        archive_dir = os.path.join(self.tempdir, "archive", "example.org")
+        os.rmdir(archive_dir)
+        self._call()
+        self.assertFalse(os.path.exists(self.config.filename))
+        self.assertFalse(os.path.exists(os.path.join(
+            self.cli_config.live_dir, "example.org")))
+        self.assertFalse(os.path.exists(archive_dir))
+
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot/util.py

@@ -38,20 +38,22 @@ ANSI_SGR_RED = "\033[31m"
 ANSI_SGR_RESET = "\033[0m"
 
 
-def run_script(params):
+def run_script(params, log=logger.error):
     """Run the script with the given params.
 
     :param list params: List of parameters to pass to Popen
+    :param logging.Logger log: Logger to use for errors
 
     """
     try:
         proc = subprocess.Popen(params,
                                 stdout=subprocess.PIPE,
-                                stderr=subprocess.PIPE)
+                                stderr=subprocess.PIPE,
+                                universal_newlines=True)
 
     except (OSError, ValueError):
         msg = "Unable to run the command: %s" % " ".join(params)
-        logger.error(msg)
+        log(msg)
         raise errors.SubprocessError(msg)
 
     stdout, stderr = proc.communicate()
@@ -60,7 +62,7 @@ def run_script(params):
         msg = "Error while running %s.\n%s\n%s" % (
             " ".join(params), stdout, stderr)
         # Enter recovery routine...
-        logger.error(msg)
+        log(msg)
         raise errors.SubprocessError(msg)
 
     return stdout, stderr

docs/cli-help.txt

@@ -178,7 +178,7 @@ renew:
   The 'renew' subcommand will attempt to renew all certificates (or more
   precisely, certificate lineages) you have previously obtained if they are
   close to expiry, and print a summary of the results. By default, 'renew'
-  will reuse the options used to create obtain or most recently successfully
+  will reuse the options used to create, obtain or most recently successfully
   renew each certificate lineage. You can try it with `--dry-run` first. For
   more fine-grained control, you can renew individual lineages with the
   `certonly` subcommand. Hooks are available to run commands before and

docs/contributing.rst

@@ -246,9 +246,8 @@ configuration checkpoints and rollback.
 Display
 ~~~~~~~
 
-We currently offer a pythondialog and "text" mode for displays. Display
-plugins implement the `~certbot.interfaces.IDisplay`
-interface.
+We currently only offer a "text" mode for displays. Display plugins
+implement the `~certbot.interfaces.IDisplay` interface.
 
 .. _dev-plugin:
 

docs/using.rst

@@ -61,7 +61,7 @@ manual_     Y    N    | Helps you obtain a cert by giving you instructions to pe
 Under the hood, plugins use one of several ACME protocol "Challenges_" to
 prove you control a domain.  The options are http-01_ (which uses port 80),
 tls-sni-01_ (port 443) and dns-01_ (requring configuration of a DNS server on
-port 53, thought that's often not the same machine as your webserver). A few
+port 53, though that's often not the same machine as your webserver). A few
 plugins support more than one challenge type, in which case you can choose one
 with ``--preferred-challenges``.
 
@@ -193,7 +193,7 @@ postfix_    N    Y    STARTTLS Everywhere is becoming a Certbot Postfix/Exim plu
 =========== ==== ==== ===============================================================
 
 .. _plesk: https://github.com/plesk/letsencrypt-plesk
-.. _haproxy: https://code.greenhost.net/open/letsencrypt-haproxy
+.. _haproxy: https://github.com/greenhost/certbot-haproxy
 .. _s3front: https://github.com/dlapiduz/letsencrypt-s3front
 .. _gandi: https://github.com/Gandi/letsencrypt-gandi
 .. _icecast: https://github.com/e00E/lets-encrypt-icecast
@@ -233,7 +233,7 @@ certificate that contains all of the old domains and one or more additional
 new domains.
 
 ``--allow-subset-of-names`` tells Certbot to continue with cert generation if
-only some of the specified domain authorazations can be obtained. This may
+only some of the specified domain authorizations can be obtained. This may
 be useful if some domains specified in a certificate no longer point at this
 system.
 

letsencrypt-auto-source/letsencrypt-auto

@@ -15,9 +15,13 @@ set -e  # Work even if somebody does "sh thisscript.sh".
 
 # Note: you can set XDG_DATA_HOME or VENV_PATH before running this script,
 # if you want to change where the virtual environment will be installed
-XDG_DATA_HOME=${XDG_DATA_HOME:-~/.local/share}
+if [ -z "$XDG_DATA_HOME" ]; then
+  XDG_DATA_HOME="~/.local/share"
+fi
 VENV_NAME="letsencrypt"
-VENV_PATH=${VENV_PATH:-"$XDG_DATA_HOME/$VENV_NAME"}
+if [ -z "$VENV_PATH" ]; then
+  VENV_PATH="$XDG_DATA_HOME/$VENV_NAME"
+fi
 VENV_BIN="$VENV_PATH/bin"
 LE_AUTO_VERSION="0.10.0.dev0"
 BASENAME=$(basename $0)
@@ -80,6 +84,17 @@ if [ $BASENAME = "letsencrypt-auto" ]; then
   HELP=0
 fi
 
+# Support for busybox and others where there is no "command",
+# but "which" instead
+if command -v command > /dev/null 2>&1 ; then
+  export EXISTS="command -v"
+elif which which > /dev/null 2>&1 ; then
+  export EXISTS="which"
+else
+  echo "Cannot find command nor which... please install one!"
+  exit 1
+fi
+
 # certbot-auto needs root access to bootstrap OS dependencies, and
 # certbot itself needs root access for almost all modes of operation
 # The "normal" case is that sudo is used for the steps that need root, but
@@ -127,7 +142,7 @@ if [ -n "${LE_AUTO_SUDO+x}" ]; then
   echo "Using preset root authorization mechanism '$LE_AUTO_SUDO'."
 else
   if test "`id -u`" -ne "0" ; then
-    if command -v sudo 1>/dev/null 2>&1; then
+    if $EXISTS sudo 1>/dev/null 2>&1; then
       SUDO=sudo
       SUDO_ENV="CERTBOT_AUTO=$0"
     else
@@ -157,7 +172,7 @@ ExperimentalBootstrap() {
 DeterminePythonVersion() {
   for LE_PYTHON in "$LE_PYTHON" python2.7 python27 python2 python; do
     # Break (while keeping the LE_PYTHON value) if found.
-    command -v "$LE_PYTHON" > /dev/null && break
+    $EXISTS "$LE_PYTHON" > /dev/null && break
   done
   if [ "$?" != "0" ]; then
     echo "Cannot find any Pythons; please install one!"
@@ -198,19 +213,22 @@ BootstrapDebCommon() {
   # distro version (#346)
 
   virtualenv=
-  if apt-cache show virtualenv > /dev/null 2>&1 && ! apt-cache --quiet=0 show virtualenv 2>&1 | grep -q 'No packages found'; then
-    virtualenv="virtualenv"
+  # virtual env is known to apt and is installable
+  if apt-cache show virtualenv > /dev/null 2>&1 ; then
+      if ! LC_ALL=C apt-cache --quiet=0 show virtualenv 2>&1 | grep -q 'No packages found'; then
+          virtualenv="virtualenv"
+      fi
   fi
 
   if apt-cache show python-virtualenv > /dev/null 2>&1; then
-    virtualenv="$virtualenv python-virtualenv"
+      virtualenv="$virtualenv python-virtualenv"
   fi
 
   augeas_pkg="libaugeas0 augeas-lenses"
-  AUGVERSION=`apt-cache show --no-all-versions libaugeas0 | grep ^Version: | cut -d" " -f2`
+  AUGVERSION=`LC_ALL=C apt-cache show --no-all-versions libaugeas0 | grep ^Version: | cut -d" " -f2`
 
   if [ "$ASSUME_YES" = 1 ]; then
-    YES_FLAG="-y"
+      YES_FLAG="-y"
   fi
 
   AddBackportRepo() {
@@ -276,7 +294,7 @@ BootstrapDebCommon() {
 
 
 
-  if ! command -v virtualenv > /dev/null ; then
+  if ! $EXISTS virtualenv > /dev/null ; then
     echo Failed to install a working \"virtualenv\" command, exiting
     exit 1
   fi
@@ -960,7 +978,28 @@ UNLIKELY_EOF
       # Report error. (Otherwise, be quiet.)
       echo "Had a problem while installing Python packages."
       if [ "$VERBOSE" != 1 ]; then
+        echo
+        echo "pip prints the following errors: "
+        echo "====================================================="
         echo "$PIP_OUT"
+        echo "====================================================="
+        echo
+        echo "Certbot has problem setting up the virtual environment."
+
+        if `echo $PIP_OUT | grep -q Killed` || `echo $PIP_OUT | grep -q "allocate memory"` ; then
+          echo
+          echo "Based on your pip output, the problem can likely be fixed by "
+          echo "increasing the available memory."
+        else
+          echo
+          echo "We were not be able to guess the right solution from your pip "
+          echo "output."
+        fi
+
+        echo
+        echo "Consult https://certbot.eff.org/docs/install.html#problems-with-python-virtual-environment"
+        echo "for possible solutions."
+        echo "You may also find some support resources at https://certbot.eff.org/support/ ."
       fi
       rm -rf "$VENV_PATH"
       exit 1

letsencrypt-auto-source/pieces/bootstrappers/deb_common.sh

@@ -23,19 +23,22 @@ BootstrapDebCommon() {
   # distro version (#346)
 
   virtualenv=
-  if apt-cache show virtualenv > /dev/null 2>&1 && ! apt-cache --quiet=0 show virtualenv 2>&1 | grep -q 'No packages found'; then
-    virtualenv="virtualenv"
+  # virtual env is known to apt and is installable
+  if apt-cache show virtualenv > /dev/null 2>&1 ; then
+      if ! LC_ALL=C apt-cache --quiet=0 show virtualenv 2>&1 | grep -q 'No packages found'; then
+          virtualenv="virtualenv"
+      fi
   fi
 
   if apt-cache show python-virtualenv > /dev/null 2>&1; then
-    virtualenv="$virtualenv python-virtualenv"
+      virtualenv="$virtualenv python-virtualenv"
   fi
 
   augeas_pkg="libaugeas0 augeas-lenses"
-  AUGVERSION=`apt-cache show --no-all-versions libaugeas0 | grep ^Version: | cut -d" " -f2`
+  AUGVERSION=`LC_ALL=C apt-cache show --no-all-versions libaugeas0 | grep ^Version: | cut -d" " -f2`
 
   if [ "$ASSUME_YES" = 1 ]; then
-    YES_FLAG="-y"
+      YES_FLAG="-y"
   fi
 
   AddBackportRepo() {

setup.py

@@ -131,7 +131,6 @@ setup(
             'null = certbot.plugins.null:Installer',
             'standalone = certbot.plugins.standalone:Authenticator',
             'webroot = certbot.plugins.webroot:Authenticator',
-            'script = certbot.plugins.script:Authenticator',
         ],
     },
 )

tests/boulder-integration.sh

@@ -33,6 +33,23 @@ common() {
         "$@"
 }
 
+CheckHooks() {
+    EXPECTED="/tmp/expected$$"
+    echo "wtf.pre" > "$EXPECTED"
+    echo "wtf2.pre" >> "$EXPECTED"
+    echo "renew" >> "$EXPECTED"
+    echo "renew" >> "$EXPECTED"
+    echo "wtf.post" > "$EXPECTED"
+    echo "wtf2.post" >> "$EXPECTED"
+    if cmp --quiet "$EXPECTED" "$HOOK_TEST" ; then
+        echo Hooks did not run as expected\; got
+        cat "$HOOK_TEST"
+        echo Expected
+        cat "$EXPECTED"
+    fi
+    [ -f "$HOOK_TEST" ] && rm -f "$HOOK_TEST"
+}
+
 # We start a server listening on the port for the
 # unrequested challenge to prevent regressions in #3601.
 python -m SimpleHTTPServer $http_01_port &
@@ -52,26 +69,14 @@ common --domains le2.wtf --preferred-challenges http-01 run \
        --renew-hook 'echo renew >> "$HOOK_TEST"'
 kill $python_server_pid
 
-common -a manual -d le.wtf auth --rsa-key-size 4096 \
-       --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \
-       --post-hook 'echo wtf2.post >> "$HOOK_TEST"'
+common certonly -a manual -d le.wtf --rsa-key-size 4096 \
+    --manual-auth-hook ./tests/manual-http-auth.sh \
+    --manual-cleanup-hook ./tests/manual-http-cleanup.sh
+    --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \
+    --post-hook 'echo wtf2.post >> "$HOOK_TEST"'
 
-CheckHooks() {
-    EXPECTED="/tmp/expected$$"
-    echo "wtf.pre" > "$EXPECTED"
-    echo "wtf2.pre" >> "$EXPECTED"
-    echo "renew" >> "$EXPECTED"
-    echo "renew" >> "$EXPECTED"
-    echo "wtf.post" > "$EXPECTED"
-    echo "wtf2.post" >> "$EXPECTED"
-    if cmp --quiet "$EXPECTED" "$HOOK_TEST" ; then
-        echo Hooks did not run as expected\; got
-        cat "$HOOK_TEST"
-        echo Expected
-        cat "$EXPECTED"
-    fi
-    [ -f "$HOOK_TEST" ] && rm -f "$HOOK_TEST"
-}
+common certonly -a manual -d dns.le.wtf --preferred-challenges dns-01 \
+    --manual-auth-hook ./tests/manual-dns-auth.sh
 
 export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \
        OPENSSL_CNF=examples/openssl.cnf

tests/display.py

@@ -18,5 +18,5 @@ def test_visual(displayer, choices):
 
 
 if __name__ == "__main__":
-    displayer = util.FileDisplay(sys.stdout):
+    displayer = util.FileDisplay(sys.stdout, False)
     test_visual(displayer, util_test.CHOICES)

tests/integration/_common.sh

@@ -25,7 +25,7 @@ certbot_test_no_force_renew () {
         --no-verify-ssl \
         --tls-sni-01-port $tls_sni_01_port \
         --http-01-port $http_01_port \
-        --manual-test-mode \
+        --manual-public-ip-logging-ok \
         $store_flags \
         --non-interactive \
         --no-redirect \

tests/manual-dns-auth.sh

@@ -0,0 +1,4 @@
+#!/bin/sh
+curl -X POST 'http://localhost:8055/set-txt' -d \
+    "{\"host\": \"_acme-challenge.$CERTBOT_DOMAIN.\", \
+     \"value\": \"$CERTBOT_VALIDATION\"}"

tests/manual-http-auth.sh

@@ -0,0 +1,12 @@
+#!/bin/sh
+uri_path=".well-known/acme-challenge/$CERTBOT_TOKEN"
+
+cd $(mktemp -d)
+mkdir -p $(dirname $uri_path)
+echo $CERTBOT_VALIDATION > $uri_path
+python -m SimpleHTTPServer $http_01_port >/dev/null 2>&1 &
+server_pid=$!
+while ! curl "http://localhost:$http_01_port/$uri_path" >/dev/null 2>&1; do
+    sleep 1s
+done
+echo $server_pid

tests/manual-http-cleanup.sh

@@ -0,0 +1,2 @@
+#!/bin/sh
+kill $CERTBOT_AUTH_OUTPUT

tox.cover.sh

@@ -36,9 +36,8 @@ cover () {
   # specific package, positional argument scopes tests only to
   # specific package directory; --cover-tests makes sure every tests
   # is run (c.f. #403)
-  nosetests -c /dev/null --with-cover --cover-tests --cover-package \
-            "$1" --cover-min-percentage="$min" "$1" --processes=-1 \
-            --process-timeout=100
+  nosetests -c /dev/null --with-cover --cover-tests --cover-package  \
+            "$1" --cover-min-percentage="$min" "$1"
 }
 
 rm -f .coverage  # --cover-erase is off, make sure stats are correct

tox.ini

@@ -63,6 +63,11 @@ commands =
     pip install -e .[dev]
     nosetests -v certbot --processes=-1 --process-timeout=100
 
+[testenv:py27_install]
+basepython = python2.7
+commands =
+    pip install -e acme[dns,dev] -e .[dev] -e certbot-apache -e certbot-nginx -e letshelp-certbot
+
 [testenv:cover]
 basepython = python2.7
 commands =