-
-
Notifications
You must be signed in to change notification settings - Fork 594
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
REST Security (opening up the API to the network) #32
Comments
I do not agree with that this should be a part of the 1.0 release. In my opinion we should try to get the 1.0 release to be a minimal fully working and stable version, and i don't think that non-local clients are a part of that so security shouldn't be necessary yet. |
@johan-bjareholt Right now, any non-local host can access the API and do anything as long as they know the port it is running on (trivial). I agree that non-local clients should not be a focus for now, but we surely need security against other hosts trying to access the API. |
@ErikBjare Which is fine since all computers nowadays are behind some kind of firewall either from their router or computer anyway, so the only ones able to access are either computers within the local network or just the localhost. If you still don't think that's fine we can instead just bind aw-server to 127.0.0.1/localhost instead of 0.0.0.0 so only local clients can access it. |
I think it's a bit naive to assume all computers are behind a trusted firewall, we do need to protect from the local network. Plenty of networks out there are completely unprotected (airport/school WiFi for instance) and a potential API breach could leak heavily sensitive information about the user. I'm not sure if binding to 127.0.0.1 protects against all cases here. It's discussed some in the Syncthing FAQ. The relevant part in the Syncthing code can be found here. Binding to 127.0.0.1 might be enough, but I'm not all that knowledgable about DNS rebinding attacks (I have a vague memory from a course in web security) so I'd rather take the certain path (auth file). |
This doesn't make sense, because you wrote yourself in the issue that "local clients can simply read it", so adding a authentication will not secure you from DNS rebinding attacks in the case when you bind to 127.0.0.1/localhost. Also, most airport/school wifi's don't allow computers within the local network to communicate with eachother. I still would propose binding to localhost in our first stable release though. On a somewhat related note, the webui works when using localhost:5600, but when you use 127.0.0.1:5600 you get cross site request issues so nothing from the rest api will load. |
I mean that local clients (for example, watchers running on the same machine) can simply read the auth file (
I know most don't, but I've been on more than one where they do. Yes, I'm not saying we shouldn't still bind to localhost, I'm just saying I don't think it is enough.
Hmm, that's weird. Just tried http://127.0.0.1:5666/ and it worked fine for me. |
Oh, alright. That sounds fine. I still don't agree that this should be a priority right now though. It's a pretty huge feature to add and would slow down the 1.0 release a lot for a security flaw that could possibly be fixed by simply binding to localhost. Even if binding to localhost doesn't fix it, activitywatch is only used by us so that security flaw would be insignificant,
My aw-webui resources.js was apparently modified, works fine now so it was all my mistake. |
The whole point of the 1.0 release is to enable others to be able to run ActivityWatch, before we can let them do that we need to have security in place. I don't think it's a very big feature to add, should be trivial. Just a
I run aw-webui as part of aw-server ( I'm not sure if |
Saw your comment edit, great! |
I just realized that the authentication scheme I had in mind complicates the webui: How would it get hold of the local auth key in An easy fix when running as So, in summary: Authentication should be easy to set up for watchers such as Also, for the authentication scheme itself (not just key management), I'd need some input and careful consideration so we don't fuck up. I'm thinking a simple challenge response scheme where the key + some nonce from the server will likely be enough. As a consequence to all this: the security required to allow non-local clients won't make it into the 1.0 release. We'll instead simply forbid non-local clients from connecting by binding to |
This article gave me some good insights into the Why's of Oauth: https://www.scottbrady91.com/OAuth/The-Wrong-Ways-to-Protect-an-API |
This thread got a bit messy so should be closed and replaced with a new one once we pick this up again. (We can have a reference back to this one there) |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
We need to set up proper API security using authentication and HMAC.
First version I propose is that the key is stored on the file system and local clients can simply read it. This is how the Deluge daemon does authentication.
To add a non-local watcher we'd want some form of pairing, but non-local watchers are not a priority and not required to work for issue to be closed.
The text was updated successfully, but these errors were encountered: