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

Twisted relies on OpenSSL to configure trust roots, resulting in an empty trust store by default on Windows and non-functional HTTPS in treq #94

Open
gdude2002 opened this issue Jun 22, 2015 · 33 comments

Comments

@gdude2002
Copy link

Not sure if this is my setup, but I've just done a fresh install of Python 2.7.10 x64 and installed Twisted and Treq (and the required libs) through pip, using MingW to compile where appropriate.

Let's say I'm attempting to access https://google.com:

url = "https://google.com"

treq.get(url).addCallback(callback).addErrback(errback)

My errback gets called with an error of type ResponseNeverReceived, and iterating over error.value.reasons gives me the following Failure:

Failure: [Failure instance: Traceback: <class 'OpenSSL.SSL.Error'>: [('SSL routines', 'ssl3_get_server_certificate', 'certificate verify failed')]
C:\Python27\lib\site-packages\twisted\internet\selectreactor.py:149:_doReadOrWrite
C:\Python27\lib\site-packages\twisted\internet\tcp.py:209:doRead
C:\Python27\lib\site-packages\twisted\internet\tcp.py:215:_dataReceived
C:\Python27\lib\site-packages\twisted\protocols\tls.py:415:dataReceived
--- <exception caught here> ---
C:\Python27\lib\site-packages\twisted\protocols\tls.py:554:_write
C:\Python27\lib\site-packages\OpenSSL\SSL.py:1271:send
C:\Python27\lib\site-packages\OpenSSL\SSL.py:1187:_raise_ssl_error
C:\Python27\lib\site-packages\OpenSSL\_util.py:48:exception_from_error_queue
]

I've tried a bunch of stuff, including trusting the cacerts.pem provided by cURL in my OS's trust store, installing idna and service_identity and reinstalling OpenSSL. Any ideas on this?


System info:

  • Windows 10 Insider Preview x64 (Build 10130)
  • Python 2.7.10 x64
  • Twisted 15.2.1
  • Treq 15.0.0
@gdude2002
Copy link
Author

Decided to test with Twisted 14.0.2, seeing as there are some build failures on Travis with the latest Twisted, and..

*--- Failure #8 ---
C:\Python27\lib\site-packages\twisted\internet\selectreactor.py:149: _doReadOrWrite(...)
C:\Python27\lib\site-packages\twisted\internet\tcp.py:214: doRead(...)
C:\Python27\lib\site-packages\twisted\internet\tcp.py:220: _dataReceived(...)
C:\Python27\lib\site-packages\twisted\protocols\tls.py:415: dataReceived(...)
 [Capture of Locals and Globals disabled (use captureVars=True)]
--- <exception caught here> ---
C:\Python27\lib\site-packages\twisted\protocols\tls.py:554: _write(...)
C:\Python27\lib\site-packages\OpenSSL\SSL.py:1271: send(...)
C:\Python27\lib\site-packages\OpenSSL\SSL.py:1187: _raise_ssl_error(...)
C:\Python27\lib\site-packages\OpenSSL\_util.py:48: exception_from_error_queue(...)
 [Capture of Locals and Globals disabled (use captureVars=True)]
OpenSSL.SSL.Error: [('SSL routines', 'ssl3_get_server_certificate', 'certificate verify failed')]
*--- End of Failure #8 ---

@rakiru
Copy link

rakiru commented Jun 25, 2015

Same error here using this code:

from twisted.internet import reactor
import treq

def done(response):
    print response.code
    reactor.stop()

def err(fail):
    print fail
    for reason in fail.value.reasons:
        print reason
    reactor.stop()

treq.get("https://www.github.com").addCallback(done).addErrback(err)

reactor.run()

Windows 7 x64
Python 2.7.8 (x86 and x64)
twisted 15.2.1
treq 15.0.0
pyOpenSSL 0.15.1

It works on my Debian box, but that's on twisted 14 and treq 0.2.1. Will update this post with newer version once I've tested it, unless @gdude2002 can.

@gdude2002
Copy link
Author

Can confirm, also works for me on my Ubuntu box using your code.

Python 2.7.9 (x64)
Twisted 15.2.1
Treq 15.0.0
PyOpenSSL 0.15.1

@gdude2002
Copy link
Author

Did some further testing..

I attempted to compile OpenSSL with MingW but after several failed attempts, more than one wasted hour, and tons of bad makefile output, I gave up and downloaded a pre-build OpenSSL for x64 Windows.

As PyOpenSSL uses the location of openssl.exe to work out where the SSL libs it needs are, I decided to check what was on my PATH and found two old versions of OpenSSL there, plus the one I'd just set up.

Removing the old versions from my PATH and reinstalling PyOpenSSL in the hopes that this would fix the issue were futile however, and the problem still continues.

@glyph
Copy link
Member

glyph commented Jun 27, 2015

Hi @rakiru - I tried running your script with your documented versions of treq, pyOpenSSL, and Twisted on OS X, and it just prints 200 to stdout, as I would expect.

@glyph
Copy link
Member

glyph commented Jun 27, 2015

@gdude2002 @rakiru - I have recently heard about some issues with Twisted's HTTPS client failing with a very specific patch-level of OpenSSL. Can you report exact OpenSSL versions (including not just the version reported by openssl version but also where you got your builds from, package versions on linux, etc) and let me know if installing the latest security patches works?

@glyph
Copy link
Member

glyph commented Jun 27, 2015

I am trying to reproduce in a windows environment with the openssl bundled with cryptography but my windows development VM is not cooperating ;)

@gdude2002
Copy link
Author

@glyph We don't have any other problems with OpenSSL in Python - Even generating and verifying keys with it works just fine. It does appear to be a Windows-only issue, but I note that stuff like txrequests works fine with https verification on Windows, which is why I feel that this is a treq problem.

@glyph
Copy link
Member

glyph commented Jun 27, 2015

txrequests just puts requests into a threadpool in the background and doesn't use any of Twisted's networking, so it doesn't use Twisted's TLS integration either. But, upon reflection, I'm probably wrong, because the issue I have heard about with micro-versions of openssl mattering is mostly on obscure sites with weird ciphersuite configurations, google.com was definitely not affected.

@gdude2002
Copy link
Author

Yeah, I figured that that's what it was doing - Although now that I think of it, I'm not sure what requests actually uses for SSL verification. Might be something to look into I guess.

Well, what might be different about how Twisted does this stuff? We are using some of the SSL stuff in other parts of our app, but nothing else explicitly does (or needs) verification right now, I guess.

@glyph
Copy link
Member

glyph commented Jun 28, 2015

Are those other parts of your app using CertificateOptions and/or optionsForClientTLS?

@gdude2002
Copy link
Author

No, it's mostly SSL sockets via reactor.connectSSL(), which I'm pretty sure does no verification using the standard context factory.

@glyph
Copy link
Member

glyph commented Jun 28, 2015

The "standard" context factory? Do you mean DefaultOpenSSLContextFactory? 😢

@rakiru
Copy link

rakiru commented Jun 28, 2015

ssl.ClientContextFactory() for one thing and a custom subclass for another. The subclass is for a protocol that allows the client to authenticate with a certificate. There is no explicit certificate verification going on, so there's no chance of running into this error there. I can use OpenSSL in my own code, so it at the very least is partially working correctly.

After saying this, I realise I should have bumped up the priority of a cert verification option... Maybe I'll look into how it works here once we figure out the issue. It was never something that seemed prominently mentioned in any docs, probably because there's no high-level context factory implementation to do all this for you in twisted? Neither of our usages are HTTP related either.

Edit: Sorry for going a bit off topic. I decided it was a good idea to check my email right before bed...

@glyph
Copy link
Member

glyph commented Jun 28, 2015

It's OK to go off-topic; the more places we can get the message of "optionsForClientTLS is the Only Right Way to do it", the better :-).

The place in the documentation where I hope this is "prominently mentioned" enough (go go gadget PageRank) is the Twisted TLS HOWTO.

@glyph
Copy link
Member

glyph commented Jun 28, 2015

OK. I can reproduce the issue now, and I am surprised we didn't get a bug report until now. It's a bug in Twisted, sort of; more like, the default configuration of Cryptography on Windows. There is a workaround.

The bug is that Twisted does not include a set of trust roots of its own, or automatically interface with any other certificate bundles, such as certifi. On Linux, the solution to this is to install something that is going to work with OpenSSL' s default paths, such as the ca-certificates package on Debian. On OS X, things are a little weird, and the solution is to use the built-in OpenSSL. There's already a bug open against Twisted for using CryptoAPI's certificate store, doing this the "right way" on Windows.

The workaround is to (A) install certifi, and (B) set the environment variable SSL_CERT_FILE to point at where it installed its pem file.

If you're using bash (say, from git for windows) then this would be

$ pip install certifi
$ SSL_CERT_FILE="$(python -m certifi)" python my_application.py

or, in cmd / batch script, it would be this somewhat obscurer snippet:

C:\...> pip install certifi
C:\...> for /f %i in ('python -m certifi') do set SSL_CERT_FILE=%i
C:\...> python my_application.py

@gdude2002
Copy link
Author

@glyph Is this something we could ship with installs of our app for all platforms, or should it only be part of the Windows version?

@rakiru
Copy link

rakiru commented Jun 28, 2015

@glyph Thanks for the link. Google wasn't in the mood to give me that last night (although I think I might have got an older version where ctrl-f "verify" had no results).

@glyph
Copy link
Member

glyph commented Jun 28, 2015

@gdude2002 It's really up to you. I would personally ship it just with the Windows version, because it is better to trust the platform-configured (and, critically, platform-updated) trust roots than to have to ship security updates every time the bundle in cerifi changes.

gdude2002 added a commit to UltrosBot/Ultros that referenced this issue Jun 29, 2015
@glyph glyph changed the title HTTPS seems broken Twisted relies on OpenSSL to configure trust roots, resulting in an empty trust store by default on Windows and non-functional HTTPS in treq Jul 2, 2015
@glyph
Copy link
Member

glyph commented Jul 2, 2015

So, this isn't really a treq bug, but I am not going to close it until we have some kind of resolution from Twisted because it is still a terrible experience from treq.

@gdude2002
Copy link
Author

Appreciated; I'm kind of surprised that Twisted has had this problem for all this time, to be honest.

@glyph
Copy link
Member

glyph commented Jul 3, 2015

@gdude2002 Me too. I think the reason is that not a lot of people use Windows and not a lot of people configure SSL properly :-.

@gdude2002
Copy link
Author

@glyph It seems that twisted's networking stuff is kind of lacking in other areas in relation to Windows - but you've seen this question before ;)

Came across this one while we were also working with async DNS, and.. yeah.

@glyph
Copy link
Member

glyph commented Jul 4, 2015

@gdude2002 did you ever file that ticket against Twisted?

@gdude2002
Copy link
Author

@glyph Twisted issue tracker is scary D:

I could I guess, I only really use GitHub though

@glyph
Copy link
Member

glyph commented Jul 4, 2015

@gdude2002 Not that scary - you don't have to submit the patch, just log in and type the description of the bug :)

@gdude2002
Copy link
Author

Maybe I just like GitHub too much. This feels archaic and like the admins are leaning over your shoulder a bit. :P

Dunno if I filled everything out correctly, but here it is I guess.

@glyph
Copy link
Member

glyph commented Jul 4, 2015

I meant for the SO question you cited above.

@gdude2002
Copy link
Author

@glyph Ah, whoops. No, it's a relatively old SO question that you appear to have commented on yourself, and it's not one of mine - I'd assumed there was already a ticket.

@gdude2002
Copy link
Author

Yeah, I don't see a ticket - I'll report this one as well then, unless you're already typing one up.

@glyph
Copy link
Member

glyph commented Jul 4, 2015

Nope - thanks!

@gdude2002
Copy link
Author

@glyph
Copy link
Member

glyph commented Jul 5, 2017

I've tried to summarize the current state of things here in the Twisted tracker, since many of the original issues are closed with only part of their scope being resolved.

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

3 participants