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

feat: support custom CTCP VERSION responses #4780

Closed

Conversation

flotwig
Copy link
Contributor

@flotwig flotwig commented Sep 11, 2023

Closes #1344

Adds a new config option, versionResponse, which is described as follows:

A template string to format thelounge's response to CTCP VERSION requests. The
tokens %product-name%, %version%, and %url% will be replaced by the
appropriate values for the currently running version of The Lounge.

You can use this to remove version information from the CTCP VERSION response
for hardening.

The default template string results in a response of
thelounge vX.Y.Z -- https://thelounge.chat/.

The goals of this, in descending order of importance, are:

  1. Make thelounge users less fingerprintable by hiding build/version info in VERSION responses
  2. Add "Security By Obscurity" by not giving the exact version of thelounge that is running
  3. Match the behavior of almost all other IRC clients (all that I've used, certainly) by allowing users to customize VERSION
  4. Allow users to have silly novelty VERSION strings (least important)

I originally wanted to just have a static versionResponse string, but I saw that defaults/config lives outside of server, so it wouldn't be right to pull in Helper.getVersion there. Another option would be to treat versionResponse as an "override" that's only used when defined so it doesn't need to be template-able, or to move getVersion to shared.

I tested this locally via node ./dist/server/command-line/index.js start -c versionResponse=foo and confirmed it works as expected, and also confirmed the default works as expected.

No support is added in this PR for disabling VERSION responses entirely. This is because CLIENTINFO and SOURCE CTCP responses also expose that thelounge is running, so there would be little point in disabling only VERSION. A future PR could add an option to disable/obfuscate all 3. Passing versionResponse= will result in a blank VERSION response only.

Please let me know if you have feedback on the API design here, I'm open to making changes.

Pre-merge tasks:

Copy link
Member

@MaxLeiter MaxLeiter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @flotwig, sorry for the delay in reviewing this. @brunnre8 might have an opinion about merging this so I'll let them comment before merging, but the code LGTM.

@brunnre8
Copy link
Member

my opinion is pretty much summarized here: #4785
I'm not going to merge this as is.

#4785 (comment)

Might be a middle ground we can do without adding options.

@flotwig
Copy link
Contributor Author

flotwig commented Feb 1, 2024

Thanks for the feedback @brunnre8 @MaxLeiter. I created a new PR to simply remove the version number and will close this PR for now: #4834

@flotwig flotwig closed this Feb 1, 2024
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.

Allow hiding version number from CTCP VERSION response.
3 participants