gh-118658: Return consistent types from get_un/verified_chain in SSLObject and SSLSocket

[matiuszka]

Fixses inconsistent return types between SSLObject and SSLSocket as described in gh-118656

• Issue: Inconsistent return types between SSLSocket and SSLObject certificate chain APIs #118658

[matiuszka]

I restored my old branch from which I created the original PR #109113. There is one file missing about NEWS, I am not sure if that was removed on purpose or not.

[sethmlarson]

Seems good to me, since these methods are now documented would it make sense to have a test case?

[matiuszka]

Seems good to me, since these methods are now documented would it make sense to have a test case?

Why not, it will not hurt. I will add it.

[felixfontein]

@matiuszka do you plan to add the test cases soon? I would love to see this PR merged.

[matiuszka]

@matiuszka do you plan to add the test cases soon? I would love to see this PR merged.

Sorry, I forgot about this issue. I will be available next week, so probably next Monday I will have it ready.

[matiuszka]

@sethmlarson, @felixfontein I tried to write tests for it but I am not sure how to start. There are no tests for the SSL module, so there is nothing that I can use to start.

I would need a boilerplate code to see how to execute such a test.

[matiuszka]

@felixfontein I applied your remarks.

[felixfontein]

From my POV this is ready to go. (I have no idea who actually has to approve/merge this though, and how to get it backported to 3.13...)

[sethmlarson]

Thanks for this, LGTM. I'm not a core developer so can't merge this myself, unfortunately.

[sethmlarson]

As far as backporting to 3.13, I'll tag in @Yhg1s. The gist is that this API solidified, but didn't get applied to both places where peer certificates can be fetched. Not backporting this will be confusing to other Python implementations which will start implementing the API now that it's public in 3.13 and potentially leave the APIs in a strange place wrt backwards compatibility.

There are only two projects which used these APIs that I'm aware of prior to their public availability and they both should be robust to a backport of this API.

[felixfontein]

I assume you mean https://github.com/sethmlarson/truststore/pull/137/files#diff-64ad03ae2eedc4414b98845398d17597aa584eccc854cabd4efe9f9445e6a9faR285 and https://github.com/ansible-collections/community.crypto/pull/784/files#diff-635a318cc493470aa9c8734ff6623a4ff998bf00cf457197469268f21bea52faR524 ?

[miss-islington-app]

Thanks @matiuszka for the PR, and @Yhg1s for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

[bedevere-app]

GH-123082 is a backport of this pull request to the 3.13 branch.

[kanavin]

Sadly, I think this has to be reverted, as it broke certificate generation script, and doesn't contain any other way to generate cert3.pem:
#107594 (comment)

Alternatively, you need to produce a fix to that script that includes correct regeneration of cert3.pem.

[felixfontein]

@kanavin how about simply disabling the broken test instead of reverting the whole PR? It would be pretty sad if the interface would be re-broken by reverting this.

[felixfontein]

I looked at the certs; cert3.pem is simply the last certificate in keycert3.pem, so extracting it from there should be easy.

[felixfontein]

#124598 should fix Lib/test/certdata/make_ssl_certs.py to extract cert3.pem from keycert3.pem.

[felixfontein]

#124599 is simpler and modifies cert3.pem (by adding the comment that's part of keycert3.pem). It shouldn't affect the test. Unfortunately it doesn't work since the comment breaks PEM_cert_to_DER_cert...