[openstack-dev] Please do *NOT* use "vendorized" versions of anything (here: glanceclient using requests.packages.urllib3)

Donald Stufft donald at stufft.io
Fri Sep 19 16:58:56 UTC 2014


> On Sep 19, 2014, at 12:42 PM, Mark Washenberger <mark.washenberger at markwash.net> wrote:
> 
> 
> 
> On Fri, Sep 19, 2014 at 8:59 AM, Donald Stufft <donald at stufft.io <mailto:donald at stufft.io>> wrote:
> 
>> On Sep 19, 2014, at 11:54 AM, Brant Knudson <blk at acm.org <mailto:blk at acm.org>> wrote:
>> 
>> 
>> I don't think anyone would be complaining if glanceclient didn't have the need to reach into and monkeypatch requests's connection pool manager[1]. Is there a way to tell requests to build the https connections differently without monkeypatching urllib3.poolmanager?
>> 
>> glanceclient's monkeypatching of the global variable here is dangerous since it will mess with the application and every other library if the application or another library uses glanceclient.
>> 
>> [1] http://git.openstack.org/cgit/openstack/python-glanceclient/tree/glanceclient/common/https.py#n75 <http://git.openstack.org/cgit/openstack/python-glanceclient/tree/glanceclient/common/https.py#n75>
>> 
> 
> Why does it need to use it’s own VerifiedHTTPSConnection class? Ironically
> reimplementing that is probably more dangerous for security than requests
> bundling urllib3 ;)
> 
> We supported the option to skip SSL compression since before adopting requests (see 556082cd6632dbce52ccb67ace57410d61057d66), useful when uploading already compressed images.
> 

Is that all it’s used for? Probably it’s sane to just delete it then.

On Python 3.2+, 2.7.9+ Python provides the APIs to do it in the stdlib and urllib3 (and thus requests) will remove TLS Compression by default.

Python 2.6, and 2.7.0-2.7.8 do not provide the APIs to do so, however on Python 2.x if you install pyOpenSSL, ndg-httpsclient, and pyasn1 then it’ll also disable TLS compression (automatically if you use requests, you have to do an import + function call with raw urllib3).

So you can remove all that code and just let requests/urllib3 handle it on 3.2+, 2.7.9+ and for anything less than that either use conditional dependencies to have glance client depend on pyOpenSSL, ndg-httpsclient, and pyasn1 on Python 2.x, or let them be optional and if people want to disable TLS compression in those versions they can install those versions themselves.

By the way, everything above holds true for SNI as well.

This seems like the best of both worlds, glance client isn’t importing stuff from the vendored requests.packages.*, people get TLS Compression disabled (by default or optional depending on the choice the project makes), and it no longer has to maintain it’s own copy of security sensitive code.

---
Donald Stufft
PGP: 7C6B 7C5D 5E2B 6356 A926 F04F 6E3C BCE9 3372 DCFA

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140919/45779441/attachment.html>


More information about the OpenStack-dev mailing list