-
Notifications
You must be signed in to change notification settings - Fork 183
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 appear to handle alternate ports #189
Comments
Can you provide actual code that reproduces this? We do not force a port and the port should be pulled from the URL that you're providing. All the adapter really does is this and leaves the rest up to urllib3. |
Apparently this hasn't been reproduced and there's been no update in 5 months. I'm closing this until we know more |
Sorry for the long delay. I needed some time to organize my thoughts on this, and I did it through a module I wrote, and I outlined troubleshooting steps here: https://github.com/JohnOmernik/jupyter_drill/blob/master/issue.md That said, to summarize Normal SSL Connection: Works fine (but due to DNS look ups, connections switch servers, losing session) Pin to IP address: When I change the URL to use an IP so it stays on a single IP, then my certificate doesn't match Pin to IP and use HostHeaderSSLAdapter with just hostname in Host header: Instead of trying to connect to IP:port, it now connects to port 443 (odd as that is not specified anywhere) Pin to IP and put the port in the Host header (mydomain.com:1234): This seems to connect, but now the certificate CN doesn't match as cert is mydomain.com and Host header is mydomain.com:1234. You can see all the examples and full tracebacks in the issue.md above and the code I am using is here. https://github.com/johnomernik/jupyter_drill Thanks! |
FYI: I am not sure how to reopen this, or should I do new issue? |
I've reopened this but in the interest of full transparency, I probably won't have time to investigate the behaviour for quite a while. |
I appreciate it. For me it's 1 part wanting to get this fixed and 2 parts curiosity. Based on the code in requests toolbelt, I don't think the issue is there, but I want to dig into requests, and urllib3 to determine if there is an improvement that could be made. Thus I need help, I tried to dig through all my options, and ran into walls. Hence starting here. |
I've done a lot of leg work and the issue comes back to when a page responds with a HTTP 303 and allow_redirects is set to true (the Default in requests). Essentially, since we are using the hostheader for the certificate authorization, and changing from the IP to the name, when the server provides a 303 redirect, it uses the hostheader in it's redirect, which doesn't have the port, and thus it returns a redirect to the name and to port 443. Perhaps this can be addressed by allowing the setting of a custom header to use the alternative name SSL verification rather than using the host header? URLLIB3 Issue: urllib3/urllib3#1411 Requests Issue: psf/requests#4730 |
Hm, what do you mean by
Do you mean checking the subjectAlternativeName (SAN) on the cert? |
No, if you look at the
Requests toolbelt is looking for a header named host, if it exists, it saying "let's assert that the host name we should validate for the purposes of SSL is this host header value" That's well and good, but the Host header is also being used by the server in crafting the 303 redirect. Basically, the server is taking what is provided by the host header, and making the redirect match the host header, but in reality we want the redirect to be going to the IP:port combination that's in the original URL. So, if we set a custom header client side like "ssl-hostname" and then requests toolbelt checked for that FIRST, if found, use that hostname for SSL Verification, if not found, use host (to keep from breaking others who are using host without the redirect). If you used ssl-hostname as the assert hostname, then the server would essentially ignore this header, and use the original host header crafted from the url. while the client-side ssl verification in urllib3 would use the value provided for ssl . Win win? |
Oooooh, I see. Yeah, something like that could definitely work. What if we also made it configurable, e.g.,
Where |
Strip_header would essentially remove the header as it sets it for
assert_hostname? Ya, that makes sense! And yes, the defaults make sense.
…On Wed, Jul 11, 2018 at 6:59 AM, Ian Stapleton Cordasco < ***@***.***> wrote:
Oooooh, I see. Yeah, something like that could definitely work. What if we
also made it configurable, e.g.,
session.mount(HostHeaderSSLAdapter(use_header='my-header-name', strip_header=True))
Where use_header would default to host and strip_header would default to
False to preserve backwards compatibility. I like this idea. It should
also clear things up by making the usage clearer.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#189 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB1zUc0fd5CEm7EvT3wJ-iOr9jRNBiIDks5uFei1gaJpZM4OCzfL>
.
|
@JohnOmernik I'd happily merge a PR that looks like that, especially if it includes tests :) |
Ooh boy, that would be really neat if my programming skills were up to that :) I've not really used git hub very much. I need to learn how to to do PR to someone elses repo (I'm really a noob, not really a programmer, more of a security guy) That said, I took a whack at the host_header_ssl.py. I changed some variable names, copied the init and class variables using other adapters as examples. I think what I have below should work. I tried to look at the tests stuff, but I couldn't just copy what you had for host ssl header tests because it looks like the individual tests instantiate the mounted adapter the same way. If each test mounted the adapter, I'd have tossed in some tests, here is the code I tried to put together, it should do what we talked about... sorry for my lack of git/python skills, tracking this problem down really pushed me to the edges of my capabilities. # -*- coding: utf-8 -*-
"""
requests_toolbelt.adapters.host_header_ssl
==========================================
This file contains an implementation of the HostHeaderSSLAdapter.
"""
from requests.adapters import HTTPAdapter
class HostHeaderSSLAdapter(HTTPAdapter):
"""
A HTTPS Adapter for Python Requests that sets the hostname for certificate
verification based on the Host header.
This allows requesting the IP address directly via HTTPS without getting
a "hostname doesn't match" exception.
Example usage:
>>> s.mount('https://', HostHeaderSSLAdapter())
>>> s.get("https://93.184.216.34", headers={"Host": "example.org"})
"""
__attrs__ = HTTPAdapter.__attrs__ + ['ssl_use_header']
__attrs__ = HTTPAdapter.__attrs__ + ['ssl_strip_header']
def __init__(self, ssl_use_header='host', ssl_strip_header=False, **kwargs):
self.ssl_use_header = ssl_use_header
self.ssl_strip_header = ssl_strip_header
super(HostHeaderSSLAdapter, self).__init__(**kwargs)
def send(self, request, **kwargs):
# HTTP headers are case-insensitive (RFC 7230)
ssl_host = None
ssl_header = None
for header in request.headers:
if header.lower() == self.ssl_use_header.lower():
ssl_header = header
ssl_host = request.headers[header]
break
if self.ssl_strip_header == True and ssl_header is not None:
del request.headers[ssl_header]
connection_pool_kwargs = self.poolmanager.connection_pool_kw
if ssl_host:
connection_pool_kwargs["assert_hostname"] = ssl_host
elif "assert_hostname" in connection_pool_kwargs:
# an assert_hostname from a previous request may have been left
connection_pool_kwargs.pop("assert_hostname", None)
return super(HostHeaderSSLAdapter, self).send(request, **kwargs) |
Hopefully https://help.github.com/articles/working-with-forks/ and https://help.github.com/articles/proposing-changes-to-your-work-with-pull-requests/ will be useful :) I have confidence that you can handle making a PR. Let me know if I can be of assistance in helping you do that. :) |
I am looking to use HostHeaderSSLAdapter to handle a rare case where I want to I have an internal hostname that's load balanced via DNS, but doesn't share sessions between nodes. Thus, I would like to use this to upon instantiation of my requests.session to do a lookup, pick one of the IPs returned at that time, and make requests send all requests to that IP address while still using the Hostname verification in the HostHeaderSSLAdapter. (i.e. I want SSL to work, while pinning the script to single IP for that session).
Example:
I have a hostname
myhosts.mydomain.com
that has three instances of a website/application running on it on nodes 192.168.0.1, 192.168.0.2, and 192.168.0.3.
When I connect, prior to setting up my session, I do a socker.getipbyhost on myhosts.mydomain.com. I get back a single IP, say 192.168.0.2
Then, I replace my url, instead of
https://myhosts.mydomain.com
I will connect to
https://192.168.0.2 for all future connections.
Using HostHeaderSSLAdapater, and setting my Hosts header, I can still have SSL validate properly.
However
If my original hostname is
https://myhosts.mydomain.com:12345
And I break out the host, look it up, get 192.168.0.2 and now make my url to be
https://192.168.0.2:12345 while setting the Hosts header to be myhosts.mydomain.com, I get a connection refused and the traceback below which seems to indicate it's using port 443 even though I specified port 12345.
ConnectionError: HTTPSConnectionPool(host='myhosts.mydomain.com', port=443): Max retries exceeded with url: / (Caused by NewConnectionError('<requests.packages.urllib3.connection.VerifiedHTTPSConnection object at 0x7fc256d180f0>: Failed to establish a new connection: [Errno 111] Connection refused',))
(That server is not listening on port 443, so that makes sense that's it a connection refused)
Any thoughts on getting that port into this adapter?
Thanks!
John
The text was updated successfully, but these errors were encountered: