ajnart / homarr

Customizable browser's home page to interact with your homeserver's Docker containers (e.g. Sonarr/Radarr)
https://homarr.dev
MIT License
6.11k stars 280 forks source link

Log authentication attempts when PASSWORD environment variable is used #506

Closed Stylback closed 1 year ago

Stylback commented 1 year ago

Description

Background: It's possible to enable password authentication to access the dashboard using the PASSWORD environment variable. If you try to authenticate with the wrong password you will be met with an error message, but nothing related to the authentication attempt will be logged.

Suggestion: Have Homarr log authentication attempts when the PASSWORD environment variable is used.

The log could be of similiar form as Sonarr, Radarr and Lidarr:

2022-11-14 15:00:00.0|Warn|Auth|Auth-Failure ip 123.123.123.123

To prevent log clutter or endlessly growing logs, one could use rotating logs or a daily cut-off point as in the case of the *arr apps. Logs older than a certain threshold could be culled with a simple cron job. For those with privacy concerns or certain threat models, logging could also be disabled with a boolean. In my mind, a docker compose with these features would look something like this:

...
- PASSWORD=SuperSecretPassword
- ENABLE_AUTH_LOG=true
- KEEP_AUTH_LOG=720h # 30 days, only necessary if ENABLE_AUTH_LOG=true
...

I believe this feature would help users become more aware of authentication attempts made against their dashboard. As an added benefit it would also allow for other services such as Fail2ban to act on failed authentication attempts.

While this feature can partly be achived already using Nginx's Basic HTTP authentication, I see no reason it should not be native to Homarr as it can only ever add to user security.

Priority

Low (Nice-to-have)

manuel-rw commented 1 year ago

Hi @Stylback , thank you very much for your detailed request. Sounds like an addition that would make sense!

@ajnart

manuel-rw commented 1 year ago

Do you have any ideas or suggestions for the cron? I'm not sure whether log management is the task of Homarr, but we can see what we can do.

Stylback commented 1 year ago

I'm by no means a cron expert, but here is a back-of-the-napkin cronjob that should work:

First, let's do some assumptions:

To cull logs that are older than let's say 7 days with cron, I would do something like this:

  1. Create a bash script:
sudo nano cull_script.sh

Paste:

#!/usr/bin/bash
find /var/logs/homarr/ -name "*.log" -type f -mtime +7 -delete 

Make it runnable:

chmod +x ~/cull_script:
  1. Create a cron job:
crontab -e
0 1 * * * ~/cull_script

0 1 * * * would run it daily at 01:00.

However, I'm not sure how one would pass an environment variable for the KEEP_AUTH_LOG suggestion I made above.

ajnart commented 1 year ago

Would just logging auth attempts in console be enough ? We don't really log anything.

ajnart commented 1 year ago
image

I've made some quick changes in https://github.com/ajnart/homarr/commit/5f61940c92a923b0d9cf89c798fd4a656fdb5553 Would that solve this issue ? I dont think a fully blown logging system is required at the moment.

Stylback commented 1 year ago

Would just logging auth attempts in console be enough ? We don't really log anything.

I know services like Gotify lets Docker handle the logging, it's then still possible to "export" the logs using the entrypoint option. A docker compose file then has the following added to it:

entrypoint: sh -c "/app/gotify-app 2>&1 | tee /app/data/gotify.log"

So as long as it's possible to somehow access the logs and write it to your own file, like with the entrypoint example above, there should be no need for a fully-blown log system.

I am not a Docker developer, so I can't help further with how that would look like.

ajnart commented 1 year ago

Would just logging auth attempts in console be enough ? We don't really log anything.

I know services like Gotify lets Docker handle the logging, it's then still possible to "export" the logs using the entrypoint option. A docker compose file then has the following added to it:


entrypoint: sh -c "/app/gotify-app 2>&1 | tee /app/data/gotify.log"

So as long as it's possible to somehow access the logs and write it to your own file, like with the entrypoint example above, there should be no need for a fully-blown log system.

I am not a Docker developer, so I can't help further with how that would look like.

These logs should show up when using docker logs and be exploited later. The changes will be added in the next patch if you confirm the format is what you were looking for.

manuel-rw commented 1 year ago
image

I've made some quick changes in https://github.com/ajnart/homarr/commit/5f61940c92a923b0d9cf89c798fd4a656fdb5553 Would that solve this issue ? I dont think a fully blown logging system is required at the moment.

Shouldn't we use a structured logger that is being recommended for production by Vercel?

https://nextjs.org/docs/going-to-production#logging

https://www.npmjs.com/package/pino

Stylback commented 1 year ago

These logs should show up when using docker logs and be exploited later. The changes will be added in the next patch if you confirm the format is what you were looking for.

I took a look at the commit and it seems to have all that anyone needs to act upon a failed authentication; date+time, warning message and a request origin. I am satsified!

Edit: From a security standpoint, I suggest removing the ${tried} variable as anyone inspecting said logs could then guess the correct password if you were to make a typo.

ajnart commented 1 year ago

These logs should show up when using docker logs and be exploited later. The changes will be added in the next patch if you confirm the format is what you were looking for.

I took a look at the commit and it seems to have all that anyone needs to act upon a failed authentication; date+time, warning message and a request origin. I am satsified!

Edit: From a security standpoint, I suggest removing the ${tried} variable as anyone inspecting said logs could then guess the correct password if you were to make a typo.

Ok I understand your comment on the pr better @manuel-rw . I'll fix it