-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Please support logging of SSL master secret by env variable SSLKEYLOGFILE #78452
Comments
As discussed on the EuroPython 2018 it would be a great improvement if the python SSL module would respect the SSLKEYLOGFILE environment variable to log the master secret and the client random for packet trace decryption. The pycurl module compiled against libopenssl 1.1.0h does already work. OpenSSL 1.1.1 will offer to register a callback that will log the keys. There is also c code available using LD_PRELOAD here: https://git.lekensteyn.nl/peter/wireshark-notes/tree/src/sslkeylog.c It would be great if a call to the requests, aiohttp, urllib3 or asks library would lead to the keys logged if the environment variable is set from within python. Thank you |
I didn't know this, but apparently the SSLKEYLOGFILE envvar is a de-facto standard: chrome, firefox, and libcurl all check for this envvar, and if found they log TLS secrets to the file in a specific format. Reports of projects supporting this:
Also, people are using gross ctypes hacks to convince Python to do this too: https://github.com/joernheissler/SslMasterKey Also, now that I know this exists I kind of wish it was supported because I've been frustrated by this problem before myself :-). My first thought was that the ssl module should provide methods to extract the various secret values (e.g., wrappers for SSL_SESSION_get_master_key and SSL_get_client_random), and leave the environment variable checking to user code. But... looking at the file format docs: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format ...it appears that TLS 1.3 has more and different secrets than previous versions, and trying to expose all these different pieces seems pretty messy. If we simply implement SSLKEYLOGFILE, that would give people what they want, and since we would be writing it out ourselves we could make it handle different TLS versions internally without exposing that complexity as part of the API. We would of course have to disable this if -E was passed on the command line. As an FYI to anyone looking at this bug, Christian (the main ssl module maintainer) is generally *very* overloaded, so I would say that the chances of this actually being implemented go *way* up if someone puts together a PR. |
Cory contributed a high level API for OpenSSL 1.1.1, https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_keylog_callback.html I already have a patch somewhere on my disk. The patch is trivial if we ignore OpenSSL < 1.1.1. |
Here is a horribly hacky and simple implementation. I have a more elaborate implementation that does correct locking and has no global state. static BIO *bio_keylog = NULL; static void keylog_callback(const SSL *ssl, const char *line)
{
BIO_printf(bio_keylog, "%s\n", line);
(void)BIO_flush(bio_keylog);
}
int PySSL_set_keylog_file(SSL_CTX *ctx, const char *keylog_file)
{
/* Close any open files */
BIO_free_all(bio_keylog);
bio_keylog = NULL;
if (ctx == NULL || keylog_file == NULL) {
/* Keylogging is disabled, OK. */
return 0;
}
/*
* Append rather than write in order to allow concurrent modification.
* Furthermore, this preserves existing keylog files which is useful when
* the tool is run multiple times.
*/
bio_keylog = BIO_new_file(keylog_file, "a");
if (bio_keylog == NULL) {
BIO *b = BIO_new_fp(stderr, BIO_NOCLOSE | BIO_FP_TEXT);
BIO_printf(b, "Error writing keylog file %s\n", keylog_file);
BIO_free_all(b);
return 1;
}
/* Write a header for seekable, empty files (this excludes pipes). */
if (BIO_tell(bio_keylog) == 0) {
BIO_puts(bio_keylog,
"# SSL/TLS secrets log file, generated by OpenSSL\n");
(void)BIO_flush(bio_keylog);
}
SSL_CTX_set_keylog_callback(ctx, keylog_callback);
return 0;
} |
Hi Christian On Wed, 26 Sep 2018 at 09:25, Christian Heimes <report@bugs.python.org>
|
Nathaniel, I created a PR with keylog and message callback patch. The message callback is really useful to investigate handshake and trace messages like close alert. I decided against making the key log feature a callback and rather make it a filename attribute. In almost all case users want to log to a file anz way. It's easier to implement and makes locking simpler, too. |
Hello Christian, much appreciated. Thank you so much. Johannes |
Perhaps https://stackoverflow.com/questions/42332792/chrome-not-firefox-are-not-dumping-to-sslkeylogfile-variable is outdated, but it suggests that: in firefox, this feature os not on by default in chrome, this feature is not available I would be vary of "too much magic"... Though I'd use this in development, I feel that's a bit risky for desktop apps, production, etc... |
It's confusing, but AFAICT what happened is that Mozilla started to disable it in release builds, but got a bunch of pushback from users and changed their minds and decided to keep it enabled. But then there was a snafu tracking the patch for that, so there ended up being a few releases where it was disabled, before everything got sorted out. But, it is enabled by default now. The very confusing thread is here: https://bugzilla.mozilla.org/show_bug.cgi?id=1188657 And I don't see how this is any riskier than other envvars like PYTHONSTARTUP, which lets you name an arbitrary file that the interpreter will execute at startup. |
It looks like this has now been done and released. Can the issue be closed? |
Yes, I didn't revisit the issue since, but Malcolm is right. Implemented in Thanks to all the contributors. On Sat, Feb 29, 2020 at 8:58 AM Malcolm Smith <report@bugs.python.org>
|
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: