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

Limit access to localhost (security related) #456

Closed
Jopyth opened this issue Sep 28, 2016 · 5 comments
Closed

Limit access to localhost (security related) #456

Jopyth opened this issue Sep 28, 2016 · 5 comments

Comments

@Jopyth
Copy link
Contributor

Jopyth commented Sep 28, 2016

Are there any merits to the usual workflow, if we limit the access to localhost? For example change this line to
server.listen(config.port, config.route); and make the default route = "127.0.0.1".

Reason: More and more modules do very cool stuff (e.g. accessing Emails), however they also need the login information in the config. Currently anyone on your network (guests - if you do not have a separate guest network) can straight up load your mirror html page or even your config.js and read sensitive information from your config file.

So if I invite Eve to my house and she goes to 192.168.0.100:8080/config/config.js (if that is my Raspberry Pi IP) she gets the good old Server not responding or whatever instead of reading all our API keys, possible usernames and password for certain websites, etc...

There is probably a much better way, but for the moment this would be very easy to implement.

@MichMich
Copy link
Collaborator

You make very valid point. But I think it might be a good idea to make this configurable. Since a lot of people access it from a external IP to test it. Maybe we should add whitelist IP's.

Most important: never allow people you don't trust on your network! 😉

@Jopyth
Copy link
Contributor Author

Jopyth commented Sep 28, 2016

Exactly that was my thought behind the config.route value. We would set this in the config file, so just like changing the port we can change the route. If this is set to "0.0.0.0" everything is like it is now, everyone can access it.

Something like a Whitelist would be awesome.

Still I think, to protect the unsuspecting user making the default (e.g. In the config.js.sample) to only allow access from localhost would be good. Plus of course documenting this change for anyone who is developing.

@MichMich
Copy link
Collaborator

Will look into this if I have time. Feel free to send a PR.

@Jopyth
Copy link
Contributor Author

Jopyth commented Sep 29, 2016

Would you prefer the simple all or nothing approach, or a more comprehensive one, for example with express-ipfilter? With this package we can use an ip_whitelist, but it adds another dependency.

@MichMich
Copy link
Collaborator

I would prefer the latter. MagicMirror² relies on a lot of dependencies already so that's not really an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants