Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ssl.create_default_context() #63888

Closed
tiran opened this issue Nov 22, 2013 · 16 comments
Closed

ssl.create_default_context() #63888

tiran opened this issue Nov 22, 2013 · 16 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@tiran
Copy link
Member

tiran commented Nov 22, 2013

BPO 19689
Nosy @gvanrossum, @pitrou, @tiran
Files
  • ssl_create_default_context.patch
  • ssl_create_default_context2.patch
  • ssl_create_default_context3.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/tiran'
    closed_at = <Date 2014-10-04.21:18:44.487>
    created_at = <Date 2013-11-22.03:22:24.405>
    labels = ['type-feature', 'library']
    title = 'ssl.create_default_context()'
    updated_at = <Date 2014-10-04.21:18:44.486>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2014-10-04.21:18:44.486>
    actor = 'pitrou'
    assignee = 'christian.heimes'
    closed = True
    closed_date = <Date 2014-10-04.21:18:44.487>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2013-11-22.03:22:24.405>
    creator = 'christian.heimes'
    dependencies = []
    files = ['32770', '32778', '32799']
    hgrepos = []
    issue_num = 19689
    keywords = ['patch']
    message_count = 16.0
    messages = ['203718', '203757', '203758', '203761', '203765', '203768', '203770', '203774', '203812', '203813', '203816', '203842', '203844', '204031', '204039', '213010']
    nosy_count = 5.0
    nosy_names = ['gvanrossum', 'janssen', 'pitrou', 'christian.heimes', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue19689'
    versions = ['Python 3.4']

    @tiran
    Copy link
    Member Author

    tiran commented Nov 22, 2013

    A few weeks ago I suggested the addition of ssl.create_default_context() to the stdlib. The patch implements my proposal. It replaces code in several of modules with one central function. The patch also removes ssl.wrap_socket() in favor for a SSLContext object.

    As soon as bpo-19292 gets accepted I'll add the additional keyword argument "purpose=None" to the arguments of ssl.create_default_context() in order to load the default system certs::

    if purpose is not None and verify_mode != CERT_NONE:
        context.load_default_certs(purpose)
    

    @tiran tiran added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Nov 22, 2013
    @pitrou
    Copy link
    Member

    pitrou commented Nov 22, 2013

    A few weeks ago I suggested the addition of
    ssl.create_default_context() to the stdlib. The patch implements my
    proposal. It replaces code in several of modules with one central
    function. The patch also removes ssl.wrap_socket() in favor for a
    SSLContext object.

    Can you call it create_default_client_context() (a bit long) and/or
    stress that it's for client use?

    Or will it be ok for server purposes too, i.e. do you promise that it'll
    never get CERT_REQUIRED by default?

    @tiran
    Copy link
    Member Author

    tiran commented Nov 22, 2013

    Good point!

    We need a purpose flag anyway in order to load the appropriate root CA certs. The purpose flag can be used for purpose-specific verify mode:

    SERVER_AUTH = _ASN1Object('1.3.6.1.5.5.7.3.1')
    CLIENT_AUTH = _ASN1Object('1.3.6.1.5.5.7.3.2')
    
        if isinstance(purpose, str):
            purpose = _ASN1Object.fromname(purpose)
        if verify_mode is None:
            if purpose == SERVER_AUTH:
                # authenticate a TLS web server (for client sockets). The default 
                # setting may change in the future.
                verify_mode = CERT_NONE
            elif purpose == CLIENT_AUTH:
                # authenticate a TLS web client (for server sockets). The default
                # setting is guaranteed to be stable and will never change.
                verify_mode = CERT_NONE
            else:
                # other (code signing, S/MIME, IPSEC, ...), default may change.
                verify_mode = CERT_NONE
        context.verify_mode = verify_mode

    @pitrou
    Copy link
    Member

    pitrou commented Nov 22, 2013

    SERVER_AUTH = _ASN1Object('1.3.6.1.5.5.7.3.1')
    CLIENT_AUTH = _ASN1Object('1.3.6.1.5.5.7.3.2')

    That's a bit ugly. How about an enum?

    @tiran
    Copy link
    Member Author

    tiran commented Nov 22, 2013

    In my opinion enums are for a closed batch of known entities. There are at least 20-30 purpose flags, maybe more. Everybody is allowed to define their own OIDs, too.

    @pitrou
    Copy link
    Member

    pitrou commented Nov 22, 2013

    In my opinion enums are for a closed batch of known entities. There
    are at least 20-30 purpose flags, maybe more. Everybody is allowed to
    define their own OIDs, too.

    Well, how many purposes are we going to expose? I don't think users
    should know what ASN1 objects are, and a nice repr() is really useful.

    (but there's no reason an enum cannot have 20 or 30 members)

    @tiran
    Copy link
    Member Author

    tiran commented Nov 22, 2013

    The objects already have a (more or less) nice representation:

    >>> ssl._ASN1Object.fromname("1.3.6.1.5.5.7.3.1")
    _ASN1Object(nid=129, shortname='serverAuth', longname='TLS Web Server Authentication', oid='1.3.6.1.5.5.7.3.1')
    >>> ssl._ASN1Object.fromname("1.3.6.1.5.5.7.3.2")
    _ASN1Object(nid=130, shortname='clientAuth', longname='TLS Web Client Authentication', oid='1.3.6.1.5.5.7.3.2')
    >>> ssl._ASN1Object.fromname("1.3.6.1.5.5.7.3.8")
    _ASN1Object(nid=133, shortname='timeStamping', longname='Time Stamping', oid='1.3.6.1.5.5.7.3.8')

    @pitrou
    Copy link
    Member

    pitrou commented Nov 22, 2013

    Ok. Note that as long as they aren't actually passed to OpenSSL, they don't need to be ASN1 objects at all, i.e. if it's only a parameter to create_default_context(), it can perfectly well be a str or enum.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 22, 2013

    New patch with enum and more cleanups.

    I'd like to explain the rationals for the purpose argument in create_default_context and the ASN1Object thing. There are multiple things involved here. First of all a certificate may have key usage and extended key usage OIDs in its X509v3 extensions. OpenSSL already checks them according to its mode.

    The purpose is also required to load the correct set of certs from a certificate provider (e.g. Windows cert store, Mozilla NSS certdata, Apple's keystore). The system or user can impose additional restrictions for certificates, e.g. disable a cert for TLS web server auth although the X.509 struct specifies 1.3.6.1.5.5.7.3.1 in its X509v3 extensions. NSS certdata also contains invalid certificates or certificates that are not suitable for server auth although the cert claims it.

    In order to load only trusted certs for a purpose the API needs a purpose flag (usually an OID or a NID). Most Linux users have never seen this differentiation because /etc/ssl/certs/ either contains only server auth certs or their distributions screw up, See https://bugs.launchpad.net/ubuntu/+source/ca-certificates/+bug/1207004 or http://www.egenix.com/company/news/eGenix-pyOpenSSL-Distribution-0.13.2.1.0.1.5.html

    @pitrou
    Copy link
    Member

    pitrou commented Nov 22, 2013

    Ok, so I still have a couple of issues with the proposed API:

    • if its purpose is to create a *default* context, create_default_context() shouldn't have that many arguments. The nice thing with contexts is that you can change their parameters later... So basically the function signature should be:
      create_default_context(purpose, *, cafile, cadata, capath)

    Also, the default for "purpose" should probably be serverAuth.

    • "PurposeEKU" is cryptic, please simply "Purpose" or "CertPurpose".

    @tiran
    Copy link
    Member Author

    tiran commented Nov 22, 2013

    Antoine and I have agreed upon a slightly different API. I'm going to split it up into one public API that creates a best practice context and one internal stdlib API to unify all places that deals with SSL sockets.

    AP:
    how about we use more strict and modern settings for the public API? TLSv1, no insecure stuff like RC4, MD5, DSS etc. https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/

    @pitrou
    Copy link
    Member

    pitrou commented Nov 22, 2013

    how about we use more strict and modern settings for the public API?
    TLSv1, no insecure stuff like RC4, MD5, DSS etc.
    https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/

    Fine, but I'd like to see something more open-ended for the ciphers
    string. e.g.
    'HIGH:!ADH:!AECDH:!MD5:!DSS:!aNULL:!eNULL:!LOW:!EXPORT:!SSLv2' ?

    @tiran
    Copy link
    Member Author

    tiran commented Nov 23, 2013

    The patch implements HIGH:!aNULL:!RC4:!DSS

    HIGH already covers !MD5:!EXPORT:!NULL:!SSLv2 and more

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 23, 2013

    New changeset 63df21e74c65 by Christian Heimes in branch 'default':
    Issue bpo-19689: Add ssl.create_default_context() factory function. It creates
    http://hg.python.org/cpython/rev/63df21e74c65

    @tiran tiran self-assigned this Nov 23, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 10, 2014

    New changeset 8b4b6609cd31 by R David Murray in branch 'default':
    whatsnew: ssl.create_default_context (bpo-19689).
    http://hg.python.org/cpython/rev/8b4b6609cd31

    @pitrou pitrou closed this as completed Oct 4, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants