Merge branch '919' into cli

2026-01-21 19:01:07 +03:00 · 2015-10-16 17:46:43 -07:00
parent dcf8ea4e97 c7732114cb
commit 9efdd095ca
28 changed files with 352 additions and 153 deletions
--- a/.travis.yml
+++ b/.travis.yml
@@ -21,6 +21,15 @@ env:
    - TOXENV=lint
    - TOXENV=cover

+
+# Only build pushes to the master branch, PRs, and branches beginning with
+# `test-`. This reduces the number of simultaneous Travis runs, which speeds
+# turnaround time on review since there is a cap of 5 simultaneous runs.
+branches:
+  only:
+    - master
+    - /^test-.*$/
+
 sudo: false  # containers
 addons:
  # make sure simplehttp simple verification works (custom /etc/hosts)
--- a/acme/acme/jose/interfaces_test.py
+++ b/acme/acme/jose/interfaces_test.py
@@ -1,6 +1,8 @@
 """Tests for acme.jose.interfaces."""
 import unittest

+import six
+

 class JSONDeSerializableTest(unittest.TestCase):
    # pylint: disable=too-many-instance-attributes
@@ -90,8 +92,9 @@ class JSONDeSerializableTest(unittest.TestCase):
        self.assertEqual('["foo1", "foo2"]', self.seq.json_dumps())

    def test_json_dumps_pretty(self):
-        self.assertEqual(
-            self.seq.json_dumps_pretty(), '[\n    "foo1", \n    "foo2"\n]')
+        filler = ' ' if six.PY2 else ''
+        self.assertEqual(self.seq.json_dumps_pretty(),
+                         '[\n    "foo1",{0}\n    "foo2"\n]'.format(filler))

    def test_json_dump_default(self):
        from acme.jose.interfaces import JSONDeSerializable
--- a/docs/contributing.rst
+++ b/docs/contributing.rst
@@ -267,6 +267,22 @@ Please:
 .. _PEP 8 - Style Guide for Python Code:
  https://www.python.org/dev/peps/pep-0008

+Submitting a pull request
+=========================
+
+Steps:
+
+1. Write your code!
+2. Make sure your environment is set up properly and that you're in your
+   virtualenv. You can do this by running ``./bootstrap/dev/venv.sh``.
+   (this is a **very important** step)
+3. Run ``./pep8.travis.sh`` to do a cursory check of your code style.
+   Fix any errors.
+4. Run ``tox -e lint`` to check for pylint errors. Fix any errors.
+5. Run ``tox`` to run the entire test suite including coverage. Fix any errors.
+6. If your code touches communication with an ACME server/Boulder, you
+   should run the integration tests, see `integration`_.
+7. Submit the PR.

 Updating the documentation
 ==========================
--- a/letsencrypt-apache/letsencrypt_apache/configurator.py
+++ b/letsencrypt-apache/letsencrypt_apache/configurator.py
@@ -1,5 +1,6 @@
 """Apache Configuration based off of Augeas Configurator."""
 # pylint: disable=too-many-lines
+import filecmp
 import itertools
 import logging
 import os
@@ -163,7 +164,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):

        temp_install(self.mod_ssl_conf)

-    def deploy_cert(self, domain, cert_path, key_path, chain_path=None):
+    def deploy_cert(self, domain, cert_path, key_path,
+            chain_path=None, fullchain_path=None): # pylint: disable=unused-argument
        """Deploys certificate to specified virtual host.

        Currently tries to find the last directives to deploy the cert in
@@ -945,9 +947,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
        """
        enabled_dir = os.path.join(self.parser.root, "sites-enabled")
        for entry in os.listdir(enabled_dir):
-            if os.path.realpath(os.path.join(enabled_dir, entry)) == avail_fp:
-                return True
-
+            try:
+                if filecmp.cmp(avail_fp, os.path.join(enabled_dir, entry)):
+                    return True
+            except OSError:
+                pass
        return False

    def enable_site(self, vhost):
--- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh
+++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh
@@ -1,33 +1,18 @@
 #!/bin/bash
 # An extremely simplified version of `a2enmod` for enabling modules in the
-# httpd docker image. First argument is the server_root and the second is the
-# module to be enabled.
+# httpd docker image. First argument is the Apache ServerRoot which should be
+# an absolute path. The second is the module to be enabled, such as `ssl`.

-APACHE_CONFDIR=$1
+confdir=$1
+module=$2

-enable () {
-    echo "LoadModule "$1"_module /usr/local/apache2/modules/mod_"$1".so" >> \
-        $APACHE_CONFDIR"/test.conf"
-    available_base="/mods-available/"$1".conf"
-    available_conf=$APACHE_CONFDIR$available_base
-    enabled_dir=$APACHE_CONFDIR"/mods-enabled"
-    enabled_conf=$enabled_dir"/"$1".conf"
-    if [ -e "$available_conf" -a -d "$enabled_dir" -a ! -e "$enabled_conf" ]
-    then
-        ln -s "..$available_base" $enabled_conf
-    fi
-}
-
-if [ $2 == "ssl" ]
+echo "LoadModule ${module}_module " \
+    "/usr/local/apache2/modules/mod_${module}.so" >> "${confdir}/test.conf"
+availbase="/mods-available/${module}.conf"
+availconf=$confdir$availbase
+enabldir="$confdir/mods-enabled"
+enablconf="$enabldir/${module}.conf"
+if [ -e $availconf -a -d $enabldir -a ! -e $enablconf ]
 then
-    # Enables ssl and all its dependencies
-    enable "setenvif"
-    enable "mime"
-    enable "socache_shmcb"
-    enable "ssl"
-elif [ $2 == "rewrite" ]
-then
-    enable "rewrite"
-else
-    exit 1
+    ln -s "..$availbase" $enablconf
 fi
--- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py
+++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py
@@ -175,12 +175,13 @@ class Proxy(configurators_common.Proxy):
        else:
            return {"example.com"}

-    def deploy_cert(self, domain, cert_path, key_path, chain_path=None):
+    def deploy_cert(self, domain, cert_path, key_path, chain_path=None,
+            fullchain_path=None):
        """Installs cert"""
        cert_path, key_path, chain_path = self.copy_certs_and_keys(
            cert_path, key_path, chain_path)
        self._apache_configurator.deploy_cert(
-            domain, cert_path, key_path, chain_path)
+            domain, cert_path, key_path, chain_path, fullchain_path)


 def _is_apache_command(command):
--- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py
+++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py
@@ -39,7 +39,7 @@ def create_le_config(parent_dir):

 def extract_configs(configs, parent_dir):
    """Extracts configs to a new dir under parent_dir and returns it"""
-    config_dir = os.path.join(parent_dir, "renewal")
+    config_dir = os.path.join(parent_dir, "configs")

    if os.path.isdir(configs):
        shutil.copytree(configs, config_dir, symlinks=True)
--- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py
+++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py
@@ -6,6 +6,7 @@ import shutil
 import socket
 import subprocess
 import sys
+import time

 import OpenSSL
 import zope.interface
@@ -117,30 +118,44 @@ class NginxConfigurator(common.Plugin):
        temp_install(self.mod_ssl_conf)

    # Entry point in main.py for installing cert
-    def deploy_cert(self, domain, cert_path, key_path, chain_path=None):
+    def deploy_cert(self, domain, cert_path, key_path,
+            chain_path, fullchain_path):
        # pylint: disable=unused-argument
        """Deploys certificate to specified virtual host.

        .. note:: Aborts if the vhost is missing ssl_certificate or
            ssl_certificate_key.

-        .. note:: Nginx doesn't have a cert chain directive, so the last
-            parameter is always ignored. It expects the cert file to have
-            the concatenated chain.
+        .. note:: Nginx doesn't have a cert chain directive.
+            It expects the cert file to have the concatenated chain.
+            However, we use the chain file as input to the
+            ssl_trusted_certificate directive, used for verify OCSP responses.

        .. note:: This doesn't save the config files!

        """
        vhost = self.choose_vhost(domain)
-        directives = [['ssl_certificate', cert_path],
-                      ['ssl_certificate_key', key_path]]
+        cert_directives = [['ssl_certificate', fullchain_path],
+                           ['ssl_certificate_key', key_path]]
+
+        # OCSP stapling was introduced in Nginx 1.3.7. If we have that version
+        # or greater, add config settings for it.
+        stapling_directives = []
+        if self.version >= (1, 3, 7):
+            stapling_directives = [
+                ['ssl_trusted_certificate', chain_path],
+                ['ssl_stapling', 'on'],
+                ['ssl_stapling_verify', 'on']]

        try:
            self.parser.add_server_directives(vhost.filep, vhost.names,
-                                              directives, True)
+                                              cert_directives, replace=True)
+            self.parser.add_server_directives(vhost.filep, vhost.names,
+                                              stapling_directives, replace=False)
            logger.info("Deployed Certificate to VirtualHost %s for %s",
                        vhost.filep, vhost.names)
-        except errors.MisconfigurationError:
+        except errors.MisconfigurationError as error:
+            logger.debug(error)
            logger.warn(
                "Cannot find a cert or key directive in %s for %s. "
                "VirtualHost was not modified.", vhost.filep, vhost.names)
@@ -598,6 +613,10 @@ def nginx_restart(nginx_ctl, nginx_conf="/etc/nginx.conf"):
    except (OSError, ValueError):
        logger.fatal("Nginx Restart Failed - Please Check the Configuration")
        sys.exit(1)
+    # Nginx can take a moment to recognize a newly added TLS SNI servername, so sleep
+    # for a second. TODO: Check for expected servername and loop until it
+    # appears or return an error if looping too long.
+    time.sleep(1)

    return True

--- a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py
+++ b/letsencrypt-nginx/letsencrypt_nginx/dvsni.py
@@ -90,14 +90,22 @@ class NginxDvsni(common.Dvsni):
        # Add the 'include' statement for the challenges if it doesn't exist
        # already in the main config
        included = False
-        directive = ['include', self.challenge_conf]
+        include_directive = ['include', self.challenge_conf]
        root = self.configurator.parser.loc["root"]
+
+        bucket_directive = ['server_names_hash_bucket_size', '128']
+
        main = self.configurator.parser.parsed[root]
-        for entry in main:
-            if entry[0] == ['http']:
-                body = entry[1]
-                if directive not in body:
-                    body.append(directive)
+        for key, body in main:
+            if key == ['http']:
+                found_bucket = False
+                for key, _ in body:
+                    if key == bucket_directive[0]:
+                        found_bucket = True
+                if not found_bucket:
+                    body.insert(0, bucket_directive)
+                if include_directive not in body:
+                    body.insert(0, include_directive)
                included = True
                break
        if not included:
--- a/letsencrypt-nginx/letsencrypt_nginx/parser.py
+++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py
@@ -476,8 +476,12 @@ def _add_directives(block, directives, replace=False):
    :param list directives: The new directives.

    """
-    if replace:
-        for directive in directives:
+    for directive in directives:
+        if not replace:
+            # We insert new directives at the top of the block, mostly
+            # to work around https://trac.nginx.org/nginx/ticket/810
+            block.insert(0, directive)
+        else:
            changed = False
            if len(directive) == 0:
                continue
@@ -489,5 +493,3 @@ def _add_directives(block, directives, replace=False):
                raise errors.MisconfigurationError(
                    'LetsEncrypt expected directive for %s in the Nginx '
                    'config but did not find it.' % directive[0])
-    else:
-        block.extend(directives)
--- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py
+++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py
@@ -63,11 +63,11 @@ class NginxConfiguratorTest(util.NginxTest):

        # pylint: disable=protected-access
        parsed = self.config.parser._parse_files(filep, override=True)
-        self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'],
+        self.assertEqual([[['server'], [['listen', '5001 ssl'],
+                                        ['listen', '69.50.225.155:9000'],
                                        ['listen', '127.0.0.1'],
                                        ['server_name', '.example.com'],
-                                        ['server_name', 'example.*'],
-                                        ['listen', '5001 ssl']]]],
+                                        ['server_name', 'example.*']]]],
                         parsed[0])

    def test_choose_vhost(self):
@@ -96,18 +96,49 @@ class NginxConfiguratorTest(util.NginxTest):
    def test_more_info(self):
        self.assertTrue('nginx.conf' in self.config.more_info())

+    def test_deploy_cert_stapling(self):
+        # Choose a version of Nginx greater than 1.3.7 so stapling code gets
+        # invoked.
+        self.config.version = (1, 9, 6)
+        example_conf = self.config.parser.abs_path('sites-enabled/example.com')
+        self.config.deploy_cert(
+            "www.example.com",
+            "example/cert.pem",
+            "example/key.pem",
+            "example/chain.pem",
+            "example/fullchain.pem")
+        self.config.save()
+        self.config.parser.load()
+        generated_conf = self.config.parser.parsed[example_conf]
+
+        self.assertTrue(util.contains_at_depth(generated_conf,
+            ['ssl_stapling', 'on'], 2))
+        self.assertTrue(util.contains_at_depth(generated_conf,
+            ['ssl_stapling_verify', 'on'], 2))
+        self.assertTrue(util.contains_at_depth(generated_conf,
+            ['ssl_trusted_certificate', 'example/chain.pem'], 2))
+
    def test_deploy_cert(self):
        server_conf = self.config.parser.abs_path('server.conf')
        nginx_conf = self.config.parser.abs_path('nginx.conf')
        example_conf = self.config.parser.abs_path('sites-enabled/example.com')
+        # Choose a version of Nginx less than 1.3.7 so stapling code doesn't get
+        # invoked.
+        self.config.version = (1, 3, 1)

        # Get the default SSL vhost
        self.config.deploy_cert(
            "www.example.com",
-            "example/cert.pem", "example/key.pem")
+            "example/cert.pem",
+            "example/key.pem",
+            "example/chain.pem",
+            "example/fullchain.pem")
        self.config.deploy_cert(
            "another.alias",
-            "/etc/nginx/cert.pem", "/etc/nginx/key.pem")
+            "/etc/nginx/cert.pem",
+            "/etc/nginx/key.pem",
+            "/etc/nginx/chain.pem",
+            "/etc/nginx/fullchain.pem")
        self.config.save()

        self.config.parser.load()
@@ -119,35 +150,34 @@ class NginxConfiguratorTest(util.NginxTest):
        access_log = os.path.join(self.work_dir, "access.log")
        error_log = os.path.join(self.work_dir, "error.log")
        self.assertEqual([[['server'],
-                           [['listen', '69.50.225.155:9000'],
+                           [['include', self.config.parser.loc["ssl_options"]],
+                            ['ssl_certificate_key', 'example/key.pem'],
+                            ['ssl_certificate', 'example/fullchain.pem'],
+                            ['error_log', error_log],
+                            ['access_log', access_log],
+
+                            ['listen', '5001 ssl'],
+                            ['listen', '69.50.225.155:9000'],
                            ['listen', '127.0.0.1'],
                            ['server_name', '.example.com'],
-                            ['server_name', 'example.*'],
-                            ['listen', '5001 ssl'],
-                            ['access_log', access_log],
-                            ['error_log', error_log],
-                            ['ssl_certificate', 'example/cert.pem'],
-                            ['ssl_certificate_key', 'example/key.pem'],
-                            ['include',
-                             self.config.parser.loc["ssl_options"]]]]],
+                            ['server_name', 'example.*']]]],
                         parsed_example_conf)
        self.assertEqual([['server_name', 'somename  alias  another.alias']],
                         parsed_server_conf)
-        self.assertEqual([['server'],
-                          [['listen', '8000'],
-                           ['listen', 'somename:8080'],
-                           ['include', 'server.conf'],
-                           [['location', '/'],
-                            [['root', 'html'],
-                             ['index', 'index.html index.htm']]],
-                           ['listen', '5001 ssl'],
-                           ['access_log', access_log],
-                           ['error_log', error_log],
-                           ['ssl_certificate', '/etc/nginx/cert.pem'],
-                           ['ssl_certificate_key', '/etc/nginx/key.pem'],
-                           ['include',
-                            self.config.parser.loc["ssl_options"]]]],
-                         parsed_nginx_conf[-1][-1][-1])
+        self.assertTrue(util.contains_at_depth(parsed_nginx_conf,
+                       [['server'],
+                        [['include', self.config.parser.loc["ssl_options"]],
+                         ['ssl_certificate_key', '/etc/nginx/key.pem'],
+                         ['ssl_certificate', '/etc/nginx/fullchain.pem'],
+                         ['error_log', error_log],
+                         ['access_log', access_log],
+                         ['listen', '5001 ssl'],
+                         ['listen', '8000'],
+                         ['listen', 'somename:8080'],
+                         ['include', 'server.conf'],
+                         [['location', '/'],
+                          [['root', 'html'], ['index', 'index.html index.htm']]]]],
+                         2))

    def test_get_all_certs_keys(self):
        nginx_conf = self.config.parser.abs_path('nginx.conf')
@@ -156,16 +186,22 @@ class NginxConfiguratorTest(util.NginxTest):
        # Get the default SSL vhost
        self.config.deploy_cert(
            "www.example.com",
-            "example/cert.pem", "example/key.pem")
+            "example/cert.pem",
+            "example/key.pem",
+            "example/chain.pem",
+            "example/fullchain.pem")
        self.config.deploy_cert(
            "another.alias",
-            "/etc/nginx/cert.pem", "/etc/nginx/key.pem")
+            "/etc/nginx/cert.pem",
+            "/etc/nginx/key.pem",
+            "/etc/nginx/chain.pem",
+            "/etc/nginx/fullchain.pem")
        self.config.save()

        self.config.parser.load()
        self.assertEqual(set([
-            ('example/cert.pem', 'example/key.pem', example_conf),
-            ('/etc/nginx/cert.pem', '/etc/nginx/key.pem', nginx_conf),
+            ('example/fullchain.pem', 'example/key.pem', example_conf),
+            ('/etc/nginx/fullchain.pem', '/etc/nginx/key.pem', nginx_conf),
        ]), self.config.get_all_certs_keys())

    @mock.patch("letsencrypt_nginx.configurator.dvsni.NginxDvsni.perform")
--- a/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py
+++ b/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py
@@ -85,7 +85,8 @@ class DvsniPerformTest(util.NginxTest):
        # Make sure challenge config is included in main config
        http = self.sni.configurator.parser.parsed[
            self.sni.configurator.parser.loc["root"]][-1]
-        self.assertTrue(['include', self.sni.challenge_conf] in http[1])
+        self.assertTrue(
+            util.contains_at_depth(http, ['include', self.sni.challenge_conf], 1))

    def test_perform2(self):
        acme_responses = []
@@ -108,7 +109,8 @@ class DvsniPerformTest(util.NginxTest):
        http = self.sni.configurator.parser.parsed[
            self.sni.configurator.parser.loc["root"]][-1]
        self.assertTrue(['include', self.sni.challenge_conf] in http[1])
-        self.assertTrue(['server_name', 'blah'] in http[1][-2][1])
+        self.assertTrue(
+            util.contains_at_depth(http, ['server_name', 'blah'], 3))

        self.assertEqual(len(sni_responses), 3)
        for i in xrange(3):
--- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py
+++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py
@@ -128,18 +128,20 @@ class NginxParserTest(util.NginxTest):
                                           r'~^(www\.)?(example|bar)\.']),
                                      [['foo', 'bar'], ['ssl_certificate',
                                                        '/etc/ssl/cert.pem']])
-        ssl_re = re.compile(r'foo bar;\n\s+ssl_certificate /etc/ssl/cert.pem')
-        self.assertEqual(1, len(re.findall(ssl_re, nginxparser.dumps(
-            nparser.parsed[nparser.abs_path('nginx.conf')]))))
-        nparser.add_server_directives(nparser.abs_path('server.conf'),
+        ssl_re = re.compile(r'\n\s+ssl_certificate /etc/ssl/cert.pem')
+        dump = nginxparser.dumps(nparser.parsed[nparser.abs_path('nginx.conf')])
+        self.assertEqual(1, len(re.findall(ssl_re, dump)))
+
+        server_conf = nparser.abs_path('server.conf')
+        nparser.add_server_directives(server_conf,
                                      set(['alias', 'another.alias',
                                           'somename']),
                                      [['foo', 'bar'], ['ssl_certificate',
                                                        '/etc/ssl/cert2.pem']])
-        self.assertEqual(nparser.parsed[nparser.abs_path('server.conf')],
-                         [['server_name', 'somename  alias  another.alias'],
+        self.assertEqual(nparser.parsed[server_conf],
+                         [['ssl_certificate', '/etc/ssl/cert2.pem'],
                          ['foo', 'bar'],
-                          ['ssl_certificate', '/etc/ssl/cert2.pem']])
+                          ['server_name', 'somename  alias  another.alias']])

    def test_add_http_directives(self):
        nparser = parser.NginxParser(self.config_path, self.ssl_options)
@@ -148,8 +150,15 @@ class NginxParserTest(util.NginxTest):
                 [['listen', '80'],
                  ['server_name', 'localhost']]]
        nparser.add_http_directives(filep, block)
-        self.assertEqual(nparser.parsed[filep][-1][0], ['http'])
-        self.assertEqual(nparser.parsed[filep][-1][1][-1], block)
+        root = nparser.parsed[filep]
+        self.assertTrue(util.contains_at_depth(root, ['http'], 1))
+        self.assertTrue(util.contains_at_depth(root, block, 2))
+
+        # Check that our server block got inserted first among all server
+        # blocks.
+        http_block = [x for x in root if x[0] == ['http']][0][1]
+        server_blocks = [x for x in http_block if x[0] == ['server']]
+        self.assertEqual(server_blocks[0], block)

    def test_replace_server_directives(self):
        nparser = parser.NginxParser(self.config_path, self.ssl_options)
--- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py
+++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py
@@ -85,3 +85,22 @@ def filter_comments(tree):
                    yield [key, values]

    return list(traverse(tree))
+
+
+def contains_at_depth(haystack, needle, n):
+    """Is the needle in haystack at depth n?
+
+    Return true if the needle is present in one of the sub-iterables in haystack
+    at depth n. Haystack must be an iterable.
+    """
+    # Specifically use hasattr rather than isinstance(..., collections.Iterable)
+    # because we want to include lists but reject strings.
+    if not hasattr(haystack, '__iter__'):
+        return False
+    if n == 0:
+        return needle in haystack
+    else:
+        for item in haystack:
+            if contains_at_depth(item, needle, n - 1):
+                return True
+        return False
--- a/letsencrypt-nginx/tests/boulder-integration.conf.sh
+++ b/letsencrypt-nginx/tests/boulder-integration.conf.sh
@@ -20,7 +20,6 @@ events {
 }

 http {
-  server_names_hash_bucket_size 2048;
  # Set an array of temp and cache file options that will otherwise default to
  # restricted locations accessible only to root.
  client_body_temp_path $root/client_body;
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -397,9 +397,9 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo

    lineage = _auth_from_domains(le_client, config, domains, plugins)

-    # TODO: We also need to pass the fullchain (for Nginx)
    le_client.deploy_certificate(
-        domains, lineage.privkey, lineage.cert, lineage.chain)
+        domains, lineage.privkey, lineage.cert,
+        lineage.chain, lineage.fullchain)
    le_client.enhance_config(domains, args.redirect)

    if len(lineage.available_versions("cert")) == 1:
@@ -452,7 +452,8 @@ def install(args, config, plugins):
        args, config, authenticator=None, installer=installer)
    assert args.cert_path is not None  # required=True in the subparser
    le_client.deploy_certificate(
-        domains, args.key_path, args.cert_path, args.chain_path)
+        domains, args.key_path, args.cert_path, args.chain_path,
+        args.fullchain_path)
    le_client.enhance_config(domains, args.redirect)


@@ -867,6 +868,8 @@ def _paths_parser(helpful):
    default_cp = None
    if verb == "auth":
        default_cp = flag_default("auth_chain_path")
+    add("paths", "--fullchain-path", default=default_cp,
+        help="Accompanying path to a full certificate chain (cert plus chain).")
    add("paths", "--chain-path", default=default_cp,
        help="Accompanying path to a certificate chain.")
    add("paths", "--config-dir", default=flag_default("config_dir"),
@@ -903,9 +906,23 @@ def _plugins_parsing(helpful, plugins):
    helpful.add_plugin_args(plugins)


-def _setup_logging(args):
-    level = -args.verbose_count * 10
-    fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s"
+def setup_log_file_handler(args, logfile, fmt):
+    """Setup file debug logging."""
+    log_file_path = os.path.join(args.logs_dir, logfile)
+    handler = logging.handlers.RotatingFileHandler(
+        log_file_path, maxBytes=2 ** 20, backupCount=10)
+    # rotate on each invocation, rollover only possible when maxBytes
+    # is nonzero and backupCount is nonzero, so we set maxBytes as big
+    # as possible not to overrun in single CLI invocation (1MB).
+    handler.doRollover()  # TODO: creates empty letsencrypt.log.1 file
+    handler.setLevel(logging.DEBUG)
+    handler_formatter = logging.Formatter(fmt=fmt)
+    handler_formatter.converter = time.gmtime  # don't use localtime
+    handler.setFormatter(handler_formatter)
+    return handler, log_file_path
+
+
+def _cli_log_handler(args, level, fmt):
    if args.text_mode:
        handler = colored_logging.StreamHandler()
        handler.setFormatter(logging.Formatter(fmt))
@@ -914,30 +931,26 @@ def _setup_logging(args):
        # dialog box is small, display as less as possible
        handler.setFormatter(logging.Formatter("%(message)s"))
    handler.setLevel(level)
+    return handler
+
+
+def setup_logging(args, cli_handler_factory, logfile):
+    """Setup logging."""
+    fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s"
+    level = -args.verbose_count * 10
+    file_handler, log_file_path = setup_log_file_handler(
+        args, logfile=logfile, fmt=fmt)
+    cli_handler = cli_handler_factory(args, level, fmt)

    # TODO: use fileConfig?

-    # unconditionally log to file for debugging purposes
-    # TODO: change before release?
-    log_file_name = os.path.join(args.logs_dir, 'letsencrypt.log')
-    file_handler = logging.handlers.RotatingFileHandler(
-        log_file_name, maxBytes=2 ** 20, backupCount=10)
-    # rotate on each invocation, rollover only possible when maxBytes
-    # is nonzero and backupCount is nonzero, so we set maxBytes as big
-    # as possible not to overrun in single CLI invocation (1MB).
-    file_handler.doRollover()  # TODO: creates empty letsencrypt.log.1 file
-    file_handler.setLevel(logging.DEBUG)
-    file_handler_formatter = logging.Formatter(fmt=fmt)
-    file_handler_formatter.converter = time.gmtime  # don't use localtime
-    file_handler.setFormatter(file_handler_formatter)
-
    root_logger = logging.getLogger()
    root_logger.setLevel(logging.DEBUG)  # send all records to handlers
-    root_logger.addHandler(handler)
+    root_logger.addHandler(cli_handler)
    root_logger.addHandler(file_handler)

    logger.debug("Root logging level set at %d", level)
-    logger.info("Saving debug log to %s", log_file_name)
+    logger.info("Saving debug log to %s", log_file_path)


 def _handle_exception(exc_type, exc_value, trace, args):
@@ -1006,7 +1019,7 @@ def main(cli_args=sys.argv[1:]):
    # private key! #525
    le_util.make_or_verify_dir(
        args.logs_dir, 0o700, os.geteuid(), "--strict-permissions" in cli_args)
-    _setup_logging(args)
+    setup_logging(args, _cli_log_handler, logfile='letsencrypt.log')

    # do not log `args`, as it contains sensitive data (e.g. revoke --key)!
    logger.debug("Arguments: %r", cli_args)
--- a/letsencrypt/client.py
+++ b/letsencrypt/client.py
@@ -336,7 +336,8 @@ class Client(object):

        return os.path.abspath(act_cert_path), cert_chain_abspath

-    def deploy_certificate(self, domains, privkey_path, cert_path, chain_path):
+    def deploy_certificate(self, domains, privkey_path,
+            cert_path, chain_path, fullchain_path):
        """Install certificate

        :param list domains: list of domains to install the certificate
@@ -357,8 +358,10 @@ class Client(object):
                # TODO: Provide a fullchain reference for installers like
                #       nginx that want it
                self.installer.deploy_cert(
-                    dom, os.path.abspath(cert_path),
-                    os.path.abspath(privkey_path), chain_path)
+                    domain=dom, cert_path=os.path.abspath(cert_path),
+                    key_path=os.path.abspath(privkey_path),
+                    chain_path=chain_path,
+                    fullchain_path=fullchain_path)

            self.installer.save("Deployed Let's Encrypt Certificate")
            # sites may have been enabled / final cleanup
--- a/letsencrypt/constants.py
+++ b/letsencrypt/constants.py
@@ -27,6 +27,7 @@ CLI_DEFAULTS = dict(

    auth_cert_path="./cert.pem",
    auth_chain_path="./chain.pem",
+    strict_permissions=False,
 )
 """Defaults for CLI flags and `.IConfig` attributes."""

--- a/letsencrypt/interfaces.py
+++ b/letsencrypt/interfaces.py
@@ -241,13 +241,15 @@ class IInstaller(IPlugin):

        """

-    def deploy_cert(domain, cert_path, key_path, chain_path=None):
+    def deploy_cert(domain, cert_path, key_path, chain_path, fullchain_path):
        """Deploy certificate.

        :param str domain: domain to deploy certificate file
        :param str cert_path: absolute path to the certificate file
        :param str key_path: absolute path to the private key file
        :param str chain_path: absolute path to the certificate chain file
+        :param str fullchain_path: absolute path to the certificate fullchain
+            file (cert plus chain)

        :raises .PluginError: when cert cannot be deployed

--- a/letsencrypt/plugins/manual.py
+++ b/letsencrypt/plugins/manual.py
@@ -129,12 +129,18 @@ binary for temporary key/certificate generation.""".replace("\n", "")
            ct=response.CONTENT_TYPE, 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
+            if sys.platform == "darwin":
+                executable = "/bin/bash"
+            else:
+                executable = None
            try:
                self._httpd = subprocess.Popen(
                    command,
                    # don't care about setting stdout and stderr,
                    # we're in test mode anyway
                    shell=True,
+                    executable=executable,
                    # "preexec_fn" is UNIX specific, but so is "command"
                    preexec_fn=os.setsid)
            except OSError as error:  # ValueError should not happen!
--- a/letsencrypt/plugins/null.py
+++ b/letsencrypt/plugins/null.py
@@ -30,7 +30,8 @@ class Installer(common.Plugin):
    def get_all_names(self):
        return []

-    def deploy_cert(self, domain, cert_path, key_path, chain_path=None):
+    def deploy_cert(self, domain, cert_path, key_path,
+            chain_path=None, fullchain_path=None):
        pass  # pragma: no cover

    def enhance(self, domain, enhancement, options=None):
--- a/letsencrypt/plugins/standalone/authenticator.py
+++ b/letsencrypt/plugins/standalone/authenticator.py
@@ -1,4 +1,5 @@
 """Standalone authenticator."""
+import logging
 import os
 import psutil
 import signal
@@ -19,6 +20,9 @@ from letsencrypt import interfaces
 from letsencrypt.plugins import common


+logger = logging.getLogger(__name__)
+
+
 class StandaloneAuthenticator(common.Plugin):
    # pylint: disable=too-many-instance-attributes
    """Standalone authenticator.
@@ -301,10 +305,21 @@ class StandaloneAuthenticator(common.Plugin):
        :param int port: The TCP port in question.
        :returns: True or False."""

-        listeners = [conn.pid for conn in psutil.net_connections()
+        try:
+            net_connections = psutil.net_connections()
+        except psutil.AccessDenied as error:
+            logger.info("Access denied when trying to list network "
+                        "connections: %s. Are you root?", error)
+            # this function is just a pre-check that often causes false
+            # positives and problems in testing (c.f. #680 on Mac, #255
+            # generally); we will fail later in bind() anyway
+            return False
+
+        listeners = [conn.pid for conn in net_connections
                     if conn.status == 'LISTEN' and
                     conn.type == socket.SOCK_STREAM and
                     conn.laddr[1] == port]
+
        try:
            if listeners and listeners[0] is not None:
                # conn.pid may be None if the current process doesn't have
--- a/letsencrypt/renewer.py
+++ b/letsencrypt/renewer.py
@@ -8,6 +8,7 @@ within lineages of successor certificates, according to configuration.

 """
 import argparse
+import logging
 import os
 import sys

@@ -17,10 +18,13 @@ import zope.component

 from letsencrypt import account
 from letsencrypt import configuration
+from letsencrypt import constants
+from letsencrypt import colored_logging
 from letsencrypt import cli
 from letsencrypt import client
 from letsencrypt import crypto_util
 from letsencrypt import errors
+from letsencrypt import le_util
 from letsencrypt import notify
 from letsencrypt import storage

@@ -28,6 +32,9 @@ from letsencrypt.display import util as display_util
 from letsencrypt.plugins import disco as plugins_disco


+logger = logging.getLogger(__name__)
+
+
 class _AttrDict(dict):
    """Attribute dictionary.

@@ -104,6 +111,12 @@ def renew(cert, old_version):
    #       (where fewer than all names were renewed)


+def _cli_log_handler(args, level, fmt):  # pylint: disable=unused-argument
+    handler = colored_logging.StreamHandler()
+    handler.setFormatter(logging.Formatter(fmt))
+    return handler
+
+
 def _paths_parser(parser):
    add = parser.add_argument_group("paths").add_argument
    add("--config-dir", default=cli.flag_default("config_dir"),
@@ -119,11 +132,16 @@ def _paths_parser(parser):
 def _create_parser():
    parser = argparse.ArgumentParser()
    #parser.add_argument("--cron", action="store_true", help="Run as cronjob.")
-    # pylint: disable=protected-access
+    parser.add_argument(
+        "-v", "--verbose", dest="verbose_count", action="count",
+        default=cli.flag_default("verbose_count"), help="This flag can be used "
+        "multiple times to incrementally increase the verbosity of output, "
+        "e.g. -vvv.")
+
    return _paths_parser(parser)


-def main(config=None, args=sys.argv[1:]):
+def main(config=None, cli_args=sys.argv[1:]):
    """Main function for autorenewer script."""
    # TODO: Distinguish automated invocation from manual invocation,
    #       perhaps by looking at sys.argv[0] and inhibiting automated
@@ -133,8 +151,13 @@ def main(config=None, args=sys.argv[1:]):

    zope.component.provideUtility(display_util.FileDisplay(sys.stdout))

-    cli_config = configuration.RenewerConfiguration(
-        _create_parser().parse_args(args))
+    args = _create_parser().parse_args(cli_args)
+
+    uid = os.geteuid()
+    le_util.make_or_verify_dir(args.logs_dir, 0o700, uid)
+    cli.setup_logging(args, _cli_log_handler, logfile='renewer.log')
+
+    cli_config = configuration.RenewerConfiguration(args)

    config = storage.config_with_defaults(config)
    # Now attempt to read the renewer config file and augment or replace
@@ -145,6 +168,9 @@ def main(config=None, args=sys.argv[1:]):
    # specify a config file on the command line, which, if provided, should
    # take precedence over this one.
    config.merge(configobj.ConfigObj(cli_config.renewer_config_file))
+    # Ensure that all of the needed folders have been created before continuing
+    le_util.make_or_verify_dir(cli_config.work_dir,
+                               constants.CONFIG_DIRS_MODE, uid)

    for i in os.listdir(cli_config.renewal_configs_dir):
        print "Processing", i
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -75,7 +75,10 @@ class CLITest(unittest.TestCase):
            output.truncate(0)
            self.assertRaises(SystemExit, self._call_stdout, ['-h', 'nginx'])
            out = output.getvalue()
-            self.assertTrue("--nginx-ctl" in out)
+            from letsencrypt.plugins import disco
+            if "nginx" in disco.PluginsRegistry.find_all():
+                # may be false while building distributions without plugins
+                self.assertTrue("--nginx-ctl" in out)
            self.assertTrue("--manual-test-mode" not in out)
            self.assertTrue("--checkpoints" not in out)
            output.truncate(0)
--- a/letsencrypt/tests/client_test.py
+++ b/letsencrypt/tests/client_test.py
@@ -177,15 +177,19 @@ class ClientTest(unittest.TestCase):

    def test_deploy_certificate(self):
        self.assertRaises(errors.Error, self.client.deploy_certificate,
-                          ["foo.bar"], "key", "cert", "chain")
+                          ["foo.bar"], "key", "cert", "chain", "fullchain")

        installer = mock.MagicMock()
        self.client.installer = installer

-        self.client.deploy_certificate(["foo.bar"], "key", "cert", "chain")
+        self.client.deploy_certificate(["foo.bar"], "key", "cert", "chain",
+            "fullchain")
        installer.deploy_cert.assert_called_once_with(
-            "foo.bar", os.path.abspath("cert"),
-            os.path.abspath("key"), os.path.abspath("chain"))
+            cert_path=os.path.abspath("cert"),
+            chain_path=os.path.abspath("chain"),
+            domain='foo.bar',
+            fullchain_path='fullchain',
+            key_path=os.path.abspath("key"))
        self.assertEqual(installer.save.call_count, 1)
        installer.restart.assert_called_once_with()

--- a/letsencrypt/tests/renewer_test.py
+++ b/letsencrypt/tests/renewer_test.py
@@ -44,8 +44,15 @@ class BaseRenewableCertTest(unittest.TestCase):
        self.tempdir = tempfile.mkdtemp()

        self.cli_config = configuration.RenewerConfiguration(
-            namespace=mock.MagicMock(config_dir=self.tempdir))
+            namespace=mock.MagicMock(
+                config_dir=self.tempdir,
+                work_dir=self.tempdir,
+                logs_dir=self.tempdir,
+            )
+        )
+
        # TODO: maybe provide RenewerConfiguration.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"))
        os.makedirs(os.path.join(self.tempdir, "renewal"))
@@ -62,6 +69,9 @@ class BaseRenewableCertTest(unittest.TestCase):
        self.test_rc = storage.RenewableCert(
            self.config, self.defaults, self.cli_config)

+    def tearDown(self):
+        shutil.rmtree(self.tempdir)
+
    def _write_out_ex_kinds(self):
        for kind in ALL_FOUR:
            where = getattr(self.test_rc, kind)
@@ -79,11 +89,6 @@ class BaseRenewableCertTest(unittest.TestCase):
 class RenewableCertTests(BaseRenewableCertTest):
    # pylint: disable=too-many-public-methods
    """Tests for letsencrypt.renewer.*."""
-    def setUp(self):
-        super(RenewableCertTests, self).setUp()
-
-    def tearDown(self):
-        shutil.rmtree(self.tempdir)

    def test_initialization(self):
        self.assertEqual(self.test_rc.lineagename, "example.org")
@@ -665,11 +670,17 @@ class RenewableCertTests(BaseRenewableCertTest):
        # This should fail because the renewal itself appears to fail
        self.assertFalse(renewer.renew(self.test_rc, 1))

+    def _common_cli_args(self):
+        return [
+            "--config-dir", self.cli_config.config_dir,
+            "--work-dir", self.cli_config.work_dir,
+            "--logs-dir", self.cli_config.logs_dir,
+        ]
+
    @mock.patch("letsencrypt.renewer.notify")
    @mock.patch("letsencrypt.storage.RenewableCert")
    @mock.patch("letsencrypt.renewer.renew")
    def test_main(self, mock_renew, mock_rc, mock_notify):
-        """Test for main() function."""
        from letsencrypt import renewer
        mock_rc_instance = mock.MagicMock()
        mock_rc_instance.should_autodeploy.return_value = True
@@ -691,8 +702,7 @@ class RenewableCertTests(BaseRenewableCertTest):
                               "example.com.conf"), "w") as f:
            f.write("cert = cert.pem\nprivkey = privkey.pem\n")
            f.write("chain = chain.pem\nfullchain = fullchain.pem\n")
-        renewer.main(self.defaults, args=[
-            '--config-dir', self.cli_config.config_dir])
+        renewer.main(self.defaults, cli_args=self._common_cli_args())
        self.assertEqual(mock_rc.call_count, 2)
        self.assertEqual(mock_rc_instance.update_all_links_to.call_count, 2)
        self.assertEqual(mock_notify.notify.call_count, 4)
@@ -705,8 +715,7 @@ class RenewableCertTests(BaseRenewableCertTest):
        mock_happy_instance.should_autorenew.return_value = False
        mock_happy_instance.latest_common_version.return_value = 10
        mock_rc.return_value = mock_happy_instance
-        renewer.main(self.defaults, args=[
-            '--config-dir', self.cli_config.config_dir])
+        renewer.main(self.defaults, cli_args=self._common_cli_args())
        self.assertEqual(mock_rc.call_count, 4)
        self.assertEqual(mock_happy_instance.update_all_links_to.call_count, 0)
        self.assertEqual(mock_notify.notify.call_count, 4)
@@ -717,8 +726,7 @@ class RenewableCertTests(BaseRenewableCertTest):
        with open(os.path.join(self.cli_config.renewal_configs_dir,
                               "bad.conf"), "w") as f:
            f.write("incomplete = configfile\n")
-        renewer.main(self.defaults, args=[
-            '--config-dir', self.cli_config.config_dir])
+        renewer.main(self.defaults, cli_args=self._common_cli_args())
        # The errors.CertStorageError is caught inside and nothing happens.


--- a/tests/boulder-integration.sh
+++ b/tests/boulder-integration.sh
@@ -14,6 +14,12 @@ export PATH="/usr/sbin:$PATH"  # /usr/sbin/nginx
 export GOPATH="${GOPATH:-/tmp/go}"
 export PATH="$GOPATH/bin:$PATH"

+if [ `uname`  == 'Darwin' ]; then
+  readlink="greadlink"
+else
+  readlink="readlink"
+fi
+
 common() {
    letsencrypt_test \
        --authenticator standalone \
@@ -49,7 +55,7 @@ dir="$root/conf/archive/le1.wtf"
 for x in cert chain fullchain privkey;
 do
    latest="$(ls -1t $dir/ | grep -e "^${x}" | head -n1)"
-    live="$(readlink -f "$root/conf/live/le1.wtf/${x}.pem")"
+    live="$($readlink -f "$root/conf/live/le1.wtf/${x}.pem")"
    [ "${dir}/${latest}" = "$live" ]  # renewer fails this test
 done

--- a/tools/dev-release.sh
+++ b/tools/dev-release.sh
@@ -88,8 +88,7 @@ mkdir ../kgs
 kgs="../kgs/$version"
 pip freeze | tee $kgs
 pip install nose
-# TODO: letsencrypt_apache fails due to symlink, c.f. #838
-nosetests letsencrypt $SUBPKGS || true
+nosetests letsencrypt $SUBPKGS

 echo "New root: $root"
 echo "KGS is at $root/kgs"