Support external SSLContext

[jdecuyper]

PR for issue #8

@Lukasa: this is preliminary work, would love to get some feedback on code and unit tests to know if I'm on the right track.

• The SSLContext can be assigned to hyper.tls._context or passed as an argument to the HTTP20Connection constructor. Both cases are covered in the unit test.
• Is it correct to assume that the SSLContext argument will later be moved to the HTTPConnection abstraction layer?

[Lukasa]

Can we keep the name of this parameter in keeping with the style of the rest? ssl_context is preferred. =)

[Lukasa]

I think this is a good start, but there's some work left to do. =)

The HTTP11Connection and HTTPConnection objects need updating to tolerate this parameter as well, and DTRT with it.

Additionally, I think we should have a factory function that creates SSLContexts and make it more obvious. This may just mean promoting _init_context() to a full-blown API that is documented and first-class. This way people can make sure they start from a position of having an SSLContext that is set up correctly for HTTP/2 (and also get one for PyOpenSSL, if they need one)

[jdecuyper]

Thanks for the quick feedback, will work on the missing parts :)

On another matter, I'm using ssl for my unit tests but that seems not such a good idea. Some of the Travis jobs using Python 2.7.8 or Python 3.3.5 are reporting errors such as object has no attribute 'SSLContext' which indicates they are using an old version of OpenSSL. Is there a workaround for this or should I build my test differently?

[Lukasa]

You should always use hyper.compat.ssl, which will either be the stdlib SSL module or a wrapper around PyOpenSSL. =)

[Lukasa]

I actually think this logic is wrong. The provided ssl_context should be used only for the specific call to wrap_socket, but this affects all subsequent calls. I think probably we should use a different, local variable in this method, and then prefer ssl_context to _context.

[jdecuyper]

I'm thinking of adding new tests to validate a provided SSLContext using HTTP11Connection and HTTPConnection, does that make sense?

[Lukasa]

@jdecuyper It does, but it's fraught with difficulty because those tests end up enormously platform dependent. I think it's probably enough to run one test that does something that can easily be detected at the socket level: maybe force a TLS version lower than 1.2?

[sigmavirus24]

So far this looks good @jdecuyper! 👍

[jdecuyper]

@sigmavirus24 thanks :)

With the last commit, a custom SSLContext can be passed to the HTTPConnection abstraction. Then it gets handed over to HTTP11 or HTTP20 accordingly.

Now I want to make _init_context() part of hyper's API (as mentioned by @Lukasa above). I suspect it would be nice to add optional parameters to this method like the location of a certificate for instance. What other parameters should we expose? I would leave the method inside the tls file and only rename it to init_context. Is that how it should be done?

Sorry for so many newbies questions and thanks a lot for you help so far!

[Lukasa]

Now I want to make _init_context() part of hyper's API (as mentioned by @Lukasa above). I suspect it would be nice to add optional parameters to this method like the location of a certificate for instance. What other parameters should we expose?

I think location of a certificate file should be the only optional parameter we expose, to satisfy #8. Anything else can be mutated on the SSLContext itself. =)

I would leave the method inside the tls file and only rename it to init_context. Is that how it should be done?

Sounds good to me! Don't forget to add a docstring that explains it, and then make sure that you document the method in the API documentation.

Sorry for so many newbies questions and thanks a lot for you help so far!

Absolutely no need to apologise! It's far better to ask the question than to muddle on confused, and we're really happy to help. You're doing great work! ✨

[jdecuyper]

@Lukasa
Sooo, what is cool and can now easily be done is the following:

from hyper.common.connection import HTTPConnection
from hyper import tls
context = tls.init_context("mycerts.pem")
c = HTTPConnection('http2bin.org', 443, ssl_context=context)
c.request('GET', '/get')
r = c.get_response()
print(r.read())

Let me know if there is anything else that needs to be changed (code or documentation).

[Lukasa]

\o/ This looks fantastic @jdecuyper, thanks so much! 🍰