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

@@ -266,6 +283,10 @@ def write_cert_reference(path):
        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 <[email protected]>

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-app bot added the awaiting review label Sep 26, 2024

bedevere-app bot 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 resolved Hide 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 resolved Hide resolved

bedevere-app bot 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 903e4ee Compare 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 2265e43 Compare 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-app bot added awaiting merge and removed awaiting core review labels Oct 4, 2024

encukou merged commit 480354d into python:main Oct 4, 2024
34 checks passed

bedevere-app bot removed the awaiting merge label Oct 4, 2024

miss-islington-app bot 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 <[email protected]>

bedevere-app bot 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)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

felixfontein commented Sep 26, 2024 •

edited by bedevere-app bot

Loading

felixfontein commented Sep 26, 2024 •

edited

Loading

kanavin commented Sep 26, 2024

kanavin commented Sep 26, 2024

encukou commented Sep 26, 2024 •

edited

Loading

felixfontein commented Sep 26, 2024

kanavin commented Sep 26, 2024

encukou commented Sep 26, 2024

encukou Oct 4, 2024

felixfontein Oct 4, 2024

miss-islington-app bot commented Oct 4, 2024

miss-islington-app bot commented Oct 4, 2024

bedevere-app bot commented Oct 4, 2024

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

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

Conversation

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

felixfontein commented Sep 26, 2024 • edited Loading

kanavin commented Sep 26, 2024

kanavin commented Sep 26, 2024

encukou commented Sep 26, 2024 • edited Loading

felixfontein commented Sep 26, 2024

kanavin commented Sep 26, 2024

encukou commented Sep 26, 2024

encukou Oct 4, 2024

Choose a reason for hiding this comment

felixfontein Oct 4, 2024

Choose a reason for hiding this comment

miss-islington-app bot commented Oct 4, 2024

miss-islington-app bot commented Oct 4, 2024

bedevere-app bot commented Oct 4, 2024

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