gh-62433: Add SSLSocket.get_verified_chain() and SSLSocket.get_unverified_chain() by chrisburr · Pull Request #17938 · python/cpython · GitHub

Based on the patch provided by Christian Heimes (christian.heimes) and updated by Mariusz Masztalerczuk (mmasztalerczuk). Updated to use SSL_get0_verified_chain in OpenSSL 1.1 as suggested by Jörn Heissler (joernheissler).

Tested with both OpenSSL 1.0.2 and 1.1.1 using the included test and:

import ssl
import socket
ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
ctx.load_default_certs()
good_hosts = [
    'google.com',
    'example.com',
]
bad_hosts = [
    'expired.badssl.com',
    'self-signed.badssl.com',
    'untrusted-root.badssl.com',
]
for host in good_hosts + bad_hosts:
    print('host =', host)
    ctx.verify_mode = ssl.CERT_NONE
    with ctx.wrap_socket(socket.socket(socket.AF_INET), server_hostname=host) as s:
        s.connect((host, 443))
        chain_no_validate = s.getpeercertchain(validate=False)
        try:
            chain_validate = s.getpeercertchain(validate=True)
        except Exception as e:
            if host in bad_hosts:
                print('Failed as expected with', e)
            else:
                raise Exception('Failed to validate good host')
        else:
            if host not in good_hosts:
                raise Exception('Managed to validate bad host')

https://bugs.python.org/issue18233

Issue: SSLSocket.getpeercertchain() #62433

I like that there's the option to get both the raw chain and the verified chain – these are different things and both are important for different purposes. But the docs should be clearer about the difference.

Maybe fetching the verified chain should only be supported when building against openssl 1.1.1? That would simplify this patch a lot, and follow the general trend of trying to get rid of custom cert verification logic in the ssl module.

For the unverified chain, the openssl docs say:

SSL_get_peer_cert_chain() returns a pointer to STACK_OF(X509) certificates forming the certificate chain sent by the peer. If called on the client side, the stack also contains the peer's certificate; if called on the server side, the peer's certificate must be obtained separately using SSL_get_peer_certificate(3).

I think this is confusing, and we should hide the confusing bit from python users. My preference would be for the python wrapper to always return the complete cert chain, including the leaf. So when necessary, the wrapper should manually call SSL_get_peer_certificate and add the leaf cert.

Thanks for taking a look so promptly.

Maybe fetching the verified chain should only be supported when building against openssl 1.1.1? That would simplify this patch a lot, and follow the general trend of trying to get rid of custom cert verification logic in the ssl module.

I'm very happy to do that if it's acceptable to only partially support openssl 1.0.2 and throw an exception if verified=True.

I think this is confusing, and we should hide the confusing bit from python users. My preference would be for the python wrapper to always return the complete cert chain, including the leaf. So when necessary, the wrapper should manually call SSL_get_peer_certificate and add the leaf cert.

I definitely agree, I'll implement it soon 👍

Maybe fetching the verified chain should only be supported when building against openssl 1.1.1? That would simplify this patch a lot, and follow the general trend of trying to get rid of custom cert verification logic in the ssl module.

I'm very happy to do that if it's acceptable to only partially support openssl 1.0.2 and throw an exception if verified=True.

I implemented it in df65d40 but I've changed my mind as it makes using getpeercertchain clunky. I don't think it should error in OpenSSL 1.0.2 with it's default arguments and I also don't think validate=False should be the default. If people disagree I'll cherry pick df65d40 into this PR.

OpenSSL 1.0.2 and 1.1.0 have reached EOL and are no longer supported by upstream. Please don't add workarounds / backports for these versions. Just make sure that Python can still be compiled with 1.0.2.

IMO there should be two new methods, one to get the raw peer cert chain and another one to get the "verified" chain. I put "verified" in quotes because the term is misleading. SSL_get0_verified_chain() can return a valid chain although the SSLSocket is configured to not require a valid chain and the verify result is not X509_V_OK. On the other hand the function can return NULL in combination with X509_V_OK, e.g. when a session is resumed.

Internally OpenSSL always builds and validates the chain, even with SSL_VERIFY_NONE. The flag SSL_VERIFY_NONE merely suppressed non-fatal validation errors.

I also like to get rid of binary madness and finally introduce proper certificate objects. I have a working implementation on my disk, but it needs a bit of polishing.

That's not necessarily correct. SSL_get_peer_cert_chain() returns whatever the client sends. It may just be the EE cert (end entity), EE+intermediates, EE+intermediate+root, EE with intermediates for primary and alternative chains, or whatever the admin for the site has configured. I guess the peer chain can even include unrelated junk.

It's also worth mentioning that the chain is not available with TLS session resumption.

SSL_R_CERTIFICATE_VERIFY_FAILED is always defined on 1.1.0+.

peer_chain can be NULL.

peer_chain can be NULL here.

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

By the way LibreSSL doesn't have SSL_get0_verified_chain(), too.

Thanks for the review @tiran!

IMO there should be two new methods, one to get the raw peer cert chain and another one to get the "verified" chain. I put "verified" in quotes because the term is misleading.

By the way LibreSSL doesn't have SSL_get0_verified_chain(), too.

That makes sense. I have two follow up questions:

Should I use snake case for the method names, i.e. getpeercertchain or get_peer_cert_chain? It seems like newer methods on SSLSocket do (e.g. get_channel_binding) but older ones like getpeercert don't.
If I split this addition and add a getverifiedchain method, is it acceptable to always raise an exception when using LibreSSL?

Please use the newer style get_egg_spam.
In case of OpenSSL 1.0.2 and LibreSSL the method should not be present. This allows users to perform a feature check with hasattr(). If you surround the whole C function with #ifdef then argument clinic does the rest for you.

Thanks for the quick reply. I've made the changes and tested them with both OpenSSL 1.0 and 1.1 and it behaves as you propose.

For the sake of the bot: I have made the requested changes; please review again

Thanks for making the requested changes!

@tiran: please review the changes made to this pull request.

You know, looking at this again today, I'd suggest naming the two methods: get_unverified_chain and get_verified_chain (or maybe get_(un)verified_peer_cert_chain or similar, depending on how verbose we want to be). The point is that if you're reading some code and see a call to the unverified method, we want it to be immediately obvious that it's unverified :-) If we call the method get_peer_cert_chain then it's easier to accidentally slip through an audit.

@tiran Sorry to pester you, but could you take another look when you get a chance?

@tiran Hi again, would you be able to take another look at this? I'd like to get it in before the 3.9 feature freeze if possible.

I stumbled on this pull request in my search for exactly this functionality. Is there anything I can do to help get this across the line?

I've just rebased to resolve a conflict from the clinic generated hash at the end of Modules/clinic/_ssl.c.h.

I'm also still very keen to get this in so I'm happy to do anything that is needed.

Just an extra push for this PR that I am really eager to see merged as well

I poked @tiran about this PR on IRC just now, and he said:

he wants to first add a (minimal) "certificate object" class, before adding new APIs that return raw PEM strings
he has a draft implementation that needs some polish, and he plans to get to it before the 3.10 feature freeze
but in case that gets delayed, he said: "If I don't get the certificate class landed before 3.10 beta freeze I'll merge the PR."

Hmm, and this makes me wonder: @tiran, do you want any help on finishing and landing the certificate object code? It seems like some folks here would be eager to help...

Based on the patch provided by Christian Heimes (christian.heimes) and updated by Mariusz Masztalerczuk (mmasztalerczuk).

…hain()

…1.1.0+

…verified_chain

…rified_chain

Hey @tiran, has there been any progress with the new certificate objects? I'm still happy to help out if it's useful.

@chrisburr I've been looking into this. https://github.com/python/cpython/blob/main/Modules/_ssl/cert.c has the new certificate class as far as I can tell. If you can get an SSLSocket object from the ssl library and do socket._sslobj.get_verified_chain() you'll see a list of them returned and calling get_info() on them gives you a similarly structured dictionary as getpeercert() does on SSLSocket.

It seems another PR got merged that provides a similar functionality: #109113

This PR is stale because it has been open for 30 days with no activity.

chrisburr requested a review from tiran as a code owner January 10, 2020 15:55

the-knights-who-say-ni added the CLA signed label Jan 10, 2020

bedevere-bot added the awaiting review label Jan 10, 2020

chrisburr force-pushed the fix-issue-18233 branch from 4f88c3d to 8586656 Compare January 10, 2020 16:22

tiran requested changes Jan 16, 2020

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Jan 16, 2020

chrisburr force-pushed the fix-issue-18233 branch 3 times, most recently from 5366c81 to bf6e309 Compare January 16, 2020 12:18

chrisburr changed the title bpo-18233: Add SSLSocket.getpeercertchain() bpo-18233: Add SSLSocket.get_verified_chain() and SSLSocket.get_peer_cert_chain() Jan 16, 2020

bedevere-bot added awaiting change review and removed awaiting changes labels Jan 16, 2020

bedevere-bot requested a review from tiran January 16, 2020 12:55

chrisburr changed the title bpo-18233: Add SSLSocket.get_verified_chain() and SSLSocket.get_peer_cert_chain() bpo-18233: Add SSLSocket.get_verified_chain() and SSLSocket.get_unverified_chain() Jan 31, 2020

chrisburr force-pushed the fix-issue-18233 branch from d0b6a5d to 3d21652 Compare June 17, 2020 08:14

chrisburr force-pushed the fix-issue-18233 branch from 3d21652 to 3a6435a Compare October 5, 2020 12:49

chrisburr closed this Oct 5, 2020

chrisburr reopened this Oct 5, 2020

blurb-it Bot and others added 6 commits December 9, 2020 15:09

📜🤖 Added by blurb_it.

33bfd92

bpo-18233: Add SSLSocket.getpeercertchain()

7008f61

Based on the patch provided by Christian Heimes (christian.heimes) and updated by Mariusz Masztalerczuk (mmasztalerczuk).

bpo-18233: Always return peer certificate from SSLSocket.getpeercertc…

4243cc6

…hain()

bpo-18233: Only support getpeercertchain(validate=True) with OpenSSL …

a7c641d

…1.1.0+

bpo-18233: Review of SSLSocket.get_peer_cert_chain and SSLSocket.get_…

3324381

…verified_chain

bpo-18233: Rename SSLSocket.get_peer_cert_chain to SSLSocket.get_unve…

1f8dfd6

…rified_chain

chrisburr force-pushed the fix-issue-18233 branch from 3a6435a to 1f8dfd6 Compare December 9, 2020 14:10

bpo-18233: Update added in version to 3.10

dd39c49

ezio-melotti removed the CLA signed label Jul 13, 2022

tiran mentioned this pull request Nov 15, 2022

SSLSocket.getpeercertchain() #62433

Open

felixfontein mentioned this pull request Feb 3, 2023

Get a certificate chain with crypto.get_certificate module ansible-collections/community.crypto#568

Closed

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 7, 2026

serhiy-storchaka changed the title bpo-18233: Add SSLSocket.get_verified_chain() and SSLSocket.get_unverified_chain() gh-62433: Add SSLSocket.get_verified_chain() and SSLSocket.get_unverified_chain() Aug 12, 2026

github-actions Bot removed the stale Stale PR or inactive for long period of time. label Aug 14, 2026

gh-62433: Add SSLSocket.get_verified_chain() and SSLSocket.get_unverified_chain() by chrisburr · Pull Request #17938 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-62433: Add SSLSocket.get_verified_chain() and SSLSocket.get_unverified_chain() - #17938

gh-62433: Add SSLSocket.get_verified_chain() and SSLSocket.get_unverified_chain()#17938chrisburr wants to merge 7 commits intopython:mainpython/cpython:mainfrom chrisburr:fix-issue-18233chrisburr/cpython:fix-issue-18233Copy head branch name to clipboard

Conversation

chrisburr commented Jan 10, 2020 • edited by serhiy-storchaka Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

njsmith commented Jan 10, 2020

Uh oh!

chrisburr commented Jan 10, 2020

Uh oh!

chrisburr commented Jan 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tiran commented Jan 16, 2020

Uh oh!

tiran Jan 16, 2020

Choose a reason for hiding this comment

Uh oh!

tiran Jan 16, 2020

Choose a reason for hiding this comment

Uh oh!

tiran Jan 16, 2020

Choose a reason for hiding this comment

Uh oh!

tiran Jan 16, 2020

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jan 16, 2020

Uh oh!

tiran commented Jan 16, 2020

Uh oh!

chrisburr commented Jan 16, 2020

Uh oh!

tiran commented Jan 16, 2020

Uh oh!

chrisburr commented Jan 16, 2020

Uh oh!

bedevere-bot commented Jan 16, 2020

Uh oh!

njsmith commented Jan 31, 2020

Uh oh!

chrisburr commented Apr 1, 2020

Uh oh!

chrisburr commented May 11, 2020

Uh oh!

dustin-johnson commented Jun 17, 2020

Uh oh!

chrisburr commented Jun 17, 2020

Uh oh!

chaen commented Jun 30, 2020

Uh oh!

njsmith commented Jul 5, 2020

Uh oh!

chrisburr commented Mar 2, 2021

Uh oh!

sigmavirus24 commented Nov 16, 2022

Uh oh!

felixfontein commented Dec 22, 2023

Uh oh!

github-actions Bot commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

gh-62433: Add SSLSocket.get_verified_chain() and SSLSocket.get_unverified_chain()#17938
chrisburr wants to merge 7 commits into
python:mainfrom
chrisburr:fix-issue-18233

chrisburr commented Jan 10, 2020 •
edited by serhiy-storchaka
Loading

chrisburr commented Jan 13, 2020 •
edited
Loading