Discussion:
[Twisted-Python] ssl APIs
meejah
2015-12-04 05:14:53 UTC
Permalink
I've been fooling around with the Twisted SSL APIs.

I'd like to add a "loadPEM" and documentation to ssl.KeyPair; shall I
open a ticket and start this, or is there a reason it doesn't have a
loadPEM() like some of the other classes (or any docstrings)? It is
exported as a public class in "ssl".

Also I'm wondering why CertificateOptions takes actual OpenSSL objects
for args, instead of the Twisted equivalents; this leads to code
accessing ".original" all the time if you use Twisted APIs to load
Certificate (and friends) which seems .. odd. Perhaps either a
Certificate *or* the correct underlying OpenSSL object could be
accepted?

There also doesn't seem to be a way around importing
OpenSSLCertificateAuthorities from _sslverify (i.e. "private" class) if
you want to give optionsForClientTLS() more than a single certificate as
trustRoot. The only way I can see is to construct one of those from a
list of OpenSSL certificate instances and pass that as trustRoot=

Thanks for any hints,
--
meejah
Glyph Lefkowitz
2015-12-04 09:34:22 UTC
Permalink
Post by meejah
I've been fooling around with the Twisted SSL APIs.
I'd like to add a "loadPEM" and documentation to ssl.KeyPair; shall I
open a ticket and start this, or is there a reason it doesn't have a
loadPEM() like some of the other classes (or any docstrings)? It is
exported as a public class in "ssl".
Please go ahead and do it.
Post by meejah
Also I'm wondering why CertificateOptions takes actual OpenSSL objects
for args, instead of the Twisted equivalents; this leads to code
accessing ".original" all the time if you use Twisted APIs to load
Certificate (and friends) which seems .. odd. Perhaps either a
Certificate *or* the correct underlying OpenSSL object could be
accepted?
Wow! I am so glad someone else finally noticed this! :-)

Basically: yes, this is odd. It is a result of a miscommunication (or rather, lack of communication) between the designers and the maintainers of that system.

A long time ago, I designed a "just enough" API for TLS (for Vertex, even, not really for Twisted), which is what got adopted into Twisted and eventually became _sslverify.py. However, this was very much incomplete; it wasn't really designed for the 99% use-case (running an HTTPS server).

OpenSSLCertificateOptions was just a private implementation detail; it was the object returned by PrivateCertificate.options(). (Hence its somewhat odd name: 'options', which has now become the convention; see optionsForClientTLS). However, the signature of .options() was tough to extend, since it just takes "*authorities". Given Vertex's use-case that sort of made sense but it really doesn't belong in the general-purpose library of twisted.internet.ssl.

Anyway, I digress: the point is that these classes were lying around, basically undocumented, when Hynek Schlawack showed up and began his otherwise excellent feature additions to Twisted's TLS stack, among . When asked "what is the 'good' ContextFactory class", the answer he got was unanimously 'OpenSSLCertificateOptions'. So he did what seemed reasonable: start adding features to that class, and then make it public. I took my eye off the ball for a release or two and by the time I checked back, "CertificateOptions" was the recommended public interface, warts and all :).

I want to be clear that despite the fact that there are problems with the factoring here, if you're writing TLS server code with Twisted today you should absolutely be using CertificateOptions; its interface is crummy but its implementation is very solid; again, Hynek did some really excellent work. As the interface changes, the implementation should stay largely the same.

If you want to fix this, rather than continue to mangle this architectural monstrosity, I think we should develop something symmetrical to the new, mostly pretty good optionsForClientTLS - https://twistedmatrix.com/documents/15.5.0/api/twisted.internet.ssl.html#optionsForClientTLS <https://twistedmatrix.com/documents/15.5.0/api/twisted.internet.ssl.html#optionsForClientTLS> - which operates entirely in terms of high-level wrapper objects defined in twisted.internet.ssl. Make an 'optionsForServerTLS', which can have a simpler interface, because it will just accept relevant configuration parameters for server TLS, and under the covers still use CertificateOptions. Initially we can provide the 'extraCertificateOptions' escape-hatch as we have with 'optionsForClientTLS', but once we've got a nice way to deal with esoterica like selecting TLS protocol versions and selecting cipher suites in those APIs, we can deprecate and remove the escape-hatch and be left with a clean function.

So, are you up for contributing an 'optionsForServerTLS'?
Post by meejah
There also doesn't seem to be a way around importing
OpenSSLCertificateAuthorities from _sslverify (i.e. "private" class) if
you want to give optionsForClientTLS() more than a single certificate as
trustRoot. The only way I can see is to construct one of those from a
list of OpenSSL certificate instances and pass that as trustRoot=
Yes, that is in fact an oversight. No need to write a ticket up yourself; you can just fix this one:

https://twistedmatrix.com/trac/ticket/7671 <https://twistedmatrix.com/trac/ticket/7671>

Thanks a ton for your interest, it would be great to get more people interested in TLS to maintain this stuff!

-glyph
meejah
2015-12-19 00:13:15 UTC
Permalink
Just to follow-up on this, I have submitted a patch for #7671 and filed
two tickets for the other two issues I brought up. These are:

#8150: twisted.internet.ssl.KeyPair should provide loadPEM
#8151: add twisted.internet.ssl.optionsForServerTLS

I will likely submit a patch for 8150 in the near term but haven't
started any direct work on 8151. I'm usually idling in OFTC and Freenode
if anyone has feedback or ideas or wants to help :)

Cheers,
--
meejah
Glyph Lefkowitz
2015-12-21 23:25:55 UTC
Permalink
Post by meejah
Just to follow-up on this, I have submitted a patch for #7671 and filed
#8150: twisted.internet.ssl.KeyPair should provide loadPEM
#8151: add twisted.internet.ssl.optionsForServerTLS
I will likely submit a patch for 8150 in the near term but haven't
started any direct work on 8151. I'm usually idling in OFTC and Freenode
if anyone has feedback or ideas or wants to help :)
Thanks for getting these filed, Meejah!

-g

Continue reading on narkive:
Loading...