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

[BUG] Backend has no authentication checks in place #668

Closed
4 tasks done
skornel02 opened this issue May 20, 2022 · 8 comments
Closed
4 tasks done

[BUG] Backend has no authentication checks in place #668

skornel02 opened this issue May 20, 2022 · 8 comments
Assignees
Labels
🐛 Bug [ISSUE] Ticket describing something that isn't working 📌 Keep Open [ISSUE][PR] Prevent auto-closing

Comments

@skornel02
Copy link

skornel02 commented May 20, 2022

Environment

Self-Hosted (Bare Metal)

Version

2.0.8

Describe the problem

To begin with I'm not sure if this should be taken as a security issue or a documentation issue.

Currently the official documentiation talks about the safety of the password and the use of keycloak.

With basic auth, all logic is happening on the client-side, which could mean a skilled user could manipulate the code to view parts of your configuration, including the hash.

This suggests that the use of basic auth has no futher security implications aside from the hash being visible. However anyone, without authentication, can still use the node server's functions. Prohibiting the use of the functions on the client side is not proper security.

I am no security expert but I think that this exposes a few immediate ways this software can exploited:

  1. Anyone can rebuild the application to their likings (without authentication)
  2. The available storage space can be filled with newly saved config files
  3. The machine can be slowed by continuous rebuilds and in case of mini PCs (like a Raspberry Pi) they can be rendered useless.

I suggest that either the security part of the documentation and the README should have a clear warning that indicates that this software should not be run as an internet facing service because of security complications OR a proper backend check for authentication is ought to be implemented (this has to be done for each security provider) OR there should be configuration for the backend to hard-disable potentially harmful endpoints and make this the default for new users.

This supersedes #590

Additional info

No response

Please tick the boxes

@skornel02 skornel02 added the 🐛 Bug [ISSUE] Ticket describing something that isn't working label May 20, 2022
@voidfire
Copy link

Any updates on this? I was really hyped to put Dashy on a server but if there are such security implications, I'd rather not have it on an internet facing server with private stuff in it :(

@liss-bot liss-bot added the 👤 Awaiting Maintainer Response [ISSUE] Response from repo author is pending label Jun 12, 2022
@liss-bot
Copy link
Collaborator

This issue has gone 6 weeks without an update. To keep the ticket open, please indicate that it is still relevant in a comment below. Otherwise it will be closed in 5 working days.

@liss-bot liss-bot added ⚰️ Stale [ISSUE] [PR] No activity for over 1 month and removed 👤 Awaiting Maintainer Response [ISSUE] Response from repo author is pending labels Jul 13, 2022
@Lissy93
Copy link
Owner

Lissy93 commented Jul 13, 2022

Will be completed (hopefully) in 2.1.2, along with a re-write of how config is loaded and managed. For details, see #799

@liss-bot liss-bot added 📌 Keep Open [ISSUE][PR] Prevent auto-closing and removed ⚰️ Stale [ISSUE] [PR] No activity for over 1 month labels Jul 14, 2022
@skornel02 skornel02 mentioned this issue Oct 5, 2022
5 tasks
@subract
Copy link

subract commented Mar 27, 2024

I think this is a more serious issue than it is being given credit for. I discovered the problem independently before finding this issue and wrote up a blog post with my thoughts, but the gist is that passing all the API keys for widgets to unauthenticated users is a huge security issue, and allowing unauthenticated config modification opens users up to DoS and phishing attacks.

@liss-bot liss-bot added 👤 Awaiting Maintainer Response [ISSUE] Response from repo author is pending and removed 👤 Awaiting Maintainer Response [ISSUE] Response from repo author is pending labels Mar 27, 2024
@CrazyWolf13
Copy link
Collaborator

Here the discussion post made by lissy:
#1469

@skornel02
Copy link
Author

I think this is a more serious issue than it is being given credit for. I discovered the problem independently before finding this issue and wrote up a blog post with my thoughts, but the gist is that passing all the API keys for widgets to unauthenticated users is a huge security issue, and allowing unauthenticated config modification opens users up to DoS and phishing attacks.

Most notably is not the problem that this is a very serious problem, but the constant downplay of this issue. The documentation is very pretty and eases the user into the false sense of security that the authentication system does anything on the backend. As recent as the above linked post suggests that the keycloak implementation will be better, but that is also frontend only.

After two years I think that anyone can decide how strong the focus on privacy is in this project.

@liss-bot liss-bot added 👤 Awaiting Maintainer Response [ISSUE] Response from repo author is pending and removed 👤 Awaiting Maintainer Response [ISSUE] Response from repo author is pending labels Mar 28, 2024
@Lissy93
Copy link
Owner

Lissy93 commented Mar 30, 2024

Hi @skornel02
I'm sorry, I haven't meant to ease anyone into a false sense of security.

The current feature is not meant to be anything more than a login page, and since auth is critical to get right, I have deprecated Dashy's built-in authentication.

My recommendation would be, if you are hosting Dashy anywhere other than within your local network, you should be protecting it with a reverse proxy and an authentication provider of your choice. And once you've finished configuring it, set the config file to read-only.

@Lissy93
Copy link
Owner

Lissy93 commented May 10, 2024

Hey @voidfire @skornel02

I hear you, and you're right. Mistakes were made (on my part), but I want to assure you that I'm doing everything I can to make them right again.

The above issue should now be fixed, documentation updates, and I've done a full write up in #1579

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug [ISSUE] Ticket describing something that isn't working 📌 Keep Open [ISSUE][PR] Prevent auto-closing
Projects
Status: Done
Development

No branches or pull requests

6 participants