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

[defect]: Password is logged to world-readable log file on startup #5241

Open
thorerik opened this issue Dec 6, 2023 · 8 comments
Open

[defect]: Password is logged to world-readable log file on startup #5241

thorerik opened this issue Dec 6, 2023 · 8 comments
Labels
defect category: a defect or misbehaviour

Comments

@thorerik
Copy link

thorerik commented Dec 6, 2023

What type of defect/bug is this?

Unexpected behaviour (obvious or verified by project member)

How can the issue be reproduced?

When you use the rlm_sql connection, the password is logged to the log file on startup in info messages.

To reproduce, set up rlm_sql_postgresql with the following configuration in /etc/freeradius/mods-enabled/sql:

driver = "rlm_sql_postgresql"

and

radius_db = "dbname=radius host=db user=radius password=Password123! port=5432"

Log output from the FreeRADIUS daemon

Wed Dec  6 13:34:51 2023 : Info: Signalled to terminate
Wed Dec  6 13:34:51 2023 : Info: Exiting normally
Wed Dec  6 13:34:51 2023 : Info: Debug state unknown (cap_sys_ptrace capability not set)
Wed Dec  6 13:34:51 2023 : Info: systemd watchdog interval is 30.00 secs
Wed Dec  6 13:34:51 2023 : Info: rlm_sql (sql): Driver rlm_sql_postgresql (module rlm_sql_postgresql) loaded and linked
Wed Dec  6 13:34:51 2023 : Info: rlm_sql (sql): Attempting to connect to database "dbname=radius host=my.database.server user=radius password=password123! port=5432 sslmode=verify-full sslrootcert=/etc/ssl/ca.crt"
Wed Dec  6 13:34:52 2023 : Info: Loaded virtual server <default>
Wed Dec  6 13:34:52 2023 : Info: Loaded virtual server default
Wed Dec  6 13:34:52 2023 : Info: Ready to process requests

Relevant log output from client utilities

No response

Backtrace from LLDB or GDB

No response

@thorerik thorerik added the defect category: a defect or misbehaviour label Dec 6, 2023
@alandekok
Copy link
Member

So? What's the problem?

The default configuration has it that the log files are not world readable. This means that anyone who can read the log files can also read the configuration files... which also has the password.

What exactly is the security issue here? What unprivileged user can read privileged information?

@thorerik
Copy link
Author

thorerik commented Dec 6, 2023

Logs can be shipped, and it's common to ship logs out of a system, the privileges might not be the same as the system running freeradius.

@alandekok
Copy link
Member

Configuration files can also be shipped off-machine.

We're happy to accept a patch which fixes the issue. This is Open Source, and we rely on contributions from the community. Barring that, this is a low-priority issue.

@thorerik
Copy link
Author

thorerik commented Dec 6, 2023

I think you misunderstood, logshipping is a common configuration for environments with a lot of servers, those logs are then sendt to eg. newrelic or an elasticsearch instance, where the ACL is different to accessing the system that runs freeradius, people who might need access to the log may not necessarily have access to the system. This would lead to a vector that doesn't require a breach of the system running freeradius.

@arr2036
Copy link
Member

arr2036 commented Dec 6, 2023

I think this is an issue if we've gone out of our way to obfuscate other config items with debug < 3. Just a manual hack here would be sufficient? Copy the parameter string to a buffer and elide everything from password= until the next space or end of the line. I don't think there's any other instance of this with other modules. It's an argument for having a "secret" flag in boxes I guess...

@arr2036 arr2036 reopened this Dec 6, 2023
@alandekok
Copy link
Member

This is on v3, so there's no "secret" flag available.

My point is really that it's a one-line patch to simply omit that message if necessary, or make it a debug message.

I would much prefer a fix for something, rather than a bug report which says "I don't like this. In some situations in my network it causes issues, please fix it."

@alandekok
Copy link
Member

I'll also add that the password field is in the output only for some databases. So simply hacking up the string for MySQL isn't a fix.

@arr2036
Copy link
Member

arr2036 commented Dec 6, 2023

I think it's just postgresql which uses a parameter string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect category: a defect or misbehaviour
Projects
None yet
Development

No branches or pull requests

3 participants