-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
Optionally log master secrets for TLS connections #2363
Comments
As this would basically throw out all security, the only way I see for a feature like that would be a compile-time option. |
I think a compile-time option would satisfy the use case for this reasonably well, though it would be less convenient than the environment variable used by Firefox and Chrome. I don't understand why you say this would throw out all security, though. The contents of the TLS connection are known to both endpoints, including the Node endpoint on which you would add this debugging variable when needed. This just allows the developer who runs the Node endpoint to store extra data about that connection. The idea is not that you would leave this on permanently in any production system. |
I'm thinking more of a scenario where a compromised server could be modified to provide these secrets to the attacker, while still serving the application. An attacker could for example modify the environment variables and force a restart on the node process. I'm not really sure the benefits outweigth the risks here. Couldn't you just run a MITM proxy for your debugging needs? |
This is entirely possible today. Anyone with access to the Node process' memory can exfiltrate key material.
For debugging of HTTP applications this is generally sufficient. But for debugging HTTP protocol issues, or especially HTTP/2 protocol issues, a proxy would not show the necessary information. The need for this flag came up for me specifically when trying to debug issues using HTTP/2 as a client against a remote server. The client knows the session key by definition, so allowing it to be (optionally) logged merely enables better debugging. |
Well, maybe a runtime flag could be done. I'm a bit more comfortable with a flag than a mere environment variable ;) |
Yep, a runtime flag would be great too! I only suggested the env variable for similarity with Firefox and Chrome. |
Sorry, but this is a terrible idea. It's a massive accident just waiting to happen. It opens so many possible attack vectors. If anyone needs to do this kind of thing they should just modify Node themselves. |
@jorangreef: Can you go into more detail about the attack vectors or accidents you forsee? Keep in mind that this has been an option in Firefox and Chrome for many years and I have not heard a single example of anyone being compromised by it. If I may guess a little: It sounds like you and @silverwind are most concerned by server operators misusing this flag. I think the most important debugging is is in TLS client code: if you control the server, you can force negotiation with a non-forward secret cipher, and since you also possess the private key, that allows the necessary analysis. However, for the client, there is no such option: the server controls the cipher negotiation and the private key. Would you be less worried about accidental misuse of a flag that affects TLS clients only, and not servers? |
@jsha there's probably an endless combination of attack vectors that an option to log the master key would support. But even if there's just one vector, that's enough, and @silverwind has already given a few. Chrome and Firefox are desktop applications so the security threats are different. I think your use-case would be solved if your Javascript could access the key from the actual TLS client instance directly, i.e. it's not necessary to ask Node to log it (via env flag or compile flag). But even then, it should only be exposed as a property by the instance if an option is explicitly provided. Something like this should err on the side of being very difficult to do. |
+1 This feature would immensely help with debugging.
Not at all. There are heisenbugs that require delicate and specific environment, and altering network flow with MITM proxies makes debugging impossible. |
+1 ... If an attacker is able to get to the running process and manipulate it, the possibilities range widely regardless. Java exposes via logging key parts which can be consumed by wireshark with a little manipulation. I know node is not java. |
Wireshark >=1.6.0 supports NSS-format log files giving the session ID and master key in the following format:
Both the session ID and master key can be obtained with So just do something like this: https.request(opts, cb)
.once('socket', (s) => {
s.once('secureConnect', () =>
let session = parseSession(s.getSession());
// session.sessionId and session.masterKey should be hex strings
fs.appendFileSync('sslkeylog.log', `RSA Session-ID:${session.sessionId} Master-Key:${session.masterKey}\n`);
});
}); This works very well for me in Wireshark 2.2.4. How thoroughly to parse the session buffer is up to you, but even something as barebones as this will work in most cases: function parseSession(buf) {
return {
sessionId: buf.slice(17, 17+32).toString('hex'),
masterKey: buf.slice(51, 51+48).toString('hex')
};
} |
@tgvarik I found your comment really useful and wrote a simple library to implement that. I was wondering if you'd be OK with me publishing it to npm. If so, how would you like attribution? https://github.com/jhford/node-https-wireshark I'm also happy to delete the code :) I changed it so that it should play nicely with older versions of node. |
@jhford Thanks for writing that up. Please feel free to publish! No attribution needed, but a link to my comment above would be appreciated. |
We've been working to debug a binary, non-HTTP, TLS-secured protocol implementation with a custom wireshark dissector; we would find this feature extremely useful. Even simple properties that gave these values without opaque buffer parsing would be very useful. I'm not sure what the negative security implications would be with this key material already available in-process from the session with no flags. Thanks for the code, @tgvarik! Going to give it a shot here. |
I've been looking into extracting the keys from a server socket, and found that parsing the SSL_SESSION structure is insufficient with modern browser clients at least. For these type of connections, Wireshark supports the SSL_SESSION only contains the master secret in these cases, so I assume SSL_get_client_random would need to be exposed as well to enable decryption of these type of sessions. At this point, I think out-of-band decryption is something we should enable userland to do. It won't have to be easy or convenient, but we should provide access to the necessary data structures to make it possible. cc: @nodejs/crypto |
Is this being worked on? Should this stay open at this point? |
I'll close it out. It's almost its 2nd anniversary and no one has put up a PR so far. If someone still wants to pursue this, open a PR and we'll take it from there. |
Recently I published https://github.com/kolontsov/node-sslkeylog to implement SSLKEYLOG. It uses little hacky approach to get access to SSL_get_client_random(), but so far it works well for my debugging purporses. I was thinking about submitting PR (to use SSL_get_client_random() without tricks), but I'm not yet sure how implementation should look like: we can write sslkeylog.txt if some env var exists, we can expose get_session_key() to be used with TLSSocket, etc.. OR we can wait until openssl 1.1.1 will be fully integrated and then just start using SSL_CTX_set_keylog_callback.. |
@kolontsov A million thanks for your module, really easy to use. function patchAgent(agent) {
const { update_log } = require("sslkeylog");
const patchSocket = socket => socket.on("secureConnect", () => update_log(socket));
const patchIfSocket = socket => socket ? patchSocket(socket) : socket;
const original = agent.createConnection.bind(agent);
agent.createConnection = (options, callback) =>
patchIfSocket(original(options, (err, socket) => callback(err, patchIfSocket(socket))));
} To sniff all HTTPS connections, just patch the global agent when your program starts: sslkeylog.set_log("/tmp/keys.log");
patchAgent(require("https").globalAgent); |
@jmendeth thank you! I'm glad you found it useful. Also thanks for the code, I'll include it into example and/or readme. |
Exposes SSL_CTX_set_keylog_callback in the form of a `keylog` event that is emitted on clients and servers. This enables easy debugging of TLS connections with i.e. Wireshark, which is a long-requested feature. Refs: nodejs#2363
Exposes SSL_CTX_set_keylog_callback in the form of a `keylog` event that is emitted on clients and servers. This enables easy debugging of TLS connections with i.e. Wireshark, which is a long-requested feature. PR-URL: nodejs#27654 Refs: nodejs#2363 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Exposes SSL_CTX_set_keylog_callback in the form of a `keylog` event that is emitted on clients and servers. This enables easy debugging of TLS connections with i.e. Wireshark, which is a long-requested feature. PR-URL: #27654 Refs: #2363 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Good news! Node.JS 12.3.0 now has a native API for TLS secret logging. Logging server connectionsSubscribe to the const myServer = https.createServer(...);
// ...
myServer.on('keylog', line =>
fs.appendFileSync('/tmp/secrets.log', line)); Then, as usual on Wireshark open Preferences → Protocols → SSL/TLS set «Pre-master-secret log filename» to This works on any instance of Logging client connectionsThe const socket = tls.connect(...);
socket.on('keylog', line =>
fs.appendFileSync('/tmp/secrets.log', line)); Or with const session = http2.connect(...);
session.socket.on('keylog', line =>
fs.appendFileSync('/tmp/secrets.log', line)); Logging HTTPS requestsThis is tricky, because an existing connection may be reused for a request. const req = https.request("https://example.org", { agent: false });
req.on('socket', socket => {
socket.on('keylog', line =>
fs.appendFileSync('/tmp/secrets.log', line));
}); Tips
|
Thanks to #30055 there's now a There's also the require('sslkeylog').hookAll(); And again, all traffic will be logged if the |
Exposes SSL_CTX_set_keylog_callback in the form of a `keylog` event that is emitted on clients and servers. This enables easy debugging of TLS connections with i.e. Wireshark, which is a long-requested feature. PR-URL: nodejs#27654 Refs: nodejs#2363 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Exposes SSL_CTX_set_keylog_callback in the form of a `keylog` event that is emitted on clients and servers. This enables easy debugging of TLS connections with i.e. Wireshark, which is a long-requested feature. PR-URL: #27654 Backport-PR-URL: #31582 Refs: #2363 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
@mildsunrise It is a very good news. But I'm having a problem. this is the server:
and this fragment is attached to the request call:
doing this I get almost always all the keys I need to decrypt the traffic, but there are sometimes when some keys get lost (related to the calls arriving at server) Hence I decided to use the --tls-keylog. This way it would dump everything, when I start the server and the calls (https) arrive the keylog is not even created, but I do get the log warning:
Can anybody tell me what's going on or what I'm doing wrong? Thanks in advance |
This could happen if the agent is reusing sockets, see "Logging HTTPS requests" in my comment above.
That shouldn't be happening at all, could you open an issue with a small snippet to reproduce? |
@luiso1979 I found the bug you're hitting, opened #33366 for a fix |
@mildsunrise thank you very much for the support!!! |
@mildsunrise I tried to follow your advice regarding the agent reusing sockets, but my problem remains, since the lost key is not from a request I'm making but from a request that arrives to my server.
This is how I'm listening for keylogs, but sometimes certain arriving POSTs are not logged. It happens rarely and I find it difficult to reproduce it. Do you have any insight that might help? Thanks |
Without much more info, I'm afraid I can't tell... Make sure you're registering the handler before |
Has anyone managed to get this working with openssl v1.1.1? I'm using a shared library that makes use of the
|
(If your reason to use |
Thanks @mildsunrise, I read this is caused by curl dynamically linking libssl whereas node statically links it so the symbols can't be patched that way. I was trying to build a solution that works with arbitrary programs written in any language, but I can customize per runtime so I will use the |
Sometimes it's necessary to decrypt your own TLS connections to debug their contents. Wireshark supports this quite nicely with its decryption feature. For non-DH key agreement, you simply provide the private key of the server. However, for DH key agreement, or when you are acting only as a client, that doesn't work. Firefox and Chrome support the environment variable SSLKEYLOGFILE to write the master secrets used to a file, for decryption by Wireshark. It would be great to support this or a similar mechanism for logging master secrets in Node.
Key log format: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format
Helpful Stack Exchange howto: https://security.stackexchange.com/questions/35639/decrypting-tls-in-wireshark-when-using-dhe-rsa-ciphersuites/42350#42350
Wireshark decryption docs: https://wiki.wireshark.org/SSL
(reposted from nodejs/node-convergence-archive#59).
The text was updated successfully, but these errors were encountered: