Merge branch 'master' into add_dns01_challenge

.travis.yml

@@ -3,6 +3,8 @@ language: python
 services:
   - rabbitmq
   - mariadb
+  # apacheconftest
+  #- apache2
 
 # http://docs.travis-ci.com/user/ci-environment/#CI-environment-OS
 # gimme has to be kept in sync with Boulder's Go version setting in .travis.yml
@@ -20,8 +22,17 @@ env:
   matrix:
     - TOXENV=py26 BOULDER_INTEGRATION=1
     - TOXENV=py27 BOULDER_INTEGRATION=1
+    - TOXENV=py33
+    - TOXENV=py34
     - TOXENV=lint
     - TOXENV=cover
+# Disabled for now due to requiring sudo -> causing more boulder integration
+# DNS timeouts :(
+#    - TOXENV=apacheconftest
+matrix:
+  include:
+    - env: TOXENV=py35
+      python: 3.5
 
 
 # Only build pushes to the master branch, PRs, and branches beginning with
@@ -58,6 +69,12 @@ addons:
     - openssl
     # For Boulder integration testing
     - rsyslog
+    # for apacheconftest
+    #- realpath
+    #- apache2
+    #- libapache2-mod-wsgi
+    #- libapache2-mod-macro
+    #- sudo
 
 install: "travis_retry pip install tox coveralls"
 script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/travis-integration.sh)'

acme/acme/__init__.py

@@ -3,10 +3,10 @@
 This module is an implementation of the `ACME protocol`_. Latest
 supported version: `draft-ietf-acme-01`_.
 
-.. _`ACME protocol`: https://github.com/ietf-wg-acme/acme/
+
+.. _`ACME protocol`: https://ietf-wg-acme.github.io/acme
 
 .. _`draft-ietf-acme-01`:
   https://github.com/ietf-wg-acme/acme/tree/draft-ietf-acme-acme-01
 
-
 """

acme/acme/jose/json_util.py

@@ -226,7 +226,7 @@ class JSONObjectWithFields(util.ImmutableMap, interfaces.JSONDeSerializable):
 
         :param str name: Name of the field to be encoded.
 
-        :raises erors.SerializationError: if field cannot be serialized
+        :raises errors.SerializationError: if field cannot be serialized
         :raises errors.Error: if field could not be found
 
         """

acme/acme/jose/util.py

@@ -130,7 +130,7 @@ class ImmutableMap(collections.Mapping, collections.Hashable):
     """Immutable key to value mapping with attribute access."""
 
     __slots__ = ()
-    """Must be overriden in subclasses."""
+    """Must be overridden in subclasses."""
 
     def __init__(self, **kwargs):
         if set(kwargs) != set(self.__slots__):

acme/acme/messages.py

@@ -25,6 +25,8 @@ class Error(jose.JSONObjectWithFields, errors.Error):
             ('connection', 'The server could not connect to the client to '
                 'verify the domain'),
             ('dnssec', 'The server could not validate a DNSSEC signed domain'),
+            ('invalidEmail',
+             'The provided email for a registration was invalid'),
             ('malformed', 'The request message was malformed'),
             ('rateLimited', 'There were too many requests of a given type'),
             ('serverInternal', 'The server experienced an internal error'),

acme/setup.py

@@ -31,7 +31,7 @@ else:
     install_requires.append('mock')
 
 if sys.version_info < (2, 7, 9):
-    # For secure SSL connexion with Python 2.7 (InsecurePlatformWarning)
+    # For secure SSL connection with Python 2.7 (InsecurePlatformWarning)
     install_requires.append('ndg-httpsclient')
     install_requires.append('pyasn1')
 
@@ -70,6 +70,7 @@ setup(
         'Programming Language :: Python :: 3',
         'Programming Language :: Python :: 3.3',
         'Programming Language :: Python :: 3.4',
+        'Programming Language :: Python :: 3.5',
         'Topic :: Internet :: WWW/HTTP',
         'Topic :: Security',
     ],

docs/contributing.rst

@@ -65,8 +65,14 @@ Testing
 
 The following tools are there to help you:
 
-- ``tox`` starts a full set of tests. Please make sure you run it
-  before submitting a new pull request.
+- ``tox`` starts a full set of tests. Please note that it includes
+  apacheconftest, which uses the system's Apache install to test config file
+  parsing, so it should only be run on systems that have an
+  experimental, non-production Apache2 install on them.  ``tox -e
+  apacheconftest`` can be used to run those specific Apache conf tests.
+
+- ``tox -e py27``, ``tox -e py26`` etc, run unit tests for specific Python
+  versions.
 
 - ``tox -e cover`` checks the test coverage only. Calling the
   ``./tox.cover.sh`` script directly (or even ``./tox.cover.sh $pkg1
@@ -365,10 +371,12 @@ are provided here mainly for the :ref:`developers <hacking>` reference.
 In general:
 
 * ``sudo`` is required as a suggested way of running privileged process
+* `Python`_ 2.6/2.7 is required
 * `Augeas`_ is required for the Python bindings
 * ``virtualenv`` and ``pip`` are used for managing other python library
   dependencies
 
+.. _Python: https://wiki.python.org/moin/BeginnersGuide/Download
 .. _Augeas: http://augeas.net/
 .. _Virtualenv: https://virtualenv.pypa.io
 

docs/using.rst

@@ -139,9 +139,20 @@ Would obtain a single certificate for all of those names, using the
 ``/var/www/example`` webroot directory for the first two, and
 ``/var/www/eg`` for the second two.
 
+The webroot plugin works by creating a temporary file for each of your requested
+domains in ``${webroot-path}/.well-known/acme-challenge``. Then the Let's
+Encrypt validation server makes HTTP requests to validate that the DNS for each
+requested domain resolves to the server running letsencrypt. An example request
+made to your web server would look like:
+
+::
+
+    66.133.109.36 - - [05/Jan/2016:20:11:24 -0500] "GET /.well-known/acme-challenge/HGr8U1IeTW4kY_Z6UIyaakzOkyQgPr_7ArlLgtZE8SX HTTP/1.1" 200 87 "-" "Mozilla/5.0 (compatible; Let's Encrypt validation server; +https://www.letsencrypt.org)"
+
 Note that to use the webroot plugin, your server must be configured to serve
 files from hidden directories.
 
+
 Manual
 ------
 
@@ -237,7 +248,9 @@ The following files are available:
   server certificate, i.e. root and intermediate certificates only.
 
   This is what Apache < 2.4.8 needs for `SSLCertificateChainFile
-  <https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcertificatechainfile>`_.
+  <https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcertificatechainfile>`_,
+  and what nginx >= 1.3.7 needs for `ssl_trusted_certificate
+  <http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_trusted_certificate>`_.
 
 ``fullchain.pem``
   All certificates, **including** server certificate. This is

examples/cli.ini

@@ -5,9 +5,6 @@
 # Use a 4096 bit RSA key instead of 2048
 rsa-key-size = 4096
 
-# Always use the staging/testing server
-server = https://acme-staging.api.letsencrypt.org/directory
-
 # Uncomment and update to register with the specified e-mail address
 # email = foo@example.com
 

examples/dev-cli.ini

@@ -1,3 +1,6 @@
+# Always use the staging/testing server - avoids rate limiting
+server = https://acme-staging.api.letsencrypt.org/directory
+
 # This is an example configuration file for developers
 config-dir = /tmp/le/conf
 work-dir = /tmp/le/conf

letsencrypt-apache/letsencrypt_apache/configurator.py

@@ -86,10 +86,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
     @classmethod
     def add_parser_arguments(cls, add):
-        add("ctl", default=constants.os_constant("ctl"),
-            help="Path to the 'apache2ctl' binary, used for 'configtest', "
-                 "retrieving the Apache2 version number, and initialization "
-                 "parameters.")
         add("enmod", default=constants.os_constant("enmod"),
             help="Path to the Apache 'a2enmod' binary.")
         add("dismod", default=constants.os_constant("dismod"),
@@ -148,10 +144,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         """
         # Verify Apache is installed
-        for exe in (self.conf("ctl"), self.conf("enmod"), self.conf("dismod")):
-            if exe is not None:
-                if not le_util.exe_exists(exe):
-                    raise errors.NoInstallationError
+        if not le_util.exe_exists(constants.os_constant("restart_cmd")[0]):
+            raise errors.NoInstallationError
 
         # Make sure configuration is valid
         self.config_test()
@@ -165,7 +159,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         self.parser = parser.ApacheParser(
             self.aug, self.conf("server-root"), self.conf("vhost-root"),
-            self.conf("ctl"), self.version)
+            self.version)
         # Check for errors in parsing files with Augeas
         self.check_parsing_errors("httpd.aug")
 
@@ -564,6 +558,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # In case no Listens are set (which really is a broken apache config)
         if not listens:
             listens = ["80"]
+        if port in listens:
+            return
         for listen in listens:
             # For any listen statement, check if the machine also listens on Port 443.
             # If not, add such a listen statement.
@@ -1277,7 +1273,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Modules can enable additional config files. Variables may be defined
         # within these new configuration sections.
         # Reload is not necessary as DUMP_RUN_CFG uses latest config.
-        self.parser.update_runtime_variables(self.conf("ctl"))
+        self.parser.update_runtime_variables()
 
     def _add_parser_mod(self, mod_name):
         """Shortcut for updating parser modules."""
@@ -1306,6 +1302,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         """
         self.config_test()
+        logger.debug(self.reverter.view_config_changes(for_logging=True))
         self._reload()
 
     def _reload(self):
@@ -1315,7 +1312,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         """
         try:
-            le_util.run_script([self.conf("ctl"), "graceful"])
+            le_util.run_script(constants.os_constant("restart_cmd"))
         except errors.SubprocessError as err:
             raise errors.MisconfigurationError(str(err))
 
@@ -1326,7 +1323,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         """
         try:
-            le_util.run_script([self.conf("ctl"), "configtest"])
+            le_util.run_script(constants.os_constant("conftest_cmd"))
         except errors.SubprocessError as err:
             raise errors.MisconfigurationError(str(err))
 
@@ -1346,7 +1343,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 constants.os_constant("version_cmd"))
         except errors.SubprocessError:
             raise errors.PluginError(
-                "Unable to run %s -v" % self.conf("ctl"))
+                "Unable to run %s -v" %
+                constants.os_constant("version_cmd"))
 
         regex = re.compile(r"Apache/([0-9\.]*)", re.IGNORECASE)
         matches = regex.findall(stdout)

letsencrypt-apache/letsencrypt_apache/constants.py

@@ -6,9 +6,11 @@ from letsencrypt import le_util
 CLI_DEFAULTS_DEBIAN = dict(
     server_root="/etc/apache2",
     vhost_root="/etc/apache2/sites-available",
-    ctl="apache2ctl",
+    vhost_files="*",
     version_cmd=['apache2ctl', '-v'],
     define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'],
+    restart_cmd=['apache2ctl', 'graceful'],
+    conftest_cmd=['apache2ctl', 'configtest'],
     enmod="a2enmod",
     dismod="a2dismod",
     le_vhost_ext="-le-ssl.conf",
@@ -19,9 +21,11 @@ CLI_DEFAULTS_DEBIAN = dict(
 CLI_DEFAULTS_CENTOS = dict(
     server_root="/etc/httpd",
     vhost_root="/etc/httpd/conf.d",
-    ctl="apachectl",
+    vhost_files="*.conf",
     version_cmd=['apachectl', '-v'],
     define_cmd=['apachectl', '-t', '-D', 'DUMP_RUN_CFG'],
+    restart_cmd=['apachectl', 'graceful'],
+    conftest_cmd=['apachectl', 'configtest'],
     enmod=None,
     dismod=None,
     le_vhost_ext="-le-ssl.conf",
@@ -32,9 +36,11 @@ CLI_DEFAULTS_CENTOS = dict(
 CLI_DEFAULTS_GENTOO = dict(
     server_root="/etc/apache2",
     vhost_root="/etc/apache2/vhosts.d",
-    ctl="apache2ctl",
+    vhost_files="*.conf",
     version_cmd=['/usr/sbin/apache2', '-v'],
     define_cmd=['/usr/sbin/apache2', '-t', '-D', 'DUMP_RUN_CFG'],
+    restart_cmd=['apache2ctl', 'graceful'],
+    conftest_cmd=['apache2ctl', 'configtest'],
     enmod=None,
     dismod=None,
     le_vhost_ext="-le-ssl.conf",

letsencrypt-apache/letsencrypt_apache/parser.py

@@ -28,7 +28,7 @@ class ApacheParser(object):
     arg_var_interpreter = re.compile(r"\$\{[^ \}]*}")
     fnmatch_chars = set(["*", "?", "\\", "[", "]"])
 
-    def __init__(self, aug, root, vhostroot, ctl, version=(2, 4)):
+    def __init__(self, aug, root, vhostroot, version=(2, 4)):
         # Note: Order is important here.
 
         # This uses the binary, so it can be done first.
@@ -37,7 +37,7 @@ class ApacheParser(object):
         # This only handles invocation parameters and Define directives!
         self.variables = {}
         if version >= (2, 4):
-            self.update_runtime_variables(ctl)
+            self.update_runtime_variables()
 
         self.aug = aug
         # Find configuration root and make sure augeas can parse it.
@@ -60,9 +60,10 @@ class ApacheParser(object):
         self.loc.update(self._set_locations())
 
         # Must also attempt to parse virtual host root
-        self._parse_file(self.vhostroot + "/*.conf")
+        self._parse_file(self.vhostroot + "/" +
+                         constants.os_constant("vhost_files"))
 
-        #check to see if there were unparsed define statements
+        # check to see if there were unparsed define statements
         if version < (2, 4):
             if self.find_dir("Define", exclude=False):
                 raise errors.PluginError("Error parsing runtime variables")
@@ -91,7 +92,7 @@ class ApacheParser(object):
                 self.modules.add(
                     os.path.basename(self.get_arg(match_filename))[:-2] + "c")
 
-    def update_runtime_variables(self, ctl):
+    def update_runtime_variables(self):
         """"
 
         .. note:: Compile time variables (apache2ctl -V) are not used within the
@@ -101,7 +102,7 @@ class ApacheParser(object):
         .. todo:: Create separate compile time variables... simply for arg_get()
 
         """
-        stdout = self._get_runtime_cfg(ctl)
+        stdout = self._get_runtime_cfg()
 
         variables = dict()
         matches = re.compile(r"Define: ([^ \n]*)").findall(stdout)
@@ -121,7 +122,7 @@ class ApacheParser(object):
 
         self.variables = variables
 
-    def _get_runtime_cfg(self, ctl):  # pylint: disable=no-self-use
+    def _get_runtime_cfg(self):  # pylint: disable=no-self-use
         """Get runtime configuration info.
 
         :returns: stdout from DUMP_RUN_CFG
@@ -136,9 +137,11 @@ class ApacheParser(object):
 
         except (OSError, ValueError):
             logger.error(
-                "Error accessing %s for runtime parameters!%s", ctl, os.linesep)
+                "Error running command %s for runtime parameters!%s",
+                constants.os_constant("define_cmd"), os.linesep)
             raise errors.MisconfigurationError(
-                "Error accessing loaded Apache parameters: %s", ctl)
+                "Error accessing loaded Apache parameters: %s",
+                constants.os_constant("define_cmd"))
         # Small errors that do not impede
         if proc.returncode != 0:
             logger.warn("Error in checking parameter list: %s", stderr)

letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test

@@ -0,0 +1,78 @@
+#!/bin/bash
+
+# A hackish script to see if the client is behaving as expected 
+# with each of the "passing" conf files.
+
+export EA=/etc/apache2/ 
+TESTDIR="`dirname $0`"
+LEROOT="`realpath \"$TESTDIR/../../../../\"`"
+cd $TESTDIR/passing
+LETSENCRYPT="${LETSENCRYPT:-$LEROOT/venv/bin/letsencrypt}"
+
+function CleanupExit() {
+    echo control c, exiting tests...
+    if [ "$f" != "" ] ; then
+        Cleanup
+    fi
+    exit 1
+}
+
+function Setup() {
+    if [ "$APPEND_APACHECONF" = "" ] ; then
+        sudo cp "$f" "$EA"/sites-available/
+        sudo ln -sf "$EA/sites-available/$f" "$EA/sites-enabled/$f"
+        sudo echo """
+<VirtualHost *:80>
+    ServerName example.com
+    DocumentRoot /tmp/
+    ErrorLog /tmp/error.log
+    CustomLog /tmp/requests.log combined
+</VirtualHost>""" >> $EA/sites-available/throwaway-example.conf
+    else
+        TMP="/tmp/`basename \"$APPEND_APACHECONF\"`.$$"
+        sudo cp -a "$APPEND_APACHECONF" "$TMP"
+        sudo bash -c "cat \"$f\" >> \"$APPEND_APACHECONF\""
+    fi
+}
+
+function Cleanup() {
+    if [ "$APPEND_APACHECONF" = "" ] ; then
+        sudo rm /etc/apache2/sites-{enabled,available}/"$f"
+        sudo rm $EA/sites-available/throwaway-example.conf
+    else
+        sudo mv "$TMP" "$APPEND_APACHECONF"
+    fi
+}
+
+# if our environment asks us to enable modules, do our best!
+if [ "$1" = --debian-modules ] ; then
+    sudo apt-get install -y libapache2-mod-wsgi
+    sudo apt-get install -y libapache2-mod-macro
+
+    for mod in  ssl rewrite macro wsgi deflate userdir version mime ; do
+        sudo a2enmod $mod
+    done
+fi
+
+
+FAILS=0
+trap CleanupExit INT
+for f in *.conf ; do 
+    echo -n  testing "$f"...
+    Setup
+    RESULT=`echo c | sudo "$LETSENCRYPT" -vvvv --debug --staging --apache --register-unsafely-without-email --agree-tos certonly -t 2>&1`
+    if echo $RESULT | grep -Eq \("Which names would you like"\|"mod_macro is not yet"\) ; then
+        echo passed
+    else
+        echo failed
+        echo $RESULT
+        echo
+        echo
+        FAILS=`expr $FAILS + 1`
+    fi
+    Cleanup
+done
+if [ "$FAILS" -ne 0 ] ; then
+    exit 1
+fi
+exit 0

letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/README.modules

@@ -0,0 +1,6 @@
+# Modules required to parse these conf files:
+ssl
+rewrite
+macro
+wsgi
+deflate

letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/passing/graphite-quote-1934.conf

@@ -1,6 +1,6 @@
 <VirtualHost *:80>
 
-        WSGIDaemonProcess _graphite processes=5 threads=5 display-name='%{GROUP}' inactivity-timeout=120 user=_graphite group=_graphite
+        WSGIDaemonProcess _graphite processes=5 threads=5 display-name='%{GROUP}' inactivity-timeout=120 user=www-data group=www-data
         WSGIProcessGroup _graphite
         WSGIImportScript /usr/share/graphite-web/graphite.wsgi process-group=_graphite application-group=%{GLOBAL}
         WSGIScriptAlias / /usr/share/graphite-web/graphite.wsgi

letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py

@@ -461,6 +461,25 @@ class TwoVhost80Test(util.ApacheTest):
         self.assertEqual(mock_add_dir.call_args_list[1][0][2], ["[::1]:8080", "https"])
         self.assertEqual(mock_add_dir.call_args_list[2][0][2], ["1.1.1.1:8080", "https"])
 
+    def test_prepare_server_https_mixed_listen(self):
+
+        mock_find = mock.Mock()
+        mock_find.return_value = ["test1", "test2"]
+        mock_get = mock.Mock()
+        mock_get.side_effect = ["1.2.3.4:8080", "443"]
+        mock_add_dir = mock.Mock()
+        mock_enable = mock.Mock()
+
+        self.config.parser.find_dir = mock_find
+        self.config.parser.get_arg = mock_get
+        self.config.parser.add_dir_to_ifmodssl = mock_add_dir
+        self.config.enable_mod = mock_enable
+
+        # Test Listen statements with specific ip listeed
+        self.config.prepare_server_https("443")
+        # Should only be 2 here, as the third interface already listens to the correct port
+        self.assertEqual(mock_add_dir.call_count, 0)
+
     def test_make_vhost_ssl(self):
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])
 

letsencrypt-apache/letsencrypt_apache/tests/constants_test.py

@@ -11,14 +11,17 @@ class ConstantsTest(unittest.TestCase):
     @mock.patch("letsencrypt.le_util.get_os_info")
     def test_get_debian_value(self, os_info):
         os_info.return_value = ('Debian', '', '')
-        self.assertEqual(constants.os_constant("ctl"), "apache2ctl")
+        self.assertEqual(constants.os_constant("vhost_root"),
+                         "/etc/apache2/sites-available")
 
     @mock.patch("letsencrypt.le_util.get_os_info")
     def test_get_centos_value(self, os_info):
         os_info.return_value = ('CentOS Linux', '', '')
-        self.assertEqual(constants.os_constant("ctl"), "apachectl")
+        self.assertEqual(constants.os_constant("vhost_root"),
+                         "/etc/httpd/conf.d")
 
     @mock.patch("letsencrypt.le_util.get_os_info")
     def test_get_default_value(self, os_info):
         os_info.return_value = ('Nonexistent Linux', '', '')
-        self.assertEqual(constants.os_constant("ctl"), "apache2ctl")
+        self.assertEqual(constants.os_constant("vhost_root"),
+                         "/etc/apache2/sites-available")

letsencrypt-apache/letsencrypt_apache/tests/parser_test.py

@@ -145,24 +145,26 @@ class BasicParserTest(util.ParserTest):
         expected_vars = {"TEST": "", "U_MICH": "", "TLS": "443",
                          "example_path": "Documents/path"}
 
-        self.parser.update_runtime_variables("ctl")
+        self.parser.update_runtime_variables()
         self.assertEqual(self.parser.variables, expected_vars)
 
     @mock.patch("letsencrypt_apache.parser.ApacheParser._get_runtime_cfg")
     def test_update_runtime_vars_bad_output(self, mock_cfg):
         mock_cfg.return_value = "Define: TLS=443=24"
-        self.parser.update_runtime_variables("ctl")
+        self.parser.update_runtime_variables()
 
         mock_cfg.return_value = "Define: DUMP_RUN_CFG\nDefine: TLS=443=24"
         self.assertRaises(
-            errors.PluginError, self.parser.update_runtime_variables, "ctl")
+            errors.PluginError, self.parser.update_runtime_variables)
 
+    @mock.patch("letsencrypt_apache.constants.os_constant")
     @mock.patch("letsencrypt_apache.parser.subprocess.Popen")
-    def test_update_runtime_vars_bad_ctl(self, mock_popen):
+    def test_update_runtime_vars_bad_ctl(self, mock_popen, mock_const):
         mock_popen.side_effect = OSError
+        mock_const.return_value = "nonexistent"
         self.assertRaises(
             errors.MisconfigurationError,
-            self.parser.update_runtime_variables, "ctl")
+            self.parser.update_runtime_variables)
 
     @mock.patch("letsencrypt_apache.parser.subprocess.Popen")
     def test_update_runtime_vars_bad_exit(self, mock_popen):
@@ -170,7 +172,7 @@ class BasicParserTest(util.ParserTest):
         mock_popen.returncode = -1
         self.assertRaises(
             errors.MisconfigurationError,
-            self.parser.update_runtime_variables, "ctl")
+            self.parser.update_runtime_variables)
 
 
 class ParserInitTest(util.ApacheTest):
@@ -191,7 +193,7 @@ class ParserInitTest(util.ApacheTest):
         self.assertRaises(
             errors.PluginError,
             ApacheParser, self.aug, os.path.relpath(self.config_path),
-            "/dummy/vhostpath", "ctl", version=(2, 2, 22))
+            "/dummy/vhostpath", version=(2, 2, 22))
 
     def test_root_normalized(self):
         from letsencrypt_apache.parser import ApacheParser
@@ -203,7 +205,7 @@ class ParserInitTest(util.ApacheTest):
                 "debian_apache_2_4/////two_vhost_80/../two_vhost_80/apache2")
 
             parser = ApacheParser(self.aug, path,
-                                  "/dummy/vhostpath", "dummy_ctl")
+                                  "/dummy/vhostpath")
 
         self.assertEqual(parser.root, self.config_path)
 
@@ -213,7 +215,7 @@ class ParserInitTest(util.ApacheTest):
                         "update_runtime_variables"):
             parser = ApacheParser(
                 self.aug, os.path.relpath(self.config_path),
-                "/dummy/vhostpath", "dummy_ctl")
+                "/dummy/vhostpath")
 
         self.assertEqual(parser.root, self.config_path)
 
@@ -223,7 +225,7 @@ class ParserInitTest(util.ApacheTest):
                         "update_runtime_variables"):
             parser = ApacheParser(
                 self.aug, self.config_path + os.path.sep,
-                "/dummy/vhostpath", "dummy_ctl")
+                "/dummy/vhostpath")
         self.assertEqual(parser.root, self.config_path)
 
 

letsencrypt-apache/letsencrypt_apache/tests/util.py

@@ -58,7 +58,7 @@ class ParserTest(ApacheTest):  # pytlint: disable=too-few-public-methods
         with mock.patch("letsencrypt_apache.parser.ApacheParser."
                         "update_runtime_variables"):
             self.parser = ApacheParser(
-                self.aug, self.config_path, self.vhost_path, "dummy_ctl_path")
+                self.aug, self.config_path, self.vhost_path)
 
 
 def get_apache_configurator(

letsencrypt-apache/letsencrypt_apache/tls_sni_01.py

@@ -1,12 +1,14 @@
 """A class that performs TLS-SNI-01 challenges for Apache"""
 
 import os
+import logging
 
 from letsencrypt.plugins import common
 
 from letsencrypt_apache import obj
 from letsencrypt_apache import parser
 
+logger = logging.getLogger(__name__)
 
 class ApacheTlsSni01(common.TLSSNI01):
     """Class that performs TLS-SNI-01 challenges within the Apache configurator
@@ -104,6 +106,7 @@ class ApacheTlsSni01(common.TLSSNI01):
         self.configurator.reverter.register_file_creation(
             True, self.challenge_conf)
 
+        logger.debug("writing a config file with text: %s", config_text)
         with open(self.challenge_conf, "w") as new_conf:
             new_conf.write(config_text)
 

letsencrypt-nginx/letsencrypt_nginx/configurator.py

@@ -122,7 +122,7 @@ class NginxConfigurator(common.Plugin):
 
     # Entry point in main.py for installing cert
     def deploy_cert(self, domain, cert_path, key_path,
-                    chain_path, fullchain_path):
+                    chain_path=None, fullchain_path=None):
         # pylint: disable=unused-argument
         """Deploys certificate to specified virtual host.
 
@@ -136,7 +136,15 @@ class NginxConfigurator(common.Plugin):
 
         .. note:: This doesn't save the config files!
 
+        :raises errors.PluginError: When unable to deploy certificate due to
+            a lack of directives or configuration
+
         """
+        if not fullchain_path:
+            raise errors.PluginError(
+                "The nginx plugin currently requires --fullchain-path to "
+                "install a cert.")
+
         vhost = self.choose_vhost(domain)
         cert_directives = [['ssl_certificate', fullchain_path],
                            ['ssl_certificate_key', key_path]]
@@ -150,6 +158,12 @@ class NginxConfigurator(common.Plugin):
                 ['ssl_stapling', 'on'],
                 ['ssl_stapling_verify', 'on']]
 
+        if len(stapling_directives) != 0 and not chain_path:
+            raise errors.PluginError(
+                "--chain-path is required to enable "
+                "Online Certificate Status Protocol (OCSP) stapling "
+                "on nginx >= 1.3.7.")
+
         try:
             self.parser.add_server_directives(vhost.filep, vhost.names,
                                               cert_directives, replace=True)
@@ -168,7 +182,7 @@ class NginxConfigurator(common.Plugin):
         self.save_notes += ("Changed vhost at %s with addresses of %s\n" %
                             (vhost.filep,
                              ", ".join(str(addr) for addr in vhost.addrs)))
-        self.save_notes += "\tssl_certificate %s\n" % cert_path
+        self.save_notes += "\tssl_certificate %s\n" % fullchain_path
         self.save_notes += "\tssl_certificate_key %s\n" % key_path
 
     #######################
@@ -311,17 +325,11 @@ class NginxConfigurator(common.Plugin):
         """
         snakeoil_cert, snakeoil_key = self._get_snakeoil_paths()
         ssl_block = [['listen', '{0} ssl'.format(self.config.tls_sni_01_port)],
-                     # access and error logs necessary for integration
-                     # testing (non-root)
-                     ['access_log', os.path.join(
-                         self.config.work_dir, 'access.log')],
-                     ['error_log', os.path.join(
-                         self.config.work_dir, 'error.log')],
                      ['ssl_certificate', snakeoil_cert],
                      ['ssl_certificate_key', snakeoil_key],
                      ['include', self.parser.loc["ssl_options"]]]
         self.parser.add_server_directives(
-            vhost.filep, vhost.names, ssl_block)
+            vhost.filep, vhost.names, ssl_block, replace=False)
         vhost.ssl = True
         vhost.raw.extend(ssl_block)
         vhost.addrs.add(obj.Addr(
@@ -384,7 +392,7 @@ class NginxConfigurator(common.Plugin):
             [['return', '301 https://$host$request_uri']]
         ]]
         self.parser.add_server_directives(
-            vhost.filep, vhost.names, redirect_block)
+            vhost.filep, vhost.names, redirect_block, replace=False)
         logger.info("Redirecting all traffic to ssl in %s", vhost.filep)
 
     ######################################
@@ -393,11 +401,10 @@ class NginxConfigurator(common.Plugin):
     def restart(self):
         """Restarts nginx server.
 
-        :returns: Success
-        :rtype: bool
+        :raises .errors.MisconfigurationError: If either the reload fails.
 
         """
-        return nginx_restart(self.conf('ctl'), self.nginx_conf)
+        nginx_restart(self.conf('ctl'), self.nginx_conf)
 
     def config_test(self):  # pylint: disable=no-self-use
         """Check the configuration of Nginx for errors.
@@ -631,19 +638,16 @@ def nginx_restart(nginx_ctl, nginx_conf="/etc/nginx.conf"):
 
             if nginx_proc.returncode != 0:
                 # Enter recovery routine...
-                logger.error("Nginx Restart Failed!\n%s\n%s", stdout, stderr)
-                return False
+                raise errors.MisconfigurationError(
+                    "nginx restart failed:\n%s\n%s" % (stdout, stderr))
 
     except (OSError, ValueError):
-        logger.fatal("Nginx Restart Failed - Please Check the Configuration")
-        sys.exit(1)
+        raise errors.MisconfigurationError("nginx restart failed")
     # 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
-
 
 def temp_install(options_ssl):
     """Temporary install for convenience."""

letsencrypt-nginx/letsencrypt_nginx/parser.py

@@ -113,7 +113,7 @@ class NginxParser(object):
         for filename in servers:
             for server in servers[filename]:
                 # Parse the server block into a VirtualHost object
-                parsed_server = _parse_server(server)
+                parsed_server = parse_server(server)
                 vhost = obj.VirtualHost(filename,
                                         parsed_server['addrs'],
                                         parsed_server['ssl'],
@@ -213,6 +213,7 @@ class NginxParser(object):
             if ext:
                 filename = filename + os.path.extsep + ext
             try:
+                logger.debug('Dumping to %s:\n%s', filename, nginxparser.dumps(tree))
                 with open(filename, 'w') as _file:
                     nginxparser.dump(tree, _file)
             except IOError:
@@ -252,7 +253,7 @@ class NginxParser(object):
         return server_names == names
 
     def add_server_directives(self, filename, names, directives,
-                              replace=False):
+                              replace):
         """Add or replace directives in the first server block with names.
 
         ..note :: If replace is True, this raises a misconfiguration error
@@ -269,20 +270,27 @@ class NginxParser(object):
         :param bool replace: Whether to only replace existing directives
 
         """
-        _do_for_subarray(self.parsed[filename],
-                         lambda x: self._has_server_names(x, names),
-                         lambda x: _add_directives(x, directives, replace))
+        try:
+            _do_for_subarray(self.parsed[filename],
+                             lambda x: self._has_server_names(x, names),
+                             lambda x: _add_directives(x, directives, replace))
+        except errors.MisconfigurationError as err:
+            raise errors.MisconfigurationError("Problem in %s: %s" % (filename, err.message))
 
     def add_http_directives(self, filename, directives):
         """Adds directives to the first encountered HTTP block in filename.
 
+        We insert new directives at the top of the block to work around
+        https://trac.nginx.org/nginx/ticket/810: If the first server block
+        doesn't enable OCSP stapling, stapling is broken for all blocks.
+
         :param str filename: The absolute filename of the config file
         :param list directives: The directives to add
 
         """
         _do_for_subarray(self.parsed[filename],
                          lambda x: x[0] == ['http'],
-                         lambda x: _add_directives(x[1], [directives], False))
+                         lambda x: x[1].insert(0, directives))
 
     def get_all_certs_keys(self):
         """Gets all certs and keys in the nginx config.
@@ -443,7 +451,7 @@ def _get_servernames(names):
     return names.split(' ')
 
 
-def _parse_server(server):
+def parse_server(server):
     """Parses a list of server directives.
 
     :param list server: list of directives in a server block
@@ -463,13 +471,20 @@ def _parse_server(server):
         elif directive[0] == 'server_name':
             parsed_server['names'].update(
                 _get_servernames(directive[1]))
+        elif directive[0] == 'ssl' and directive[1] == 'on':
+            parsed_server['ssl'] = True
 
     return parsed_server
 
 
-def _add_directives(block, directives, replace=False):
-    """Adds or replaces directives in a block. If the directive doesn't exist in
-    the entry already, raises a misconfiguration error.
+def _add_directives(block, directives, replace):
+    """Adds or replaces directives in a config block.
+
+    When replace=False, it's an error to try and add a directive that already
+    exists in the config block with a conflicting value.
+
+    When replace=True, a directive with the same name MUST already exist in the
+    config block, and the first instance will be replaced.
 
     ..todo :: Find directives that are in included files.
 
@@ -478,21 +493,43 @@ def _add_directives(block, directives, replace=False):
 
     """
     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
-            # Only add directive if its not already in the block
-            if directive not in block:
-                block.insert(0, directive)
-        else:
-            changed = False
-            if len(directive) == 0:
-                continue
-            for index, line in enumerate(block):
-                if len(line) > 0 and line[0] == directive[0]:
-                    block[index] = directive
-                    changed = True
-            if not changed:
+        _add_directive(block, directive, replace)
+
+repeatable_directives = set(['server_name', 'listen', 'include'])
+
+def _add_directive(block, directive, replace):
+    """Adds or replaces a single directive in a config block.
+
+    See _add_directives for more documentation.
+
+    """
+    location = -1
+    # Find the index of a config line where the name of the directive matches
+    # the name of the directive we want to add.
+    for index, line in enumerate(block):
+        if len(line) > 0 and line[0] == directive[0]:
+            location = index
+            break
+    if replace:
+        if location == -1:
+            raise errors.MisconfigurationError(
+                'expected directive for %s in the Nginx '
+                'config but did not find it.' % directive[0])
+        block[location] = directive
+    else:
+        # Append directive. Fail if the name is not a repeatable directive name,
+        # and there is already a copy of that directive with a different value
+        # in the config file.
+        directive_name = directive[0]
+        directive_value = directive[1]
+        if location != -1 and directive_name.__str__() not in repeatable_directives:
+            if block[location][1] == directive_value:
+                # There's a conflict, but the existing value matches the one we
+                # want to insert, so it's fine.
+                pass
+            else:
                 raise errors.MisconfigurationError(
-                    'Let\'s Encrypt expected directive for %s in the Nginx '
-                    'config but did not find it.' % directive[0])
+                    'tried to insert directive "%s" but found conflicting "%s".' % (
+                    directive, block[location]))
+        else:
+            block.append(directive)

letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py

@@ -40,6 +40,23 @@ class NginxConfiguratorTest(util.NginxTest):
         self.assertEquals((1, 6, 2), self.config.version)
         self.assertEquals(5, len(self.config.parser.parsed))
 
+    @mock.patch("letsencrypt_nginx.configurator.le_util.exe_exists")
+    @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen")
+    def test_prepare_initializes_version(self, mock_popen, mock_exe_exists):
+        mock_popen().communicate.return_value = (
+            "", "\n".join(["nginx version: nginx/1.6.2",
+                           "built by clang 6.0 (clang-600.0.56)"
+                           " (based on LLVM 3.5svn)",
+                           "TLS SNI support enabled",
+                           "configure arguments: --prefix=/usr/local/Cellar/"
+                           "nginx/1.6.2 --with-http_ssl_module"]))
+
+        mock_exe_exists.return_value = True
+
+        self.config.version = None
+        self.config.prepare()
+        self.assertEquals((1, 6, 2), self.config.version)
+
     @mock.patch("letsencrypt_nginx.configurator.socket.gethostbyaddr")
     def test_get_all_names(self, mock_gethostbyaddr):
         mock_gethostbyaddr.return_value = ('155.225.50.69.nephoscale.net', [], [])
@@ -65,16 +82,19 @@ class NginxConfiguratorTest(util.NginxTest):
         filep = self.config.parser.abs_path('sites-enabled/example.com')
         self.config.parser.add_server_directives(
             filep, set(['.example.com', 'example.*']),
-            [['listen', '5001 ssl']])
+            [['listen', '5001 ssl']],
+            replace=False)
         self.config.save()
 
         # pylint: disable=protected-access
         parsed = self.config.parser._parse_files(filep, override=True)
-        self.assertEqual([[['server'], [['listen', '5001 ssl'],
+        self.assertEqual([[['server'], [
                                         ['listen', '69.50.225.155:9000'],
                                         ['listen', '127.0.0.1'],
                                         ['server_name', '.example.com'],
-                                        ['server_name', 'example.*']]]],
+                                        ['server_name', 'example.*'],
+                                        ['listen', '5001 ssl']
+                                        ]]],
                          parsed[0])
 
     def test_choose_vhost(self):
@@ -91,12 +111,26 @@ class NginxConfiguratorTest(util.NginxTest):
                    'test.www.example.com': foo_conf,
                    'abc.www.foo.com': foo_conf,
                    'www.bar.co.uk': localhost_conf}
+
+        conf_path = {'localhost': "etc_nginx/nginx.conf",
+                   'alias': "etc_nginx/nginx.conf",
+                   'example.com': "etc_nginx/sites-enabled/example.com",
+                   'example.com.uk.test': "etc_nginx/sites-enabled/example.com",
+                   'www.example.com': "etc_nginx/sites-enabled/example.com",
+                   'test.www.example.com': "etc_nginx/foo.conf",
+                   'abc.www.foo.com': "etc_nginx/foo.conf",
+                   'www.bar.co.uk': "etc_nginx/nginx.conf"}
+
         bad_results = ['www.foo.com', 'example', 't.www.bar.co',
                        '69.255.225.155']
 
         for name in results:
-            self.assertEqual(results[name],
-                             self.config.choose_vhost(name).names)
+            vhost = self.config.choose_vhost(name)
+            path = os.path.relpath(vhost.filep, self.temp_dir)
+
+            self.assertEqual(results[name], vhost.names)
+            self.assertEqual(conf_path[name], path)
+
         for name in bad_results:
             self.assertEqual(set([name]), self.config.choose_vhost(name).names)
 
@@ -125,6 +159,24 @@ class NginxConfiguratorTest(util.NginxTest):
         self.assertTrue(util.contains_at_depth(generated_conf,
                                                ['ssl_trusted_certificate', 'example/chain.pem'], 2))
 
+    def test_deploy_cert_stapling_requires_chain_path(self):
+        self.config.version = (1, 3, 7)
+        self.assertRaises(errors.PluginError, self.config.deploy_cert,
+            "www.example.com",
+            "example/cert.pem",
+            "example/key.pem",
+            None,
+            "example/fullchain.pem")
+
+    def test_deploy_cert_requires_fullchain_path(self):
+        self.config.version = (1, 3, 1)
+        self.assertRaises(errors.PluginError, self.config.deploy_cert,
+            "www.example.com",
+            "example/cert.pem",
+            "example/key.pem",
+            "example/chain.pem",
+            None)
+
     def test_deploy_cert(self):
         server_conf = self.config.parser.abs_path('server.conf')
         nginx_conf = self.config.parser.abs_path('nginx.conf')
@@ -154,38 +206,36 @@ class NginxConfiguratorTest(util.NginxTest):
         parsed_server_conf = util.filter_comments(self.config.parser.parsed[server_conf])
         parsed_nginx_conf = util.filter_comments(self.config.parser.parsed[nginx_conf])
 
-        access_log = os.path.join(self.work_dir, "access.log")
-        error_log = os.path.join(self.work_dir, "error.log")
         self.assertEqual([[['server'],
-                           [['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.*']]]],
+                            ['server_name', 'example.*'],
+
+                            ['listen', '5001 ssl'],
+                            ['ssl_certificate', 'example/fullchain.pem'],
+                            ['ssl_certificate_key', 'example/key.pem'],
+                            ['include', self.config.parser.loc["ssl_options"]]
+                            ]]],
                          parsed_example_conf)
         self.assertEqual([['server_name', 'somename  alias  another.alias']],
                          parsed_server_conf)
-        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))
+        self.assertTrue(util.contains_at_depth(
+            parsed_nginx_conf,
+            [['server'],
+             [
+              ['listen', '8000'],
+              ['listen', 'somename:8080'],
+              ['include', 'server.conf'],
+              [['location', '/'],
+               [['root', 'html'],
+                ['index', 'index.html index.htm']]],
+              ['listen', '5001 ssl'],
+              ['ssl_certificate', '/etc/nginx/fullchain.pem'],
+              ['ssl_certificate_key', '/etc/nginx/key.pem'],
+              ['include', self.config.parser.loc["ssl_options"]]]],
+            2))
 
     def test_get_all_certs_keys(self):
         nginx_conf = self.config.parser.abs_path('nginx.conf')
@@ -297,19 +347,19 @@ class NginxConfiguratorTest(util.NginxTest):
         mocked = mock_popen()
         mocked.communicate.return_value = ('', '')
         mocked.returncode = 0
-        self.assertTrue(self.config.restart())
+        self.config.restart()
 
     @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen")
     def test_nginx_restart_fail(self, mock_popen):
         mocked = mock_popen()
         mocked.communicate.return_value = ('', '')
         mocked.returncode = 1
-        self.assertFalse(self.config.restart())
+        self.assertRaises(errors.MisconfigurationError, self.config.restart)
 
     @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen")
     def test_no_nginx_start(self, mock_popen):
         mock_popen.side_effect = OSError("Can't find program")
-        self.assertRaises(SystemExit, self.config.restart)
+        self.assertRaises(errors.MisconfigurationError, self.config.restart)
 
     @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen")
     def test_config_test(self, mock_popen):
@@ -330,6 +380,17 @@ class NginxConfiguratorTest(util.NginxTest):
             OpenSSL.crypto.load_privatekey(
                 OpenSSL.crypto.FILETYPE_PEM, key_file.read())
 
+    def test_redirect_enhance(self):
+        expected = [
+            ['if', '($scheme != "https")'],
+            [['return', '301 https://$host$request_uri']]
+        ]
+
+        example_conf = self.config.parser.abs_path('sites-enabled/example.com')
+        self.config.enhance("www.example.com", "redirect")
+
+        generated_conf = self.config.parser.parsed[example_conf]
+        self.assertTrue(util.contains_at_depth(generated_conf, expected, 2))
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py

@@ -127,7 +127,8 @@ class NginxParserTest(util.NginxTest):
                                       set(['localhost',
                                            r'~^(www\.)?(example|bar)\.']),
                                       [['foo', 'bar'], ['ssl_certificate',
-                                                        '/etc/ssl/cert.pem']])
+                                                        '/etc/ssl/cert.pem']],
+                                      replace=False)
         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)))
@@ -136,12 +137,15 @@ class NginxParserTest(util.NginxTest):
         names = set(['alias', 'another.alias', 'somename'])
         nparser.add_server_directives(server_conf, names,
                                       [['foo', 'bar'], ['ssl_certificate',
-                                                        '/etc/ssl/cert2.pem']])
-        nparser.add_server_directives(server_conf, names, [['foo', 'bar']])
+                                                        '/etc/ssl/cert2.pem']],
+                                      replace=False)
+        nparser.add_server_directives(server_conf, names, [['foo', 'bar']],
+                                      replace=False)
         self.assertEqual(nparser.parsed[server_conf],
-                         [['ssl_certificate', '/etc/ssl/cert2.pem'],
+                         [['server_name', 'somename  alias  another.alias'],
                           ['foo', 'bar'],
-                          ['server_name', 'somename  alias  another.alias']])
+                          ['ssl_certificate', '/etc/ssl/cert2.pem']
+                          ])
 
     def test_add_http_directives(self):
         nparser = parser.NginxParser(self.config_path, self.ssl_options)
@@ -165,17 +169,19 @@ class NginxParserTest(util.NginxTest):
         target = set(['.example.com', 'example.*'])
         filep = nparser.abs_path('sites-enabled/example.com')
         nparser.add_server_directives(
-            filep, target, [['server_name', 'foo bar']], True)
+            filep, target, [['server_name', 'foobar.com']], replace=True)
         self.assertEqual(
             nparser.parsed[filep],
             [[['server'], [['listen', '69.50.225.155:9000'],
                            ['listen', '127.0.0.1'],
-                           ['server_name', 'foo bar'],
-                           ['server_name', 'foo bar']]]])
+                           ['server_name', 'foobar.com'],
+                           ['server_name', 'example.*'],
+                           ]]])
         self.assertRaises(errors.MisconfigurationError,
                           nparser.add_server_directives,
-                          filep, set(['foo', 'bar']),
-                          [['ssl_certificate', 'cert.pem']], True)
+                          filep, set(['foobar.com', 'example.*']),
+                          [['ssl_certificate', 'cert.pem']],
+                          replace=True)
 
     def test_get_best_match(self):
         target_name = 'www.eff.org'
@@ -217,10 +223,31 @@ class NginxParserTest(util.NginxTest):
                                       set(['.example.com', 'example.*']),
                                       [['ssl_certificate', 'foo.pem'],
                                        ['ssl_certificate_key', 'bar.key'],
-                                       ['listen', '443 ssl']])
+                                       ['listen', '443 ssl']],
+                                      replace=False)
         c_k = nparser.get_all_certs_keys()
         self.assertEqual(set([('foo.pem', 'bar.key', filep)]), c_k)
 
+    def test_parse_server_ssl(self):
+        server = parser.parse_server([
+	    ['listen', '443']
+	])
+        self.assertFalse(server['ssl'])
+
+        server = parser.parse_server([
+	    ['listen', '443 ssl']
+	])
+        self.assertTrue(server['ssl'])
+
+        server = parser.parse_server([
+	    ['listen', '443'], ['ssl', 'off']
+	])
+        self.assertFalse(server['ssl'])
+
+        server = parser.parse_server([
+	    ['listen', '443'], ['ssl', 'on']
+	])
+        self.assertTrue(server['ssl'])
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

letsencrypt-nginx/tests/boulder-integration.conf.sh

@@ -20,13 +20,14 @@ events {
 }
 
 http {
-  # Set an array of temp and cache file options that will otherwise default to
+  # Set an array of temp, cache and log file options that will otherwise default to
   # restricted locations accessible only to root.
   client_body_temp_path $root/client_body;
   fastcgi_temp_path $root/fastcgi_temp;
   proxy_temp_path $root/proxy_temp;
   #scgi_temp_path $root/scgi_temp;
   #uwsgi_temp_path $root/uwsgi_temp;
+  access_log $root/error.log;
 
   # This should be turned off in a Virtualbox VM, as it can cause some
   # interesting issues with data corruption in delivered files.
@@ -54,9 +55,6 @@ http {
 
     root $root/webroot;
 
-    access_log $root/access.log;
-    error_log $root/error.log;
-
     location / {
       # First attempt to serve request as file, then as directory, then fall
       # back to index.html.

letsencrypt/auth_handler.py

@@ -540,16 +540,11 @@ def _generate_failed_chall_msg(failed_achalls):
 
     """
     typ = failed_achalls[0].error.typ
-    msg = [
-        "The following '{0}' errors were reported by the server:".format(typ)]
+    msg = ["The following errors were reported by the server:"]
 
-    problems = dict()
     for achall in failed_achalls:
-        problems.setdefault(achall.error.description, set()).add(achall.domain)
-    for problem in problems:
-        msg.append("\n\nDomains: ")
-        msg.append(", ".join(sorted(problems[problem])))
-        msg.append("\nError: {0}".format(problem))
+        msg.append("\n\nDomain: %s\nType:   %s\nDetail: %s" % (
+            achall.domain, achall.error.typ, achall.error.detail))
 
     if typ in _ERROR_HELP:
         msg.append("\n\n")

letsencrypt/reverter.py

@@ -94,7 +94,7 @@ class Reverter(object):
                     "Unable to load checkpoint during rollback")
             rollback -= 1
 
-    def view_config_changes(self):
+    def view_config_changes(self, for_logging=False):
         """Displays all saved checkpoints.
 
         All checkpoints are printed by
@@ -144,6 +144,8 @@ class Reverter(object):
 
             output.append(os.linesep)
 
+        if for_logging:
+            return os.linesep.join(output)
         zope.component.getUtility(interfaces.IDisplay).notification(
             os.linesep.join(output), display_util.HEIGHT)
 

letsencrypt/tests/auth_handler_test.py

@@ -467,7 +467,7 @@ class ReportFailedChallsTest(unittest.TestCase):
         auth_handler._report_failed_challs([self.http01, self.tls_sni_same])
         call_list = mock_zope().add_message.call_args_list
         self.assertTrue(len(call_list) == 1)
-        self.assertTrue("Domains: example.com\n" in call_list[0][0][0])
+        self.assertTrue("Domain: example.com\nType:   tls\nDetail: detail" in call_list[0][0][0])
 
     @mock.patch("letsencrypt.auth_handler.zope.component.getUtility")
     def test_different_errors_and_domains(self, mock_zope):

tests/apache-conf-files/hackish-apache-test

@@ -1,28 +0,0 @@
-#!/bin/bash
-
-# A hackish script to see if the client is behaving as expected 
-# with each of the "passing" conf files.
-
-# TODO presently this requires interaction and human judgement to 
-# assess, but it should be automated
-export EA=/etc/apache2/ 
-TESTDIR="`dirname $0`"
-LEROOT="`realpath \"$TESTDIR/../../\"`"
-cd $TESTDIR/passing
-
-function CleanupExit() {
-    echo control c, exiting tests...
-    if [ "$f" != "" ] ; then
-        sudo rm /etc/apache2/sites-{enabled,available}/"$f"
-    fi
-    exit 1
-}
-
-trap CleanupExit INT
-for f in *.conf ; do 
-    echo testing "$f"
-    sudo cp "$f" "$EA"/sites-available/
-    sudo ln -s "$EA/sites-available/$f" "$EA/sites-enabled/$f"
-    sudo "$LEROOT"/venv/bin/letsencrypt --apache  certonly -t
-    sudo rm /etc/apache2/sites-{enabled,available}/"$f"
-done

tests/apache-conf-files/passing/README.modules

@@ -1,7 +0,0 @@
-Modules required to parse these conf files:
-
-ssl
-rewrite
-macro
-wsgi
-deflate

tests/letstest/multitester.py

@@ -77,6 +77,15 @@ parser.add_argument('--saveinstances',
 parser.add_argument('--alt_pip',
                     default='',
                     help="server from which to pull candidate release packages")
+parser.add_argument('--killboulder',
+                    action='store_true',
+                    help="do not leave a persistent boulder server running")
+parser.add_argument('--boulderonly',
+                    action='store_true',
+                    help="only make a boulder server")
+parser.add_argument('--fast',
+                    action='store_true',
+                    help="use larger instance types to run faster (saves about a minute, probably not worth it)")
 cl_args = parser.parse_args()
 
 # Credential Variables
@@ -292,6 +301,30 @@ def grab_letsencrypt_log():
     sudo('if [ -f ./letsencrypt.log ]; then \
     cat ./letsencrypt.log; else echo "[nolocallog]"; fi')
 
+def create_client_instances(targetlist):
+    "Create a fleet of client instances"
+    instances = []
+    print("Creating instances: ", end="")
+    for target in targetlist:
+        if target['virt'] == 'hvm':
+            machine_type = 't2.medium' if cl_args.fast else 't2.micro'
+        else:
+            # 32 bit systems
+            machine_type = 'c1.medium' if cl_args.fast else 't1.micro'
+        if 'userdata' in target.keys():
+            userdata = target['userdata']
+        else:
+            userdata = ''
+        name = 'le-%s'%target['name']
+        print(name, end=" ")
+        instances.append(make_instance(name,
+                                       target['ami'],
+                                       KEYNAME,
+                                       machine_type=machine_type,
+                                       userdata=userdata))
+    print()
+    return instances
+
 #-------------------------------------------------------------------------------
 # SCRIPT BEGINS
 #-------------------------------------------------------------------------------
@@ -352,30 +385,28 @@ if not sg_exists:
     make_security_group()
     time.sleep(30)
 
+boulder_preexists = False
+boulder_servers = EC2.instances.filter(Filters=[
+    {'Name': 'tag:Name',            'Values': ['le-boulderserver']},
+    {'Name': 'instance-state-name', 'Values': ['running']}])
+
+boulder_server = next(iter(boulder_servers), None)
+
 print("Requesting Instances...")
-boulder_server = make_instance('le-boulderserver',
-                               BOULDER_AMI,
-                               KEYNAME,
-                               #machine_type='t2.micro',
-                               machine_type='t2.medium',
-                               security_groups=['letsencrypt_test'])
-
-instances = []
-for target in targetlist:
-    if target['virt'] == 'hvm':
-        machine_type = 't2.micro'
-    else:
-        machine_type = 't1.micro'
-    if 'userdata' in target.keys():
-        userdata = target['userdata']
-    else:
-        userdata = ''
-    instances.append(make_instance('le-%s'%target['name'],
-                                   target['ami'],
+if boulder_server:
+    print("Found existing boulder server:", boulder_server)
+    boulder_preexists = True
+else:
+    print("Can't find a boulder server, starting one...")
+    boulder_server = make_instance('le-boulderserver',
+                                   BOULDER_AMI,
                                    KEYNAME,
-                                   machine_type=machine_type,
-                                   userdata=userdata))
+                                   machine_type='t2.micro',
+                                   #machine_type='t2.medium',
+                                   security_groups=['letsencrypt_test'])
 
+if not cl_args.boulderonly:
+    instances = create_client_instances(targetlist)
 
 # Configure and launch boulder server
 #-------------------------------------------------------------------------------
@@ -383,21 +414,24 @@ print("Waiting on Boulder Server")
 boulder_server = block_until_instance_ready(boulder_server)
 print(" server %s"%boulder_server)
 
-print("Configuring and Launching Boulder")
 
 # env.host_string defines the ssh user and host for connection
 env.host_string = "ubuntu@%s"%boulder_server.public_ip_address
 print("Boulder Server at (SSH):", env.host_string)
-config_and_launch_boulder(boulder_server)
-# blocking often unnecessary, but cheap EC2 VMs can get very slow
-block_until_http_ready('http://%s:4000'%boulder_server.public_ip_address,
-                       wait_time=10,
-                       timeout=500)
+if not boulder_preexists:
+    print("Configuring and Launching Boulder")
+    config_and_launch_boulder(boulder_server)
+    # blocking often unnecessary, but cheap EC2 VMs can get very slow
+    block_until_http_ready('http://%s:4000'%boulder_server.public_ip_address,
+                           wait_time=10, timeout=500)
 
 boulder_url = "http://%s:4000/directory"%boulder_server.private_ip_address
 print("Boulder Server at (public ip): http://%s:4000/directory"%boulder_server.public_ip_address)
 print("Boulder Server at (EC2 private ip): %s"%boulder_url)
 
+if cl_args.boulderonly:
+    sys.exit(0)
+
 # Install and launch client scripts in parallel
 #-------------------------------------------------------------------------------
 print("Uploading and running test script in parallel: %s"%cl_args.test_script)
@@ -480,7 +514,8 @@ results_file.close()
 if not cl_args.saveinstances:
     print('Logs in ', LOGDIR)
     print('Terminating EC2 Instances and Cleaning Dangling EBS Volumes')
-    boulder_server.terminate()
+    if cl_args.killboulder:
+        boulder_server.terminate()
     terminate_and_clean(instances)
 else:
     # print login information for the boxes for debugging

tests/letstest/scripts/test_leauto_upgrades.sh

@@ -7,7 +7,9 @@ cd letsencrypt
 #git checkout v0.1.0     use --branch instead
 SAVE="$PIP_EXTRA_INDEX_URL"
 unset PIP_EXTRA_INDEX_URL
+export PIP_INDEX_URL="https://isnot.org/pip/0.1.0/"
 ./letsencrypt-auto -v --debug --version 
+unset PIP_INDEX_URL
 
 export PIP_EXTRA_INDEX_URL="$SAVE"
 

tests/letstest/scripts/test_letsencrypt_auto_venv_only.sh

@@ -4,4 +4,4 @@
 
 cd letsencrypt
 # help installs virtualenv and does nothing else
-./letsencrypt-auto -v --help all
+./letsencrypt-auto -v --debug --help all

tox.ini

@@ -67,3 +67,11 @@ commands =
     pylint --rcfile=.pylintrc letsencrypt-nginx/letsencrypt_nginx
     pylint --rcfile=.pylintrc letsencrypt-compatibility-test/letsencrypt_compatibility_test
     pylint --rcfile=.pylintrc letshelp-letsencrypt/letshelp_letsencrypt
+
+[testenv:apacheconftest]
+#basepython = python2.7
+setenv =
+    LETSENCRYPT=/home/travis/build/letsencrypt/letsencrypt/.tox/apacheconftest/bin/letsencrypt
+commands =
+    pip install -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx -e letsencrypt-compatibility-test -e letshelp-letsencrypt
+    sudo ./letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test --debian-modules