classification
Title: Please support logging of SSL master secret by env variable SSLKEYLOGFILE
Type: enhancement Stage: patch review
Components: SSL Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: Dima.Tisnek, christian.heimes, jmfrank63, jonozzz, njs, sascha_silbe, yan12125
Priority: normal Keywords: patch

Created on 2018-07-29 12:38 by jmfrank63, last changed 2019-10-07 14:56 by sascha_silbe.

Files
File name Uploaded Description Edit
pycurl-get.py jmfrank63, 2018-07-29 12:38 Working example with pycurl module compiled against openssl 1.1.0h
Pull Requests
URL Status Linked Edit
PR 10031 merged christian.heimes, 2018-10-21 15:42
PR 13728 merged christian.heimes, 2019-06-01 17:54
Messages (11)
msg322632 - (view) Author: Johannes Frank (jmfrank63) * Date: 2018-07-29 12:38
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
msg326427 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-09-26 08:05
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:

- https://www.imperialviolet.org/2012/06/25/wireshark.html
- https://jimshaver.net/2015/02/11/decrypting-tls-browser-traffic-with-wireshark-the-easy-way/
- https://ec.haxx.se/tls-sslkeylogfile.html

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.
msg326429 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2018-09-26 08:17
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.
msg326430 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2018-09-26 08:25
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;
}
msg326431 - (view) Author: Johannes Frank (jmfrank63) * Date: 2018-09-26 08:35
Hi Christian
I would be willing to give this a try, could you publish or send me that
more elaborate code?
Thanks Johannes

On Wed, 26 Sep 2018 at 09:25, Christian Heimes <report@bugs.python.org>
wrote:

>
> Christian Heimes <lists@cheimes.de> added the comment:
>
> 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;
> }
>
> ----------
> stage:  -> needs patch
> versions: +Python 3.8 -Python 3.7
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue34271>
> _______________________________________
>
msg328219 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2018-10-21 16:02
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.
msg328221 - (view) Author: Johannes Frank (jmfrank63) * Date: 2018-10-21 16:15
Hello Christian,

much appreciated. Thank you so much.

Johannes
msg332181 - (view) Author: Dima Tisnek (Dima.Tisnek) * Date: 2018-12-20 06:21
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...
msg332196 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-12-20 08:13
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.
msg344050 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2019-05-31 09:44
New changeset c7f7069e77c58e83b847c0bfe4d5aadf6add2e68 by Christian Heimes in branch 'master':
bpo-34271: Add ssl debugging helpers (GH-10031)
https://github.com/python/cpython/commit/c7f7069e77c58e83b847c0bfe4d5aadf6add2e68
msg344459 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2019-06-03 18:40
New changeset e35d1ba9eab07a59b98b700c5e18ceb13b2561a6 by Christian Heimes in branch 'master':
bpo-34271: Fix compatibility with 1.0.2 (GH-13728)
https://github.com/python/cpython/commit/e35d1ba9eab07a59b98b700c5e18ceb13b2561a6
History
Date User Action Args
2019-10-07 14:56:08sascha_silbesetnosy: + sascha_silbe
2019-06-03 18:40:18christian.heimessetmessages: + msg344459
2019-06-01 17:54:03christian.heimessetpull_requests: + pull_request13612
2019-05-31 09:44:08christian.heimessetmessages: + msg344050
2018-12-20 08:13:34njssetmessages: + msg332196
2018-12-20 06:21:15Dima.Tisneksetnosy: + Dima.Tisnek
messages: + msg332181
2018-10-21 16:15:52jmfrank63setmessages: + msg328221
2018-10-21 16:02:24christian.heimessetmessages: + msg328219
2018-10-21 15:42:37christian.heimessetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request9369
2018-09-26 08:35:45jmfrank63setmessages: + msg326431
2018-09-26 08:25:56christian.heimessetstage: needs patch
messages: + msg326430
versions: + Python 3.8, - Python 3.7
2018-09-26 08:17:25christian.heimessetmessages: + msg326429
2018-09-26 08:05:25njssetnosy: + njs
messages: + msg326427
2018-07-31 23:09:47jonozzzsetnosy: + jonozzz
2018-07-29 17:04:58yan12125setnosy: + yan12125
2018-07-29 12:38:43jmfrank63settitle: Please support logging of SSL master secret by env variable SSLKEYLOGFILe -> Please support logging of SSL master secret by env variable SSLKEYLOGFILE
2018-07-29 12:38:09jmfrank63create