gh-150898: Assume OpenSSL supports keylogging

[robsdedude]

Since version 3.10, CPython requires OpenSSL 1.1.1 or higher.
Therefore, support for keylogging is always available.

Initially I was confused by the docs for create_default_context stating

When keylog_filename is supported [...]

but the docs for keylog_filename not mentioning why or how it could be
unsupported.

Looking at the C code also confirms that keylog_filename is always defined:

cpython/Modules/_ssl.c

Lines 5733 to 5734 in 7a468a1

	{"keylog_filename", _PySSLContext_get_keylog_filename,
	_PySSLContext_set_keylog_filename, NULL},

vs in 3.9 where it was not always present:

cpython/Modules/_ssl.c

Lines 4891 to 4894 in 0bbaf5d

	#ifdef HAVE_OPENSSL_KEYLOG
	{"keylog_filename", (getter) _PySSLContext_get_keylog_filename,
	(setter) _PySSLContext_set_keylog_filename, NULL},
	#endif

Closes: gh-150898

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #33042844 | 📁 Comparing 7ea6fcb against main (7a468a1)

  🔍 Preview build  

20 files changed · ± 20 modified

± Modified

• c-api/complex.html
• c-api/long.html
• c-api/typeobj.html
• extending/first-extension-module.html
• howto/functional.html
• library/bisect.html
• library/collections.html
• library/difflib.html
• library/importlib.resources.html
• library/io.html
• and 10 more...

[picnixz]

Yes please create an issue and a NEWS entry. I will not backport this though.

[robsdedude]

Issue created and linked. For the news-entry, I'm not so sure what to put in there. Nothing of this really concerns the user (besides the docs). https://devguide.python.org/getting-started/pull-request-lifecycle/#updating-news-and-what-s-new-in-python says about this:

Most changes made to the codebase deserve an entry in Misc/NEWS.d, except for the following:

• documentation changes
• test changes
• strictly internal changes with no user-visible effects
• [...]

This PR is exactly all of those 3. I've drafted a news entry anyway. Let me know what you think.

[picnixz]

I would prefer having it for those who use an alternative implementation, just in case. Idk if all AWS-LC versions support this or if BoringSSL does. So for a NEWS entry, just say that it is now unconditionally assumed to exist.

[robsdedude]

@picnixz I think I've addressed all comments. If you have time, I'd appreciate a review.

[vstinner]

LGTM.

cc @picnixz

[vstinner]

I don't think that a NEWS entry is needed, you can remove it. The change doesn't impact users.

[robsdedude]

I added it specifically as requested by @picnixz (#150870 (comment))

I'd like to wait for his input on this thread before removing it again 😇

[picnixz]

I'd prefer that we keep it, so that surprises know when it was changed. I'm also afraid of hunting regressions when there are OpenSSL forks involved for instance.

[vstinner]

"cc @picnixz": oh, @picnixz already approved the PR :-)