Merge remote-tracking branch 'origin/namespace' into renew_verb

.travis.yml

@@ -23,6 +23,7 @@ env:
   global:
     - GOPATH=/tmp/go
     - PATH=$GOPATH/bin:$PATH
+
 matrix:
   include:
     - python: "2.6"
@@ -47,6 +48,10 @@ matrix:
       env: TOXENV=py34
     - python: "3.5"
       env: TOXENV=py35
+    - sudo: required
+      env: TOXENV=le_auto
+      services: docker
+      before_install:
 
 # Only build pushes to the master branch, PRs, and branches beginning with
 # `test-`. This reduces the number of simultaneous Travis runs, which speeds

letsencrypt-apache/letsencrypt_apache/centos-options-ssl-apache.conf

@@ -0,0 +1,21 @@
+# Baseline setting to Include for SSL sites
+
+SSLEngine on
+
+# Intermediate configuration, tweak to your needs
+SSLProtocol             all -SSLv2 -SSLv3
+SSLCipherSuite          ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:kEDH+AESGCM:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA:ECDHE-ECDSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA:DHE-RSA-AES256-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:AES:CAMELLIA:DES-CBC3-SHA:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!MD5:!PSK:!aECDH:!EDH-DSS-DES-CBC3-SHA:!EDH-RSA-DES-CBC3-SHA:!KRB5-DES-CBC3-SHA
+SSLHonorCipherOrder     on
+
+SSLOptions +StrictRequire
+
+# Add vhost name to log entries:
+LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined
+LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common
+
+#CustomLog /var/log/apache2/access.log vhost_combined
+#LogLevel warn
+#ErrorLog /var/log/apache2/error.log
+
+# Always ensure Cookies have "Secure" set (JAH 2012/1)
+#Header edit Set-Cookie (?i)^(.*)(;\s*secure)??((\s*;)?(.*)) "$1; Secure$3$4"

letsencrypt-apache/letsencrypt_apache/configurator.py

@@ -155,7 +155,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Set Version
         if self.version is None:
             self.version = self.get_version()
-        if self.version < (2, 4):
+        if self.version < (2, 2):
             raise errors.NotSupportedError(
                 "Apache Version %s not supported.", str(self.version))
 
@@ -305,6 +305,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             if not vhost.ssl:
                 vhost = self.make_vhost_ssl(vhost)
 
+            self._add_servername_alias(target_name, vhost)
             self.assoc[target_name] = vhost
             return vhost
 
@@ -335,6 +336,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 raise errors.PluginError(
                     "VirtualHost not able to be selected.")
 
+        self._add_servername_alias(target_name, vhost)
         self.assoc[target_name] = vhost
         return vhost
 
@@ -353,7 +355,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Points 1 - Address name with no SSL
         best_candidate = None
         best_points = 0
-
         for vhost in self.vhosts:
             if vhost.modmacro is True:
                 continue
@@ -692,7 +693,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         # Reload augeas to take into account the new vhost
         self.aug.load()
-
         # Get Vhost augeas path for new vhost
         vh_p = self.aug.match("/files%s//* [label()=~regexp('%s')]" %
                               (ssl_fp, parser.case_i("VirtualHost")))
@@ -709,6 +709,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         # Add directives
         self._add_dummy_ssl_directives(vh_p)
+        self.save()
 
         # Log actions and create save notes
         logger.info("Created an SSL vhost at %s", ssl_fp)
@@ -859,6 +860,22 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                             "insert_key_file_path")
         self.parser.add_dir(vh_path, "Include", self.mod_ssl_conf)
 
+    def _add_servername_alias(self, target_name, vhost):
+        fp = vhost.filep
+        vh_p = self.aug.match("/files%s//* [label()=~regexp('%s')]" %
+                              (fp, parser.case_i("VirtualHost")))
+        if not vh_p:
+            return
+        vh_path = vh_p[0]
+        if (self.parser.find_dir("ServerName", target_name, start=vh_path, exclude=False)
+           or self.parser.find_dir("ServerAlias", target_name, start=vh_path, exclude=False)):
+            return
+        if not self.parser.find_dir("ServerName", None, start=vh_path, exclude=False):
+            self.parser.add_dir(vh_path, "ServerName", target_name)
+        else:
+            self.parser.add_dir(vh_path, "ServerAlias", target_name)
+        self._add_servernames(vhost)
+
     def _add_name_vhost_if_necessary(self, vhost):
         """Add NameVirtualHost Directives if necessary for new vhost.
 
@@ -874,9 +891,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # See if the exact address appears in any other vhost
         # Remember 1.1.1.1:* == 1.1.1.1 -> hence any()
         for addr in vhost.addrs:
+            # In Apache 2.2, when a NameVirtualHost directive is not
+            # set, "*" and "_default_" will conflict when sharing a port
+            if addr.get_addr() in ("*", "_default_"):
+                addrs = [obj.Addr((a, addr.get_port(),))
+                         for a in ("*", "_default_")]
+
             for test_vh in self.vhosts:
                 if (vhost.filep != test_vh.filep and
-                        any(test_addr == addr for
+                        any(test_addr in addrs for
                             test_addr in test_vh.addrs) and
                         not self.is_name_vhost(addr)):
                     self.add_name_vhost(addr)
@@ -1587,4 +1610,4 @@ def install_ssl_options_conf(options_ssl):
 
     # Check to make sure options-ssl.conf is installed
     if not os.path.isfile(options_ssl):
-        shutil.copyfile(constants.MOD_SSL_CONF_SRC, options_ssl)
+        shutil.copyfile(constants.os_constant("MOD_SSL_CONF_SRC"), options_ssl)

letsencrypt-apache/letsencrypt_apache/constants.py

@@ -16,7 +16,9 @@ CLI_DEFAULTS_DEBIAN = dict(
     le_vhost_ext="-le-ssl.conf",
     handle_mods=True,
     handle_sites=True,
-    challenge_location="/etc/apache2"
+    challenge_location="/etc/apache2",
+    MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+    "letsencrypt_apache", "options-ssl-apache.conf")
 )
 CLI_DEFAULTS_CENTOS = dict(
     server_root="/etc/httpd",
@@ -31,7 +33,9 @@ CLI_DEFAULTS_CENTOS = dict(
     le_vhost_ext="-le-ssl.conf",
     handle_mods=False,
     handle_sites=False,
-    challenge_location="/etc/httpd/conf.d"
+    challenge_location="/etc/httpd/conf.d",
+    MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+    "letsencrypt_apache", "centos-options-ssl-apache.conf")
 )
 CLI_DEFAULTS_GENTOO = dict(
     server_root="/etc/apache2",
@@ -46,7 +50,9 @@ CLI_DEFAULTS_GENTOO = dict(
     le_vhost_ext="-le-ssl.conf",
     handle_mods=False,
     handle_sites=False,
-    challenge_location="/etc/apache2/vhosts.d"
+    challenge_location="/etc/apache2/vhosts.d",
+    MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+    "letsencrypt_apache", "options-ssl-apache.conf")
 )
 CLI_DEFAULTS = {
     "debian": CLI_DEFAULTS_DEBIAN,
@@ -62,11 +68,6 @@ CLI_DEFAULTS = {
 MOD_SSL_CONF_DEST = "options-ssl-apache.conf"
 """Name of the mod_ssl config file as saved in `IConfig.config_dir`."""
 
-MOD_SSL_CONF_SRC = pkg_resources.resource_filename(
-    "letsencrypt_apache", "options-ssl-apache.conf")
-"""Path to the Apache mod_ssl config file found in the Let's Encrypt
-distribution."""
-
 AUGEAS_LENS_DIR = pkg_resources.resource_filename(
     "letsencrypt_apache", "augeas_lens")
 """Path to the Augeas lens directory"""

letsencrypt-apache/letsencrypt_apache/display_ops.py

@@ -85,12 +85,12 @@ def _vhost_menu(domain, vhosts):
             "or Address of {0}.{1}Which virtual host would you "
             "like to choose?".format(domain, os.linesep),
             choices, help_label="More Info", ok_label="Select")
-    except errors.MissingCommandlineFlag, e:
+    except errors.MissingCommandlineFlag as e:
         msg = ("Failed to run Apache plugin non-interactively{1}{0}{1}"
                "(The best solution is to add ServerName or ServerAlias "
                "entries to the VirtualHost directives of your apache "
                "configuration files.)".format(e, os.linesep))
-        raise errors.MissingCommandlineFlag, msg
+        raise errors.MissingCommandlineFlag(msg)
 
     return code, tag
 

letsencrypt-apache/letsencrypt_apache/parser.py

@@ -597,7 +597,7 @@ class ApacheParser(object):
         .. todo:: Make sure that files are included
 
         """
-        default = self._set_user_config_file()
+        default = self.loc["root"]
 
         temp = os.path.join(self.root, "ports.conf")
         if os.path.isfile(temp):
@@ -618,23 +618,6 @@ class ApacheParser(object):
 
         raise errors.NoInstallationError("Could not find configuration root")
 
-    def _set_user_config_file(self):
-        """Set the appropriate user configuration file
-
-        .. todo:: This will have to be updated for other distros versions
-
-        :param str root: pathname which contains the user config
-
-        """
-        # Basic check to see if httpd.conf exists and
-        # in hierarchy via direct include
-        # httpd.conf was very common as a user file in Apache 2.2
-        if (os.path.isfile(os.path.join(self.root, "httpd.conf")) and
-                self.find_dir("Include", "httpd.conf", self.loc["root"])):
-            return os.path.join(self.root, "httpd.conf")
-        else:
-            return os.path.join(self.root, "apache2.conf")
-
 
 def case_i(string):
     """Returns case insensitive regex.

letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py

@@ -161,6 +161,7 @@ class TwoVhost80Test(util.ApacheTest):
     def test_choose_vhost_select_vhost_non_ssl(self, mock_select):
         mock_select.return_value = self.vh_truth[0]
         chosen_vhost = self.config.choose_vhost("none.com")
+        self.vh_truth[0].aliases.add("none.com")
         self.assertEqual(
             self.vh_truth[0].get_names(), chosen_vhost.get_names())
 
@@ -192,8 +193,8 @@ class TwoVhost80Test(util.ApacheTest):
         self.assertEqual(
             self.vh_truth[0],
             self.config._find_best_vhost("encryption-example.demo"))
-        self.assertTrue(
-            self.config._find_best_vhost("does-not-exist.com") is None)
+        self.assertEqual(
+            self.config._find_best_vhost("does-not-exist.com"), None)
 
     def test_find_best_vhost_variety(self):
         # pylint: disable=protected-access
@@ -606,6 +607,14 @@ class TwoVhost80Test(util.ApacheTest):
         self.config._add_name_vhost_if_necessary(self.vh_truth[0])
         self.assertTrue(self.config.save.called)
 
+        new_addrs = set()
+        for addr in self.vh_truth[0].addrs:
+            new_addrs.add(obj.Addr(("_default_", addr.get_port(),)))
+
+        self.vh_truth[0].addrs = new_addrs
+        self.config._add_name_vhost_if_necessary(self.vh_truth[0])
+        self.assertEqual(self.config.save.call_count, 2)
+
     @mock.patch("letsencrypt_apache.configurator.tls_sni_01.ApacheTlsSni01.perform")
     @mock.patch("letsencrypt_apache.configurator.ApacheConfigurator.restart")
     def test_perform(self, mock_restart, mock_perform):

letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py

@@ -37,7 +37,7 @@ class SelectVhostTest(unittest.TestCase):
         mock_util().menu.side_effect = errors.MissingCommandlineFlag("no vhost default")
         try:
             self._call(self.vhosts)
-        except errors.MissingCommandlineFlag, e:
+        except errors.MissingCommandlineFlag as e:
             self.assertTrue("VirtualHost directives" in e.message)
 
     @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility")

letsencrypt-apache/letsencrypt_apache/tests/parser_test.py

@@ -106,7 +106,7 @@ class BasicParserTest(util.ParserTest):
     def test_set_locations(self):
         with mock.patch("letsencrypt_apache.parser.os.path") as mock_path:
 
-            mock_path.isfile.side_effect = [True, False, False]
+            mock_path.isfile.side_effect = [False, False]
 
             # pylint: disable=protected-access
             results = self.parser._set_locations()
@@ -114,16 +114,6 @@ class BasicParserTest(util.ParserTest):
             self.assertEqual(results["default"], results["listen"])
             self.assertEqual(results["default"], results["name"])
 
-    def test_set_user_config_file(self):
-        # pylint: disable=protected-access
-        path = os.path.join(self.parser.root, "httpd.conf")
-        open(path, 'w').close()
-        self.parser.add_dir(self.parser.loc["default"], "Include",
-                            "httpd.conf")
-
-        self.assertEqual(
-            path, self.parser._set_user_config_file())
-
     @mock.patch("letsencrypt_apache.parser.ApacheParser._get_runtime_cfg")
     def test_update_runtime_variables(self, mock_cfg):
         mock_cfg.return_value = (

letsencrypt-apache/letsencrypt_apache/tests/util.py

@@ -33,7 +33,7 @@ class ApacheTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
             pkg="letsencrypt_apache.tests")
 
         self.ssl_options = common.setup_ssl_options(
-            self.config_dir, constants.MOD_SSL_CONF_SRC,
+            self.config_dir, constants.os_constant("MOD_SSL_CONF_SRC"),
             constants.MOD_SSL_CONF_DEST)
 
         self.config_path = os.path.join(self.temp_dir, config_root)
@@ -150,7 +150,7 @@ def get_vh_truth(temp_dir, config_name):
                 os.path.join(prefix, "default-ssl-port-only.conf"),
                 os.path.join(aug_pre, ("default-ssl-port-only.conf/"
                                        "IfModule/VirtualHost")),
-                set([obj.Addr.fromstring("_default_:443")]), True, False),
+                set([obj.Addr.fromstring("_default_:443")]), True, False)
         ]
         return vh_truth
 

letsencrypt-auto-source/Dockerfile

@@ -7,7 +7,6 @@ FROM ubuntu:trusty
 RUN useradd --create-home --home-dir /home/lea --shell /bin/bash --groups sudo --uid 1000 lea
 
 # Let that user sudo:
-RUN adduser lea sudo
 RUN sed -i.bkp -e \
       's/%sudo\s\+ALL=(ALL\(:ALL\)\?)\s\+ALL/%sudo ALL=NOPASSWD:ALL/g' \
       /etc/sudoers

letsencrypt-auto-source/letsencrypt-auto

@@ -19,7 +19,7 @@ XDG_DATA_HOME=${XDG_DATA_HOME:-~/.local/share}
 VENV_NAME="letsencrypt"
 VENV_PATH=${VENV_PATH:-"$XDG_DATA_HOME/$VENV_NAME"}
 VENV_BIN=${VENV_PATH}/bin
-LE_AUTO_VERSION="0.3.0"
+LE_AUTO_VERSION="0.4.0.dev0"
 
 # This script takes the same arguments as the main letsencrypt program, but it
 # additionally responds to --verbose (more output) and --debug (allow support
@@ -374,7 +374,7 @@ Bootstrap() {
   elif [ -f /etc/redhat-release ]; then
     echo "Bootstrapping dependencies for RedHat-based OSes..."
     BootstrapRpmCommon
-  elif `grep -q openSUSE /etc/os-release` ; then
+  elif [ -f /etc/os-release] && `grep -q openSUSE /etc/os-release` ; then
     echo "Bootstrapping dependencies for openSUSE-based OSes..."
     BootstrapSuseCommon
   elif [ -f /etc/arch-release ]; then

letsencrypt-auto-source/letsencrypt-auto.template

@@ -130,7 +130,7 @@ Bootstrap() {
   elif [ -f /etc/redhat-release ]; then
     echo "Bootstrapping dependencies for RedHat-based OSes..."
     BootstrapRpmCommon
-  elif `grep -q openSUSE /etc/os-release` ; then
+  elif [ -f /etc/os-release] && `grep -q openSUSE /etc/os-release` ; then
     echo "Bootstrapping dependencies for openSUSE-based OSes..."
     BootstrapSuseCommon
   elif [ -f /etc/arch-release ]; then

letsencrypt-nginx/letsencrypt_nginx/configurator.py

@@ -310,10 +310,10 @@ class NginxConfigurator(common.Plugin):
         key = OpenSSL.crypto.load_privatekey(
             OpenSSL.crypto.FILETYPE_PEM, le_key.pem)
         cert = acme_crypto_util.gen_ss_cert(key, domains=[socket.gethostname()])
-        cert_path = os.path.join(tmp_dir, "cert.pem")
         cert_pem = OpenSSL.crypto.dump_certificate(
             OpenSSL.crypto.FILETYPE_PEM, cert)
-        with open(cert_path, 'w') as cert_file:
+        cert_file, cert_path = le_util.unique_file(os.path.join(tmp_dir, "cert.pem"))
+        with cert_file:
             cert_file.write(cert_pem)
         return cert_path, le_key.file
 

letsencrypt/cli.py

@@ -113,11 +113,15 @@ def usage_strings(plugins):
     return USAGE % (apache_doc, nginx_doc), SHORT_USAGE
 
 
-def _find_domains(args, installer):
-    if not args.domains:
+def _find_domains(config, installer):
+    if not config.domains:
         domains = display_ops.choose_names(installer)
+        # record in config.domains (so that it can be serialised in renewal config files),
+        # and set webroot_map entries if applicable
+        for d in domains:
+            _process_domain(config, d)
     else:
-        domains = args.domains
+        domains = config.domains
 
     if not domains:
         raise errors.Error("Please specify --domains, or --installer that "
@@ -540,7 +544,7 @@ def choose_configurator_plugins(args, config, plugins, verb):
                    '{1} and "--help plugins" for more information.)'.format(
                    req_auth, os.linesep, cli_command))
 
-            raise errors.MissingCommandlineFlag, msg
+            raise errors.MissingCommandlineFlag(msg)
     else:
         need_inst = need_auth = False
     if verb == "certonly":
@@ -588,10 +592,10 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     """Obtain a certificate and install."""
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "run")
-    except errors.PluginSelectionError, e:
+    except errors.PluginSelectionError as e:
         return e.message
 
-    domains = _find_domains(args, installer)
+    domains = _find_domains(config, installer)
 
     # TODO: Handle errors from _init_le_client?
     le_client = _init_le_client(args, config, authenticator, installer)
@@ -613,7 +617,8 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
 
 
 def obtain_cert(args, config, plugins):
-    """Authenticate & obtain cert, but do not install it."""
+    """Implements "certonly": authenticate & obtain cert, but do not install it."""
+
     if args.domains and args.csr is not None:
         # TODO: --csr could have a priority, when --domains is
         # supplied, check if CSR matches given domains?
@@ -622,7 +627,7 @@ def obtain_cert(args, config, plugins):
     try:
         # installers are used in auth mode to determine domain names
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "certonly")
-    except errors.PluginSelectionError, e:
+    except errors.PluginSelectionError as e:
         return e.message
 
     # TODO: Handle errors from _init_le_client?
@@ -636,7 +641,7 @@ def obtain_cert(args, config, plugins):
             certr, chain, args.cert_path, args.chain_path, args.fullchain_path)
         _report_new_cert(cert_path, cert_fullchain)
     else:
-        domains = _find_domains(args, installer)
+        domains = _find_domains(config, installer)
         _auth_from_domains(le_client, config, domains)
 
     _suggest_donate()
@@ -651,10 +656,10 @@ def install(args, config, plugins):
     try:
         installer, _ = choose_configurator_plugins(args, config,
                                                    plugins, "install")
-    except errors.PluginSelectionError, e:
+    except errors.PluginSelectionError as e:
         return e.message
 
-    domains = _find_domains(args, installer)
+    domains = _find_domains(config, installer)
     le_client = _init_le_client(
         args, config, authenticator=None, installer=installer)
     assert args.cert_path is not None  # required=True in the subparser
@@ -901,6 +906,12 @@ class HelpfulArgumentParser(object):
 
         # Do any post-parsing homework here
 
+        # we get domains from -d, but also from the webroot map...
+        if parsed_args.webroot_map:
+            for domain in parsed_args.webroot_map.keys():
+                if domain not in parsed_args.domains:
+                    parsed_args.domains.append(domain)
+
         # argparse seemingly isn't flexible enough to give us this behaviour easily...
         if parsed_args.staging:
             if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
@@ -1323,11 +1334,12 @@ def _plugins_parsing(helpful, plugins):
                      "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`")
-    parse_dict = lambda s: dict(json.loads(s))
     # --webroot-map still has some awkward properties, so it is undocumented
-    helpful.add("webroot", "--webroot-map", default={}, type=parse_dict,
-                help=argparse.SUPPRESS)
-
+    helpful.add("webroot", "--webroot-map", default={}, action=WebrootMapProcessor,
+        help="JSON dictionary mapping domains to webroot paths; this implies -d "
+             "for each entry. You may need to escape this from your shell. "
+             """Eg: --webroot-map '{"eg1.is,m.eg1.is":"/www/eg1/", "eg2.is":"/www/eg2"}' """
+             "This option is merged with, but takes precedence over, -w / -d entries")
 
 class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
     def __init__(self, *args, **kwargs):
@@ -1356,18 +1368,36 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstring
         config.webroot_path.append(webroot)
 
 
+_undot = lambda domain: domain[:-1] if domain.endswith('.') else domain
+
+def _process_domain(config, domain_arg, webroot_path=None):
+    """
+    Process a new -d flag, helping the webroot plugin construct a map of
+    {domain : webrootpath} if -w / --webroot-path is in use
+    """
+    webroot_path = webroot_path if webroot_path else config.webroot_path
+
+    for domain in (d.strip() for d in domain_arg.split(",")):
+        if domain not in config.domains:
+            domain = _undot(domain)
+            config.domains.append(domain)
+            # Each domain has a webroot_path of the most recent -w flag
+            # unless it was explicitly included in webroot_map
+            if webroot_path:
+                config.webroot_map.setdefault(domain, webroot_path[-1])
+
+
+class WebrootMapProcessor(argparse.Action): # pylint: disable=missing-docstring
+    def __call__(self, parser, config, webroot_map_arg, option_string=None):
+        webroot_map = json.loads(webroot_map_arg)
+        for domains, webroot_path in webroot_map.iteritems():
+            _process_domain(config, domains, [webroot_path])
+
+
 class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring
     def __call__(self, parser, config, domain_arg, option_string=None):
-        """
-        Process a new -d flag, helping the webroot plugin construct a map of
-        {domain : webrootpath} if -w / --webroot-path is in use
-        """
-        for domain in (d.strip() for d in domain_arg.split(",")):
-            if domain not in config.domains:
-                config.domains.append(domain)
-                # Each domain has a webroot_path of the most recent -w flag
-                if config.webroot_path:
-                    config.webroot_map[domain] = config.webroot_path[-1]
+        """Just wrap _process_domain in argparseese."""
+        _process_domain(config, domain_arg)
 
 
 def setup_log_file_handler(args, logfile, fmt):

letsencrypt/client.py

@@ -146,7 +146,7 @@ def perform_registration(acme, config):
     """
     try:
         return acme.register(messages.NewRegistration.from_data(email=config.email))
-    except messages.Error, e:
+    except messages.Error as e:
         err = repr(e)
         if "MX record" in err or "Validation of contact mailto" in err:
             config.namespace.email = display_ops.get_email(more=True, invalid=True)

letsencrypt/configuration.py

@@ -43,6 +43,12 @@ class NamespaceConfig(object):
         # Check command line parameters sanity, and error out in case of problem.
         check_config_sanity(self)
 
+        # We're done setting up the attic. Now pull up the ladder after ourselves...
+        self.__setattr__ = self.__setattr_implementation__
+
+    def __setattr_implementation__(self, var, value):
+        return self.namespace.__setattr__(var, value)
+
     def __getattr__(self, name):
         return getattr(self.namespace, name)
 

letsencrypt/crypto_util.py

@@ -53,8 +53,8 @@ def init_save_key(key_size, key_dir, keyname="key-letsencrypt.pem"):
                                config.strict_permissions)
     key_f, key_path = le_util.unique_file(
         os.path.join(key_dir, keyname), 0o600)
-    key_f.write(key_pem)
-    key_f.close()
+    with key_f:
+        key_f.write(key_pem)
 
     logger.info("Generating key (%d bits): %s", key_size, key_path)
 

letsencrypt/display/ops.py

@@ -78,11 +78,12 @@ def pick_plugin(config, default, plugins, question, ifaces):
             # it's really bad to auto-select the single available plugin in
             # non-interactive mode, because an update could later add a second
             # available plugin
-            raise errors.MissingCommandlineFlag, ("Missing command line flags. For non-interactive "
-                "execution, you will need to specify a plugin on the command line.  Run with "
-                "'--help plugins' to see a list of options, and see "
-                " https://eff.org/letsencrypt-plugins for more detail on what the plugins "
-                "do and how to use them.")
+            raise errors.MissingCommandlineFlag(
+                "Missing command line flags. For non-interactive execution, "
+                "you will need to specify a plugin on the command line.  Run "
+                "with '--help plugins' to see a list of options, and see "
+                "https://eff.org/letsencrypt-plugins for more detail on what "
+                "the plugins do and how to use them.")
 
         filtered = plugins.visible().ifaces(ifaces)
 
@@ -158,7 +159,7 @@ def get_email(more=False, invalid=False):
     except errors.MissingCommandlineFlag:
         msg = ("You should register before running non-interactively, or provide --agree-tos"
                " and --email <email_address> flags")
-        raise errors.MissingCommandlineFlag, msg
+        raise errors.MissingCommandlineFlag(msg)
 
     if code == display_util.OK:
         if le_util.safe_email(email):

letsencrypt/display/util.py

@@ -428,7 +428,7 @@ class NoninteractiveDisplay(object):
             msg += "\n" + extra
         if cli_flag:
             msg += "\n\n(You can set this with the {0} flag)".format(cli_flag)
-        raise errors.MissingCommandlineFlag, msg
+        raise errors.MissingCommandlineFlag(msg)
 
     def notification(self, message, height=10, pause=False):
         # pylint: disable=unused-argument

letsencrypt/le_util.py

@@ -298,18 +298,19 @@ def check_domain_sanity(domain):
     # Check if there's a wildcard domain
     if domain.startswith("*."):
         raise errors.ConfigurationError(
-            "Wildcard domains are not supported")
+            "Wildcard domains are not supported: {0}".format(domain))
     # Punycode
     if "xn--" in domain:
         raise errors.ConfigurationError(
-            "Punycode domains are not presently supported")
+            "Punycode domains are not presently supported: {0}".format(domain))
 
     # Unicode
     try:
         domain.encode('ascii')
     except UnicodeDecodeError:
         raise errors.ConfigurationError(
-            "Internationalized domain names are not presently supported")
+            "Internationalized domain names are not presently supported: {0}"
+            .format(domain))
 
     # FQDN checks from
     # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/
@@ -317,4 +318,4 @@ def check_domain_sanity(domain):
     #  first and last char is not "-"
     fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(?<!-)\\.)+[A-Za-z]{2,63}$")
     if not fqdn.match(domain):
-        raise errors.ConfigurationError("Requested domain is not a FQDN")
+        raise errors.ConfigurationError("Requested domain {0} is not a FQDN".format(domain))

letsencrypt/storage.py

@@ -128,6 +128,17 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
         self.fullchain = self.configuration["fullchain"]
 
         self._fix_symlinks()
+        self._check_symlinks()
+
+    def _check_symlinks(self):
+        """Raises an exception if a symlink doesn't exist"""
+        def check(link):
+            """Checks if symlink points to a file that exists"""
+            return os.path.exists(os.path.realpath(link))
+        for kind in ALL_FOUR:
+            if not check(getattr(self, kind)):
+                raise errors.CertStorageError(
+                        "link: {0} does not exist".format(getattr(self, kind)))
 
     def _consistent(self):
         """Are the files associated with this lineage self-consistent?

letsencrypt/tests/cli_test.py

@@ -138,7 +138,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         try:
             with mock.patch('letsencrypt.cli.sys.stderr'):
                 cli.main(self.standard_args + args[:])  # NOTE: parser can alter its args!
-        except errors.MissingCommandlineFlag, exc:
+        except errors.MissingCommandlineFlag as exc:
             self.assertTrue(message in str(exc))
         self.assertTrue(exc is not None)
 
@@ -356,6 +356,10 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         namespace = cli.prepare_and_parse_args(plugins, short_args)
         self.assertEqual(namespace.domains, ['example.com'])
 
+        short_args = ['-d', 'trailing.period.com.']
+        namespace = cli.prepare_and_parse_args(plugins, short_args)
+        self.assertEqual(namespace.domains, ['trailing.period.com'])
+
         short_args = ['-d', 'example.com,another.net,third.org,example.com']
         namespace = cli.prepare_and_parse_args(plugins, short_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net',
@@ -365,6 +369,10 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         namespace = cli.prepare_and_parse_args(plugins, long_args)
         self.assertEqual(namespace.domains, ['example.com'])
 
+        long_args = ['--domains', 'trailing.period.com.']
+        namespace = cli.prepare_and_parse_args(plugins, long_args)
+        self.assertEqual(namespace.domains, ['trailing.period.com'])
+
         long_args = ['--domains', 'example.com,another.net,example.com']
         namespace = cli.prepare_and_parse_args(plugins, long_args)
         self.assertEqual(namespace.domains, ['example.com', 'another.net'])
@@ -382,11 +390,26 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         short_args = ['--staging', '--server', 'example.com']
         self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args)
 
+    def _webroot_map_test(self, map_arg, path_arg, domains_arg, # pylint: disable=too-many-arguments
+                          expected_map, expectect_domains, extra_args=None):
+        plugins = disco.PluginsRegistry.find_all()
+        webroot_map_args = extra_args if extra_args else []
+        if map_arg:
+            webroot_map_args.extend(["--webroot-map", map_arg])
+        if path_arg:
+            webroot_map_args.extend(["-w", path_arg])
+        if domains_arg:
+            webroot_map_args.extend(["-d", domains_arg])
+        namespace = cli.prepare_and_parse_args(plugins, webroot_map_args)
+        domains = cli._find_domains(namespace, mock.MagicMock()) # pylint: disable=protected-access
+        self.assertEqual(namespace.webroot_map, expected_map)
+        self.assertEqual(set(domains), set(expectect_domains))
+
     def test_parse_webroot(self):
         plugins = disco.PluginsRegistry.find_all()
         webroot_args = ['--webroot', '-w', '/var/www/example',
             '-d', 'example.com,www.example.com', '-w', '/var/www/superfluous',
-            '-d', 'superfluo.us', '-d', 'www.superfluo.us']
+            '-d', 'superfluo.us', '-d', 'www.superfluo.us.']
         namespace = cli.prepare_and_parse_args(plugins, webroot_args)
         self.assertEqual(namespace.webroot_map, {
             'example.com': '/var/www/example',
@@ -397,9 +420,29 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         webroot_args = ['-d', 'stray.example.com'] + webroot_args
         self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, webroot_args)
 
-        webroot_map_args = ['--webroot-map', '{"eg.com" : "/tmp"}']
+        simple_map = '{"eg.com" : "/tmp"}'
+        expected_map = {"eg.com": "/tmp"}
+        self._webroot_map_test(simple_map, None, None, expected_map, ["eg.com"])
+
+        # test merging webroot maps from the cli and a webroot map
+        expected_map["eg2.com"] = "/tmp2"
+        domains = ["eg.com", "eg2.com"]
+        self._webroot_map_test(simple_map, "/tmp2", "eg2.com,eg.com", expected_map, domains)
+
+        # test inclusion of interactively specified domains in the webroot map
+        with mock.patch('letsencrypt.cli.display_ops.choose_names') as mock_choose:
+            mock_choose.return_value = domains
+            expected_map["eg2.com"] = "/tmp"
+            self._webroot_map_test(None, "/tmp", None, expected_map, domains)
+
+        extra_args = ['-c', test_util.vector_path('webrootconftest.ini')]
+        self._webroot_map_test(None, None, None, expected_map, domains, extra_args)
+
+        webroot_map_args = ['--webroot-map',
+                            '{"eg.com.,www.eg.com": "/tmp", "eg.is.": "/tmp2"}']
         namespace = cli.prepare_and_parse_args(plugins, webroot_map_args)
-        self.assertEqual(namespace.webroot_map, {u"eg.com": u"/tmp"})
+        self.assertEqual(namespace.webroot_map,
+                         {"eg.com": "/tmp", "www.eg.com": "/tmp", "eg.is": "/tmp2"})
 
     @mock.patch('letsencrypt.cli._suggest_donate')
     @mock.patch('letsencrypt.crypto_util.notAfter')

letsencrypt/tests/renewer_test.py

@@ -76,7 +76,10 @@ class BaseRenewableCertTest(unittest.TestCase):
         junk.close()
 
         self.defaults = configobj.ConfigObj()
-        self.test_rc = storage.RenewableCert(config.filename, self.cli_config)
+
+        with mock.patch("letsencrypt.storage.RenewableCert._check_symlinks") as check:
+            check.return_value = True
+            self.test_rc = storage.RenewableCert(config.filename, self.cli_config)
 
     def tearDown(self):
         shutil.rmtree(self.tempdir)
@@ -786,6 +789,12 @@ class RenewableCertTests(BaseRenewableCertTest):
         renewer.main(cli_args=self._common_cli_args())
         # The errors.CertStorageError is caught inside and nothing happens.
 
+    def test_missing_cert(self):
+        from letsencrypt import storage
+        self.assertRaises(errors.CertStorageError,
+                          storage.RenewableCert,
+                          self.config.filename, self.cli_config)
+
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

letsencrypt/tests/testdata/webrootconftest.ini

@@ -0,0 +1,3 @@
+webroot
+webroot-path = /tmp
+domains = eg.com, eg2.com

tests/letstest/multitester.py

@@ -141,7 +141,7 @@ def make_instance(instance_name,
     # give instance a name
     try:
         new_instance.create_tags(Tags=[{'Key': 'Name', 'Value': instance_name}])
-    except botocore.exceptions.ClientError, e:
+    except botocore.exceptions.ClientError as e:
         if "InvalidInstanceID.NotFound" in str(e):
             # This seems to be ephemeral... retry
             time.sleep(1)