gh-118658: Modify cert generation script to extract cert3.pem#124598

felixfontein · 2024-09-26T13:30:51Z

This updates #118669 to extract the added file cert3.pem from keycert3.pem.

I'm not sure whether using the same issue number in the title is ok...

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

felixfontein · 2024-09-26T13:32:25Z

I cannot really test this since the script doesn't work for me at all (it produces an invalid OpenSSL command line before reaching the part I modified), but I ran the code I added/modified manually and it seems to work.

Lib/test/certdata/make_ssl_certs.py

kanavin · 2024-09-26T14:16:32Z

I have cherry-picked this. Sadly there are failures:

====================================================================== ERROR: test_certificate_chain (test.test_ssl.TestPostHandshakeAuth.test_certificate_chain) ---------------------------------------------------------------------- Traceback (most recent call last): File "/srv/work/alex/cpython/Lib/test/test_ssl.py", line 4708, in test_certificate_chain expected_ee_cert = ssl.PEM_cert_to_DER_cert(f.read()) File "/srv/work/alex/cpython/Lib/ssl.py", line 1498, in PEM_cert_to_DER_cert raise ValueError("Invalid PEM encoding; must start with %s" % PEM_HEADER) ValueError: Invalid PEM encoding; must start with -----BEGIN CERTIFICATE----- ====================================================================== FAIL: test_https_client_non_tls_response_ignored (test.test_ssl.TestPreHandshakeClose.test_https_client_non_tls_response_ignored) ---------------------------------------------------------------------- Traceback (most recent call last): File "/srv/work/alex/cpython/Lib/test/test_ssl.py", line 5150, in test_https_client_non_tls_response_ignored with warnings_helper.check_no_resource_warning(self), \ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^ File "/srv/work/alex/cpython/Lib/contextlib.py", line 148, in __exit__ next(self.gen) ~~~~^^^^^^^^^^ File "/srv/work/alex/cpython/Lib/test/support/warnings_helper.py", line 148, in check_no_resource_warning with check_no_warnings(testcase, category=ResourceWarning, force_gc=True): ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/srv/work/alex/cpython/Lib/contextlib.py", line 148, in __exit__ next(self.gen) ~~~~^^^^^^^^^^ File "/srv/work/alex/cpython/Lib/test/support/warnings_helper.py", line 131, in check_no_warnings testcase.assertEqual(warns, []) ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^ AssertionError: Lists differ: [<warnings.WarningMessage object at 0x7fe633996510>] != [] First list contains 1 additional elements. First extra element 0: <warnings.WarningMessage object at 0x7fe633996510> - [<warnings.WarningMessage object at 0x7fe633996510>] + [] ---------------------------------------------------------------------- Ran 2 tests in 0.081s

The generated cert3.pem looks like this:

Lib/test/certdata/make_ssl_certs.py

kanavin · 2024-09-26T14:57:43Z

CC @kanavin
I cannot really test this since the script doesn't work for me at all (it produces an invalid OpenSSL command line before reaching the part I modified), but I ran the code I added/modified manually and it seems to work.

I confirmed via #107594 that this fixes the problem. But you do need the above mentioned change, and the NEWS.d entry.

encukou · 2024-09-26T16:55:18Z

@gpshead Do you still have the context for this in your head? :)

felixfontein · 2024-09-26T18:35:31Z

Since the skip news label has been added, do I still have to add a NEWS.d entry?

kanavin · 2024-09-26T18:43:21Z

Since the skip news label has been added, do I still have to add a NEWS.d entry?

I don't think so, the CI check for that now passes. But this does need to be backported to 3.13.

encukou · 2024-09-26T18:48:10Z

It's a a test-only change, so no NEWS entry is needed , and a backport can go in 3.13.1.

encukou · 2024-10-04T09:30:50Z

Lib/test/certdata/make_ssl_certs.py

 f.write(key)
 f.write(cert)

+cert=extract_cert(cert, 0)


Instead of text manipulation, could we ask openssl to extract it?
As far as I can see, the incantation is:
openssl x509 -outform pem -in keycert3.pem -out cert3.pem

Great idea! Implemented in 021af09.

miss-islington-app · 2024-10-04T11:15:11Z

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

miss-islington-app · 2024-10-04T11:15:14Z

Sorry, @felixfontein and @encukou, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 480354dc236af9ae9d47b2520aa85fb7293c7b68 3.13

…3.pem (pythonGH-124598) (cherry picked from commit 480354d) Co-authored-by: Felix Fontein <felix@fontein.de>

bedevere-app · 2024-10-04T12:46:21Z

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

…H-124598) (GH-124972) (cherry picked from commit 480354d)

bedevere-appbot added the awaiting review label Sep 26, 2024

bedevere-appbot mentioned this pull request Sep 26, 2024
Inconsistent return types between SSLSocket and SSLObject certificate chain APIs #118658
Closed

felixfontein mentioned this pull request Sep 26, 2024
gh-118658: Return consistent types from get_un/verified_chain in SSLObject and SSLSocket#118669
Merged

felixfontein commented Sep 26, 2024
View reviewed changes

Lib/test/certdata/make_ssl_certs.py Outdated Show resolvedHide resolved

felixfontein mentioned this pull request Sep 26, 2024
gh-118658: Modify cert generation script to extract cert3.pem #124599
Closed

kanavin suggested changes Sep 26, 2024
View reviewed changes

Lib/test/certdata/make_ssl_certs.py Outdated Show resolvedHide resolved

bedevere-appbot added awaiting core review and removed awaiting review labels Sep 26, 2024

kanavin mentioned this pull request Sep 26, 2024
gh-107562: Lib/test: update test certificates to expire far in the future #107594
Merged

encukou added skip news topic-SSL labels Sep 26, 2024

felixfontein force-pushed the cert3 branch from 2ea9ff9 to 903e4eeCompare September 26, 2024 18:28

encukou added the needs backport to 3.13 bugs and security fixes label Sep 26, 2024

Modify cert generation script to extract cert3.pem.
2265e43

felixfontein force-pushed the cert3 branch from 903e4ee to 2265e43Compare September 28, 2024 05:39

encukou reviewed Oct 4, 2024
View reviewed changes

Use openssl CLI tool instead of text manipulation.
021af09

encukou approved these changes Oct 4, 2024
View reviewed changes

bedevere-appbot added awaiting merge and removed awaiting core review labels Oct 4, 2024

encukou merged commit 480354d into python:mainOct 4, 2024

bedevere-appbot removed the awaiting merge label Oct 4, 2024

miss-islington-appbot assigned encukou Oct 4, 2024

felixfontein deleted the cert3 branch October 4, 2024 12:24

felixfontein added a commit to felixfontein/cpython that referenced this pull request Oct 4, 2024
[3.13]pythongh-118658: Modify cert generation script to extract cert…
106f476
…3.pem (pythonGH-124598) (cherry picked from commit 480354d) Co-authored-by: Felix Fontein <felix@fontein.de>

bedevere-appbot removed the needs backport to 3.13 bugs and security fixes label Oct 4, 2024

encukou pushed a commit that referenced this pull request Oct 8, 2024
[3.13]gh-118658: Modify cert generation script to extract cert3.pem (G…
4eab6e8
…H-124598) (GH-124972) (cherry picked from commit 480354d)

Uh oh!

gh-118658: Modify cert generation script to extract cert3.pem#124598

gh-118658: Modify cert generation script to extract cert3.pem #124598

Uh oh!

Conversation

felixfontein commented Sep 26, 2024• edited by bedevere-app botLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felixfontein commented Sep 26, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kanavin commented Sep 26, 2024

Uh oh!

Uh oh!

kanavin commented Sep 26, 2024

Uh oh!

encukou commented Sep 26, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felixfontein commented Sep 26, 2024

Uh oh!

kanavin commented Sep 26, 2024

Uh oh!

encukou commented Sep 26, 2024

Uh oh!

encukouOct 4, 2024

Choose a reason for hiding this comment

Uh oh!

felixfonteinOct 4, 2024

Choose a reason for hiding this comment

Uh oh!

miss-islington-appbot commented Oct 4, 2024

Uh oh!

miss-islington-appbot commented Oct 4, 2024

Uh oh!

bedevere-appbot commented Oct 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

felixfontein commented Sep 26, 2024•
edited by bedevere-app bot
Loading

felixfontein commented Sep 26, 2024•
edited
Loading

encukou commented Sep 26, 2024•
edited
Loading