[Feature Request] Add support for OpenSSL SSLKEYLOGFILE

[WSL-Ben]

[Feature Request] Add support for OpenSSL SSLKEYLOGFILE

[bsergean]

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]

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]

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

[WSL-Ben]

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]

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]

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]

I think static member is pretty equivalent, unless you initialize it with a getenv(“OPENSSL_VALUE”), but we need some logic to run for that. What I am after is something that runs in the constructor of the class, and to make that _isNewOpenSSLThing a normal member variable.

…

On Feb 12, 2024, at 2:58 AM, WSL-Ben ***@***.***> wrote: 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. — Reply to this email directly, view it on GitHub <#499 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC2O6UMITZYGIJV3YKC5R73YTHYV3AVCNFSM6AAAAABDCREEDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZYGQ2TGMRZGA>. You are receiving this because you commented.

[WSL-Ben]

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]

Yeah you’re right.

…

On Feb 12, 2024, at 2:04 PM, WSL-Ben ***@***.***> wrote: 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 - 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. — Reply to this email directly, view it on GitHub <#499 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC2O6UOPCWP7G5IWQHEKZ6LYTKGXVAVCNFSM6AAAAABDCREEDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZZGY3TANRZHA>. You are receiving this because you commented.