gh-107361: strengthen default SSL context flags #112389

woodruffw · 2023-11-25T04:35:58Z

See #107361: this adds VERIFY_X509_STRICT to make the default SSL context perform stricter (per RFC 5280) validation, as well as VERIFY_X509_PARTIAL_CHAIN to enforce more standards-compliant path-building behavior.

As part of this changeset, I had to tweak make_ssl_certs.py slightly to emit 5280-conforming CA certs. This changeset includes the regenerated certificates after that change.

CC @sethmlarson

Issue: ssl.create_default_context(): add VERIFY_X509_STRICT and VERIFY_X509_PARTIAL_CHAIN to the default verify_flagsÂ #107361

ðŸ“š Documentation preview ðŸ“š: https://cpython-previews--112389.org.readthedocs.build/

See python#107361: this adds `VERIFY_X509_STRICT` to make the default SSL context perform stricter (per RFC 5280) validation, as well as `VERIFY_X509_PARTIAL_CHAIN` to enforce more standards-compliant path-building behavior. As part of this changeset, I had to tweak `make_ssl_certs.py` slightly to emit 5280-conforming CA certs. This changeset includes the regenerated certificates after that change.

Doc/library/ssl.rst

Misc/NEWS.d/next/Library/2023-11-24-23-40-00.gh-issue-107361.v54gh46.rst

bedevere-app · 2023-11-29T08:15:36Z

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.

And if you don't make the requested changes, you will be poked with soft cushions!

pitrou · 2023-11-29T09:04:02Z

Side note: perhaps we mark all PEM files as generated?

Signed-off-by: William Woodruff <[email protected]>

woodruffw · 2023-11-29T09:24:50Z

Side note: perhaps we mark all PEM files as generated?

Done: I've marked certdata/*.{pem,0} as generated (the .0 files should also really be .pem I think -- I can do some cleanup there in a follow-up PR, if there's interest).

vstinner · 2023-11-29T12:05:15Z

Doc/whatsnew/3.13.rst

@@ -125,6 +124,11 @@ Other Language Changes
  equivalent of the :option:`-X frozen_modules <-X>` command-line option.
  (Contributed by Yilei Yang in :gh:`111374`.)

+* The :func:`ssl.create_default_context` API now includes
+  :data:`ssl.VERIFY_X509_PARTIAL_CHAIN` and :data:`ssl.VERIFY_X509_STRICT`
+  in its default flags.


Can you please explain (give instructions) how to get the previous behavior?

Also, how can someone write future-proof code by switching to these defaults in Python 3.12 and older? Maybe also provide instructions for that?

Would you mind to explain which kinds of certificates are no longer accepted? If someone is impacted, how certificates should be generated to fit with stricter security policy?

Can you please explain (give instructions) how to get the previous behavior?

Sure, I can add a note callout similar to the one for SSLv3.

Also, how can someone write future-proof code by switching to these defaults in Python 3.12 and older? Maybe also provide instructions for that?

The simplest way would probably be to either use create_default_context() or SSLContext() and twiddle ctx.verify_flags directly.

Do you think it makes sense to include that in the ssl.rst docs? It looks like there isn't precedent for doing so (unlike the callout for SSLv3). If so, I'm happy to do it ðŸ™‚

Would you mind to explain which kinds of certificates are no longer accepted? If someone is impacted, how certificates should be generated to fit with stricter security policy?

This has two parts:

VERIFY_X509_PARTIAL_CHAIN makes chain building strictly more permissive, not less. Specifically, it tells OpenSSL to build chains the way RFC 5280 tells it to, not the bespoke way it does by default. So this flag alone shouldn't cause any certificate rejections, only discoveries of more valid chains ðŸ™‚ -- no changes should be required on users' parts.

VERIFY_X509_STRICT tells OpenSSL to validate X.509 certificates per the RFC 5280 profile, which has more extension/criticality requirements than OpenSSL checks by default. In practice, this should have no effect on >99% of users: most root programs use 5280 as their baseline, and the Web PKI's CABF rules are also built on 5280.

TL;DR: No generation changes need to happen for VERIFY_X509_PARTIAL_CHAIN. For VERIFY_X509_STRICT, the small number of users doing things like openssl req manually should ensure that their generated certs conform to RFC 5280 (which they already need to, if they're being used in browsers or other contexts where 5280 or a superset is assumed).

Do you think it makes sense to include that in the ssl.rst docs?

Doing it in the what's new makes sense for sure - It doesn't hurt to also include it in the ssl.rst versionchanged section. People come at docs from multiple angles depending on what they're trying to do or when and how they are debugging things.

I was suggesting to add these information in Python doc. Since tuning a SLContext is complex, IMO it's worth it to elaborate that in ssl.rst and point to that from What's New In Python.

Either link to ssl doc which explains how to get Python 3.12 behavior, or repeat the instructions here. Something like:

context.verify_flags &= ~(_ssl.VERIFY_X509_PARTIAL_CHAIN | _ssl.VERIFY_X509_STRICT)

Added a callout similar to the one for SSLv3 here: 4ae44d1

That'll get linked via the ref to :func:ssl.create_default_context -- do you think another link makes sense, or is that sufficient?

Would it be possible to add a link from here to the this new doc? Or copy/paste it here?

I've copied the same .. note and example here, PTAL ðŸ™‚

vstinner · 2023-11-30T17:30:25Z

In practice, this should have no effect on >99% of users: most root programs use 5280 as their baseline, and the Web PKI's CABF rules are also built on 5280.

Your PR required to regenerate all PEM test certificates. Does it mean that they belong to the 1% of affected users?

woodruffw · 2023-11-30T21:19:50Z

Your PR required to regenerate all PEM test certificates. Does it mean that they belong to the 1% of affected users?

I made the numbers up for emphasis, but I'll stand by this having no effect on the vast majority of users: the vast majority use case is the Web PKI, which is significantly stricter than even VERIFY_X509_STRICT. The fact that the PEMs had to be regenerated is more a testament to how lax OpenSSL is by default than any actual common practice ðŸ™‚

Or put another way: most users are not manually generating certs with openssl req or, if they are, they're massaging them into RFC 5280 compliance because other ecosystems (like Go, Java, and OSes/major software) are much more strict.

Additional datapoints:

The MS root program is at least as strict as these changes: https://learn.microsoft.com/en-us/security/trusted-root/program-requirements
The Java root program specifies RFC 5280, making it at least as strict: https://www.oracle.com/java/technologies/javase/carootcertsprogram.html
Same with Adobe: https://helpx.adobe.com/acrobat/kb/approved-trust-list2.html

(I picked these semi-arbitrarily from https://pkic.org/ltl/)

sethmlarson

I'm ++ on the partial chain changes, my comments below apply to VERIFY_X509_STRICT:

Since OpenSSL will likely be the way folks are generating certificates and like you said, OpenSSL allows you to do things that isn't acceptable for Web PKI, is there any value in providing guidance on how one does generate a certificate with OpenSSL that works with VERIFY_X509_STRICT?

Also since we're recommending folks down a new code path to restore old behavior, should we create a new test which exercises that code path and verifies that "bad" certificates would be accepted?

What about non-Web PKI, does this change have any affect for certificates in that domain? I am less versed for this, so am relying a bit on others' experience.

pitrou · 2023-12-01T08:46:57Z

Since OpenSSL will likely be the way folks are generating certificates

Is that still the case? Aren't most users relying on something like Let's Encrypt nowadays (at least for Web certificates, but also perhaps e-mail)?

is there any value in providing guidance on how one does generate a certificate with OpenSSL that works with VERIFY_X509_STRICT?

I don't think writing an OpenSSL command line tutorial is really a responsibility for CPython maintainers.

woodruffw · 2023-12-01T09:18:53Z

Since OpenSSL will likely be the way folks are generating certificates and like you said, OpenSSL allows you to do things that isn't acceptable for Web PKI, is there any value in providing guidance on how one does generate a certificate with OpenSSL that works with VERIFY_X509_STRICT?

To clarify: I think most users won't be using OpenSSL to generate certs -- I think the majority will be getting their certs from a CA service (like Let's Encrypt), with a very small minority doing bespoke things. Putting an OpenSSL example in the docs might cause additional confusion, and IMO won't be too helpful anyways (since the context here is verification, in which case the user might not have any control over the certs they're being presented with).

What about non-Web PKI, does this change have any affect for certificates in that domain? I am less versed for this, so am relying a bit on others' experience

Nope, this should be wholly compatible with those -- "strict" validation for OpenSSL means "approximate RFC 5280," and the Web PKI profiles are a superset of RFC 5280 ðŸ™‚

In other words: all public web certificates should be compatible with these changes.

malemburg · 2023-12-01T10:12:02Z

These are the checks that OpenSSL applies when VERIFY_X509_STRICT is set: https://www.openssl.org/docs/man3.0/man1/openssl-verification-options.html#x509_strict (search for -x509_strict in case the link doesn't take you there directly)

These checks all make sense for certs generated with a trusted CA, e.g. Let's Encrypt.

The other common use case is having self-signed certs (e.g. for intranet/VPC devices). Those will already fail to verify with the current standard settings, since self-signed certs are rejected by OpenSSL's verify. You have to use the ssl._create_unverified_context() or a custom one to the same effect for those.

Note: The POP/FTP/IMAP and SMTP modules use the unverified context, since intranet/VPC mail and FTP servers will often not use certs created by a trusted CA for various reasons or they are referenced using IP addresses, which don't work with certs at all.

vstinner

I'm in favor of making Python more secure by default, but your PR shows that even Python core devs who maintain PEM files failed to generate certificates properly. So it's likely that at least another user will be affected by stricter default. I'm just asking for clear instructions on how to identify if you are are affected and you're affected, explain how to get the old behavior.

I don't think that Python should explain how to regenerate PEM certificates passing stricter default. I prefer to let users make their own choices.

Doc/library/ssl.rst

vstinner · 2023-12-01T11:38:59Z

Doc/whatsnew/3.13.rst

@@ -125,6 +124,11 @@ Other Language Changes
  equivalent of the :option:`-X frozen_modules <-X>` command-line option.
  (Contributed by Yilei Yang in :gh:`111374`.)

+* The :func:`ssl.create_default_context` API now includes
+  :data:`ssl.VERIFY_X509_PARTIAL_CHAIN` and :data:`ssl.VERIFY_X509_STRICT`
+  in its default flags.


Either link to ssl doc which explains how to get Python 3.12 behavior, or repeat the instructions here. Something like:

context.verify_flags &= ~(_ssl.VERIFY_X509_PARTIAL_CHAIN | _ssl.VERIFY_X509_STRICT)

woodruffw · 2023-12-01T11:53:34Z

Makes sense, I'll make those documentation changes today.

malemburg · 2023-12-01T13:00:02Z

Just to clarify: The CA certs used for testing had to be updated, since the strict check requires these additional constraints:

The basicConstraints of CA certificates must be marked critical.
CA certificates must explicitly include the keyUsage extension.

This was not the case for the root CA certs in the test data. After updating the CA certs, all other certs signed with these CA certs had to be resigned.

People will not often create root CA certs by hand (but yes, they might for test cases). So instead of creating uncertainty for regular user certificates, perhaps it's a good idea to highlight these stricter requirements for root CA certs.

Co-authored-by: Victor Stinner <[email protected]>

Signed-off-by: William Woodruff <[email protected]>

sethmlarson · 2023-12-05T16:21:56Z

@woodruffw I would still like a new test case for ensuring that disabling X509_VERIFY_STRICT would allow a "bad" certificate to be used. After that is added I am satisfied with this PR, thanks much!

Doc/library/ssl.rst

vstinner · 2023-12-06T00:10:12Z

Doc/whatsnew/3.13.rst

@@ -125,6 +124,11 @@ Other Language Changes
  equivalent of the :option:`-X frozen_modules <-X>` command-line option.
  (Contributed by Yilei Yang in :gh:`111374`.)

+* The :func:`ssl.create_default_context` API now includes
+  :data:`ssl.VERIFY_X509_PARTIAL_CHAIN` and :data:`ssl.VERIFY_X509_STRICT`
+  in its default flags.


Would it be possible to add a link from here to the this new doc? Or copy/paste it here?

Lib/ssl.py

Co-authored-by: Victor Stinner <[email protected]>

woodruffw · 2023-12-06T01:07:30Z

@woodruffw I would still like a new test case for ensuring that disabling X509_VERIFY_STRICT would allow a "bad" certificate to be used. After that is added I am satisfied with this PR, thanks much!

Making sure I understand: the idea is a "backstop" test to make sure that users have a downgrade path, right? If so, makes sense, and I'll work on that now.

â€¦flags

woodruffw · 2024-02-02T02:26:34Z

Lib/test/test_ssl.py

@@ -2945,6 +2956,36 @@ def test_ecc_cert(self):
                cipher = s.cipher()[0].split('-')
                self.assertTrue(cipher[:2], ('ECDHE', 'ECDSA'))

+    def test_verify_strict(self):


@sethmlarson This test provides a backstop check on VERIFY_X509_STRICT, PTAL!

woodruffw · 2024-02-02T02:28:00Z

Sorry for the delay here! I've updated this PR to include a testcase for strict verification, including a "backstop" test that ensures that disabling VERIFY_X509_STRICT allows this to verify as it did before the change.

OpenSSL is not consistent about error types between versions.

woodruffw · 2024-02-02T02:38:18Z

I'm a little mystified by this error, which only happens on OpenSSL 1.1.1w in CI:

 0:00:10 load avg: 2.63 [18/43/1] test_ssl failed (1 failure)
test test_ssl failed -- Traceback (most recent call last):
  File "/home/runner/work/cpython/cpython/Lib/test/test_ssl.py", line 2971, in test_verify_strict
    with self.assertRaises(ssl.SSLError):
        s.connect((HOST, server.port))
AssertionError: SSLError not raised

AFAICT, the strict flag exists in that version and has the same semantics as in OpenSSL 3.0 and 3.1, which both pass.

Edit: This is probably why: openssl/openssl@1e41dad (and openssl/openssl#10688)

â€¦flags

woodruffw · 2024-02-16T17:58:56Z

This should be good to go again: I've added a "backstop" test that confirms that VERIFY_X509_STRICT enables and disables as expected.

bedevere-app bot added the awaiting review label Nov 25, 2023

bedevere-app bot mentioned this pull request Nov 25, 2023

ssl.create_default_context(): add VERIFY_X509_STRICT and VERIFY_X509_PARTIAL_CHAIN to the default verify_flags #107361

Open

woodruffw added 2 commits November 24, 2023 23:47

add news entry

4782048

ssl: expand comment

732b953

sethmlarson added the type-security A security issue label Nov 25, 2023

gpshead requested review from sethmlarson and gpshead November 29, 2023 02:17

gpshead self-assigned this Nov 29, 2023

gpshead approved these changes Nov 29, 2023

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Nov 29, 2023

malemburg requested changes Nov 29, 2023

View reviewed changes

Doc/library/ssl.rst Show resolved Hide resolved

Misc/NEWS.d/next/Library/2023-11-24-23-40-00.gh-issue-107361.v54gh46.rst Show resolved Hide resolved

bedevere-app bot added awaiting changes and removed awaiting merge labels Nov 29, 2023

woodruffw added 4 commits November 29, 2023 10:12

ssl.rst: further explain new X509 flags

15c0313

Signed-off-by: William Woodruff <[email protected]>

whatsnew: add ssl.create_default_context changes

2aceb81

Signed-off-by: William Woodruff <[email protected]>

gitattributes: mark certdata as generated

eb2c6e4

Signed-off-by: William Woodruff <[email protected]>

gitattributes: avoid {} glob syntax

668803c

Signed-off-by: William Woodruff <[email protected]>

vstinner reviewed Nov 29, 2023

View reviewed changes

sethmlarson reviewed Dec 1, 2023

View reviewed changes

malemburg approved these changes Dec 1, 2023

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting changes labels Dec 1, 2023

vstinner reviewed Dec 1, 2023

View reviewed changes

woodruffw and others added 2 commits December 1, 2023 14:03

Update Doc/library/ssl.rst

da318a6

Co-authored-by: Victor Stinner <[email protected]>

ssl.rst: explain how to disable VERIFY_X509_STRICT

4ae44d1

Signed-off-by: William Woodruff <[email protected]>

vstinner reviewed Dec 6, 2023

View reviewed changes

woodruffw and others added 2 commits December 5, 2023 20:00

Apply suggestions from code review

792383a

Co-authored-by: Victor Stinner <[email protected]>

whatsnew/3.13: add note for disabling VERIFY_X509_STRICT

f1b59ed

Merge remote-tracking branch 'upstream/main' into default-ssl-verify-â€„1¤7

c8a7bb5

â€¦flags

This was referenced Feb 1, 2024

Figure out a good way to keep leaf cert keys around C2SP/x509-limbo#186

Closed

limbo: include peer keys, when possible C2SP/x509-limbo#187

Merged

test: add a backstop test for VERIFY_X509_STRICT

f6c3af3

woodruffw commented Feb 2, 2024

View reviewed changes

test: try a higher base error

ff087fb

OpenSSL is not consistent about error types between versions.

woodruffw added 3 commits February 15, 2024 20:52

test: require OpenSSL 3+ for test_verify_strict

e46a672

Merge remote-tracking branch 'upstream/main' into default-ssl-verify-â€„1¤7

fe42d9a

â€¦flags

test: whitespace

1a3e037

Jan	FEB	Apr
	17
2023	2024	2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-107361: strengthen default SSL context flags #112389

gh-107361: strengthen default SSL context flags #112389

woodruffw commented Nov 25, 2023 •

edited by github-actions bot

bedevere-app bot commented Nov 29, 2023

pitrou commented Nov 29, 2023

woodruffw commented Nov 29, 2023

vstinner Nov 29, 2023

vstinner Nov 29, 2023

woodruffw Nov 29, 2023

gpshead Nov 30, 2023

vstinner Nov 30, 2023

vstinner Dec 1, 2023

woodruffw Dec 1, 2023

vstinner Dec 6, 2023

woodruffw Dec 6, 2023

vstinner commented Nov 30, 2023

woodruffw commented Nov 30, 2023

sethmlarson left a comment •

edited

pitrou commented Dec 1, 2023

woodruffw commented Dec 1, 2023

malemburg commented Dec 1, 2023

vstinner left a comment

vstinner Dec 1, 2023

woodruffw commented Dec 1, 2023

malemburg commented Dec 1, 2023

sethmlarson commented Dec 5, 2023 •

edited

vstinner Dec 6, 2023

woodruffw commented Dec 6, 2023

woodruffw Feb 2, 2024

woodruffw commented Feb 2, 2024

woodruffw commented Feb 2, 2024 •

edited

woodruffw commented Feb 16, 2024

gh-107361: strengthen default SSL context flags #112389

Are you sure you want to change the base?

gh-107361: strengthen default SSL context flags #112389

Conversation

woodruffw commented Nov 25, 2023 • edited by github-actions bot

bedevere-app bot commented Nov 29, 2023

pitrou commented Nov 29, 2023

woodruffw commented Nov 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner commented Nov 30, 2023

woodruffw commented Nov 30, 2023

sethmlarson left a comment • edited

Choose a reason for hiding this comment

pitrou commented Dec 1, 2023

woodruffw commented Dec 1, 2023

malemburg commented Dec 1, 2023

vstinner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woodruffw commented Dec 1, 2023

malemburg commented Dec 1, 2023

sethmlarson commented Dec 5, 2023 • edited

Choose a reason for hiding this comment

woodruffw commented Dec 6, 2023

Choose a reason for hiding this comment

woodruffw commented Feb 2, 2024

woodruffw commented Feb 2, 2024 • edited

woodruffw commented Feb 16, 2024

woodruffw commented Nov 25, 2023 •

edited by github-actions bot

sethmlarson left a comment •

edited

sethmlarson commented Dec 5, 2023 •

edited

woodruffw commented Feb 2, 2024 •

edited