Skip to content
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

[Feature Request] Add support for OpenSSL SSLKEYLOGFILE #499

Open
WSL-Ben opened this issue Feb 10, 2024 · 9 comments
Open

[Feature Request] Add support for OpenSSL SSLKEYLOGFILE #499

WSL-Ben opened this issue Feb 10, 2024 · 9 comments

Comments

@WSL-Ben
Copy link

WSL-Ben commented Feb 10, 2024

Firstly, thank you very much for writing such a simple to use and well documented C++ Websocket library. As someone who's relatively new to the language I've found that to something of a rarity, and I really appreciate it.

With that out of the way, would you be willing to consider implementing support for OpenSSL's SSLKEYLOGFILE? :)

I've hacked it into a quick fork I setup for a POC I'm writing, but think this might be something that other people would be interested in too.

My specific use-case is a little niche - we passively tap and capture packets that go to/from our websocket client application and would like to be able to log the ssl decryption keys in order to compare the applications output against the raw traffic - but I'm sure it would be helpful in all kinds of data collection contexts.

Thanks!

@bsergean
Copy link
Collaborator

Thanks for the good words.

If you can think of a generic way to handle that it would be good. Is it more focused on the client or on the server at this point ? Is your POC really complicated.

Apparently curl supports this, can you see how they do it ?
https://wiki.wireshark.org/TLS

@WSL-Ben
Copy link
Author

WSL-Ben commented Feb 11, 2024

The POC is pretty simple and can be compared against your master branch here: master...WSL-Ben:IXWebSocket:ssl-keylogfile

Not sure it's the best solution, but I can confirm that it works. Ensure you're using OpenSSL and set the tlsOptions.keyLogFile and you should see the keys written out to the specified location.

@bsergean
Copy link
Collaborator

Code is pretty simple, can you explain why you need the mutex and the global variable ?

@WSL-Ben
Copy link
Author

WSL-Ben commented Feb 11, 2024

Not really. Being honest, I have no idea what I'm doing and just kind of stopped once it worked.

Regarding the mutex, I was under the impression that due to multiple SSL contexts existing and being used in different threads that I'd need to protect it from a race condition. Sounds like I might have been wrong.

@bsergean
Copy link
Collaborator

lol it's fine not to be sure ... programming is hard :)

There should be one context by IXSocket object already, so I'm confident we don't need the mutex.

@WSL-Ben
Copy link
Author

WSL-Ben commented Feb 12, 2024

Ok, I've dropped the mutex and replaced the global variable with a static class member. Thanks for the direction, I always appreciate an opportunity to learn.

@bsergean
Copy link
Collaborator

bsergean commented Feb 12, 2024 via email

@WSL-Ben
Copy link
Author

WSL-Ben commented Feb 12, 2024

The callback for OpenSSL needs to be static in order to match the expected callback signature. It also doesn't accept a void* callback, which in my limited experience means that it won't really be possible to store it as a member variable, at least not in a way where it's accessible to the callback - though I'll be happy to find I'm incorrect as I've used this pattern in the past and find it less than ideal.

To clarify on the feature as well, I don't think there is any necessity to support the SSLKEYLOGFILE environment variable, simply to expose it as an option via the TLSOptions class.

I've also noticed with my patch that the first SSL key is written out, but subsequent ones fail to either call the callback or find a mapping from the SSL context to the filename. Are SSL context's designed to last the lifetime of the connection?

@bsergean
Copy link
Collaborator

bsergean commented Feb 13, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants