Merge lp:~jml/launchpadlib/ssl-creds into lp:launchpadlib

Proposed by Jonathan Lange
Status: Merged
Merged at revision: 128
Proposed branch: lp:~jml/launchpadlib/ssl-creds
Merge into: lp:launchpadlib
Diff against target: 61 lines (+31/-8)
1 file modified
src/launchpadlib/credentials.py (+31/-8)
To merge this branch: bzr merge lp:~jml/launchpadlib/ssl-creds
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+110848@code.launchpad.net

Description of the change

Same thing as https://code.launchpad.net/~jml/lazr.restfulclient/disable-ssl-cert-verification/+merge/110503 but for launchpadlib.

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

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

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)
Revision history for this message
Jonathan Lange (jml) wrote :

Good point re discoverability. I've annotated the exception.

Revision history for this message
Benji York (benji) wrote :

The extra info in the exception looks great.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/launchpadlib/credentials.py'
--- src/launchpadlib/credentials.py 2011-12-05 18:53:25 +0000
+++ src/launchpadlib/credentials.py 2012-06-18 15:52:56 +0000
@@ -61,6 +61,35 @@
61EXPLOSIVE_ERRORS = (MemoryError, KeyboardInterrupt, SystemExit)61EXPLOSIVE_ERRORS = (MemoryError, KeyboardInterrupt, SystemExit)
6262
6363
64def _ssl_certificate_validation_disabled():
65 """Whether the user has disabled SSL certificate connection.
66
67 Some testing servers have broken certificates. Rather than raising an
68 error, we allow an environment variable,
69 ``LP_DISABLE_SSL_CERTIFICATE_VALIDATION`` to disable the check.
70 """
71 # XXX: Copied from lazr/restfulclient/_browser.py. Once it appears in a
72 # released version of lazr.restfulclient, depend on that new version and
73 # delete this copy.
74 return bool(
75 os.environ.get('LP_DISABLE_SSL_CERTIFICATE_VALIDATION', False))
76
77
78def _http_post(url, headers, params):
79 """POST to ``url`` with ``headers`` and a body of urlencoded ``params``.
80
81 Wraps it up to make sure we avoid the SSL certificate validation if our
82 environment tells us to. Also, raises an error on non-200 statuses.
83 """
84 cert_disabled = _ssl_certificate_validation_disabled()
85 response, content = httplib2.Http(
86 disable_ssl_certificate_validation=cert_disabled).request(
87 url, method='POST', headers=headers, body=urlencode(params))
88 if response.status != 200:
89 raise HTTPError(response, content)
90 return response, content
91
92
64class Credentials(OAuthAuthorizer):93class Credentials(OAuthAuthorizer):
65 """Standard credentials storage and usage class.94 """Standard credentials storage and usage class.
6695
@@ -131,10 +160,7 @@
131 headers = {'Referer': web_root}160 headers = {'Referer': web_root}
132 if token_format == self.DICT_TOKEN_FORMAT:161 if token_format == self.DICT_TOKEN_FORMAT:
133 headers['Accept'] = 'application/json'162 headers['Accept'] = 'application/json'
134 response, content = httplib2.Http().request(163 response, content = _http_post(url, headers, params)
135 url, method='POST', headers=headers, body=urlencode(params))
136 if response.status != 200:
137 raise HTTPError(response, content)
138 if token_format == self.DICT_TOKEN_FORMAT:164 if token_format == self.DICT_TOKEN_FORMAT:
139 params = simplejson.loads(content)165 params = simplejson.loads(content)
140 if context is not None:166 if context is not None:
@@ -172,10 +198,7 @@
172 oauth_signature='&%s' % self._request_token.secret)198 oauth_signature='&%s' % self._request_token.secret)
173 url = web_root + access_token_page199 url = web_root + access_token_page
174 headers = {'Referer': web_root}200 headers = {'Referer': web_root}
175 response, content = httplib2.Http().request(201 response, content = _http_post(url, headers, params)
176 url, method='POST', headers=headers, body=urlencode(params))
177 if response.status != 200:
178 raise HTTPError(response, content)
179 self.access_token = AccessToken.from_string(content)202 self.access_token = AccessToken.from_string(content)
180203
181204

Subscribers

People subscribed via source and target branches