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

sslkeylog: Allows toggling of SSL key logging on or off without restarting the application #3267

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

VaibhavTekale1
Copy link

@lws-team
Added Support for LWS_KEYLOGFILE with Dynamic SSL Key Logging :

Problem: In the existing SSL key log feature, the environment variable is set or unset only during vhost initialization, which requires the application to restart to start or stop diagnostic sessions. Many customer environments do not allow restarting the application solely to initiate a diagnostic session.

Solution: To allow toggling of session key logging without restarting the application:

  1. Added a boolean flag in usr_ctx, If the flag is true, the environment variable file path is extracted and assigned to lws-keylog_file, starting the diagnostic session. If the flag is false, the existing flow continues. This enables dynamic toggling of SSL key logging.
  2. Introduced a new environment variable, LWS_SSLKEYLOGFILE. On Windows, applications often lock the SSLKEYLOGFILE variable, preventing access to the log file until all processes are closed. Using LWS_SSLKEYLOGFILE avoids this issue while maintaining backward compatibility with SSLKEYLOGFILE.

Note: The changes have been validated across multiple scenarios, confirming that the new functionality works without disrupting existing behavior.

@lws-team
Copy link
Member

lws-team commented Nov 8, 2024

Not sure I understand why env vars are "locked" by some other process.... env vars are local to each process, right? Do you mean that the file at the path given is what is locked?

So you can (even on windows, presumably) set SSLKEYLOGFILE=mypath myapp and no other app who happens to use SSLKEYLOGFILE should be able to interfere or see that process SSLKEYLOGFILE? Other processes have their own env vars and can have their own SSLKEYLOGFILE with same or different data? If the filename for the log is specific to myapp, no other process can lock it (since it doesn't know what path you used for that process' log).

This idea of mandating what is in user pointers in the core code is not a good way to turn things on and off. Isn't it better to be able to set an option bit when you create the client wsi to indicate you want that particular connection to be logged? You can have a new vhost option bit as well to indicate that it should not default to logging everything because SSLKEYLOGFILE is set, and then a bit in the client info struct indicating that one should be logged.

@lws-team
Copy link
Member

Just some gentle advice if you want to get your patch in, it's better to engage with what the project is saying about it. For your situation it might be preferable to ignore anything other than what you have done, but it leads to the patch only making sense for yourself. It is free software and you can add whatever you want on top yourself only without needing anyone's permission.

But if you want the changes already included in lws, it has to make sense for the maintainer to explain how others should use it, and to look after the code if it later is involved in changes.

@VaibhavTekale1
Copy link
Author

@lws-team Thank you for the suggestions .

Our use case involves handling all client logging on the server side, which is why we can’t make any modifications in the client wsi—it simply isn’t relevant for this setup.

On the server, we have access to all connected client information, and if we need to log SSL keys for a specific client, we must disconnect it to achieve this. Many customer environments don’t allow restarting the application just to start a diagnostic session. By disconnecting the client and setting a flag based on user input, we’re able to implement the feature of selectively logging SSL keys for specific nodes.

with respect to : Commit ID
All control of reconnection for specific clients are managed entirely on the server side, with no options provided to the client nodes.

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

Successfully merging this pull request may close these issues.

2 participants