Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HostHeaderSSLAdapter doesn't seem to respect SNI #159

Open
mattrobenolt opened this issue Jul 25, 2016 · 14 comments
Open

HostHeaderSSLAdapter doesn't seem to respect SNI #159

mattrobenolt opened this issue Jul 25, 2016 · 14 comments

Comments

@mattrobenolt
Copy link

mattrobenolt commented Jul 25, 2016

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. :)

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/36339780-hostheaderssladapter-doesn-t-seem-to-respect-sni?utm_campaign=plugin&utm_content=tracker%2F418367&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F418367&utm_medium=issues&utm_source=github).
@sigmavirus24
Copy link
Collaborator

Do you have pyOpenSSL, ndg-httpsclient, and pyasn1 installed as well?

@mattrobenolt
Copy link
Author

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]>

@mattrobenolt
Copy link
Author

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

@sigmavirus24
Copy link
Collaborator

Cool. Just wanted to make sure :)

I'll test this tonight or tomorrow night unless @Lukasa beats me to it.

@mattrobenolt
Copy link
Author

And for posterity, latest requests, 2.10.0.

@Lukasa
Copy link
Member

Lukasa commented Jul 26, 2016

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

@mattrobenolt
Copy link
Author

Would that need to be patched in urllib3 itself to expose the API? Or can it be done in requests?

@Lukasa
Copy link
Member

Lukasa commented Jul 26, 2016

It would need to be done in urllib3.

@mattrobenolt
Copy link
Author

Awesome, thanks. 👍 I'll take a look over on that side then!

@dwfreed
Copy link

dwfreed commented Apr 20, 2017

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.

@NyanKiyoshi
Copy link

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.

@NyanKiyoshi
Copy link

@sigmavirus24 thank you for the merge! Could it be possible to release them? Thanks!

@ian28223
Copy link

@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!

@imackintouch
Copy link

I tried the code fix in PR 293 and still ended up with an error like this:

requests.exceptions.SSLError: HTTPSConnectionPool(host='host.xyz.com', port=443): Max retries exceeded with url: /dlps/20071001/ps (Caused by SSLError(SSLCertVerificationError(1, "[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch, certificate is not valid for 'host.xyz.com'. (_ssl.c:1007)")))

host.xyz.com is basically acting as a proxy in front of the actual web server that is being accessed.

I am very, very confused.

My code call looks like this:

headers['Host'] = 'realhostname.com'
session.mount('https://', host_header_ssl.HostHeaderSSLAdapter())
response = session.post("https://host.xyz.com", headers=headers, data=somedata, timeout=some_timeout, verify=True)

Is there just no way to do this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants