Merge master in before computing a known-good set for 0.2.0.

This also serves as a suitable base to build sdists for isnot.org, so we can try the old le-auto script against mockless versions of the LE packages.

acme/.pylintrc

@@ -100,7 +100,7 @@ comment=no
 [FORMAT]
 
 # Maximum number of characters on a single line.
-max-line-length=100
+max-line-length=80
 
 # Regexp for a line that is allowed to be longer than the limit.
 ignore-long-lines=^\s*(# )?<?https?://\S+>?$

acme/acme/challenges.py

@@ -236,10 +236,8 @@ def simple_verify(self, chall, domain, account_public_key, port=None):
 
         :param challenges.SimpleHTTP chall: Corresponding challenge.
         :param unicode domain: Domain name being verified.
-        :param account_public_key: Public key for the key pair
-            being authorized. If ``None`` key verification is not
-            performed!
-        :param JWK account_public_key:
+        :param JWK account_public_key: Public key for the key pair
+            being authorized.
         :param int port: Port used in the validation.
 
         :returns: ``True`` iff validation is successful, ``False``

acme/acme/challenges_test.py

@@ -73,7 +73,8 @@ def test_verify_wrong_thumbprint(self):
     def test_verify_wrong_form(self):
         from acme.challenges import KeyAuthorizationChallengeResponse
         response = KeyAuthorizationChallengeResponse(
-            key_authorization='.foo.oKGqedy-b-acd5eoybm2f-NVFxvyOoET5CNy3xnv8WY')
+            key_authorization='.foo.oKGqedy-b-acd5eoybm2f-'
+            'NVFxvyOoET5CNy3xnv8WY')
         self.assertFalse(response.verify(self.chall, KEY.public_key()))
 
 
@@ -273,10 +274,12 @@ def test_simple_verify_bad_key_authorization(self):
     @mock.patch('acme.challenges.TLSSNI01Response.verify_cert', autospec=True)
     def test_simple_verify(self, mock_verify_cert):
         mock_verify_cert.return_value = mock.sentinel.verification
-        self.assertEqual(mock.sentinel.verification, self.response.simple_verify(
-            self.chall, self.domain, KEY.public_key(),
-            cert=mock.sentinel.cert))
-        mock_verify_cert.assert_called_once_with(self.response, mock.sentinel.cert)
+        self.assertEqual(
+            mock.sentinel.verification, self.response.simple_verify(
+                self.chall, self.domain, KEY.public_key(),
+                cert=mock.sentinel.cert))
+        mock_verify_cert.assert_called_once_with(
+            self.response, mock.sentinel.cert)
 
     @mock.patch('acme.challenges.TLSSNI01Response.probe_cert')
     def test_simple_verify_false_on_probe_error(self, mock_probe_cert):
@@ -590,7 +593,8 @@ def test_check_validation_wrong_payload(self):
 
     def test_check_validation_wrong_fields(self):
         bad_validation = jose.JWS.sign(
-            payload=self.msg.update(token=b'x' * 20).json_dumps().encode('utf-8'),
+            payload=self.msg.update(
+                token=b'x' * 20).json_dumps().encode('utf-8'),
             alg=jose.RS256, key=KEY)
         self.assertFalse(self.msg.check_validation(
             bad_validation, KEY.public_key()))

acme/acme/client.py

@@ -66,15 +66,13 @@ def __init__(self, directory, key, alg=jose.RS256, verify_ssl=True,
     @classmethod
     def _regr_from_response(cls, response, uri=None, new_authzr_uri=None,
                             terms_of_service=None):
-        terms_of_service = (
-            response.links['terms-of-service']['url']
-            if 'terms-of-service' in response.links else terms_of_service)
+        if 'terms-of-service' in response.links:
+            terms_of_service = response.links['terms-of-service']['url']
+        if 'next' in response.links:
+            new_authzr_uri = response.links['next']['url']
 
         if new_authzr_uri is None:
-            try:
-                new_authzr_uri = response.links['next']['url']
-            except KeyError:
-                raise errors.ClientError('"next" link missing')
+            raise errors.ClientError('"next" link missing')
 
         return messages.RegistrationResource(
             body=messages.Registration.from_json(response.json()),
@@ -483,7 +481,7 @@ def revoke(self, cert):
                 'Successful revocation must return HTTP OK status')
 
 
-class ClientNetwork(object):
+class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
     """Client network."""
     JSON_CONTENT_TYPE = 'application/json'
     JSON_ERROR_CONTENT_TYPE = 'application/problem+json'
@@ -539,7 +537,7 @@ def _check_response(cls, response, content_type=None):
             # TODO: response.json() is called twice, once here, and
             # once in _get and _post clients
             jobj = response.json()
-        except ValueError as error:
+        except ValueError:
             jobj = None
 
         if not response.ok:

acme/acme/client_test.py

@@ -34,8 +34,10 @@ def setUp(self):
         self.net.get.return_value = self.response
 
         self.directory = messages.Directory({
-            messages.NewRegistration: 'https://www.letsencrypt-demo.org/acme/new-reg',
-            messages.Revocation: 'https://www.letsencrypt-demo.org/acme/revoke-cert',
+            messages.NewRegistration:
+                'https://www.letsencrypt-demo.org/acme/new-reg',
+            messages.Revocation:
+                'https://www.letsencrypt-demo.org/acme/revoke-cert',
         })
 
         from acme.client import Client
@@ -127,6 +129,13 @@ def test_query_registration(self):
         self.response.json.return_value = self.regr.body.to_json()
         self.assertEqual(self.regr, self.client.query_registration(self.regr))
 
+    def test_query_registration_updates_new_authzr_uri(self):
+        self.response.json.return_value = self.regr.body.to_json()
+        self.response.links = {'next': {'url': 'UPDATED'}}
+        self.assertEqual(
+            'UPDATED',
+            self.client.query_registration(self.regr).new_authzr_uri)
+
     def test_agree_to_tos(self):
         self.client.update_registration = mock.Mock()
         self.client.agree_to_tos(self.regr)
@@ -331,7 +340,8 @@ def request_issuance(csr, authzrs):  # pylint: disable=missing-docstring
         self.assertEqual(clock.dt, datetime.datetime(2015, 3, 27, 0, 1, 7))
 
         # CA sets invalid | TODO: move to a separate test
-        invalid_authzr = mock.MagicMock(times=[], retries=[messages.STATUS_INVALID])
+        invalid_authzr = mock.MagicMock(
+            times=[], retries=[messages.STATUS_INVALID])
         self.assertRaises(
             errors.PollError, self.client.poll_and_request_issuance,
             csr, authzrs=(invalid_authzr,), mintime=mintime)

acme/acme/messages.py

@@ -130,8 +130,9 @@ def _canon_key(cls, key):
     @classmethod
     def register(cls, resource_body_cls):
         """Register resource."""
-        assert resource_body_cls.resource_type not in cls._REGISTERED_TYPES
-        cls._REGISTERED_TYPES[resource_body_cls.resource_type] = resource_body_cls
+        resource_type = resource_body_cls.resource_type
+        assert resource_type not in cls._REGISTERED_TYPES
+        cls._REGISTERED_TYPES[resource_type] = resource_body_cls
         return resource_body_cls
 
     def __init__(self, jobj):

acme/acme/standalone_test.py

@@ -32,11 +32,10 @@ class TLSSNI01ServerTest(unittest.TestCase):
     """Test for acme.standalone.TLSSNI01Server."""
 
     def setUp(self):
-        self.certs = {
-            b'localhost': (test_util.load_pyopenssl_private_key('rsa512_key.pem'),
-                           # pylint: disable=protected-access
-                           test_util.load_cert('cert.pem')),
-        }
+        self.certs = {b'localhost': (
+            test_util.load_pyopenssl_private_key('rsa512_key.pem'),
+            test_util.load_cert('cert.pem'),
+        )}
         from acme.standalone import TLSSNI01Server
         self.server = TLSSNI01Server(("", 0), certs=self.certs)
         # pylint: disable=no-member
@@ -49,7 +48,8 @@ def tearDown(self):
 
     def test_it(self):
         host, port = self.server.socket.getsockname()[:2]
-        cert = crypto_util.probe_sni(b'localhost', host=host, port=port, timeout=1)
+        cert = crypto_util.probe_sni(
+            b'localhost', host=host, port=port, timeout=1)
         self.assertEqual(jose.ComparableX509(cert),
                          jose.ComparableX509(self.certs[b'localhost'][1]))
 
@@ -140,7 +140,8 @@ def test_it(self):
         while max_attempts:
             max_attempts -= 1
             try:
-                cert = crypto_util.probe_sni(b'localhost', b'0.0.0.0', self.port)
+                cert = crypto_util.probe_sni(
+                    b'localhost', b'0.0.0.0', self.port)
             except errors.Error:
                 self.assertTrue(max_attempts > 0, "Timeout!")
                 time.sleep(1)  # wait until thread starts

letsencrypt-apache/letsencrypt_apache/configurator.py

@@ -8,6 +8,7 @@
 import socket
 import time
 
+import zope.component
 import zope.interface
 
 from acme import challenges
@@ -709,6 +710,39 @@ def _get_ssl_vhost_path(self, non_ssl_vh_fp):
         else:
             return non_ssl_vh_fp + self.conf("le_vhost_ext")
 
+    def _sift_line(self, line):
+        """Decides whether a line should be copied to a SSL vhost.
+
+        A canonical example of when sifting a line is required:
+        When the http vhost contains a RewriteRule that unconditionally
+        redirects any request to the https version of the same site.
+        e.g:
+        RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [L,QSA,R=permanent]
+        Copying the above line to the ssl vhost would cause a
+        redirection loop.
+
+        :param str line: a line extracted from the http vhost.
+
+        :returns: True - don't copy line from http vhost to SSL vhost.
+        :rtype: bool
+
+        """
+        if not line.lstrip().startswith("RewriteRule"):
+            return False
+
+        # According to: http://httpd.apache.org/docs/2.4/rewrite/flags.html
+        # The syntax of a RewriteRule is:
+        # RewriteRule pattern target [Flag1,Flag2,Flag3]
+        # i.e. target is required, so it must exist.
+        target = line.split()[2].strip()
+
+        # target may be surrounded with quotes
+        if target[0] in ("'", '"') and target[0] == target[-1]:
+            target = target[1:-1]
+
+        # Sift line if it redirects the request to a HTTPS site
+        return target.startswith("https://")
+
     def _copy_create_ssl_vhost_skeleton(self, avail_fp, ssl_fp):
         """Copies over existing Vhost with IfModule mod_ssl.c> skeleton.
 
@@ -721,18 +755,38 @@ def _copy_create_ssl_vhost_skeleton(self, avail_fp, ssl_fp):
         # First register the creation so that it is properly removed if
         # configuration is rolled back
         self.reverter.register_file_creation(False, ssl_fp)
+        sift = False
 
         try:
             with open(avail_fp, "r") as orig_file:
                 with open(ssl_fp, "w") as new_file:
                     new_file.write("<IfModule mod_ssl.c>\n")
                     for line in orig_file:
-                        new_file.write(line)
+                        if self._sift_line(line):
+                            if not sift:
+                                new_file.write(
+                                    "# Some rewrite rules in this file were "
+                                    "were disabled on your HTTPS site,\n"
+                                    "# because they have the potential to "
+                                    "create redirection loops.\n")
+                                sift = True
+                            new_file.write("# " + line)
+                        else:
+                            new_file.write(line)
                     new_file.write("</IfModule>\n")
         except IOError:
             logger.fatal("Error writing/reading to file in make_vhost_ssl")
             raise errors.PluginError("Unable to write/read in make_vhost_ssl")
 
+        if sift:
+            reporter = zope.component.getUtility(interfaces.IReporter)
+            reporter.add_message(
+                "Some rewrite rules copied from {0} were disabled in the "
+                "vhost for your HTTPS site located at {1} because they have "
+                "the potential to create redirection loops.".format(avail_fp,
+                                                                    ssl_fp),
+                reporter.MEDIUM_PRIORITY)
+
     def _update_ssl_vhosts_addrs(self, vh_path):
         ssl_addrs = set()
         ssl_addr_p = self.aug.match(vh_path + "/arg")

letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py

@@ -847,7 +847,8 @@ def test_redirect_with_existing_rewrite(self, mock_exe, _):
 
         # Create a preexisting rewrite rule
         self.config.parser.add_dir(
-            self.vh_truth[3].path, "RewriteRule", ["Unknown"])
+            self.vh_truth[3].path, "RewriteRule", ["UnknownPattern",
+                "UnknownTarget"])
         self.config.save()
 
         # This will create an ssl vhost for letsencrypt.demo
@@ -862,7 +863,7 @@ def test_redirect_with_existing_rewrite(self, mock_exe, _):
 
         self.assertEqual(len(rw_engine), 1)
         # three args to rw_rule + 1 arg for the pre existing rewrite
-        self.assertEqual(len(rw_rule), 4)
+        self.assertEqual(len(rw_rule), 5)
 
         self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path))
         self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path))
@@ -911,6 +912,44 @@ def test_create_own_redirect_for_old_apache_version(self):
         self.config._enable_redirect(self.vh_truth[1], "")  # pylint: disable=protected-access
         self.assertEqual(len(self.config.vhosts), 7)
 
+    def test_sift_line(self):
+        # pylint: disable=protected-access
+        small_quoted_target = "RewriteRule ^ \"http://\""
+        self.assertFalse(self.config._sift_line(small_quoted_target))
+
+        https_target = "RewriteRule ^ https://satoshi"
+        self.assertTrue(self.config._sift_line(https_target))
+
+        normal_target = "RewriteRule ^/(.*) http://www.a.com:1234/$1 [L,R]"
+        self.assertFalse(self.config._sift_line(normal_target))
+
+    @mock.patch("letsencrypt_apache.configurator.zope.component.getUtility")
+    def test_make_vhost_ssl_with_existing_rewrite_rule(self, mock_get_utility):
+        self.config.parser.modules.add("rewrite_module")
+
+        http_vhost = self.vh_truth[0]
+
+        self.config.parser.add_dir(
+            http_vhost.path, "RewriteEngine", "on")
+
+        self.config.parser.add_dir(
+                http_vhost.path, "RewriteRule",
+                ["^",
+                "https://%{SERVER_NAME}%{REQUEST_URI}",
+                "[L,QSA,R=permanent]"])
+        self.config.save()
+
+        ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])
+
+        self.assertTrue(self.config.parser.find_dir(
+            "RewriteEngine", "on", ssl_vhost.path, False))
+
+        conf_text = open(ssl_vhost.filep).read()
+        commented_rewrite_rule = \
+        "# RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [L,QSA,R=permanent]"
+        self.assertTrue(commented_rewrite_rule in conf_text)
+        mock_get_utility().add_message.assert_called_once_with(mock.ANY,
+                                                               mock.ANY)
 
     def get_achalls(self):
         """Return testing achallenges."""