[Twisted-Python] The role of twisted.internet._sslverify.IOpenSSLTrustRoot

[Adi Roiban]

Hi,

On 25 December 2015 at 11:20, Glyph Lefkowitz <glyph at twistedmatrix.com>
wrote:

>
> On Dec 20, 2015, at 9:05 AM, Adi Roiban <adi at roiban.ro> wrote:
>
> Hi,
>
> What is the purpose of IOpenSSLTrustRoot ?
>
>
> The idea is that we have public functions, mainly `optionsForClientTLS´,
> which need to take a thing that represents a "trust root".  We want this to
> be something that can abstractly be described at a high level, but then in
> reality we need to do with gross implementation details of OpenSSL.  So
> this interface describes what you pass.
>

Reading the docstring of IOpenSSLTrustRoot it does not give any hints about
optionsForClientTLS

So instead of

    Trust settings for an OpenSSL context.

Maybe it should be something like:

   Marker only interface for private implementations of OpenSSL trust root
things.

It it documented as a private interface, it has only private methods, but
> then it is exposed in  twisted.internet.ssl.optionsForClientTLS
>
> Why?
>
>
> Yes, this is intentional.  It is a private interface, so you can't check
> if something provides it, you aren't allowed to know what attributes it
> has, and you can't implement it.  However, you *can* call a function that
> is documented to return a value that provides it (such as
> `twisted.internet.ssl.platformTrust´) and pass that value to a function
> documented to accept it (such as
> `twisted.internet.ssl.optionsForClientTLS`).
>
> It's private because we weren't sure if we'd want to change it.  At the
> time it was implemented, the only two cases were OpenSSLDefaultPaths and
> Certificate.  In the case of Certificate, you know what certificate you're
> adding, but in the case of OpenSSLDefaultPaths, you just call a method on
> the context object to mutate it, and you can't extract information about
> which certificates are trusted past that.  The method we came up
> with, _addCACertsToContext, was a gross compromise which allowed for
> implementing this but could not be made abstract, because it reflects a
> bizarre flaw in the OpenSSL API, and it by necessity exposes pyOpenSSL
> objects, which we are trying to do less of.  For one thing, we'd eventually
> like to support TLS via OpenSSL using an API provided by Cryptography; for
> another, we'd like to one day provide TLS from an API that might not be
> backed by OpenSSL at all.  So reducing the surface area of our public API
> that touches pyOpenSSL is important.
>
> Hopefully this thoroughly explains the decision?
>

I understand now, but I find it hard to extract this information just from
the code... maybe I am a bad code reader, or maybe the  IOpenSSLTrustRoot
docstring should inform that it is a gross compromise so that other people
will understand that this is not really intentional ... and invited to find
something better :)

I am happy to see more backend neutral API but unless we have at least
another use case for non OpenSSL backed TLS I don't know if it worth
designing an API for that unknown API.

I am confused while trying to review
> https://twistedmatrix.com/trac/ticket/7671
>
> Please take a look at my review and add your wisdom :)
>
>
> This is quite a detailed review :-).
>
> Merry christmas,
>
>
Many thanks for your comment.

-- 
Adi Roiban
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20151226/51b36aac/attachment-0002.html>