Merge lp:~jml/lazr.restfulclient/disable-ssl-cert-verification into lp:lazr.restfulclient

[Jonathan Lange]

The SSL certificates for launchpad.dev and dogfood are invalid, but we still want to test against them. Add an environment variable to disable certificate checking.

<jml> oh right.
<jml> I can't seem to use the API against launchpad.dev
<jml> I get an error about SSL.
<wgrant> Yeah, cert verification will break that.
<jml> httplib2.SSLHandshakeError: [Errno 1] _ssl.c:504: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed
<jml> wgrant: is there a work around?
<wgrant> I either hack launchpadlib to not verify the cert, or generate a new cert for launchpad.dev and tell it to trust that.
<cjwatson> I often end up hacking httplib2.
<cjwatson> (search for disable_ssl and change False defaults to True)
<cjwatson> I wish there were an environment variable override.
<cjwatson> dogfood has the same problem.
<jml> wgrant, cjwatson: thanks.
<wgrant> jml: In lazr.restfulclient._browser.RestfulHttp.__init__, add disable_ssl_certificate_validation=True to the super call
<jml> wgrant: even better, thanks.
<cjwatson> There are three sites you need to change, IME.
<cjwatson> I always forget the full list.
<cjwatson> Which is why I've started editing httplib2 instead.
<wgrant> Yeah
<wgrant> Trial and error works for me...
<cjwatson> Because not all the call sites are in the same file.
<wgrant> Yep

[Jonathan Lange (jml)]

New changes factor the code slightly better. I haven't added tests.

[Jonathan Lange (jml)]

This needs https://code.launchpad.net/~jml/launchpadlib/ssl-creds/+merge/110848 to work properly

[Benji York (benji)]

Looks good.

It's too bad that the environment variable isn't discoverable. Maybe we could annotate SSH certificate errors with a message about how to disable validation. Or we could just whitelist the development sites with known-bad certs and the user won't see the errors to start with.

review: Approve (code)

[lp:~jml/lazr.restfulclient/disable-ssl-cert-verification]

132. By Jonathan Lange on 2012-06-19

Make the env var discoverable.

[Benji York (benji)]

The extra info in the exception looks great.

=== modified file 'src/lazr/restfulclient/_browser.py'
--- src/lazr/restfulclient/_browser.py	2012-03-25 14:19:29 +0000
+++ src/lazr/restfulclient/_browser.py	2012-06-19 11:46:20 +0000
@@ -27,6 +27,7 @@
 __all__ = [
     'Browser',
     'RestfulHttp',
+    'ssl_certificate_validation_disabled',
     ]
 
 import atexit
@@ -40,6 +41,7 @@
 from time import sleep
 from httplib2 import (
     Http,
+    SSLHandshakeError,
     urlnorm,
     )
 import simplejson
@@ -97,6 +99,17 @@
     return ",".join((filename, filemd5))
 
 
+def ssl_certificate_validation_disabled():
+    """Whether the user has disabled SSL certificate connection.
+
+    Some testing servers have broken certificates.  Rather than raising an
+    error, we allow an environment variable,
+    ``LP_DISABLE_SSL_CERTIFICATE_VALIDATION`` to disable the check.
+    """
+    return bool(
+        os.environ.get('LP_DISABLE_SSL_CERTIFICATE_VALIDATION', False))
+
+
 class RestfulHttp(Http):
     """An Http subclass with some custom behavior.
 
@@ -109,7 +122,10 @@
 
     def __init__(self, authorizer=None, cache=None, timeout=None,
                  proxy_info=None):
-        super(RestfulHttp, self).__init__(cache, timeout, proxy_info)
+        cert_disabled = ssl_certificate_validation_disabled()
+        super(RestfulHttp, self).__init__(
+            cache, timeout, proxy_info,
+            disable_ssl_certificate_validation=cert_disabled)
         self.authorizer = authorizer
         if self.authorizer is not None:
             self.authorizer.authorizeSession(self)
@@ -333,8 +349,17 @@
         if extra_headers is not None:
             headers.update(extra_headers)
         # Make the request.
-        response, content = self._request_and_retry(
-            str(url), method=method, body=data, headers=headers)
+        try:
+            response, content = self._request_and_retry(
+                str(url), method=method, body=data, headers=headers)
+        except SSLHandshakeError, e:
+            msg = str(e)
+            if "SSL3_GET_SERVER_CERTIFICATE:certificate verify failed" in msg:
+                raise SSLHandshakeError(
+                    "%s: perhaps set LP_DISABLE_SSL_CERTIFICATE_VALIDATION if "
+                    "certificate validation failed and you genuinely trust the "
+                    "remote server." % (e,))
+            raise
         if response.status == 304:
             # The resource didn't change.
             if content == '':