chore: tidy up SSL context creation now that we've dropped Python 3.8 by james-garner-canonical · Pull Request #2364 · canonical/operator

james-garner-canonical · 2026-03-05T03:09:07Z

This PR tidies up SSL context in ops-tracing, using create_default_context now that we've dropped Python 3.8.

Due to changes to the default behaviour in Python 3.13, we still need two lines of manual changes afterwards. The docs say:

The protocol, options, cipher and other settings may change to more restrictive values anytime without prior deprecation. The values represent a fair balance between compatibility and security.
If your application needs specific settings, you should create a SSLContext and apply the settings yourself.

This makes me wonder if we should stick to manually creating the SSLContext ourselves, though the docs do indicate that applying the settings is OK too -- I'm just wondering if anything else relevant might change underneath us.

Resolves #2055

james-garner-canonical · 2026-03-05T03:16:40Z

tracing/ops_tracing/_export.py

-        # Note that ssl.create_default_context() doesn't allow setting the context.protocol in a
-        # way that's the same across Python 3.8 and 3.10 onwards. Whip the context up by hand.
-        context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
+        context = ssl.create_default_context(cadata=ca)


ssl.create_default_context defaults to purpose=Purpose.SERVER_AUTH. This uses SSLContext(ssl.PROTOCOL_TLS_CLIENT).

james-garner-canonical · 2026-03-05T03:19:12Z

tracing/ops_tracing/_export.py

+        # The partial chain flag allows trusting intermediate CAs in the CA list
+        # without the matching root CA.
+        # Enabled by default in Python 3.13+, needed explicitly before that.
+        context.verify_flags |= ssl.VERIFY_X509_PARTIAL_CHAIN


This diff is annoyingly interleaved here. This chunk of additions corresponds to these lines below:

- if partial_chain := getattr(ssl, 'VERIFY_X509_PARTIAL_CHAIN', None): - # Available starting from Python 3.10. The partial chain flag allows trusting an - # intermediate CAs in the CA list without the matching root CA. - context.verify_flags |= partial_chain

james-garner-canonical · 2026-03-05T03:21:06Z

tracing/ops_tracing/_export.py

+        # Enabled by default in Python 3.13+, so must be explicitly cleared.
+        context.verify_flags &= ~ssl.VERIFY_X509_STRICT


This diff is annoyingly interleaved here. This addition corresponds to this line above:

- # context.verify_flags |= ssl.VERIFY_X509_STRICT

And matches this retained comment:

# Can't use strict certificate chain validation until our self-signed ca is fixed # https://github.com/canonical/self-signed-certificates-operator/issues/330 # https://github.com/canonical/tls-certificates-interface/pull/333

james-garner-canonical · 2026-03-05T03:22:15Z

tracing/ops_tracing/_export.py

-        if ca is not None:
-            context.load_verify_locations(cadata=ca)
-        else:
-            context.load_default_certs()


This is now handled by create_default_context. Per the docs:

Passing SERVER_AUTH as purpose sets verify_mode to CERT_REQUIRED and either loads CA certificates (when at least one of cafile, capath or cadata is given) or uses SSLContext.load_default_certs() to load default CA certificates.

dimaqq · 2026-03-05T06:20:41Z

tracing/ops_tracing/_export.py

+        # Enabled by default in Python 3.13+, needed explicitly before that.
+        context.verify_flags |= ssl.VERIFY_X509_PARTIAL_CHAIN
        # Can't use strict certificate chain validation until our self-signed ca is fixed
        # https://github.com/canonical/self-signed-certificates-operator/issues/330


I though we'd be able to drop that hack by now, but alas!
https://matrix.to/#/!yAkGlrYcBFYzYRvOlQ:ubuntu.com/$ibCVWod_F_dRl7afdHzfgDI5lGTbFhZI3jKtaQVbX50?via=ubuntu.com&via=matrix.org&via=laquadrature.net

dimaqq · 2026-03-05T06:26:33Z

How I would validate this change:

Hack up Ops (ipython or unit test) to try to connect to (somewhere).
Run the {old,new} code x Python {3.8, 3.10, 3.12}.
Capture the traffic (e.g. wireshark) and inspect the initial handshake

How to inspect: one of

A public HTTPS endpoint (like google.com) + Wireshark, or
openssl s_server with some cert and cmd line flags that show decoded packets

(We've got integration tests, one uses self-signed-certs, but we don't have an integration test against a real cert provider).

james-garner-canonical · 2026-03-05T07:21:50Z

How I would validate this change:

Hack up Ops (ipython or unit test) to try to connect to (somewhere).

Run the {old,new} code x Python {3.8, 3.10, 3.12}.

Capture the traffic (e.g. wireshark) and inspect the initial handshake

How to inspect: one of

A public HTTPS endpoint (like google.com) + Wireshark, or

openssl s_server with some cert and cmd line flags that show decoded packets

I'm not sure that this is necessary, as the SSLContext objects created with code on main and the code from this PR should be identical on all the Python versions we test on (Python 3.10, 3.12, 3.14). I've added a test to demonstrate this, though I'm not sure it's worth keeping it around.

(We've got integration tests, one uses self-signed-certs, but we don't have an integration test against a real cert provider).

I forgot that we don't run integration tests on PRs, I've kicked off a run here.

dimaqq · 2026-03-05T07:25:19Z

tracing/test/test_backend.py

+    for attr in dir(new):
+        new_attr = getattr(new, attr)
+        if callable(new_attr):
+            continue
+        assert new_attr == getattr(original, attr)


If this only were so simple.

IIRC we set ALPN or NPN protocols, but those can't be inspected.
I can't be sure if that's the only setting like that.

The point is that the change needs to be validated outside of Python process.

P.S. this equivalence test should be transient, I don't think that it needs to be checked in.

True, one of the unchanged lines in in updated method is context.set_alpn_protocols(['http/1.1']), and it doesn't look like we can get them back from the context. But given that context is still an ssl.SSLContext, why would we expect this line to do anything different now? create_default_context doesn't touch ALPN protocols.

I guess the real concern is that there are some other under the hood settings that we care about that are broken for us with create_default_context. If that's the case, then given that the docs indicate that create_default_context may change settings to be 'more restrictive' at any time, I wonder if create_default_context is the right fit for us, as we'd need to repeat manual validation whenever ssl changes.

P.S. this equivalence test should be transient, I don't think that it needs to be checked in.

Agreed.

dimaqq · 2026-03-05T07:31:14Z

tracing/ops_tracing/_export.py

+        context = ssl.create_default_context(cadata=ca)
        context.minimum_version = ssl.TLSVersion.TLSv1_2  # See comment at the top of module
        context.set_alpn_protocols(['http/1.1'])
+        # The partial chain flag allows trusting intermediate CAs in the CA list
+        # without the matching root CA.
+        # Enabled by default in Python 3.13+, needed explicitly before that.
+        context.verify_flags |= ssl.VERIFY_X509_PARTIAL_CHAIN
        # Can't use strict certificate chain validation until our self-signed ca is fixed
        # https://github.com/canonical/self-signed-certificates-operator/issues/330
        # https://github.com/canonical/tls-certificates-interface/pull/333
-        # context.verify_flags |= ssl.VERIFY_X509_STRICT
-        if partial_chain := getattr(ssl, 'VERIFY_X509_PARTIAL_CHAIN', None):
-            # Available starting from Python 3.10. The partial chain flag allows trusting an
-            # intermediate CAs in the CA list without the matching root CA.
-            context.verify_flags |= partial_chain
-
-        if ca is not None:
-            context.load_verify_locations(cadata=ca)
-        else:
-            context.load_default_certs()
-
+        # Enabled by default in Python 3.13+, so must be explicitly cleared.
+        context.verify_flags &= ~ssl.VERIFY_X509_STRICT


Having eyeballed this, it looks good.

tonyandrewmeyer

I haven't reviewed this in detail - one question first: it feels like the goal of the issue was to get rid of creating a custom context that was required to support 3.8. However, because of other issues, we're still creating a custom context. What are we gaining here? Should we be waiting for some future Python version instead?

james-garner-canonical · 2026-03-06T00:27:36Z

I haven't reviewed this in detail - one question first: it feels like the goal of the issue was to get rid of creating a custom context that was required to support 3.8. However, because of other issues, we're still creating a custom context. What are we gaining here? Should we be waiting for some future Python version instead?

Yeah I feel the same way, and I suspect that create_default_context is always going to have this issue, or at least require careful attention with every Python release.

tonyandrewmeyer · 2026-03-06T00:34:17Z

Yeah I feel the same way, and I suspect that create_default_context is always going to have this issue, or at least require careful attention with every Python release.

Does that mean that it's not worth doing this issue, or that we need to have an issue like this with each Python release to handle whatever quirks are there? Or something else?

dimaqq · 2026-03-09T01:08:49Z

Given that all we promise is this: https://documentation.ubuntu.com/ops/latest/reference/ops-tracing/#ops-tracing-security

I think that our CI should cover what we need.

Let's remove the transient equivalence test and merge.

dimaqq · 2026-03-09T01:13:53Z

P.S. we could push min TLS to 1.3, because:

https://charmhub.io/tempo-coordinator-k8s (min published revision is 79)
canonical/tempo-coordinator-k8s-operator@978259a (merged in rev 75)

However, that could be a subtle breaking change for someone with an older, deployed COS.
So I'll punt that change to Ops 4.0 list.

dimaqq

Let's remove the equivalence test though.

chore: tidy up SSL context creation now that we've dropped Python 3.8

b226d6a

james-garner-canonical commented Mar 5, 2026

View reviewed changes

james-garner-canonical requested review from dimaqq and tonyandrewmeyer March 5, 2026 03:26

james-garner-canonical marked this pull request as ready for review March 5, 2026 03:26

dimaqq reviewed Mar 5, 2026

View reviewed changes

test: add a test to assert that the context we create hasn't changed

49d8ae8

dimaqq reviewed Mar 5, 2026

View reviewed changes

tonyandrewmeyer reviewed Mar 6, 2026

View reviewed changes

dimaqq approved these changes Mar 19, 2026

View reviewed changes

		# Enabled by default in Python 3.13+, so must be explicitly cleared.
		context.verify_flags &= ~ssl.VERIFY_X509_STRICT

Conversation

james-garner-canonical commented Mar 5, 2026 • edited by benhoyt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimaqq commented Mar 5, 2026

Uh oh!

james-garner-canonical commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonyandrewmeyer left a comment

Choose a reason for hiding this comment

Uh oh!

james-garner-canonical commented Mar 6, 2026

Uh oh!

tonyandrewmeyer commented Mar 6, 2026

Uh oh!

dimaqq commented Mar 9, 2026

Uh oh!

dimaqq commented Mar 9, 2026

Uh oh!

dimaqq left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

james-garner-canonical commented Mar 5, 2026 •

edited by benhoyt

Loading

james-garner-canonical commented Mar 5, 2026 •

edited

Loading