Opened 15 months ago

Last modified 8 days ago

#6800 enhancement new

Expand `CertificateOptions` to include a simple API for specifying what protocols to support

Reported by: exarkun Owned by:
Priority: normal Milestone:
Component: core Keywords: ssl
Cc: hs@…, dpn@… Branch:
Author: Launchpad Bug:

Description (last modified by exarkun)

twisted.internet.ssl.CertificateOptions is used to define the connection parameters for a family of protocols. SSL versions 21 and 3 and TLS versions 1.0, 1.1, and 1.2.

The current API for choosing between these protocols is quite baroque. It's also not very expressive.

  • CertificateOptions(method=SSLv2_METHOD) - this negotiates SSLv2
  • CertificateOptions(method=SSLv3_METHOD) - this negotiates SSLv3
  • CertificateOptions(method=TLSv1_METHOD) - this negotiates TLS v1.0.
  • CertificateOptions(method=SSLv23_METHOD) - this negotiates SSLv2, SSLv3, TLSv1.0, TLSv1.1, or TLSv1.2.
  • CertificateOptions(method=None) (or CertificateOptions()) - this negotiates TLSv1.0, TLSv1.1, or TLSv1.22

Given a sufficiently new version of pyOpenSSL, these are also possible:

  • CertificateOptions(method=TLSv1_1_METHOD) - this negotiates TLSv1.1
  • CertificateOptions(method=TLSv1_2_METHOD) - this negotiates TLSv1.2

Notice that out of 5 protocols there are only 7 possible protocol combinations supported (rather than the complete set of 31 combinations3).

I suggest that CertificateOptions(protocols=[...]) is a better API. The list can specify exactly which protocols to support in any combination. This avoids the need for more symbols than there are protocols (eg SSLv23_METHOD) and makes it extremely obvious what protocols are being selected.

It also steps away from the OpenSSL specific terminology of "methods" and it will force us to step away from the pyOpenSSL supplied method constants since will not longer be useful and not all of them (SSLv23_METHOD) will make sense with this API. These are good things since CertificateOptions is not supposed to be OpenSSL-specific.

1: Of course, SSLv2 is disabled in most real-world OpenSSL builds now so all mentions of SSLv2 in this description are for the sake of completeness. It's most likely not possible to ever negotiate this protocol anymore - nor should it be.

2: Prior to the merge of #6772 it actually negotiates only TLSv1.0.

3: Or 15 if we disregard SSLv2

Change History (7)

comment:1 Changed 15 months ago by exarkun

  • Description modified (diff)

comment:2 Changed 15 months ago by exarkun

  • Description modified (diff)

comment:3 Changed 15 months ago by exarkun

  • Description modified (diff)

comment:4 Changed 15 months ago by hynek

  • Cc hs@… added

comment:5 Changed 8 days ago by glyph

Apropos of some of my comments on #7752, here's a proposal:

#!python
from twisted.python.constants import Names, NamedConstant
class TLSVersion(Names):
    TLSv1 = NamedConstant()
    TLSv1_1 = NamedConstant()
    TLSv1_2 = NamedConstant()

class KnownVulnerableVersion(Names):
    SSLv2 = NamedConstant()
    SSLv3 = NamedConstant()

_versionOrdering = (list(KnownVulnerableVersion.iterconstants()) +
                    list(TLSVersion.iterconstants()))

# these will all need fallbacks for older pyOpenSSL probably
from OpenSSL.SSL import (SSLv23_METHOD as NEGOTIATE_VERSIONS,
                         OP_NO_SSLv2, OP_NO_SSLv3, OP_NO_TLSv1, OP_NO_TLSv1_1,
                         OP_NO_TLSv1_2, Context)

_disableFlags = {
    KnownVulnerableVersion.SSLv2: OP_NO_SSLv2,
    KnownVulnerableVersion.SSLv3: OP_NO_SSLv3,

    TLSVersion.TLSv1: OP_NO_TLSv1,
    TLSVersion.TLSv1_1: OP_NO_TLSv1_1,
    TLSVersion.TLSv1_2: OP_NO_TLSv1_2,
}

def versionRange(minimum, maximum=TLSVersion.TLSv1_2):
    left = _versionOrdering.index(minimum)
    right = _versionOrdering.index(maximum)+1
    return set(_versionOrdering[left:right])

print(versionRange(KnownVulnerableVersion.SSLv3))


def _contextAndOptions(versions):
    """
    pyOpenSSL does not expose SSL_set_ssl_method or SSL_get_options, so there's
    literally no way to represent this sanely.  We can't fully create a context
    because then the caller can't modify the options, and we can't modify an
    existing context because we can't change its method *or* read its existing
    options.
    """
    ctx = Context(NEGOTIATE_VERSIONS)
    options = 0x0000
    for version in set(_versionOrdering) - set(versions):
        options |= _disableFlags[version]
    return ctx, options

comment:6 Changed 8 days ago by hynek

I’m not happy with the division between class TLSVersion(Names) and class KnownVulnerableVersion(Names): since those sets are in no way final.

Can’t we have

class TLSVersion(Names):
    SSLv2 = NamedConstant()
    SSLv3 = NamedConstant()
    TLSv1 = NamedConstant()
    TLSv1_1 = NamedConstant()
    TLSv1_2 = NamedConstant()

and sort out the current vulnerabilities elsewhere?

Or is it by design, that software that is written using TLSVersion.TLSv1 today breaks and has to be manually fixed once TLSv1 wanders over to KnownVulnerableVersion?

In any case, I’d guess that 99% of users will only use the default anyways.

comment:7 Changed 8 days ago by dpn

  • Cc dpn@… added
Note: See TracTickets for help on using tickets.