Merge pull request #567 from kuba/nginx-integration

Nginx bug fixes and integration tests

.travis.yml

@@ -1,7 +1,9 @@
 language: python
 
 # http://docs.travis-ci.com/user/ci-environment/#CI-environment-OS
-before_install: travis_retry sudo ./bootstrap/ubuntu.sh
+before_install:
+  - travis_retry sudo ./bootstrap/ubuntu.sh
+  - travis_retry sudo apt-get install --no-install-recommends nginx-light openssl
 
 # using separate envs with different TOXENVs creates 4x1 Travis build
 # matrix, which allows us to clearly distinguish which component under

letsencrypt/plugins/disco.py

@@ -75,7 +75,7 @@ class PluginEntryPoint(object):
                 if iface.implementedBy(self.plugin_cls):
                     logger.debug(
                         "%s implements %s but object does not verify: %s",
-                        self.plugin_cls, iface.__name__, error)
+                        self.plugin_cls, iface.__name__, error, exc_info=True)
                 return False
         return True
 
@@ -93,10 +93,14 @@ class PluginEntryPoint(object):
             try:
                 self._initialized.prepare()
             except errors.MisconfigurationError as error:
-                logger.debug("Misconfigured %r: %s", self, error)
+                logger.debug("Misconfigured %r: %s", self, error, exc_info=True)
                 self._prepared = error
             except errors.NoInstallationError as error:
-                logger.debug("No installation (%r): %s", self, error)
+                logger.debug(
+                    "No installation (%r): %s", self, error, exc_info=True)
+                self._prepared = error
+            except errors.PluginError as error:
+                logger.debug("Other error:(%r): %s", self, error, exc_info=True)
                 self._prepared = error
             else:
                 self._prepared = True

letsencrypt/plugins/disco_test.py

@@ -144,6 +144,16 @@ class PluginEntryPointTest(unittest.TestCase):
         self.assertFalse(self.plugin_ep.misconfigured)
         self.assertFalse(self.plugin_ep.available)
 
+    def test_prepare_generic_plugin_error(self):
+        plugin = mock.MagicMock()
+        plugin.prepare.side_effect = errors.PluginError
+        # pylint: disable=protected-access
+        self.plugin_ep._initialized = plugin
+        self.assertTrue(isinstance(self.plugin_ep.prepare(), errors.PluginError))
+        self.assertTrue(self.plugin_ep.prepared)
+        self.assertFalse(self.plugin_ep.misconfigured)
+        self.assertFalse(self.plugin_ep.available)
+
     def test_repr(self):
         self.assertEqual("PluginEntryPoint#sa", repr(self.plugin_ep))
 

letsencrypt_nginx/configurator.py

@@ -13,6 +13,7 @@ from acme import challenges
 
 from letsencrypt import achallenges
 from letsencrypt import constants as core_constants
+from letsencrypt import crypto_util
 from letsencrypt import errors
 from letsencrypt import interfaces
 from letsencrypt import le_util
@@ -63,6 +64,11 @@ class NginxConfigurator(common.Plugin):
             "'nginx' binary, used for 'configtest' and retrieving nginx "
             "version number.")
 
+    @property
+    def nginx_conf(self):
+        """Nginx config file path."""
+        return os.path.join(self.conf("server_root"), "nginx.conf")
+
     def __init__(self, *args, **kwargs):
         """Initialize an Nginx Configurator.
 
@@ -74,8 +80,7 @@ class NginxConfigurator(common.Plugin):
         super(NginxConfigurator, self).__init__(*args, **kwargs)
 
         # Verify that all directories and files exist with proper permissions
-        if os.geteuid() == 0:
-            self._verify_setup()
+        self._verify_setup()
 
         # Files to save
         self.save_notes = ""
@@ -263,9 +268,23 @@ class NginxConfigurator(common.Plugin):
 
         return all_names
 
+    def _get_snakeoil_paths(self):
+        # TODO: generate only once
+        tmp_dir = os.path.join(self.config.work_dir, "snakeoil")
+        key = crypto_util.init_save_key(
+            key_size=1024, key_dir=tmp_dir, keyname="key.pem")
+        cert_pem = crypto_util.make_ss_cert(
+            key.pem, domains=[socket.gethostname()])
+        cert = os.path.join(tmp_dir, "cert.pem")
+        with open(cert, 'w') as cert_file:
+            cert_file.write(cert_pem)
+        return cert, key.file
+
     def _make_server_ssl(self, vhost):
-        """Makes a server SSL based on server_name and filename by adding
-        a 'listen 443 ssl' directive to the server block.
+        """Make a server SSL.
+
+        Make a server SSL based on server_name and filename by adding a
+        ``listen IConfig.dvsni_port ssl`` directive to the server block.
 
         .. todo:: Maybe this should create a new block instead of modifying
             the existing one?
@@ -274,17 +293,22 @@ class NginxConfigurator(common.Plugin):
         :type vhost: :class:`~letsencrypt_nginx.obj.VirtualHost`
 
         """
-        ssl_block = [['listen', '443 ssl'],
-                     ['ssl_certificate',
-                      '/etc/ssl/certs/ssl-cert-snakeoil.pem'],
-                     ['ssl_certificate_key',
-                      '/etc/ssl/private/ssl-cert-snakeoil.key'],
+        snakeoil_cert, snakeoil_key = self._get_snakeoil_paths()
+        ssl_block = [['listen', '{0} ssl'.format(self.config.dvsni_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.ssl = True
         vhost.raw.extend(ssl_block)
-        vhost.addrs.add(obj.Addr('', '443', True, False))
+        vhost.addrs.add(obj.Addr('', str(self.config.dvsni_port), True, False))
 
     def get_all_certs_keys(self):
         """Find all existing keys, certs from configuration.
@@ -335,7 +359,7 @@ class NginxConfigurator(common.Plugin):
         :rtype: bool
 
         """
-        return nginx_restart(self.conf('ctl'))
+        return nginx_restart(self.conf('ctl'), self.nginx_conf)
 
     def config_test(self):  # pylint: disable=no-self-use
         """Check the configuration of Nginx for errors.
@@ -346,7 +370,7 @@ class NginxConfigurator(common.Plugin):
         """
         try:
             proc = subprocess.Popen(
-                [self.conf('ctl'), "-t"],
+                [self.conf('ctl'), "-c", self.nginx_conf, "-t"],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE)
             stdout, stderr = proc.communicate()
@@ -391,11 +415,12 @@ class NginxConfigurator(common.Plugin):
         """
         try:
             proc = subprocess.Popen(
-                [self.conf('ctl'), "-V"],
+                [self.conf('ctl'), "-c", self.nginx_conf, "-V"],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE)
             text = proc.communicate()[1]  # nginx prints output to stderr
-        except (OSError, ValueError):
+        except (OSError, ValueError) as error:
+            logging.debug(error, exc_info=True)
             raise errors.PluginError(
                 "Unable to run %s -V" % self.conf('ctl'))
 
@@ -544,7 +569,7 @@ class NginxConfigurator(common.Plugin):
             self.restart()
 
 
-def nginx_restart(nginx_ctl):
+def nginx_restart(nginx_ctl, nginx_conf="/etc/nginx.conf"):
     """Restarts the Nginx Server.
 
     .. todo:: Nginx restart is fatal if the configuration references
@@ -555,14 +580,14 @@ def nginx_restart(nginx_ctl):
 
     """
     try:
-        proc = subprocess.Popen([nginx_ctl, "-s", "reload"],
+        proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf, "-s", "reload"],
                                 stdout=subprocess.PIPE,
                                 stderr=subprocess.PIPE)
         stdout, stderr = proc.communicate()
 
         if proc.returncode != 0:
             # Maybe Nginx isn't running
-            nginx_proc = subprocess.Popen([nginx_ctl],
+            nginx_proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf],
                                           stdout=subprocess.PIPE,
                                           stderr=subprocess.PIPE)
             stdout, stderr = nginx_proc.communicate()

letsencrypt_nginx/dvsni.py

@@ -47,7 +47,8 @@ class NginxDvsni(common.Dvsni):
         self.configurator.save()
 
         addresses = []
-        default_addr = "443 default_server ssl"
+        default_addr = "{0} default_server ssl".format(
+            self.configurator.config.dvsni_port)
 
         for achall in self.achalls:
             vhost = self.configurator.choose_vhost(achall.domain)
@@ -133,6 +134,12 @@ class NginxDvsni(common.Dvsni):
 
         block.extend([['server_name', achall.nonce_domain],
                       ['include', self.configurator.parser.loc["ssl_options"]],
+                      # access and error logs necessary for
+                      # integration testing (non-root)
+                      ['access_log', os.path.join(
+                          self.configurator.config.work_dir, 'access.log')],
+                      ['error_log', os.path.join(
+                          self.configurator.config.work_dir, 'error.log')],
                       ['ssl_certificate', self.get_cert_file(achall)],
                       ['ssl_certificate_key', achall.key.file],
                       [['location', '/'], [['root', document_root]]]])

letsencrypt_nginx/tests/configurator_test.py

@@ -1,8 +1,10 @@
 """Test for letsencrypt_nginx.configurator."""
+import os
 import shutil
 import unittest
 
 import mock
+import OpenSSL
 
 from acme import challenges
 from acme import messages
@@ -55,7 +57,7 @@ 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', '443 ssl']])
+            [['listen', '5001 ssl']])
         self.config.save()
 
         # pylint: disable=protected-access
@@ -64,7 +66,7 @@ class NginxConfiguratorTest(util.NginxTest):
                                         ['listen', '127.0.0.1'],
                                         ['server_name', '.example.com'],
                                         ['server_name', 'example.*'],
-                                        ['listen', '443 ssl']]]],
+                                        ['listen', '5001 ssl']]]],
                          parsed[0])
 
     def test_choose_vhost(self):
@@ -98,7 +100,7 @@ class NginxConfiguratorTest(util.NginxTest):
         nginx_conf = self.config.parser.abs_path('nginx.conf')
         example_conf = self.config.parser.abs_path('sites-enabled/example.com')
 
-        # Get the default 443 vhost
+        # Get the default SSL vhost
         self.config.deploy_cert(
             "www.example.com",
             "example/cert.pem", "example/key.pem")
@@ -109,12 +111,16 @@ class NginxConfiguratorTest(util.NginxTest):
 
         self.config.parser.load()
 
+        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'],
                             ['listen', '127.0.0.1'],
                             ['server_name', '.example.com'],
                             ['server_name', 'example.*'],
-                            ['listen', '443 ssl'],
+                            ['listen', '5001 ssl'],
+                            ['access_log', access_log],
+                            ['error_log', error_log],
                             ['ssl_certificate', 'example/cert.pem'],
                             ['ssl_certificate_key', 'example/key.pem'],
                             ['include',
@@ -129,7 +135,9 @@ class NginxConfiguratorTest(util.NginxTest):
                            [['location', '/'],
                             [['root', 'html'],
                              ['index', 'index.html index.htm']]],
-                           ['listen', '443 ssl'],
+                           ['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',
@@ -140,7 +148,7 @@ class NginxConfiguratorTest(util.NginxTest):
         nginx_conf = self.config.parser.abs_path('nginx.conf')
         example_conf = self.config.parser.abs_path('sites-enabled/example.com')
 
-        # Get the default 443 vhost
+        # Get the default SSL vhost
         self.config.deploy_cert(
             "www.example.com",
             "example/cert.pem", "example/key.pem")
@@ -266,6 +274,18 @@ class NginxConfiguratorTest(util.NginxTest):
         mocked.returncode = 0
         self.assertTrue(self.config.config_test())
 
+    def test_get_snakeoil_paths(self):
+        # pylint: disable=protected-access
+        cert, key = self.config._get_snakeoil_paths()
+        self.assertTrue(os.path.exists(cert))
+        self.assertTrue(os.path.exists(key))
+        with open(cert) as cert_file:
+            OpenSSL.crypto.load_certificate(
+                OpenSSL.crypto.FILETYPE_PEM, cert_file.read())
+        with open(key) as key_file:
+            OpenSSL.crypto.load_privatekey(
+                OpenSSL.crypto.FILETYPE_PEM, key_file.read())
+
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

letsencrypt_nginx/tests/util.py

@@ -53,6 +53,7 @@ def get_nginx_configurator(
             backup_dir=backups,
             temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"),
             in_progress_dir=os.path.join(backups, "IN_PROGRESS"),
+            dvsni_port=5001,
         ),
         name="nginx",
         version=version)

tests/boulder-integration.sh

@@ -10,24 +10,15 @@
 #
 # Note: this script is called by Boulder integration test suite!
 
-root="$(mktemp -d)"
-echo "\nRoot integration tests directory: $root"
-store_flags="--config-dir $root/conf --work-dir $root/work"
-store_flags="$store_flags --logs-dir $root/logs"
+. ./tests/integration/_common.sh
+export PATH="/usr/sbin:$PATH"  # /usr/sbin/nginx
+
 
 common() {
-    # first three flags required, rest is handy defaults
-    letsencrypt \
-        --server "${SERVER:-http://localhost:4000/acme/new-reg}" \
-        --no-verify-ssl \
-        --dvsni-port 5001 \
-        $store_flags \
-        --text \
-        --agree-eula \
-        --email "" \
+    letsencrypt_test \
         --authenticator standalone \
         --installer null \
-        -vvvvvvv "$@"
+        "$@"
 }
 
 common --domains le1.wtf auth
@@ -60,3 +51,9 @@ do
     live="$(readlink -f "$root/conf/live/le1.wtf/${x}.pem")"
     [ "${dir}/${latest}" = "$live" ]  # renewer fails this test
 done
+
+
+if type nginx;
+then
+    . ./tests/integration/nginx.sh
+fi

tests/integration/_common.sh

@@ -0,0 +1,25 @@
+#!/bin/sh
+
+if [ "xxx$root" = "xxx" ];
+then
+    root="$(mktemp -d)"
+    echo "Root integration tests directory: $root"
+fi
+store_flags="--config-dir $root/conf --work-dir $root/work"
+store_flags="$store_flags --logs-dir $root/logs"
+export root store_flags
+
+letsencrypt_test () {
+    # first three flags required, rest is handy defaults
+    letsencrypt \
+        --server "${SERVER:-http://localhost:4000/acme/new-reg}" \
+        --no-verify-ssl \
+        --dvsni-port 5001 \
+        $store_flags \
+        --text \
+        --agree-eula \
+        --email "" \
+        --debug \
+        -vvvvvvv \
+        "$@"
+}

tests/integration/nginx.conf.sh

@@ -0,0 +1,67 @@
+# Based on
+# https://www.exratione.com/2014/03/running-nginx-as-a-non-root-user/
+# https://github.com/exratione/non-root-nginx/blob/9a77f62e5d5cb9c9026fd62eece76b9514011019/nginx.conf
+
+cat <<EOF
+# This error log will be written regardless of server scope error_log
+# definitions, so we have to set this here in the main scope.
+#
+# Even doing this, Nginx will still try to create the default error file, and
+# log a non-fatal error when it fails. After that things will work, however.
+error_log $root/error.log;
+
+# The pidfile will be written to /var/run unless this is set.
+pid $root/nginx.pid;
+
+worker_processes 1;
+
+events {
+  worker_connections 1024;
+}
+
+http {
+  # 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;
+  fastcgi_temp_path $root/fastcgi_temp;
+  proxy_temp_path $root/proxy_temp;
+  #scgi_temp_path $root/scgi_temp;
+  #uwsgi_temp_path $root/uwsgi_temp;
+
+  # This should be turned off in a Virtualbox VM, as it can cause some
+  # interesting issues with data corruption in delivered files.
+  sendfile off;
+
+  tcp_nopush on;
+  tcp_nodelay on;
+  keepalive_timeout 65;
+  types_hash_max_size 2048;
+
+  include /etc/nginx/mime.types;
+  index index.html index.htm index.php;
+
+  log_format   main '\$remote_addr - \$remote_user [\$time_local] \$status '
+    '"\$request" \$body_bytes_sent "\$http_referer" '
+    '"\$http_user_agent" "\$http_x_forwarded_for"';
+
+  default_type application/octet-stream;
+
+  server {
+    # IPv4.
+    listen 8080;
+    # IPv6.
+    listen [::]:8080 default ipv6only=on;
+
+    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.
+      try_files \$uri \$uri/ /index.html;
+    }
+  }
+}
+EOF

tests/integration/nginx.sh

@@ -0,0 +1,28 @@
+#!/bin/sh -xe
+# prerequisite: apt-get install --no-install-recommends nginx-light openssl
+
+. ./tests/integration/_common.sh
+
+export PATH="/usr/sbin:$PATH"  # /usr/sbin/nginx
+nginx_root="$root/nginx"
+mkdir $nginx_root
+root="$nginx_root" ./tests/integration/nginx.conf.sh > $nginx_root/nginx.conf
+
+killall nginx || true
+nginx -c $nginx_root/nginx.conf
+
+letsencrypt_test_nginx () {
+    letsencrypt_test \
+        --configurator nginx \
+        --nginx-server-root $nginx_root \
+        "$@"
+}
+
+letsencrypt_test_nginx --domains nginx.wtf run
+echo | openssl s_client -connect localhost:5001 \
+    | openssl x509 -out $root/nginx.pem
+diff -q $root/nginx.pem $root/conf/live/nginx.wtf/cert.pem
+
+# note: not reached if anything above fails, hence "killall" at the
+# top
+nginx -c $nginx_root/nginx.conf -s stop