[SquadJS Logging] RCON password shouldnt be output in plaintext at log level 2

[DrKittens]

Description of Issue

Use a systemd service to run the bot (node index.js) with rcon log level set >=2 \ with servicename foo

Run journalctl -xeu foo and note that the rcon password used has been dutifully noted down and logged as at rcon log level 2 the application outputs it in plaintext which gets logged by journald

Errors or Screenshots of Issue

RCON password shouldnt be written out in plaintext to system as journalctl can be queried by non-proviledged users

Basic workarounds are to lower rcon logging level to 1 or redirect stdout for the nodejs application to not log at all Both are undesireable because if something breaks the user would have less logs to review / submit for fault diagnosis without needing to reproduce

Squad Information

• SquadJS Version: 3.6.1
• Squad Version: N/A

If potentially relevant, please provide regarding the state of the Squad server at the time of error, e.g. the current layer.

System Information

• Operating System: RHEL9
• NodeJS Version: v18.12.1
• Yarn Version: 1.22.19

[ect0s]

RCON 1 is default.

You chose level 2.

This is akin to setting SSHD to log more information and being upset when it does.

[ect0s]

Assuming we do want to fix this,

https://github.com/Team-Silver-Sphere/SquadJS/blob/dc2bc1d663fb5b4aecab39d9072ef51dad089fae/core/rcon.js#L270

I assume the log line is similar to:

Logger.verbose('RCON', 2, Writing packet with type "${type}" and body "${body}".);

Perhaps we can toss a branch here?

[Thomas-Smyth]

I think this is worth fixing, but I think we need to have the option to keep it enabled in case we need to debug RCON authentication in the future.

[DrKittens]

RCON 1 is default.

You chose level 2.

This is akin to setting SSHD to log more information and being upset when it does.

I'm very aware this is a problem of my own design, only raised an issue because rcon log level 2 is a single step above 1, there is no documentation saying to NOT change it above 1 and that single step causes arguably privileged information to be logged. If it were doing this at log level 4 i wouldn't have batted an eye / it would have been expected because I'd have then chosen to request (what ive read) to be the highest level of verbosity.

This is akin to setting SSHD to log more information and being upset when it does.

yes and no, man sshd_config at least advises that setting logging to any debug level violates user privacy and isnt recommended, it also provides a "safe" verbose setting to use.

So yes your analogy is correct, but i think comparing sshd debug3 to rcon 2 is a stretch

E:

I think this is worth fixing, but I think we need to have the option to keep it enabled in case we need to debug RCON authentication in the future.

I definitely agree it should be retained as a visual "oops thats not my password" on launch is helpful.

As mentioned above, i think tying it to rcon 4 is probably a good idea assuming there is useful info developers would like users to log at debug levels 2-3 that can be safely recorded / provided along with fault reports.

if developers want a user to skip straight to setting rcon 4 for fault troubleshooting instead of using those intermediary steps then this can probably be closed with a doc update ex here saying to not fiddle with the logging defaults unless reporting a problem?

[ect0s]

yes and no, man sshd_config at least advises that setting logging to any debug level violates user privacy and isnt recommended, it also provides a "safe" verbose setting to use.

So yes your analogy is correct, but i think comparing sshd debug3 to rcon 2 is a stretch

I didn't mean to sound short, and I grant all that

I definitely agree it should be retained as a visual "oops thats not my password" on launch is helpful.

As mentioned above, i think tying it to rcon 4 is probably a good idea assuming there is useful info developers would like users to log at debug levels 2-3 that can be safely recorded / provided along with fault reports.

if developers want a user to skip straight to setting rcon 4 for fault troubleshooting instead of using those intermediary steps then this can probably be closed with a doc update ex here saying to not fiddle with the logging defaults unless reporting a problem?

If we do branch there, by checking the packet type (auth), we can have an RCON 2, "sending auth" and an RCON 4 with the true password?

That still allows leeway for troubleshooting while solving this issue?

[ect0s]

https://github.com/Team-Silver-Sphere/SquadJS/blob/dc2bc1d663fb5b4aecab39d9072ef51dad089fae/core/rcon.js#L292

We already branch here, so just moving the logging inside each side would be a quick fix?

[DrKittens]

I didn't mean to sound short, and I grant all that

All good, re-reading my issue i too came off poorly. I am sorry aswell.

If we do branch there, by checking the packet type (auth), we can have an RCON 2, "sending auth" and an RCON 4 with the true password?

That still allows leeway for troubleshooting while solving this issue?

I like this solution.

[ect0s]

https://github.com/Team-Silver-Sphere/SquadJS/pull/261

Thoughts?

I'm not sure I can condense to one line with a ternary operator, as I don't think we see the log level as set in the config