Re: [Twisted-Python] Twisted 16.3.0 Prerelease 2 Announcement

2016-07-12 Thread Paweł Miech
> In an earlier e-mail you mentioned that you were using Python 3.  Is that
still true?

I can reproduce this in Python 2.7.11 and Python 3.5.2. In both of them
Chrome responds with ERR_SPDY_INADEQUATE_TRANSPORT_SECURITY.  When I test
with curl with verbose flag I see that it also shows information about
ciphers used:

Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH

...
SSL connection using TLSv1.2 / AES256-GCM-SHA384


I see ciphers are set at this point here:
https://github.com/twisted/twisted/blob/556f0f24df2eba2f38ec7f0fa422c4aa7df07fec/twisted/internet/_sslverify.py#L1660
and Twisted cipher is described here:
https://github.com/twisted/twisted/blob/556f0f24df2eba2f38ec7f0fa422c4aa7df07fec/twisted/internet/_sslverify.py#L1851
so probably this is the area to look for in case there is something going
awry in setting ciphers.

One thing to note is that I use DefaultOpenSSLContextFactory and do
something like this:

context_factory = DefaultOpenSSLContextFactory("key.pem", "cert.pem")
reactor.listenSSL(8080, site, context_factory)

Twisted docs for SSL
https://twistedmatrix.com/documents/current/core/howto/ssl.html suggest to
try something like this:

certData = getModule(__name__).filePath.sibling('server.pem').getContent()
certificate = ssl.PrivateCertificate.loadPEM(certData)
factory = protocol.Factory.forProtocol(echoserv.Echo)
reactor.listenSSL(8000, factory, certificate.options())

but those code samples from docs appeared broken. I was not able to run
them I was planning to review those docs later, find out what is wrong and
create PR for that.

Is it possible that using DefaultOpenSSLContextFactory instead of
certificate.options() affects something here? I can see my Twisted-SSL code
works ok in Chrome with HTTP 1.1 ( I can see green "secure" icon in url bar
and confirm that requests flies all right with ssl in dev tools) only fails
with HTTP2. This seems to suggest that using DefaultSSLContextFactory is ok
(even if it's not documented officially),  but maybe execution path is
different for contextFactory and certificate.options()?


2016-07-12 1:47 GMT+02:00 Glyph Lefkowitz :

>
> On Jul 11, 2016, at 4:42 PM, Craig Rodrigues 
> wrote:
>
> In an earlier e-mail you mentioned that you were using Python 3.  Is that
> still true?
>
>
> Seconded - it would be very interesting to know if switching to python 2
> fixes your issue. :)
>
> ___
> Twisted-Python mailing list
> Twisted-Python@twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
>
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


Re: [Twisted-Python] Twisted 16.3.0 Prerelease 2 Announcement

2016-07-12 Thread Cory Benfield

> On 11 Jul 2016, at 22:04, Paweł Miech  wrote:
> 
> This seems to suggest that Ubuntu 16.04 (the system I'm testing) does not 
> support ciphers required by HTTP2. But nginx article about HTTP2 lists ubuntu 
> as only linux like system that is able to support HTTP2 over ALPN which is 
> required by Chrome: 
> https://www.nginx.com/blog/supporting-http2-google-chrome-users/ 
> 
Sorry. To be clear, I was not responding to your specific needs but discussing 
Glyph’s wider point about alerting when bad configuration is present.

> I decided to verify tnginx statements and I tried to set up nginx with HTTP2 
> on ubuntu 16.04. It turns out this is possible and it works ok. I just 
> followed this article here: 
> https://www.digitalocean.com/community/tutorials/how-to-set-up-nginx-with-http-2-support-on-ubuntu-16-04
>  
> 
>  This means that in principle Ubuntu 16.04 should be able to support HTTP2 
> and it has required TLS ciphers.
> 
> So the problem here is not about lack of OS support.
> 
> Looking into this nginx article they recommend two things that are part of 
> manual setup which (maybe?) are required?
> 
> 1) They say ciphers should be set to ssl_ciphers 
> EECDH+CHACHA20:EECDH+AES128:RSA+AES128:EECDH+AES256:RSA+AES256:EECDH+3DES:RSA+3DES:!MD5;
> 
> This long string does not mean much to me, but reading email from Amber again 
> I see it differs slightly from what she says Twisted uses. But one thing I'm 
> wondering about is how do you guys know which ciphers are set in Twisted? 
> Looking into source code of DefaultOpenSSLContextFactory I see context is 
> created here: 
> https://github.com/twisted/twisted/blob/3455a902fb15e732ee43b59f4d82a66b105351ba/twisted/internet/ssl.py#L107
>  
> 
>  I dont see any point where there is a call that sets ciphers. Maybe this is 
> done somewhere else? I tried grepping source for string mentioned by Amber 
> but cant find it.

Ok, this is your problem.

DefaultOpenSSLContextFactory should have been deprecated a long time ago. It’s 
insecure, and in particular does not set a cipher string, so it uses DEFAULT. 
That will have all kinds of messed up priorities. For that reason, you should 
adjust your code to use OpenSSLCertificateOptions or, even better, use the TLS 
endpoint directly.

The TL;DR is: yes, it seems that DefaultOpenSSLContextFactory produces a 
context that is genuinely unacceptable for HTTP/2.

Cory



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


Re: [Twisted-Python] Twisted 16.3.0 Prerelease 2 Announcement

2016-07-12 Thread Cory Benfield

> On 11 Jul 2016, at 20:22, Glyph Lefkowitz  wrote:
> 
> So pyOpenSSL/Cryptography doesn't have SSL_get_current_cipher anywhere?

get_current_cipher isn’t helpful. In particular, it puts us in an awkward place 
where we have a connection that has been negotiated for HTTP/2, but we cannot 
use it. The only action Twisted can meaningfully take at that point is to log 
and tear the connection down, which doesn’t really solve our problems.

We can do that, for sure, but it wouldn’t be much clearer than what happened 
here.

Cory


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


Re: [Twisted-Python] Twisted 16.3.0 Prerelease 2 Announcement

2016-07-12 Thread Tristan Seligmann
On Tue, 12 Jul 2016 at 09:43 Cory Benfield  wrote:

> For that reason, you should adjust your code to use
> OpenSSLCertificateOptions or, even better, use the TLS endpoint directly.
>
> The exported name of this class is actually just "CertificateOptions",
fwiw.
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


Re: [Twisted-Python] Twisted 16.3.0 Prerelease 2 Announcement

2016-07-12 Thread Paweł Miech
> DefaultOpenSSLContextFactory should have been deprecated a long time ago.
It’s insecure, and in particular does not set a cipher string, so it uses
DEFAULT. That will have all kinds of messed up priorities. For that reason,
you should adjust your code to use OpenSSLCertificateOptions or, even
better, use the TLS endpoint directly.The TL;DR is: yes, it seems that
DefaultOpenSSLContextFactory produces a context that is genuinely
unacceptable for HTTP/2.

Indeed it all works fine with endpoints. Thanks!

I was not aware that DefaultOpenSSLContextFactory is deprecated. There is
no warning about it anywhere. It seems that is is very widely used by
users, I just did some github search now and found around 5k occurences of
people using it:

https://github.com/search?utf8=%E2%9C%93&q=defaultopensslcontextfactory&type=Code&ref=searchresults

If you google for "ssl in twisted" you will also find articles that
recommend it. Since so many people use it, maybe it could be updated to be
more secure? If it does not make sense to update it then perhaps it would
be good to deprecate it so that it does not confuse users?

2016-07-12 9:56 GMT+02:00 Tristan Seligmann :

> On Tue, 12 Jul 2016 at 09:43 Cory Benfield  wrote:
>
>> For that reason, you should adjust your code to use
>> OpenSSLCertificateOptions or, even better, use the TLS endpoint directly.
>>
>> The exported name of this class is actually just "CertificateOptions",
> fwiw.
>
> ___
> Twisted-Python mailing list
> Twisted-Python@twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
>
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


Re: [Twisted-Python] Twisted 16.3.0 Prerelease 2 Announcement

2016-07-12 Thread Cory Benfield

> On 12 Jul 2016, at 09:33, Paweł Miech  wrote:
> 
> If you google for "ssl in twisted" you will also find articles that recommend 
> it. Since so many people use it, maybe it could be updated to be more secure? 
> If it does not make sense to update it then perhaps it would be good to 
> deprecate it so that it does not confuse users?

Agreed. I’m planning to begin the deprecation process, though it will take a 
little while as we need to remove all uses of it from within the Twisted 
codebase itself, as well as from the documentation. That turns out to be a 
bigger task than expected!



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


Re: [Twisted-Python] Twisted 16.3.0 Prerelease 2 Announcement

2016-07-12 Thread Paweł Miech
> Agreed. I’m planning to begin the deprecation process, though it will
take a little while as we need to remove all uses of it from within the
Twisted codebase itself, as well as from the documentation. That turns out
to be a bigger task than expected!

+1

One final point that I glossed over earlier

> To be clear, I was not responding to your specific needs but discussing
Glyph’s wider point about alerting when bad configuration is present.

When using Twisted endpoints (e.g. serverFromString) the problem with bad
openssl configuration is not bad. If OS does not support ALPN (OpenSSL
versions below 1.0.2) so in vast majority of Linux systems currently in use
Chrome connection simply falls back to HTTP 1.1 (I tested this on Ubuntu
14.04), This means there is no error and content is served, so it's some
sort of graceful degradation. This behavior is identical to nginx. I'm not
sure if Twisted can and should do something about this. Maybe it can print
some warning or maybe it can just let users know in documentation that
HTTP2 support via ALPN (which is required in Chrome) requires Openssl
1.0.2? Adding warnings to code might require some extra development but it
does not look that difficult. If you think about this, you probably dont
need to check ciphers available in system, you can probably only
check OpenSSL version available and check if client attempts to use ALPN.

2016-07-12 17:13 GMT+02:00 Cory Benfield :

>
> On 12 Jul 2016, at 09:33, Paweł Miech  wrote:
>
> If you google for "ssl in twisted" you will also find articles that
> recommend it. Since so many people use it, maybe it could be updated to be
> more secure? If it does not make sense to update it then perhaps it would
> be good to deprecate it so that it does not confuse users?
>
>
> Agreed. I’m planning to begin the deprecation process, though it will take
> a little while as we need to remove all uses of it from within the Twisted
> codebase itself, as well as from the documentation. That turns out to be a
> bigger task than expected!
>
>
> ___
> Twisted-Python mailing list
> Twisted-Python@twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
>
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


Re: [Twisted-Python] Twisted 16.3.0 Prerelease 2 Announcement

2016-07-12 Thread Cory Benfield

> On 12 Jul 2016, at 17:42, Paweł Miech  wrote:
> 
> > Agreed. I’m planning to begin the deprecation process, though it will take 
> > a little while as we need to remove all uses of it from within the Twisted 
> > codebase itself, as well as from the documentation. That turns out to be a 
> > bigger task than expected!
> 
> +1
> 
> One final point that I glossed over earlier
> 
> > To be clear, I was not responding to your specific needs but discussing 
> > Glyph’s wider point about alerting when bad configuration is present.
> 
> When using Twisted endpoints (e.g. serverFromString) the problem with bad 
> openssl configuration is not bad. If OS does not support ALPN (OpenSSL 
> versions below 1.0.2) so in vast majority of Linux systems currently in use 
> Chrome connection simply falls back to HTTP 1.1 (I tested this on Ubuntu 
> 14.04), This means there is no error and content is served, so it's some sort 
> of graceful degradation. This behavior is identical to nginx. I'm not sure if 
> Twisted can and should do something about this. Maybe it can print some 
> warning or maybe it can just let users know in documentation that HTTP2 
> support via ALPN (which is required in Chrome) requires Openssl 1.0.2? Adding 
> warnings to code might require some extra development but it does not look 
> that difficult. If you think about this, you probably dont need to check 
> ciphers available in system, you can probably only check OpenSSL version 
> available and check if client attempts to use ALPN.

We can actually do better than that.

The way the Twisted APIs are constructed, it knows if it’s got NPN, ALPN, 
neither, or both. So Twisted is capable of warning in a situation where it has 
protocols to advertise/negotiate, but no mechanism with which to do it. 
Unfortunately, I’m not sure of a way of doing it that isn’t intrusive: users 
opt in to HTTP/2 only by having the HTTP/2 dependencies installed, which they 
may have for other reasons (they’re common code used by other tools). That 
means that you could have a situation where you have the HTTP/2 dependencies 
installed, install Twisted, and then get spammed with warnings because you have 
older OpenSSL’s.

I’m definitely open to it, but I’m not sure that the user experience is good. 
If anyone has suggestions of how to get a better UX, I’m open to it.

Cory



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


Re: [Twisted-Python] Twisted 16.3.0 Prerelease 2 Announcement

2016-07-12 Thread Glyph Lefkowitz

> On Jul 12, 2016, at 12:43 AM, Cory Benfield  wrote:
> 
> DefaultOpenSSLContextFactory should have been deprecated a long time ago. 

2 years ago, to be precise: 

https://twistedmatrix.com/trac/ticket/6923

Someone fixing this would be tremendously useful.

-glyph

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


[Twisted-Python] removing twistedchecker buildbot for the time being

2016-07-12 Thread Glyph Lefkowitz
Right now it seems the difference-computation logic on the twistedchecker 
buildbot has just broken completely.  It's introducing useless noise into the 
build results because it makes every actually-passing build into a big red 'X' 
on the pull request status page.  I think I'm going to remove it.  Any 
objections?

-glyph
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python


Re: [Twisted-Python] removing twistedchecker buildbot for the time being

2016-07-12 Thread Adi Roiban
On 13 July 2016 at 00:37, Glyph Lefkowitz  wrote:

> Right now it seems the difference-computation logic on the twistedchecker
> buildbot has just broken completely.  It's introducing useless noise into
> the build results because it makes every actually-passing build into a big
> red 'X' on the pull request status page.  I think I'm going to remove it.
> Any objections?
>
>
1. Maybe use the --diff option on Buildbot.

2. We can move it to Travis based on the --diff option, ticket pending
review https://twistedmatrix.com/trac/ticket/8572
And if we move to Travis, get rid of tox-travis so that we can have
multiple jobs for the same python version,
https://twistedmatrix.com/trac/ticket/8535

--

I am +1 for removing the buildbot specific logic and allow to run a diff on
the local branch, so that it is much easier to work on improving the diff
functionality.

-- 
Adi Roiban
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python