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

Use Windows' certificate store for CA certs #61336

Closed
tiran opened this issue Feb 5, 2013 · 22 comments
Closed

Use Windows' certificate store for CA certs #61336

tiran opened this issue Feb 5, 2013 · 22 comments
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@tiran
Copy link
Member

tiran commented Feb 5, 2013

BPO 17134
Nosy @doko42, @pitrou, @vstinner, @tiran, @merwok, @dstufft
Files
  • enumcertstore3.patch
  • enum_cert_trust2.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 = None
    closed_at = <Date 2013-12-04.07:24:30.950>
    created_at = <Date 2013-02-05.15:29:03.785>
    labels = ['extension-modules', 'type-feature']
    title = "Use Windows' certificate store for CA certs"
    updated_at = <Date 2013-12-04.07:24:30.949>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2013-12-04.07:24:30.949>
    actor = 'christian.heimes'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-12-04.07:24:30.950>
    closer = 'christian.heimes'
    components = ['Extension Modules']
    creation = <Date 2013-02-05.15:29:03.785>
    creator = 'christian.heimes'
    dependencies = []
    files = ['30507', '32430']
    hgrepos = []
    issue_num = 17134
    keywords = ['patch']
    message_count = 22.0
    messages = ['181445', '181459', '181463', '181467', '190743', '190744', '190753', '190757', '190812', '190864', '190865', '190894', '193833', '199341', '200529', '201778', '203153', '203709', '203736', '203753', '203781', '205202']
    nosy_count = 8.0
    nosy_names = ['doko', 'exarkun', 'pitrou', 'vstinner', 'christian.heimes', 'eric.araujo', 'python-dev', 'dstufft']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17134'
    versions = ['Python 3.4']

    @tiran
    Copy link
    Member Author

    tiran commented Feb 5, 2013

    I found a recipe how to access the Windows certificate store and dump its content as PEM. The code doesn't look complicated and could be added to _ssl.c

    http://fixunix.com/openssl/254866-re-can-openssl-use-windows-certificate-store.html

    @tiran tiran added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Feb 5, 2013
    @merwok
    Copy link
    Member

    merwok commented Feb 5, 2013

    Isn’t this part of bpo-13655? One feature is usually discussed for all platforms in one bug report. (Sorry for all the bureaucracy in your recent reports, but it helps keep things manageable :)

    @tiran
    Copy link
    Member Author

    tiran commented Feb 5, 2013

    I like to split up tasks in small subtasks.

    It's true that bpo-13655 benefits from this feature but it can be implemented without this ticket. This enhancement also requires some addition to API and bindings to Windows' crypt32.dll. It might be inappropriate to add it to bpo-13655 because we need to backport bpo-13655 to Python 2.6 to 3.3.

    @pitrou
    Copy link
    Member

    pitrou commented Feb 5, 2013

    Sounds promising. Do you think this should be hooked into SSLContext.set_default_verify_paths, or be exposed as a separate method?

    @exarkun
    Copy link
    Mannequin

    exarkun mannequin commented Jun 7, 2013

    Sounds promising. Do you think this should be hooked into SSLContext.set_default_verify_paths, or be exposed as a separate method?

    If there were an API which exposed the certificate material, then this would be more useful to libraries trying to do other things (present debugging information, use an alternate SSL implementation *wink*, etc). If this is *only* wrapped up inside set_default_verify_paths then many of these extra things are impossible with a seconding binding to the same API.

    @tiran
    Copy link
    Member Author

    tiran commented Jun 7, 2013

    Yes, I'm planing to expose the low level API. I prefer to do as much work in Python space as possible. The information is just too useful to 3rd parties, too.

    I'm thinking about one low level function that interfaces Windows's cert store. The rest can be build on top of this function and bpo-18138.

    enum_system_store(store_name, cert_type="certificate") -> [(cert_data, encoding_type), ...]

    store_name:
    name of the store (e.g. "CA", "MY", "ROOT"), see http://msdn.microsoft.com/en-us/library/windows/desktop/aa376560%28v=vs.85%29.aspx
    cert_type:
    "certificate" or "crl"
    data:
    certificate bytes (as far as I know the certs are stored in DER format)
    encoding_type:
    integer encoding X509_ASN_ENCODING or PKCS_7_ASN_ENCODING

    @tiran
    Copy link
    Member Author

    tiran commented Jun 7, 2013

    First patch. I have not yet verified that the return data can be loaded by openssl. Also I need to verify the error paths and add some tests, too.

    @tiran
    Copy link
    Member Author

    tiran commented Jun 7, 2013

    I fixed a ref leak and added some tests.

    @tiran
    Copy link
    Member Author

    tiran commented Jun 8, 2013

    New patch with fixed doc string and indention.

    http://msdn.microsoft.com/en-us/library/windows/desktop/aa377189%28v=vs.85%29.aspx explains how encoding type shall be interpreted. I haven't seen PKCS#7 certs on my Windows system, though.

    Instead of a flag I could also return a string: "CERTIFICATE" for X509_ASN_ENCODING cert, "X509 CRL" for X509_ASN_ENCODING CRL or "PKCS7" for PKCS#7 encoded certs.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 9, 2013

    New changeset 10d325f674f5 by Christian Heimes in branch 'default':
    Issue bpo-17134: Add ssl.enum_cert_store() as interface to Windows' cert store.
    http://hg.python.org/cpython/rev/10d325f674f5

    @pitrou
    Copy link
    Member

    pitrou commented Jun 9, 2013

    New changeset 10d325f674f5 by Christian Heimes in branch 'default':
    Issue bpo-17134: Add ssl.enum_cert_store() as interface to Windows' cert store.
    http://hg.python.org/cpython/rev/10d325f674f5

    I don't want to sound annoying, but I would have liked to review this
    before it goes in. Could it wait a few days? (I'm sure it can :-))

    @tiran
    Copy link
    Member Author

    tiran commented Jun 10, 2013

    Ezio already reviewed my code. But sure I can wait a couple of days. The second part of the patch depends on bpo-18138 anyway.

    @tiran
    Copy link
    Member Author

    tiran commented Jul 28, 2013

    I guess I have to revise my patch and go throw Windows' crypto lookup functions...

    Automatic CA root certificate updates on Windows http://netsekure.org/2011/04/automatic-ca-root-certificate-updates-on-windows/

    @tiran
    Copy link
    Member Author

    tiran commented Oct 9, 2013

    The current implementation doesn't check the trust settings and purpose of certs.

    CertGetCertificateContextProperty() with CERT_ENHKEY_USAGE_PROP_ID returns a ASN.1 structure. I just have to figure out how to parse the CTL_USAGE struct ...

    http://msdn.microsoft.com/en-us/library/aa376079%28v=vs.85%29.aspx
    http://msdn.microsoft.com/en-us/library/aa381493%28v=vs.85%29.aspx
    http://www.alvestrand.no/objectid/1.3.6.1.5.5.7.3.html

    @tiran
    Copy link
    Member Author

    tiran commented Oct 20, 2013

    The new patch splits up the one function into enum_certificates() and enum_crls(). enum_certificates() now returns also trust settings for the certificate. Internally it maps the most common OIDs to human readable names.

    The patch comes without doc updates yet.

    @tiran
    Copy link
    Member Author

    tiran commented Oct 30, 2013

    Here is a simplified version of my patch with doc updates.

    Changes:

    • Different functions for certs and CRLs: enum_certificates() / enum_crls()

    • encoding is now a string ('x509_asn' or 'pkcs_7_asn')

    • for certificates trust information is either a set of OIDs or True. The OIDs can be interpreter with the new functions bpo-19448.

    Both functions are intended to be low level interfaces to Window's cert store.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 17, 2013

    The feature is not yet production-ready but part of the feature is already in 3.4. It depends on bpo-19448 and bpo-16487, too. What shall I do about it?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 22, 2013

    New changeset 9adcb61ea741 by Christian Heimes in branch 'default':
    Issue bpo-17134: Finalize interface to Windows' certificate store. Cert and
    http://hg.python.org/cpython/rev/9adcb61ea741

    @tiran tiran closed this as completed Nov 22, 2013
    @vstinner
    Copy link
    Member

    The test is failing:

    http://buildbot.python.org/all/builders/x86%20Windows%20Server%202003%20%5BSB%5D%203.x/builds/1758/steps/test/logs/stdio

    ======================================================================
    FAIL: test_enum_certificates (test.test_ssl.BasicSocketTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "E:\Data\buildslave\cpython\3.x.snakebite-win2k3r2sp2-x86\build\lib\test\test_ssl.py", line 553, in test_enum_certificates
        self.assertIn(serverAuth, names)
    AssertionError: '1.3.6.1.5.5.7.3.1' not found in {'1.3.6.1.5.5.7.3.3', '1.3.6.1.4.1.311.10.3.5', '2.16.840.1.113730.4.1', '2.16.840.1.113733.1.8.1'}

    @vstinner vstinner reopened this Nov 22, 2013
    @tiran
    Copy link
    Member Author

    tiran commented Nov 22, 2013

    That's strange. It looks like the Win2k box has no root CA certs for serverAuth installed whatsoever. I'm adding Matthias to this ticket.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 22, 2013

    New changeset de65df13ed50 by Christian Heimes in branch 'default':
    Issue bpo-17134: check certs of CA and ROOT system store
    http://hg.python.org/cpython/rev/de65df13ed50

    @tiran
    Copy link
    Member Author

    tiran commented Dec 4, 2013

    The tests are passing again. Thanks!

    @tiran tiran closed this as completed Dec 4, 2013
    @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 type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants