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

TLS SNI Support #749

Closed
pythonmobile opened this issue Aug 1, 2012 · 47 comments
Closed

TLS SNI Support #749

pythonmobile opened this issue Aug 1, 2012 · 47 comments

Comments

@pythonmobile
Copy link

It seems I'm having problems using requests with servers that need TLS SNI support. When will requests have this feature? Is this something that is planned?

Thanks,
--Ram

@mitsuhiko
Copy link
Contributor

Not posisble in Python 2.x on top of the stdlib's ssl module.

@pythonmobile
Copy link
Author

Twisted has it, right? Perhaps time to depend on pyopenssl or some other ssl module?

@shazow
Copy link
Contributor

shazow commented Aug 2, 2012

There's a pull request for urllib3 adding SNI support for Py32+, but still needs test coverage. urllib3/urllib3#89

@pythonmobile
Copy link
Author

shzow: Thanks for the pointer. I'm unfortunately using Twisted/Flask, which means I'm stuck to 2.7. Twisted it seems can do something like this, but i've not seen examples, and it would need pyopenssl + twisted decoding to do this. Am not an expert in either to pull this off. At this point, just thinking of wrapping some command line utility like wget... :(

@mitsuhiko
Copy link
Contributor

PyOpenSSL should be able to do it on 2.x. So there is hope.

@pythonmobile
Copy link
Author

I could make it work with pyopenssl. But I still am having a hard time getting it done in twisted correctly. http://pbin.be/show/719/ -- kind of works. But then one has to build an API on it for it to be any useful. I still cant figure out how to pass the correct context to Agent.

@Lukasa Lukasa closed this as completed Nov 17, 2012
@pythonmobile
Copy link
Author

Lukasa: was this resolved?

@Lukasa
Copy link
Member

Lukasa commented Dec 16, 2012

No. We're blocked on urllib3/urllib3#89.

@shazow
Copy link
Contributor

shazow commented Dec 16, 2012

Merged. Carry on. :)

@Lukasa Lukasa reopened this Dec 16, 2012
@shazow
Copy link
Contributor

shazow commented Dec 16, 2012

Note that it's only supported on Py32+ for now. Should be enough to get started though.

@shazow
Copy link
Contributor

shazow commented Dec 16, 2012

Another fair warning: Some tests on Py32 are failing on urllib3 master right now. Some help resolving this would be greatly appreciated (and might affect this functionality): urllib3/urllib3#128

@t-8ch
Copy link
Contributor

t-8ch commented Dec 16, 2012

works for me on archlinux x64 on py26, py27, py32, py33

@shazow
Copy link
Contributor

shazow commented Dec 16, 2012

Ah I should have qualified that this is on OSX. Scumbag OSX.

@pythonmobile
Copy link
Author

t-8ch: Are the sni tests passing on py26/27/32 and 33 for you?

@t-8ch
Copy link
Contributor

t-8ch commented Dec 19, 2012

The SNI tests aren't even run on py2, but on py32 and py33 it works.
There is no way to do SNI with the ssl module of the standard library before py32.

@Lukasa
Copy link
Member

Lukasa commented Jan 21, 2013

@shazow: Presumably you have no interest in making urllib3 depend on PyOpenSSL?

@shazow
Copy link
Contributor

shazow commented Jan 21, 2013

I'd be happy to have a separate library which adds SSL via PyOpenSSL support to urllib3.

@sigmavirus24
Copy link
Contributor

It looks like @t-8ch is knocking it out of the park with the work on urllib3. I'll let him tell us when this can be closed.

@WhyNotHugo
Copy link
Contributor

urllib3 has optional support for SNI with python2, though with a few optional dependencies. (See urllib3#156).

I belive this should work using:

from requests.packages.urllib3.contrib import pyopenssl
pyopenssl.inject_into_urllib3

After adding this to packages in setup.py

requests.packages.urllib3.contrib

Note that the contrib package is not imported by default.

There are basically two alternatives to enable SNI in requests:

  • Create function to manually enable SNI that basically runs the above code. (the downside is that users will need to explicitly enable SNI support).
  • Try to import the pre-requisites. If they exists, enable SNI, else don't.

I've no problem implementing any of both alternatives, though I'd prefer to know which would be the prefer one for requests. I personally vote for the second alternative.

BTW: can we reopen the issue, now that we have support from urllib3?

@sigmavirus24
Copy link
Contributor

I actually prefer the former because the people who need it are the ones who know they'll need it and they're few enough that it won't cause too many complaints. However, if we're going to be consistent with the existing API, the latter would be the way to implement it. Currently, urllib3 (and requests) will provide the API to send HTTPS requests without the ssl module present but will fail in the case where the ssl module is not available. In other words, the API is there and you can use it but it just won't work and that is indicated by an exception.

As to re-opening this, that's up to @kennethreitz. The issue of course (with all of this) is that we're currently under a feature freeze (#1165, #1168) so I'm not sure if the work is even worth doing because it might not be accepted.

@WhyNotHugo
Copy link
Contributor

Let me be a bit clearer: what I meant by the second alternative was "Try to use SNI if the optional deps are available, but still use SSL as we always have it they aren't.". This shouldn't break anything existing up to now.

As for reopening (or not) the issue, IMHO, this issue shouldn't be taken lightly. requests can become useless without SNI support for many scenarios. In particular, multiple domains on a single IPv4 host without SNI become impossible. And additional IPv4 addresses are out of the questions for many users (due to their cost).

In any case, this is half feature, half bug, though I'll await @kennethreitz's reply regarding reopening the issue.

@pythonmobile
Copy link
Author

Hugo: I just did pip install -U requests, and when I can't import contrib.
How do I fix that? (Importerror : No module named contrib).

On Fri, May 3, 2013 at 1:12 PM, Hugo Osvaldo Barrera <
[email protected]> wrote:

urllib3 has optional support for SNI with python2, though with a few
optional dependencies. (See urllib3#156urllib3/urllib3#156
).

I belive this should work using:

from requests.packages.urllib3.contrib import pyopenssl
pyopenssl.inject_into_urllib3

Note that the contrib package is not imported by default.

There are basically two alternatives to enable SNI in requests:

  • Create function to manually enable SNI that basically runs the above
    code. (the downside is that users will need to explicitly enable SNI
    support).
  • Try to import the pre-requisites. If they exists, enable SNI, else
    don't.

I've no problem implementing any of both alternatives, though I'd prefer
to know which would be the prefer one for requests. I personally vote for
the second alternative.


Reply to this email directly or view it on GitHubhttps://github.com/kennethreitz/requests/issues/749#issuecomment-17406518
.

@WhyNotHugo
Copy link
Contributor

request's setup.py doesn't include that package (the source it's there, but it's excluded when installing/packaging), so it's not installed, that's why I mentioned:

After adding this to packages in setup.py
requests.packages.urllib3.contrib

You'll basically need to install from source.
Check out my pull request if you're interested, making this work is just a couple of lines. :)

@sigmavirus24
Copy link
Contributor

"Try to use SNI if the optional deps are available, but still use SSL as we always have it they aren't."

SSL is not always there though. That was my point. Currently we try and use it but fail hard and fast if we don't have it and a user tries to make an https request. The way you had described it though, I was under the impression SNI would require the user to pass some extra notion to urllib3 and you were focusing on just the set-up for it. If all that is necessary is #1347 then I'm 100% behind this.

@pythonmobile
Copy link
Author

It would be really nice if this was the default (SNI Support) and requests
just works without any extra code or setup from source. For urllib3 its 2
lines of code. Why not just add that to requests. Am I day dreaming? :)

On Fri, May 3, 2013 at 10:43 PM, Ian Cordasco [email protected]:

"Try to use SNI if the optional deps are available, but still use SSL as
we always have it they aren't."

SSL is not always there though. That was my point. Currently we try and
use it but fail hard and fast if we don't have it and a user tries to make
an https request. The way you had described it though, I was under the
impression SNI would require the user to pass some extra notion to urllib3
and you were focusing on just the set-up for it. If all that is necessary
is #1347 https://github.com/kennethreitz/requests/issues/1347 then I'm
100% behind this.


Reply to this email directly or view it on GitHubhttps://github.com/kennethreitz/requests/issues/749#issuecomment-17426626
.

tradej referenced this issue in tradej/pykickstart Feb 13, 2015
Because urlgrabber is being dropped by many significant software projects in
Fedora, Pykickstart can't keep depending on it once it is converted to Python 3
(rhbz#985310).

This commit replaces the dep with requests with SSL verification turned on by
default. This could pose trouble in Python 2 if proper underlying packages
aren't installed [1]. In case this becomes a problem, the SSL_VERIFY constant
in constants.py may be switched to False.

[1] https://github.com/kennethreitz/requests/issues/749#issuecomment-19187417
tradej referenced this issue in tradej/pykickstart Feb 13, 2015
Because urlgrabber is being dropped by many significant software projects in
Fedora, Pykickstart can't keep depending on it once it is converted to Python 3
(rhbz#985310).

This commit replaces the dep with requests with SSL verification turned on by
default. This could pose trouble in Python 2 if proper underlying packages
aren't installed [1]. In case this becomes a problem, the SSL_VERIFY constant
in constants.py may be switched to False.

[1] https://github.com/kennethreitz/requests/issues/749#issuecomment-19187417
tradej referenced this issue in tradej/pykickstart Feb 13, 2015
Because urlgrabber is being dropped by many significant software projects in
Fedora, Pykickstart can't keep depending on it once it is converted to Python 3
(rhbz#985310).

This commit replaces the dep with requests with SSL verification turned on by
default. This could pose trouble in Python 2 if proper underlying packages
aren't installed [1]. In case this becomes a problem, the SSL_VERIFY constant
in constants.py may be switched to False.

[1] https://github.com/kennethreitz/requests/issues/749#issuecomment-19187417
tradej referenced this issue in tradej/pykickstart Feb 16, 2015
Because urlgrabber is being dropped by many significant software projects in
Fedora, Pykickstart can't keep depending on it once it is converted to Python 3
(rhbz#985310).

This commit replaces the dep with requests with SSL verification turned on by
default. This could pose trouble in Python 2 if proper underlying packages
aren't installed [1]. In case this becomes a problem, the SSL_VERIFY constant
in load.py may be switched to False.

[1] https://github.com/kennethreitz/requests/issues/749#issuecomment-19187417
tradej referenced this issue in tradej/pykickstart Feb 17, 2015
Because urlgrabber is being dropped by many significant software projects in
Fedora, Pykickstart can't keep depending on it once it is converted to Python 3
(rhbz#985310).

This commit replaces the dep with requests with SSL verification turned on by
default. This could pose trouble in Python 2 if proper underlying packages
aren't installed [1]. In case this becomes a problem, the SSL_VERIFY constant
in load.py may be switched to False.

[1] https://github.com/kennethreitz/requests/issues/749#issuecomment-19187417
tradej referenced this issue in tradej/pykickstart Feb 17, 2015
Because urlgrabber is being dropped by many significant software projects in
Fedora, Pykickstart can't keep depending on it once it is converted to Python 3
(rhbz#985310).

This commit replaces the dep with requests with SSL verification turned on by
default. This could pose trouble in Python 2 if proper underlying packages
aren't installed [1]. In case this becomes a problem, the SSL_VERIFY constant
in load.py may be switched to False.

[1] https://github.com/kennethreitz/requests/issues/749#issuecomment-19187417
remh referenced this issue in DataDog/dd-agent Feb 26, 2015
Fixes

* Fix #1196
* Fix #1398

Along with the self contained agent (pyopenssl etc) it will support SNI.

See https://github.com/kennethreitz/requests/issues/749 for more
information
remh referenced this issue in DataDog/dd-agent Feb 27, 2015
* Fix #1196
* Fix #1398

Along with the self contained agent (pyopenssl etc) it will support SNI.

See https://github.com/kennethreitz/requests/issues/749 for more information
dalf referenced this issue in searx/searx Jan 6, 2017
dalf referenced this issue in searx/searx Jan 6, 2017
@tekram
Copy link

tekram commented Feb 18, 2019

I am running Python 2.7.6 and can not install pyOpenSSL. I have tried the work arounds in the htis post -> https://stackoverflow.com/questions/18578439/using-requests-with-tls-doesnt-give-sni-support/18579484#18579484

I can not also upgrade by Python. Any other solutions?

@JustinMaxwell
Copy link

JustinMaxwell commented Feb 24, 2020

I'm requesting we reconsider the unconditional use (if present) of "inject_into_urllib3()".

Added 7 years ago, the goal was to "add SNI support for Python 2"

"urllib3.contrib.pyopenssl.inject_into_urllib3()" is described to "Monkey-patch urllib3 with PyOpenSSL-backed SSL-support.":
(https://github.com/urllib3/urllib3/blob/master/src/urllib3/contrib/pyopenssl.py)

Justification:
1: Security/stability: Monkey patching to swap out standard library use for a 3rd party library should arguably be more surgical, especially for ssl. If the patch is for support in Python2, then at least check for a major version. Or even better, check if SNI support is already present ssl.HAS_SNI.

2: Unnecessary: As of Dec. 10, 2014, the entirety of Python 3.4's ssl module has been backported for Python 2.7.9. See PEP 466 for justification. https://www.python.org/downloads/release/python-279/. This enables SNI support in the standard library for > v2.7.9

3: Inflexible: As implemented, there is no way to disable this behavior. The only option to prevent requests use of pyopenssl context is to uninstall pyopenssl for my entire python environment.

@Tectract
Copy link

Hi guys.

Sorry to rehash an old thread but this just became relevant for a lot of users again, because python2.7 support has reached end-of-life and specifically SSLv2 cert and non-SNI requests are being dropped from a lot of servers this year.

I'm now getting this error with my python2.7.6 stack:
[Errno 1] _ssl.c:510: error:14077410:SSL routines:SSL23_GET_SERVER_HELLO:sslv3 alert handshake failure

Sadly, nothing I'm trying is working, including installing various versions of requests and requests[security] via pip.

Is there some way to recover SNI support in python2.7.6 / pip and get past this TLS handshake failure? Upgrading to a different version of python or a different operating system are not an option for me right now.

@psf psf locked as resolved and limited conversation to collaborators Feb 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests