toolbelt
toolbelt copied to clipboard
requests
HostHeaderSSLAdapter doesn't seem to respect SNI
On a host that doesn't rely on SNI, it seems to behave correctly:
>>> s.mount('https://', HostHeaderSSLAdapter())
>>> s.get('https://208.101.49.126', headers={'Host': 'www.getsentry.com'})
<Response [200]>
But on a host that requires SNI, it doesn't seem to do the right thing.
>>> s.get('https://216.58.218.142', headers={'Host': 'google.com'})
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 487, in get
return self.request('GET', url, **kwargs)
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 475, in request
resp = self.send(prep, **send_kwargs)
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 585, in send
r = adapter.send(request, **kwargs)
File "<stdin>", line 15, in send
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/adapters.py", line 452, in send
raise SSLError(e, request=request)
requests.exceptions.SSLError: hostname 'google.com' doesn't match 'www.google.com'
>>> s.get('https://216.58.218.142', headers={'Host': 'www.google.com'})
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 487, in get
return self.request('GET', url, **kwargs)
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 475, in request
resp = self.send(prep, **send_kwargs)
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 585, in send
r = adapter.send(request, **kwargs)
File "<stdin>", line 15, in send
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/adapters.py", line 452, in send
raise SSLError(e, request=request)
requests.exceptions.SSLError: hostname 'google.com' doesn't match 'www.google.com'
I'm happy to help however I can. :)
This also seems to be dependent on the order, so something is persisting between requests.
>>> s.get('https://216.58.218.142', headers={'Host': 'www.google.com'})
<Response [200]>
>>> s.get('https://216.58.218.142', headers={'Host': 'google.com'})
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 487, in get
return self.request('GET', url, **kwargs)
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 475, in request
resp = self.send(prep, **send_kwargs)
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 606, in send
history = [resp for resp in gen] if allow_redirects else []
File "/Users/matt/.virtualenvs/sentry/lib/python2.7/site-packages/requests/sessions.py", line 113, in resolve_redirects
raise TooManyRedirects('Exceeded %s redirects.' % self.max_redirects, response=resp)
requests.exceptions.TooManyRedirects: Exceeded 30 redirects.
The latter is odd, since it's getting a redirect loop. This behavior is correct otherwise:
>>> s.get('https://google.com')
<Response [200]>
Do you have pyOpenSSL, ndg-httpsclient, and pyasn1 installed as well?
Yes.
$ pip freeze | egrep 'pyOpenSSL|ndg-httpsclient|pyasn1'
ndg-httpsclient==0.3.3
pyasn1==0.1.8
pyOpenSSL==0.15.1
Cool. Just wanted to make sure :)
I'll test this tonight or tomorrow night unless @Lukasa beats me to it.
Yes, as written that's exactly what the code does. urllib3 has no hooks for overriding the SNI used: it will always use whatever the connection pool believes that the host is, regardless of the headers. We could accept a patch to add an sni kwarg to the connection pool
Would that need to be patched in urllib3 itself to expose the API? Or can it be done in requests?
I needed a way to modify the SNI too, so I just subclassed SSLContext, and made the HTTPAdapter use an instance of my subclass:
import requests
import ssl
class MySSLContext(ssl.SSLContext):
def __new__(cls, server_hostname):
return super(MySSLContext, cls).__new__(cls, ssl.PROTOCOL_SSLv23)
def __init__(self, server_hostname):
super(MySSLContext, self).__init__(ssl.PROTOCOL_SSLv23)
self._my_server_hostname = server_hostname
def change_server_hostname(self, server_hostname):
self._my_server_hostname = server_hostname
def wrap_socket(self, *args, **kwargs):
kwargs['server_hostname'] = self._my_server_hostname
return super(MySSLContext, self).wrap_socket(*args, **kwargs)
session = requests.Session()
adapter = requests.adapters.HTTPAdapter()
context = MySSLContext(host)
adapter.init_poolmanager(10, 10, ssl_context=context)
session.mount('https://', adapter)
result = session.get(url, headers={'host': host}, verify=False)
In this case I didn't care about cert verification, I just wanted to fix 400 errors, hence the verify=False, but having looked at HostHeaderSSLAdapter, this should work with that too, and should allow you to leave verification enabled. This doesn't seem to be python3-only, but I only casually tested it just now (originally wrote this code for python3), so I don't know how well it works in all cases.
If you want to include this in requests-toolbelt, you're welcome to (feel free to change up the API a little bit). Legal crap if you plan to include it: this code is my own work, created from reading the requests and urllib3 sources and Python documentation (and probably also the Python source at some point, I forget); I license it to you under Apache License Version 2.0.
Any chance to get reviews for #293? We have been heavily using this PR's code for a few months now, and it works perfectly.
@sigmavirus24 thank you for the merge! Could it be possible to release them? Thanks!
@sigmavirus24 thank you for the merge! Could it be possible to release them? Thanks!
piggybacking on this. Would it be possible to make a release with the fix included? Much appreciated!
