Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Doc/library/ssl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ purposes.
*cadata* is given) or uses :meth:`SSLContext.load_default_certs` to load
default CA certificates.

When :attr:`~SSLContext.keylog_filename` is supported and the environment
variable :envvar:`SSLKEYLOGFILE` is set, :func:`create_default_context`
enables key logging.
When the environment variable :envvar:`!SSLKEYLOGFILE` is set,
:func:`create_default_context` enables key logging by setting
:attr:`~SSLContext.keylog_filename` to the variable's value.

The default settings for this context include
:data:`VERIFY_X509_PARTIAL_CHAIN` and :data:`VERIFY_X509_STRICT`.
Expand Down
14 changes: 6 additions & 8 deletions Lib/ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -721,10 +721,9 @@ def create_default_context(purpose=Purpose.SERVER_AUTH, *, cafile=None,
# root CA certificates for the given purpose. This may fail silently.
context.load_default_certs(purpose)
# OpenSSL 1.1.1 keylog file
if hasattr(context, 'keylog_filename'):
keylogfile = os.environ.get('SSLKEYLOGFILE')
if keylogfile and not sys.flags.ignore_environment:
context.keylog_filename = keylogfile
keylogfile = os.environ.get('SSLKEYLOGFILE')
if keylogfile and not sys.flags.ignore_environment:
context.keylog_filename = keylogfile
return context

def _create_unverified_context(protocol=None, *, cert_reqs=CERT_NONE,
Expand Down Expand Up @@ -775,10 +774,9 @@ def _create_unverified_context(protocol=None, *, cert_reqs=CERT_NONE,
# root CA certificates for the given purpose. This may fail silently.
context.load_default_certs(purpose)
# OpenSSL 1.1.1 keylog file
if hasattr(context, 'keylog_filename'):
keylogfile = os.environ.get('SSLKEYLOGFILE')
if keylogfile and not sys.flags.ignore_environment:
context.keylog_filename = keylogfile
keylogfile = os.environ.get('SSLKEYLOGFILE')
if keylogfile and not sys.flags.ignore_environment:
context.keylog_filename = keylogfile
return context

# Used by http.client if no context is explicitly passed.
Expand Down
8 changes: 1 addition & 7 deletions Lib/test/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,7 @@
CAN_GET_SELECTED_OPENSSL_SIGALG = ssl.OPENSSL_VERSION_INFO >= (3, 5)
PY_SSL_DEFAULT_CIPHERS = sysconfig.get_config_var('PY_SSL_DEFAULT_CIPHERS')

HAS_KEYLOG = hasattr(ssl.SSLContext, 'keylog_filename')
requires_keylog = unittest.skipUnless(
HAS_KEYLOG, 'test requires OpenSSL 1.1.1 with keylog callback')
CAN_SET_KEYLOG = HAS_KEYLOG and os.name != "nt"
CAN_SET_KEYLOG = (os.name != "nt")
requires_keylog_setter = unittest.skipUnless(
CAN_SET_KEYLOG,
"cannot set 'keylog_filename' on Windows"
Expand Down Expand Up @@ -5453,7 +5450,6 @@ def keylog_lines(self, fname=os_helper.TESTFN):
with open(fname) as f:
return len(list(f))

@requires_keylog
def test_keylog_defaults(self):
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
Expand Down Expand Up @@ -5481,7 +5477,6 @@ def test_keylog_defaults(self):
with self.assertRaises(TypeError):
ctx.keylog_filename = 1

@requires_keylog
def test_keylog_filename(self):
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
client_context, server_context, hostname = testing_context()
Expand Down Expand Up @@ -5522,7 +5517,6 @@ def test_keylog_filename(self):
client_context.keylog_filename = None
server_context.keylog_filename = None

@requires_keylog
@unittest.skipIf(sys.flags.ignore_environment,
"test is not compatible with ignore_environment")
def test_keylog_env(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Unconditionally assume :attr:`ssl.SSLContext.keylog_filename` exists.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😇

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Loading