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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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
|
msg362992 - (view) |
Author: Malcolm Smith (Malcolm Smith) |
Date: 2020-02-29 16:57 |
It looks like this has now been done and released. Can the issue be closed?
|
msg363007 - (view) |
Author: Johannes Frank (jmfrank63) * |
Date: 2020-02-29 19:26 |
Yes, I didn't revisit the issue since, but Malcolm is right. Implemented in
python 3.8.
Thanks to all the contributors.
On Sat, Feb 29, 2020 at 8:58 AM Malcolm Smith <report@bugs.python.org>
wrote:
>
> Malcolm Smith <malcolm.smith@gmail.com> added the comment:
>
> It looks like this has now been done and released. Can the issue be closed?
>
> ----------
> nosy: +Malcolm Smith
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue34271>
> _______________________________________
>
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:04 | admin | set | github: 78452 |
2021-04-17 19:36:53 | christian.heimes | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
2020-02-29 19:26:32 | jmfrank63 | set | messages:
+ msg363007 |
2020-02-29 16:57:50 | Malcolm Smith | set | nosy:
+ Malcolm Smith messages:
+ msg362992
|
2019-10-07 14:56:08 | sascha_silbe | set | nosy:
+ sascha_silbe
|
2019-06-03 18:40:18 | christian.heimes | set | messages:
+ msg344459 |
2019-06-01 17:54:03 | christian.heimes | set | pull_requests:
+ pull_request13612 |
2019-05-31 09:44:08 | christian.heimes | set | messages:
+ msg344050 |
2018-12-20 08:13:34 | njs | set | messages:
+ msg332196 |
2018-12-20 06:21:15 | Dima.Tisnek | set | nosy:
+ Dima.Tisnek messages:
+ msg332181
|
2018-10-21 16:15:52 | jmfrank63 | set | messages:
+ msg328221 |
2018-10-21 16:02:24 | christian.heimes | set | messages:
+ msg328219 |
2018-10-21 15:42:37 | christian.heimes | set | keywords:
+ patch stage: needs patch -> patch review pull_requests:
+ pull_request9369 |
2018-09-26 08:35:45 | jmfrank63 | set | messages:
+ msg326431 |
2018-09-26 08:25:56 | christian.heimes | set | stage: needs patch messages:
+ msg326430 versions:
+ Python 3.8, - Python 3.7 |
2018-09-26 08:17:25 | christian.heimes | set | messages:
+ msg326429 |
2018-09-26 08:05:25 | njs | set | nosy:
+ njs messages:
+ msg326427
|
2018-07-31 23:09:47 | jonozzz | set | nosy:
+ jonozzz
|
2018-07-29 17:04:58 | yan12125 | set | nosy:
+ yan12125
|
2018-07-29 12:38:43 | jmfrank63 | set | title: 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:09 | jmfrank63 | create | |