activitywatch icon indicating copy to clipboard operation
activitywatch copied to clipboard

REST Security (opening up the API to the network)

Open ErikBjare opened this issue 7 years ago • 13 comments

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.

ErikBjare avatar Mar 09 '17 15:03 ErikBjare

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 avatar Mar 10 '17 10:03 johan-bjareholt

@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 avatar Mar 10 '17 10:03 ErikBjare

@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.

johan-bjareholt avatar Mar 10 '17 10:03 johan-bjareholt

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).

ErikBjare avatar Mar 10 '17 11:03 ErikBjare

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.

johan-bjareholt avatar Mar 10 '17 17:03 johan-bjareholt

@johan-bjareholt

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.

I mean that local clients (for example, watchers running on the same machine) can simply read the auth file (~/.config/activitywatch/auth for example) and use the localclient user specified in there since they have access to the filesystem. Clients on the same local network would not, and would need to be paired (possibly a solution for a future Android watcher since we're unlikely to get aw-server running well on Android anytime soon).

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.

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.

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.

Hmm, that's weird. Just tried http://127.0.0.1:5666/ and it worked fine for me.

ErikBjare avatar Mar 10 '17 18:03 ErikBjare

I mean that local clients (for example, watchers running on the same machine) can simply read the auth file (~/.config/activitywatch/auth for example) and use the localclient user specified in there since they have access to the filesystem

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,

Hmm, that's weird. Just tried http://127.0.0.1:5666/ and it worked fine for me.

~~Are you sure that you havn't commented the CORS line in aw-server? Running it in testing also allows cross site requests.~~

My aw-webui resources.js was apparently modified, works fine now so it was all my mistake.

johan-bjareholt avatar Mar 10 '17 19:03 johan-bjareholt

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

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 auth file, a few lines in aw-client setting the proper request header and a few lines to verify the header in aw-server.

That definitely shouldn't work (i assume you run aw-webui with npm since its 5666?) since aw-server is on 5600 so that should trigger a cross site request. Are you sure that you havn't commented the CORS line in aw-server? Running it in testing also allows cross site requests.

I run aw-webui as part of aw-server (aw-server --testing uses port 5666, no npm/node involved) and I just tested running without the --testing flag, still works fine on port 5600. And yes, I've checked that CORS is only allowed in testing mode.

I'm not sure if 127.0.0.1:5600 is considered to be a different origin than localhost:5600 since localhost -> 127.0.0.1 in the hosts file.

ErikBjare avatar Mar 10 '17 20:03 ErikBjare

Saw your comment edit, great!

ErikBjare avatar Mar 10 '17 20:03 ErikBjare

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 ~/.config/activitywatch/auth?

An easy fix when running as npm run dev: Let the webpack build script load the key from the auth file and put it in a global which we then put in the header of every API call (after hashing and whatnot). But for the webui bundled with aw-server, I'm not sure how we could pass the key into the webapp. We might have to put the webapp itself behind a login screen if we want to inject (depending on where the request is coming from).

So, in summary: Authentication should be easy to set up for watchers such as aw-watcher-afk, aw-watcher-window but harder for aw-watcher-web since it can't read the filesystem (We might opt for a one-time setup). How it'll work with the webui is still unclear and will likely be the primary cause of complexity.

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 127.0.0.1/localhost.

ErikBjare avatar Mar 14 '17 18:03 ErikBjare

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

ErikBjare avatar Jul 09 '17 16:07 ErikBjare

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)

ErikBjare avatar Aug 01 '17 20:08 ErikBjare

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.

stale[bot] avatar Feb 15 '20 12:02 stale[bot]