From 23d27f4659c19aa66bdc1d3a05d090c3238064ed Mon Sep 17 00:00:00 2001
From: yan <yzhu@yahoo-inc.com>
Date: Tue, 28 Apr 2015 13:49:12 -0700
Subject: [PATCH 1/7] Bump min nginx version to 0.8.48

We are assuming that if a server_name isn't specified, it matches the empty
string. Prior to 0.8.48, it would match the machine's hostname.
---
 letsencrypt/client/plugins/nginx/configurator.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/client/plugins/nginx/configurator.py b/letsencrypt/client/plugins/nginx/configurator.py
index 5f49ca8ee..158feb32c 100644
--- a/letsencrypt/client/plugins/nginx/configurator.py
+++ b/letsencrypt/client/plugins/nginx/configurator.py
@@ -399,10 +399,11 @@ class NginxConfigurator(object):
 
         nginx_version = tuple([int(i) for i in version_matches[0].split(".")])
 
-        # nginx < 0.8.21 doesn't use default_server
-        if nginx_version < (0, 8, 21):
+        # nginx < 0.8.48 uses machine hostname as default server_name instead of
+        # the empty string
+        if nginx_version < (0, 8, 48):
             raise errors.LetsEncryptConfiguratorError(
-                "Nginx version must be 0.8.21+")
+                "Nginx version must be 0.8.48+")
 
         return nginx_version
 

From 6f4af62f61bfacb63d5b141a4f9cbc17729fdf17 Mon Sep 17 00:00:00 2001
From: yan <yzhu@yahoo-inc.com>
Date: Tue, 28 Apr 2015 18:38:28 -0700
Subject: [PATCH 2/7] Add dvsni tests

---
 letsencrypt/client/plugins/nginx/dvsni.py     |  99 ++++++++++--
 letsencrypt/client/plugins/nginx/obj.py       |  14 +-
 letsencrypt/client/plugins/nginx/parser.py    |   6 +-
 .../client/plugins/nginx/tests/dvsni_test.py  | 148 ++++++++++++++----
 .../client/plugins/nginx/tests/obj_test.py    |   6 +-
 5 files changed, 224 insertions(+), 49 deletions(-)

diff --git a/letsencrypt/client/plugins/nginx/dvsni.py b/letsencrypt/client/plugins/nginx/dvsni.py
index 7233d7c62..0eab4d402 100644
--- a/letsencrypt/client/plugins/nginx/dvsni.py
+++ b/letsencrypt/client/plugins/nginx/dvsni.py
@@ -1,7 +1,11 @@
 """NginxDVSNI"""
 import logging
+import os
 
+from letsencrypt.client import errors
 from letsencrypt.client.plugins.apache.dvsni import ApacheDvsni
+from letsencrypt.client.plugins.nginx import obj
+from letsencrypt.client.plugins.nginx.nginxparser import dump
 
 
 class NginxDvsni(ApacheDvsni):
@@ -29,35 +33,110 @@ class NginxDvsni(ApacheDvsni):
     """
 
     def perform(self):
-        """Perform a DVSNI challenge on Nginx."""
+        """Perform a DVSNI challenge on Nginx.
+
+        :returns: list of :class:`letsencrypt.acme.challenges.DVSNIResponse`
+        :rtype: list
+
+        """
         if not self.achalls:
             return []
 
         self.configurator.save()
 
         addresses = []
+        default_addr = "443 default_server ssl"
+
         for achall in self.achalls:
             vhost = self.configurator.choose_vhost(achall.domain)
             if vhost is None:
                 logging.error(
-                    "No nginx vhost exists with servername or alias of: %s",
+                    "No nginx vhost exists with server_name or alias of: %s",
                     achall.domain)
                 logging.error("No default 443 nginx vhost exists")
-                logging.error("Please specify servernames in the Nginx config")
+                logging.error("Please specify server_names in the Nginx config")
                 return None
+
+            for addr in vhost.addrs:
+                if addr.default:
+                    addresses.append([obj.Addr.fromstring(default_addr)])
+                    break
             else:
                 addresses.append(list(vhost.addrs))
 
-        responses = []
+        # Create challenge certs
+        responses = [self._setup_challenge_cert(x) for x in self.achalls]
 
-        # Create all of the challenge certs
-        # for achall in self.achalls:
-        #     responses.append(self._setup_challenge_cert(achall))
-
-        # Setup the configuration
-        # self._mod_config(addresses)
+        # Set up the configuration
+        self._mod_config(addresses)
 
         # Save reversible changes
         self.configurator.save("SNI Challenge", True)
 
         return responses
+
+    def _mod_config(self, ll_addrs):
+        """Modifies Nginx config to include challenge server blocks.
+
+        :param list ll_addrs: list of lists of
+            :class:`letsencrypt.client.plugins.apache.obj.Addr` to apply
+
+        :raises errors.LetsEncryptMisconfigurationError:
+            Unable to find a suitable HTTP block to include DVSNI hosts.
+
+        """
+        # Add the 'include' statement for the challenges if it doesn't exist
+        # already in the main config
+        included = False
+        directive = ['include', self.challenge_conf]
+        root = self.configurator.parser.loc["root"]
+        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)
+                included = True
+                break
+        if not included:
+            raise errors.LetsEncryptMisconfigurationError(
+                'LetsEncrypt could not find an HTTP block to include DVSNI '
+                'challenges in %s.' % root)
+
+        config = []
+        for idx, addrs in enumerate(ll_addrs):
+            config.append(self._make_server_block(self.achalls[idx], addrs))
+
+        self.configurator.reverter.register_file_creation(
+            True, self.challenge_conf)
+
+        with open(self.challenge_conf, "w") as new_conf:
+            dump(config, new_conf)
+
+    def _make_server_block(self, achall, addrs):
+        """Creates a server block for a DVSNI challenge.
+
+        :param achall: Annotated DVSNI challenge.
+        :type achall: :class:`letsencrypt.client.achallenges.DVSNI`
+
+        :param list addrs: addresses of challenged domain
+            :class:`list` of type :class:`~nginx.obj.Addr`
+
+        :returns: server block for the challenge host
+        :rtype: list
+
+        """
+        block = []
+        for addr in addrs:
+            block.append(['listen', str(addr)])
+
+        block.append(['server_name', achall.nonce_domain])
+        block.append(['include', self.configurator.parser.loc["ssl_options"]])
+        block.append(['ssl_certificate', self.get_cert_file(achall)])
+        block.append(['ssl_certificate_key', achall.key.file])
+
+        document_root = os.path.join(
+            self.configurator.config.config_dir, "dvsni_page")
+        block.append([['location', '/'], [['root', document_root]]])
+
+        return [['server'], block]
diff --git a/letsencrypt/client/plugins/nginx/obj.py b/letsencrypt/client/plugins/nginx/obj.py
index acaacb3b0..f4b2f0f57 100644
--- a/letsencrypt/client/plugins/nginx/obj.py
+++ b/letsencrypt/client/plugins/nginx/obj.py
@@ -67,12 +67,20 @@ class Addr(ApacheAddr):
         return cls(host, port, ssl, default)
 
     def __str__(self):
+        parts = ''
         if self.tup[0] and self.tup[1]:
-            return "%s:%s" % self.tup
+            parts = "%s:%s" % self.tup
         elif self.tup[0]:
-            return self.tup[0]
+            parts = self.tup[0]
         else:
-            return self.tup[1]
+            parts = self.tup[1]
+
+        if self.default:
+            parts += ' default_server'
+        if self.ssl:
+            parts += ' ssl'
+
+        return parts
 
     def __eq__(self, other):
         if isinstance(other, self.__class__):
diff --git a/letsencrypt/client/plugins/nginx/parser.py b/letsencrypt/client/plugins/nginx/parser.py
index 55a0b01e8..8de705dde 100644
--- a/letsencrypt/client/plugins/nginx/parser.py
+++ b/letsencrypt/client/plugins/nginx/parser.py
@@ -33,6 +33,7 @@ class NginxParser(object):
         """Loads Nginx files into a parsed tree.
 
         """
+        self.parsed = {}
         self._parse_recursively(self.loc["root"])
 
     def _parse_recursively(self, filepath):
@@ -252,8 +253,9 @@ class NginxParser(object):
 
     def add_server_directives(self, filename, names, directives,
                               replace=False):
-        """Add or replace directives in server blocks whose server_name set
-        is 'names'. If replace is True, this raises a misconfiguration error
+        """Add or replace directives in server blocks identified by server_name.
+
+        ..note :: If replace is True, this raises a misconfiguration error
         if the directive does not already exist.
 
         ..todo :: Doesn't match server blocks whose server_name directives are
diff --git a/letsencrypt/client/plugins/nginx/tests/dvsni_test.py b/letsencrypt/client/plugins/nginx/tests/dvsni_test.py
index bf66367e6..a44e0fcd6 100644
--- a/letsencrypt/client/plugins/nginx/tests/dvsni_test.py
+++ b/letsencrypt/client/plugins/nginx/tests/dvsni_test.py
@@ -6,13 +6,16 @@ import shutil
 import mock
 
 from letsencrypt.acme import challenges
-from letsencrypt.acme import messages2
 
 from letsencrypt.client import achallenges
+from letsencrypt.client import errors
 from letsencrypt.client import le_util
 
+from letsencrypt.client.plugins.nginx.obj import Addr
 from letsencrypt.client.plugins.nginx.tests import util
 
+from letsencrypt.client.tests import acme_util
+
 
 class DvsniPerformTest(util.NginxTest):
     """Test the NginxDVSNI challenge."""
@@ -36,25 +39,29 @@ class DvsniPerformTest(util.NginxTest):
 
         self.achalls = [
             achallenges.DVSNI(
-                challb=messages2.ChallengeBody(
-                    chall=challenges.DVSNI(
+                challb=acme_util.chall_to_challb(
+                    challenges.DVSNI(
                         r="foo",
-                        nonce="bar",
-                    ),
-                    uri="https://letsencrypt-ca.org/chall0_uri",
-                    status=messages2.Status("pending"),
-                ), domain="www.example.com", key=auth_key),
+                        nonce="bar"
+                    ), "pending"),
+                domain="www.example.com", key=auth_key),
             achallenges.DVSNI(
-                challb=messages2.ChallengeBody(
-                    chall=challenges.DVSNI(
+                challb=acme_util.chall_to_challb(
+                    challenges.DVSNI(
                         r="\xba\xa9\xda?<m\xaewmx\xea\xad\xadv\xf4\x02\xc9y\x80"
                           "\xe2_X\t\xe7\xc7\xa4\t\xca\xf7&\x945",
                         nonce="Y\xed\x01L\xac\x95\xf7pW\xb1\xd7"
-                              "\xa1\xb2\xc5\x96\xba",
-                    ),
-                    uri="https://letsencrypt-ca.org/chall1_uri",
-                    status=messages2.Status("pending"),
-                ), domain="blah", key=auth_key),
+                              "\xa1\xb2\xc5\x96\xba"
+                    ), "pending"),
+                domain="blah", key=auth_key),
+            achallenges.DVSNI(
+                challb=acme_util.chall_to_challb(
+                    challenges.DVSNI(
+                        r="\x8c\x8a\xbf_-f\\cw\xee\xd6\xf8/\xa5\xe3\xfd\xeb9"
+                          "\xf1\xf5\xb9\xefVM\xc9w\xa4u\x9c\xe1\x87\xb4",
+                        nonce="7\xbc^\xb7]>\x00\xa1\x9bOcU\x84^Z\x18"
+                    ), "pending"),
+                domain="www.example.org", key=auth_key)
         ]
 
     def tearDown(self):
@@ -69,26 +76,105 @@ class DvsniPerformTest(util.NginxTest):
 
     @mock.patch("letsencrypt.client.plugins.nginx.configurator."
                 "NginxConfigurator.save")
-    def test_perform0(self, mock_save):
-        self.sni.add_chall(self.achalls[0])
-        responses = self.sni.perform()
-        self.assertEqual([], responses)
-        self.assertEqual(mock_save.call_count, 2)
-
-    def test_setup_challenge_cert(self):
-        # This is a helper function that can be used for handling
-        # open context managers more elegantly. It avoids dealing with
-        # __enter__ and __exit__ calls.
-        # http://www.voidspace.org.uk/python/mock/helpers.html#mock.mock_open
-        pass
-
-    @mock.patch("letsencrypt.client.plugins.nginx.configurator."
-                "NginxConfigurator.save")
-    def test_perform1(self, mock_save):
+    def test_perform(self, mock_save):
         self.sni.add_chall(self.achalls[1])
         responses = self.sni.perform()
         self.assertEqual(None, responses)
         self.assertEqual(mock_save.call_count, 1)
 
+    def test_perform0(self):
+        responses = self.sni.perform()
+        self.assertEqual([], responses)
+
+    @mock.patch("letsencrypt.client.plugins.nginx.configurator."
+                "NginxConfigurator.save")
+    def test_perform1(self, mock_save):
+        self.sni.add_chall(self.achalls[0])
+        mock_setup_cert = mock.MagicMock(
+            return_value=challenges.DVSNIResponse(s="nginxS1"))
+
+        # pylint: disable=protected-access
+        self.sni._setup_challenge_cert = mock_setup_cert
+
+        responses = self.sni.perform()
+
+        mock_setup_cert.assert_called_once_with(self.achalls[0])
+        self.assertEqual([challenges.DVSNIResponse(s="nginxS1")], responses)
+        self.assertEqual(mock_save.call_count, 2)
+
+        # 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])
+
+    def test_perform2(self):
+        self.sni.add_chall(self.achalls[0])
+        self.sni.add_chall(self.achalls[2])
+
+        mock_setup_cert = mock.MagicMock(side_effect=[
+            challenges.DVSNIResponse(s="nginxS0"),
+            challenges.DVSNIResponse(s="nginxS1")])
+        # pylint: disable=protected-access
+        self.sni._setup_challenge_cert = mock_setup_cert
+
+        responses = self.sni.perform()
+
+        self.assertEqual(mock_setup_cert.call_count, 2)
+
+        self.assertEqual(
+            mock_setup_cert.call_args_list[0], mock.call(self.achalls[0]))
+        self.assertEqual(
+            mock_setup_cert.call_args_list[1], mock.call(self.achalls[2]))
+
+        http = self.sni.configurator.parser.parsed[
+            self.sni.configurator.parser.loc["root"]][-1]
+        self.assertTrue(['include', self.sni.challenge_conf] in http[1])
+
+        self.assertEqual(len(responses), 2)
+        for i in xrange(2):
+            self.assertEqual(responses[i].s, "nginxS%d" % i)
+
+    def test_mod_config(self):
+        self.sni.add_chall(self.achalls[0])
+        self.sni.add_chall(self.achalls[2])
+
+        v_addr1 = [Addr("69.50.225.155", "9000", True, False),
+                   Addr("127.0.0.1", "", False, False)]
+        v_addr2 = [Addr("myhost", "", False, True)]
+        ll_addr = [v_addr1, v_addr2]
+        self.sni._mod_config(ll_addr)  # pylint: disable=protected-access
+
+        self.sni.configurator.save()
+
+        self.sni.configurator.parser.load()
+
+        http = self.sni.configurator.parser.parsed[
+            self.sni.configurator.parser.loc["root"]][-1]
+        self.assertTrue(['include', self.sni.challenge_conf] in http[1])
+
+        vhosts = self.sni.configurator.parser.get_vhosts()
+        vhs = []
+        for vhost in vhosts:
+            if vhost.filep == self.sni.challenge_conf:
+                vhs.append(vhost)
+
+        for vhost in vhs:
+            if vhost.addrs == set(v_addr1):
+                self.assertEqual(
+                    vhost.names, set([self.achalls[0].nonce_domain]))
+            else:
+                self.assertEqual(vhost.addrs, set(v_addr2))
+                self.assertEqual(
+                    vhost.names, set([self.achalls[2].nonce_domain]))
+
+        self.assertEqual(len(vhs), 2)
+
+    def test_mod_config_fail(self):
+        root = self.sni.configurator.parser.loc["root"]
+        self.sni.configurator.parser.parsed[root] = [['include', 'foo.conf']]
+        # pylint: disable=protected-access
+        self.assertRaises(errors.LetsEncryptMisconfigurationError,
+                          self.sni._mod_config, [])
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/letsencrypt/client/plugins/nginx/tests/obj_test.py b/letsencrypt/client/plugins/nginx/tests/obj_test.py
index d5591c763..833be1619 100644
--- a/letsencrypt/client/plugins/nginx/tests/obj_test.py
+++ b/letsencrypt/client/plugins/nginx/tests/obj_test.py
@@ -49,11 +49,11 @@ class AddrTest(unittest.TestCase):
 
     def test_str(self):
         self.assertEqual(str(self.addr1), "192.168.1.1")
-        self.assertEqual(str(self.addr2), "192.168.1.1:*")
+        self.assertEqual(str(self.addr2), "192.168.1.1:* ssl")
         self.assertEqual(str(self.addr3), "192.168.1.1:80")
-        self.assertEqual(str(self.addr4), "*:80")
+        self.assertEqual(str(self.addr4), "*:80 default_server ssl")
         self.assertEqual(str(self.addr5), "myhost")
-        self.assertEqual(str(self.addr6), "80")
+        self.assertEqual(str(self.addr6), "80 default_server")
 
     def test_eq(self):
         from letsencrypt.client.plugins.nginx.obj import Addr

From cf487916868d3525c3307bb6f7b17a7a7ab2591d Mon Sep 17 00:00:00 2001
From: yan <yzhu@yahoo-inc.com>
Date: Tue, 5 May 2015 23:57:40 -0700
Subject: [PATCH 3/7] Address @kuba review comments

---
 letsencrypt/client/plugins/nginx/dvsni.py     | 24 +++++++++----------
 .../client/plugins/nginx/tests/dvsni_test.py  |  7 ++----
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/letsencrypt/client/plugins/nginx/dvsni.py b/letsencrypt/client/plugins/nginx/dvsni.py
index 0eab4d402..30af2e7a1 100644
--- a/letsencrypt/client/plugins/nginx/dvsni.py
+++ b/letsencrypt/client/plugins/nginx/dvsni.py
@@ -1,4 +1,5 @@
 """NginxDVSNI"""
+import itertools
 import logging
 import os
 
@@ -103,9 +104,8 @@ class NginxDvsni(ApacheDvsni):
                 'LetsEncrypt could not find an HTTP block to include DVSNI '
                 'challenges in %s.' % root)
 
-        config = []
-        for idx, addrs in enumerate(ll_addrs):
-            config.append(self._make_server_block(self.achalls[idx], addrs))
+        config = [self._make_server_block(pair[0], pair[1])
+                  for pair in itertools.izip(self.achalls, ll_addrs)]
 
         self.configurator.reverter.register_file_creation(
             True, self.challenge_conf)
@@ -126,17 +126,15 @@ class NginxDvsni(ApacheDvsni):
         :rtype: list
 
         """
-        block = []
-        for addr in addrs:
-            block.append(['listen', str(addr)])
-
-        block.append(['server_name', achall.nonce_domain])
-        block.append(['include', self.configurator.parser.loc["ssl_options"]])
-        block.append(['ssl_certificate', self.get_cert_file(achall)])
-        block.append(['ssl_certificate_key', achall.key.file])
-
         document_root = os.path.join(
             self.configurator.config.config_dir, "dvsni_page")
-        block.append([['location', '/'], [['root', document_root]]])
+
+        block = [['listen', str(addr)] for addr in addrs]
+
+        block.extend([['server_name', achall.nonce_domain],
+                      ['include', self.configurator.parser.loc["ssl_options"]],
+                      ['ssl_certificate', self.get_cert_file(achall)],
+                      ['ssl_certificate_key', achall.key.file],
+                      [['location', '/'], [['root', document_root]]]])
 
         return [['server'], block]
diff --git a/letsencrypt/client/plugins/nginx/tests/dvsni_test.py b/letsencrypt/client/plugins/nginx/tests/dvsni_test.py
index a44e0fcd6..15d7dbdb5 100644
--- a/letsencrypt/client/plugins/nginx/tests/dvsni_test.py
+++ b/letsencrypt/client/plugins/nginx/tests/dvsni_test.py
@@ -79,7 +79,7 @@ class DvsniPerformTest(util.NginxTest):
     def test_perform(self, mock_save):
         self.sni.add_chall(self.achalls[1])
         responses = self.sni.perform()
-        self.assertEqual(None, responses)
+        self.assertTrue(responses is None)
         self.assertEqual(mock_save.call_count, 1)
 
     def test_perform0(self):
@@ -153,10 +153,7 @@ class DvsniPerformTest(util.NginxTest):
         self.assertTrue(['include', self.sni.challenge_conf] in http[1])
 
         vhosts = self.sni.configurator.parser.get_vhosts()
-        vhs = []
-        for vhost in vhosts:
-            if vhost.filep == self.sni.challenge_conf:
-                vhs.append(vhost)
+        vhs = [vh for vh in vhosts if vh.filep == self.sni.challenge_conf]
 
         for vhost in vhs:
             if vhost.addrs == set(v_addr1):

From 4dc566a871a26d7a7f61e0c807abb4dc9a514e8a Mon Sep 17 00:00:00 2001
From: yan <yzhu@yahoo-inc.com>
Date: Thu, 7 May 2015 18:30:50 -0700
Subject: [PATCH 4/7] Update vhost object when making nginx SSL block

---
 .../client/plugins/nginx/configurator.py      | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/letsencrypt/client/plugins/nginx/configurator.py b/letsencrypt/client/plugins/nginx/configurator.py
index 158feb32c..ac6671178 100644
--- a/letsencrypt/client/plugins/nginx/configurator.py
+++ b/letsencrypt/client/plugins/nginx/configurator.py
@@ -20,6 +20,7 @@ from letsencrypt.client import reverter
 
 from letsencrypt.client.plugins.nginx import dvsni
 from letsencrypt.client.plugins.nginx import parser
+from letsencrypt.client.plugins.nginx import obj
 
 
 class NginxConfigurator(object):
@@ -170,7 +171,7 @@ class NginxConfigurator(object):
 
         if vhost is not None:
             if not vhost.ssl:
-                self._make_server_ssl(vhost.filep, vhost.names)
+                self._make_server_ssl(vhost)
 
         return vhost
 
@@ -245,23 +246,28 @@ class NginxConfigurator(object):
 
         return all_names
 
-    def _make_server_ssl(self, filename, names):
+    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.
 
         .. todo:: Maybe this should create a new block instead of modifying
         the existing one?
 
-        :param str filename: The absolute filename of the config file.
-        :param set names: The server names of the block to add SSL in
+        :param vhost: The vhost to add SSL to.
+        :type vhost: :class:`~letsencrypt.client.plugins.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'],
+                     ['include', self.parser.loc["ssl_options"]]]
         self.parser.add_server_directives(
-            filename, names,
-            [['listen', '443 ssl'],
-             ['ssl_certificate', '/etc/ssl/certs/ssl-cert-snakeoil.pem'],
-             ['ssl_certificate_key', '/etc/ssl/private/ssl-cert-snakeoil.key'],
-             ['include', self.parser.loc["ssl_options"]]])
+            vhost.filep, vhost.names, ssl_block)
+        vhost.ssl = True
+        vhost.raw.extend(ssl_block)
+        vhost.addrs.add(obj.Addr('', '443', True, False))
 
     def get_all_certs_keys(self):
         """Find all existing keys, certs from configuration.

From 1533eea35143d439442ec84473d0b3fbe78041b5 Mon Sep 17 00:00:00 2001
From: yan <yzhu@yahoo-inc.com>
Date: Fri, 8 May 2015 12:34:48 -0700
Subject: [PATCH 5/7] Start nginx if it's not already running

---
 .../client/plugins/nginx/configurator.py        | 17 ++++++++++++-----
 .../plugins/nginx/tests/configurator_test.py    | 14 ++++++++++++++
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/client/plugins/nginx/configurator.py b/letsencrypt/client/plugins/nginx/configurator.py
index ac6671178..3c00e5e50 100644
--- a/letsencrypt/client/plugins/nginx/configurator.py
+++ b/letsencrypt/client/plugins/nginx/configurator.py
@@ -545,11 +545,18 @@ def nginx_restart(nginx_ctl):
         stdout, stderr = proc.communicate()
 
         if proc.returncode != 0:
-            # Enter recovery routine...
-            logging.error("Nginx Restart Failed!")
-            logging.error(stdout)
-            logging.error(stderr)
-            return False
+            # Maybe Nginx isn't running
+            nginx_proc = subprocess.Popen([nginx_ctl],
+                                          stdout=subprocess.PIPE,
+                                          stderr=subprocess.PIPE)
+            stdout, stderr = nginx_proc.communicate()
+
+            if nginx_proc.returncode != 0:
+                # Enter recovery routine...
+                logging.error("Nginx Restart Failed!")
+                logging.error(stdout)
+                logging.error(stderr)
+                return False
 
     except (OSError, ValueError):
         logging.fatal(
diff --git a/letsencrypt/client/plugins/nginx/tests/configurator_test.py b/letsencrypt/client/plugins/nginx/tests/configurator_test.py
index cb5fef6bf..9399d42a6 100644
--- a/letsencrypt/client/plugins/nginx/tests/configurator_test.py
+++ b/letsencrypt/client/plugins/nginx/tests/configurator_test.py
@@ -259,6 +259,20 @@ class NginxConfiguratorTest(util.NginxTest):
         mocked.returncode = 0
         self.assertTrue(self.config.restart())
 
+    @mock.patch("letsencrypt.client.plugins.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())
+
+    @mock.patch("letsencrypt.client.plugins.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)
+
     @mock.patch("letsencrypt.client.plugins.nginx.configurator."
                 "subprocess.Popen")
     def test_config_test(self, mock_popen):

From f7116a738806e6dab3198b53535b56dd14a8933a Mon Sep 17 00:00:00 2001
From: yan <yzhu@yahoo-inc.com>
Date: Fri, 8 May 2015 12:36:32 -0700
Subject: [PATCH 6/7] Reduce logging severity for unparseable config files

---
 letsencrypt/client/plugins/nginx/parser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/client/plugins/nginx/parser.py b/letsencrypt/client/plugins/nginx/parser.py
index 8de705dde..16480d75f 100644
--- a/letsencrypt/client/plugins/nginx/parser.py
+++ b/letsencrypt/client/plugins/nginx/parser.py
@@ -166,7 +166,7 @@ class NginxParser(object):
             except IOError:
                 logging.warn("Could not open file: %s", item)
             except pyparsing.ParseException:
-                logging.warn("Could not parse file: %s", item)
+                logging.debug("Could not parse file: %s", item)
         return trees
 
     def _set_locations(self, ssl_options):

From dfb94613bf2dda3ef8577a167bea511d2fd9afc1 Mon Sep 17 00:00:00 2001
From: yan <yzhu@yahoo-inc.com>
Date: Fri, 8 May 2015 17:17:24 -0700
Subject: [PATCH 7/7] Add nginx server block for target_name if one doesn't
 exist

---
 .../client/plugins/nginx/configurator.py      |  8 ++-
 letsencrypt/client/plugins/nginx/dvsni.py     |  3 +-
 letsencrypt/client/plugins/nginx/obj.py       |  2 +-
 letsencrypt/client/plugins/nginx/parser.py    | 56 +++++++++++--------
 .../plugins/nginx/tests/configurator_test.py  |  2 +-
 .../client/plugins/nginx/tests/dvsni_test.py  | 31 +++++-----
 .../client/plugins/nginx/tests/parser_test.py | 10 ++++
 7 files changed, 68 insertions(+), 44 deletions(-)

diff --git a/letsencrypt/client/plugins/nginx/configurator.py b/letsencrypt/client/plugins/nginx/configurator.py
index 3c00e5e50..88a440f21 100644
--- a/letsencrypt/client/plugins/nginx/configurator.py
+++ b/letsencrypt/client/plugins/nginx/configurator.py
@@ -159,8 +159,12 @@ class NginxConfigurator(object):
 
         matches = self._get_ranked_matches(target_name)
         if not matches:
-            # No matches at all :'(
-            pass
+            # No matches. Create a new vhost with this name in nginx.conf.
+            filep = self.parser.loc["root"]
+            new_block = [['server'], [['server_name', target_name]]]
+            self.parser.add_http_directives(filep, new_block)
+            vhost = obj.VirtualHost(filep, set([]), False, True,
+                                    set([target_name]), list(new_block[1]))
         elif matches[0]['rank'] in xrange(2, 6):
             # Wildcard match - need to find the longest one
             rank = matches[0]['rank']
diff --git a/letsencrypt/client/plugins/nginx/dvsni.py b/letsencrypt/client/plugins/nginx/dvsni.py
index 30af2e7a1..0e4f125f6 100644
--- a/letsencrypt/client/plugins/nginx/dvsni.py
+++ b/letsencrypt/client/plugins/nginx/dvsni.py
@@ -52,9 +52,8 @@ class NginxDvsni(ApacheDvsni):
             vhost = self.configurator.choose_vhost(achall.domain)
             if vhost is None:
                 logging.error(
-                    "No nginx vhost exists with server_name or alias of: %s",
+                    "No nginx vhost exists with server_name matching: %s",
                     achall.domain)
-                logging.error("No default 443 nginx vhost exists")
                 logging.error("Please specify server_names in the Nginx config")
                 return None
 
diff --git a/letsencrypt/client/plugins/nginx/obj.py b/letsencrypt/client/plugins/nginx/obj.py
index f4b2f0f57..b2db6522a 100644
--- a/letsencrypt/client/plugins/nginx/obj.py
+++ b/letsencrypt/client/plugins/nginx/obj.py
@@ -97,7 +97,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
     :ivar set addrs: Virtual Host addresses (:class:`set` of :class:`Addr`)
     :ivar set names: Server names/aliases of vhost
         (:class:`list` of :class:`str`)
-    :ivar array raw: The raw form of the parsed server block
+    :ivar list raw: The raw form of the parsed server block
 
     :ivar bool ssl: SSLEngine on in vhost
     :ivar bool enabled: Virtual host is enabled
diff --git a/letsencrypt/client/plugins/nginx/parser.py b/letsencrypt/client/plugins/nginx/parser.py
index 16480d75f..099a8e36d 100644
--- a/letsencrypt/client/plugins/nginx/parser.py
+++ b/letsencrypt/client/plugins/nginx/parser.py
@@ -253,7 +253,7 @@ class NginxParser(object):
 
     def add_server_directives(self, filename, names, directives,
                               replace=False):
-        """Add or replace directives in server blocks identified by server_name.
+        """Add or replace directives in the first server block with names.
 
         ..note :: If replace is True, this raises a misconfiguration error
         if the directive does not already exist.
@@ -267,14 +267,20 @@ class NginxParser(object):
         :param bool replace: Whether to only replace existing directives
 
         """
-        if replace:
-            _do_for_subarray(self.parsed[filename],
-                             lambda x: self._has_server_names(x, names),
-                             lambda x: _replace_directives(x, directives))
-        else:
-            _do_for_subarray(self.parsed[filename],
-                             lambda x: self._has_server_names(x, names),
-                             lambda x: x.extend(directives))
+        _do_for_subarray(self.parsed[filename],
+                         lambda x: self._has_server_names(x, names),
+                         lambda x: _add_directives(x, directives, replace))
+
+    def add_http_directives(self, filename, directives):
+        """Adds directives to the first encountered HTTP block in filename.
+
+        :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))
 
     def get_all_certs_keys(self):
         """Gets all certs and keys in the nginx config.
@@ -463,24 +469,28 @@ def _parse_server(server):
     return parsed_server
 
 
-def _replace_directives(block, directives):
-    """Replaces directives in a block. If the directive doesn't exist in
+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.
 
     ..todo :: Find directives that are in included files.
 
     :param list block: The block to replace in
     :param list directives: The new directives.
+
     """
-    for directive in directives:
-        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:
-            raise errors.LetsEncryptMisconfigurationError(
-                'LetsEncrypt expected directive for %s in the Nginx config '
-                'but did not find it.' % directive[0])
+    if replace:
+        for directive in directives:
+            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:
+                raise errors.LetsEncryptMisconfigurationError(
+                    'LetsEncrypt expected directive for %s in the Nginx '
+                    'config but did not find it.' % directive[0])
+    else:
+        block.extend(directives)
diff --git a/letsencrypt/client/plugins/nginx/tests/configurator_test.py b/letsencrypt/client/plugins/nginx/tests/configurator_test.py
index 9399d42a6..a17fbb611 100644
--- a/letsencrypt/client/plugins/nginx/tests/configurator_test.py
+++ b/letsencrypt/client/plugins/nginx/tests/configurator_test.py
@@ -91,7 +91,7 @@ class NginxConfiguratorTest(util.NginxTest):
             self.assertEqual(results[name],
                              self.config.choose_vhost(name).names)
         for name in bad_results:
-            self.assertEqual(None, self.config.choose_vhost(name))
+            self.assertEqual(set([name]), self.config.choose_vhost(name).names)
 
     def test_more_info(self):
         self.assertTrue('nginx.conf' in self.config.more_info())
diff --git a/letsencrypt/client/plugins/nginx/tests/dvsni_test.py b/letsencrypt/client/plugins/nginx/tests/dvsni_test.py
index 15d7dbdb5..7505b3751 100644
--- a/letsencrypt/client/plugins/nginx/tests/dvsni_test.py
+++ b/letsencrypt/client/plugins/nginx/tests/dvsni_test.py
@@ -75,12 +75,12 @@ class DvsniPerformTest(util.NginxTest):
         self.assertEqual([0], self.sni.indices)
 
     @mock.patch("letsencrypt.client.plugins.nginx.configurator."
-                "NginxConfigurator.save")
-    def test_perform(self, mock_save):
+                "NginxConfigurator.choose_vhost")
+    def test_perform(self, mock_choose):
         self.sni.add_chall(self.achalls[1])
-        responses = self.sni.perform()
-        self.assertTrue(responses is None)
-        self.assertEqual(mock_save.call_count, 1)
+        mock_choose.return_value = None
+        result = self.sni.perform()
+        self.assertTrue(result is None)
 
     def test_perform0(self):
         responses = self.sni.perform()
@@ -108,30 +108,31 @@ class DvsniPerformTest(util.NginxTest):
         self.assertTrue(['include', self.sni.challenge_conf] in http[1])
 
     def test_perform2(self):
-        self.sni.add_chall(self.achalls[0])
-        self.sni.add_chall(self.achalls[2])
+        for achall in self.achalls:
+            self.sni.add_chall(achall)
 
         mock_setup_cert = mock.MagicMock(side_effect=[
             challenges.DVSNIResponse(s="nginxS0"),
-            challenges.DVSNIResponse(s="nginxS1")])
+            challenges.DVSNIResponse(s="nginxS1"),
+            challenges.DVSNIResponse(s="nginxS2")])
         # pylint: disable=protected-access
         self.sni._setup_challenge_cert = mock_setup_cert
 
         responses = self.sni.perform()
 
-        self.assertEqual(mock_setup_cert.call_count, 2)
+        self.assertEqual(mock_setup_cert.call_count, 3)
 
-        self.assertEqual(
-            mock_setup_cert.call_args_list[0], mock.call(self.achalls[0]))
-        self.assertEqual(
-            mock_setup_cert.call_args_list[1], mock.call(self.achalls[2]))
+        for index, achall in enumerate(self.achalls):
+            self.assertEqual(
+                mock_setup_cert.call_args_list[index], mock.call(achall))
 
         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.assertEqual(len(responses), 2)
-        for i in xrange(2):
+        self.assertEqual(len(responses), 3)
+        for i in xrange(3):
             self.assertEqual(responses[i].s, "nginxS%d" % i)
 
     def test_mod_config(self):
diff --git a/letsencrypt/client/plugins/nginx/tests/parser_test.py b/letsencrypt/client/plugins/nginx/tests/parser_test.py
index 21e96aa26..51ca03e5e 100644
--- a/letsencrypt/client/plugins/nginx/tests/parser_test.py
+++ b/letsencrypt/client/plugins/nginx/tests/parser_test.py
@@ -140,6 +140,16 @@ class NginxParserTest(util.NginxTest):
                           ['foo', 'bar'],
                           ['ssl_certificate', '/etc/ssl/cert2.pem']])
 
+    def test_add_http_directives(self):
+        nparser = parser.NginxParser(self.config_path, self.ssl_options)
+        filep = nparser.abs_path('nginx.conf')
+        block = [['server'],
+                 [['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)
+
     def test_replace_server_directives(self):
         nparser = parser.NginxParser(self.config_path, self.ssl_options)
         target = set(['.example.com', 'example.*'])