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

SSLContext doesn't support loading a CRL #53059

Closed
vstinner opened this issue May 24, 2010 · 27 comments
Closed

SSLContext doesn't support loading a CRL #53059

vstinner opened this issue May 24, 2010 · 27 comments
Assignees
Labels
extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

BPO 8813
Nosy @pitrou, @vstinner, @larryhastings, @giampaolo, @tiran, @ned-deily, @dstufft
Files
  • verify_flags_crl.patch
  • verify_flags_crl2.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-03-18.09:41:38.862>
    created_at = <Date 2010-05-24.21:17:08.659>
    labels = ['extension-modules', 'type-feature', 'library']
    title = "SSLContext doesn't support loading a CRL"
    updated_at = <Date 2014-03-18.10:05:00.667>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2014-03-18.10:05:00.667>
    actor = 'vstinner'
    assignee = 'christian.heimes'
    closed = True
    closed_date = <Date 2014-03-18.09:41:38.862>
    closer = 'christian.heimes'
    components = ['Extension Modules', 'Library (Lib)']
    creation = <Date 2010-05-24.21:17:08.659>
    creator = 'vstinner'
    dependencies = []
    files = ['32744', '32760']
    hgrepos = []
    issue_num = 8813
    keywords = ['patch']
    message_count = 27.0
    messages = ['106393', '143358', '203170', '203562', '203627', '203663', '203664', '203666', '203678', '203680', '203681', '203688', '203689', '203691', '203694', '203895', '203904', '203905', '203966', '203983', '203984', '212998', '213920', '213951', '213952', '213953', '213954']
    nosy_count = 9.0
    nosy_names = ['pitrou', 'vstinner', 'larry', 'giampaolo.rodola', 'christian.heimes', 'ned.deily', 'dandrzejewski', 'python-dev', 'dstufft']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue8813'
    versions = ['Python 3.4']

    @vstinner
    Copy link
    Member Author

    SSL Context should support loading a CRL. See M2Crypto patches:
    https://bugzilla.osafoundation.org/show_bug.cgi?id=12954
    https://bugzilla.osafoundation.org/show_bug.cgi?id=11694

    Or PyOpenSSL branch supporting CRL:
    https://launchpad.net/~rick-fdd/pyopenssl/crl_and_revoked

    @vstinner vstinner added the stdlib Python modules in the Lib dir label May 24, 2010
    @pitrou pitrou added the type-feature A feature request or enhancement label May 24, 2010
    @pitrou
    Copy link
    Member

    pitrou commented Sep 1, 2011

    Is it enough to just load a CRL file, or is other functionality usually needed?

    The following APIs should help us do it:

    • X509_STORE *SSL_CTX_get_cert_store(const SSL_CTX *ctx);
    • int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x);
    • X509_CRL *d2i_X509_CRL_fp(FILE *fp,X509_CRL **crl);

    And also for configuration (enable CRL checking on the context):

    • X509_VERIFY_PARAM *X509_STORE_CTX_get0_param(X509_STORE_CTX *ctx);
    • int X509_VERIFY_PARAM_set_flags(X509_VERIFY_PARAM *param, unsigned long flags);

    @tiran tiran added the extension-modules C modules in the Modules dir label Jul 8, 2013
    @tiran
    Copy link
    Member

    tiran commented Nov 17, 2013

    Yes, you are right. OpenSSL uses the same API to load certs and CRLs. CRL checks must be enabled, though.

    @tiran
    Copy link
    Member

    tiran commented Nov 21, 2013

    The patch implements SSLContext.verify_flags in order to enable CRL checks. It comes with documentation, a unit test and a new CRL file.

    @tiran
    Copy link
    Member

    tiran commented Nov 21, 2013

    My patch is inspired by mod_ssl:

    http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_init.c?view=markup#l697

    CRLs can already be loaded with SSLContext.load_verify_locations(). The patch exposes the verification flags of SSLContext's X509_STORE. With X509_V_FLAG_CRL_CHECK OpenSSL requires (!) a CRL that matches the issuer of leaf certificate of the chain (the peer's cert). X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL also requires CRLs for all intermediate certs of the peer's cert chain.

    @tiran
    Copy link
    Member

    tiran commented Nov 21, 2013

    The new patch addresses your review. I have altered the new to FLAGS_NONE, FLAGS_CLR_CHECK_LEAF etc.

    @pitrou
    Copy link
    Member

    pitrou commented Nov 21, 2013

    That sounds too generic. How about VERIFY_CRL_NONE, etc.

    @tiran
    Copy link
    Member

    tiran commented Nov 21, 2013

    It *is* generic. The flags are not about CRL alone, http://www.openssl.org/docs/crypto/X509_VERIFY_PARAM_set_flags.html#VERIFICATION_FLAGS

    @pitrou
    Copy link
    Member

    pitrou commented Nov 21, 2013

    It *is* generic. The flags are not about CRL alone,

    That's why I proposed VERIFY_xxx, e.g. VERIFY_CRL_NONE.

    Calling some flags "FLAGS" is senseless, it's like calling an integer
    "INTEGER".

    @tiran
    Copy link
    Member

    tiran commented Nov 21, 2013

    s/FLAGS_/VERIFY_/g ? OK, I don't have hard feelings. :)

    @pitrou
    Copy link
    Member

    pitrou commented Nov 21, 2013

    s/FLAGS_/VERIFY_/g ? OK, I don't have hard feelings. :)

    And VERIFY_NONE should be VERIFY_CRL_NONE IMO.

    @tiran
    Copy link
    Member

    tiran commented Nov 21, 2013

    But it's not about CRL alone. How about VERIFY_DEFAULT = 0 ?

    @pitrou
    Copy link
    Member

    pitrou commented Nov 21, 2013

    But it's not about CRL alone. How about VERIFY_DEFAULT = 0 ?

    Sounds good.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 21, 2013

    New changeset 83805c9d1f05 by Christian Heimes in branch 'default':
    Issue bpo-8813: Add SSLContext.verify_flags to change the verification flags
    http://hg.python.org/cpython/rev/83805c9d1f05

    @tiran
    Copy link
    Member

    tiran commented Nov 21, 2013

    memo to me: add whatsnew entry

    @tiran tiran self-assigned this Nov 21, 2013
    @ned-deily
    Copy link
    Member

    This change seems to have broken the OS X 10.4 Tiger buildbot:

    _ssl.c:2240: error: 'struct x509_store_st' has no member named 'param'
    _ssl.c:2253: error: 'struct x509_store_st' has no member named 'param'
    _ssl.c:2257: error: 'struct x509_store_st' has no member named 'param'
    _ssl.c:2263: error: 'struct x509_store_st' has no member named 'param'

    http://buildbot.python.org/all/builders/x86%20Tiger%203.x/builds/7370

    @tiran
    Copy link
    Member

    tiran commented Nov 22, 2013

    :(

    I seriously need access to a Darwin or OSX box. This is the second time I broke the build on OSX.

    Ned Deily <report@bugs.python.org> schrieb:

    Ned Deily added the comment:

    This change seems to have broken the OS X 10.4 Tiger buildbot:

    _ssl.c:2240: error: 'struct x509_store_st' has no member named 'param'
    _ssl.c:2253: error: 'struct x509_store_st' has no member named 'param'
    _ssl.c:2257: error: 'struct x509_store_st' has no member named 'param'
    _ssl.c:2263: error: 'struct x509_store_st' has no member named 'param'

    http://buildbot.python.org/all/builders/x86%20Tiger%203.x/builds/7370

    ----------
    nosy: +ned.deily
    resolution: fixed ->
    status: pending -> open


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue8813\>


    @ned-deily
    Copy link
    Member

    10.4 is *very* old:

    $ /usr/bin/openssl version
    OpenSSL 0.9.7l 28 Sep 2006

    If you kept around that version of the headers and libs, you'd probably catch most of the problems.

    @ned-deily
    Copy link
    Member

    This problem also breaks the 32-bit OS X installer build.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 23, 2013

    New changeset 40d4be2b7258 by Christian Heimes in branch 'default':
    Issue bpo-8813: X509_VERIFY_PARAM is only available on OpenSSL 0.9.8+
    http://hg.python.org/cpython/rev/40d4be2b7258

    @tiran
    Copy link
    Member

    tiran commented Nov 23, 2013

    The _ssl module compiles again with OpenSSL 0.9.7.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 9, 2014

    New changeset 1508c4c9e747 by R David Murray in branch 'default':
    whatsnew: SSLContext.verify_flags and constants. (bpo-8813)
    http://hg.python.org/cpython/rev/1508c4c9e747

    @vstinner
    Copy link
    Member Author

    What is the status of this issue? Is it fixed or not?

    The What's New in Python 3.4 document says that Python 3.4 can load CRL.

    @tiran
    Copy link
    Member

    tiran commented Mar 18, 2014

    Yes, Python 3.4 can load and use CRLs.

    @tiran tiran closed this as completed Mar 18, 2014
    @vstinner
    Copy link
    Member Author

    Yes, Python 3.4 can load and use CRLs.

    Great work Christian, I was expecting this feature since many years :-)

    @tiran
    Copy link
    Member

    tiran commented Mar 18, 2014

    It was *really* trivial. I just had to expose two simple OpenSSL APIs to enable / disable CRL. All versions of Python could already load the CRLs but CRL checks could not be enabled.

    @vstinner
    Copy link
    Member Author

    It was *really* trivial. I just had to expose two simple OpenSSL APIs to enable / disable CRL.

    It was trivial thanks to all the work done before around SSLContext. For example, Python 2.7 doesn't have SSLContext, so adding support for CRL in Python 2.7 is non-trivial :-/

    @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
    extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants