keymaster icon indicating copy to clipboard operation
keymaster copied to clipboard

Improve Default Configuration - Add admin interface listening to localhost only by default

Open benmap-brex opened this issue 5 years ago • 7 comments

The admin panel on port 6920 is by default bound to all interfaces. Instead, bind the address to localhost only to provide for defense in depth and use a proxy or iptables to expose the port to the outside world if desired.

benmap-brex avatar Feb 11 '20 21:02 benmap-brex

I would disagree with this one. The admin port is also used to collect metrics for the system and to perform unsealing operations. I would agree that:

  1. Logs /sensitive data should be not exposed by default
  2. Any write operation MUST be authenticated.

Would metrics be considered sensitive data? I would probably say no, but I am willing to hear why this kind of data can be considered sensitive.

Another option would be to keep opening the port to 0.0.0.0 by locally filter to rfc1918 addresses (and rfc rfc6598).

Can you please elaborate on this one more?

cviecco avatar Feb 11 '20 23:02 cviecco

I changed the title because logs arn't exposed by default and write operations are authenticated. That's great, I assumed the default config exposed logs because of our environment!

Would it be possible for this admin port to listen on localhost by default? It just helps reduce the attack surface on keymaster a bit by not exposing the port by default. Think fresh ec2-instance spun up with no firewall because it's dev and security is a bit more lax or a dev instance running on a laptop where the dev maybe forgot to enable their firewall or something.

Thank you!

benmap-brex avatar Feb 12 '20 06:02 benmap-brex

I don't think it's a good idea to bind this to localhost by default, since the purpose of this port is to provide off-machine visibility (i.e. metrics). We would just be adding to the burden of setting up the service, since basically everyone who wants to run this will need to undo the default.

rgooch avatar Feb 13 '20 00:02 rgooch

If you want to expose admin functionality, I think it's a good thing that administrators are forced to carefully consider what they are exposing and to whom. The setup is a one-time thing, but an insecure configuration an administrator is not aware of lasts forever. I think we should default to protecting our customers as much as possible by not exposing administrative interfaces to the world out of the box.

benmap-brex avatar Feb 13 '20 17:02 benmap-brex

I don't agree, sorry. What you are proposing will force every admin who is running a system with expectations of uptime to fix what they will see as a broken configuration option (because they need to enable metrics collection). The proposed configuration option does not present a meaningful choice, since they have to enable it in practice.

I think the right place to deal with this is by using security groups to limit access to subnets with the metrics collectors and the desktop machines used by the administrators.

rgooch avatar Feb 13 '20 18:02 rgooch

One thing that's concerning to me is the metrics and admin functionality have been co-mingled onto this single port. It's one thing to have a metrics endpoint be publicly available, but another thing to also have the metrics endpoint have the ability to reconfigure the app, which is how I believe it currently is, and why I think it should be protected by default.

This is a defense in depth issue, not a bug, so it's okay to disagree here and not do this. If keymaster wants to have the strongest security posture out of the box, metrics should be split from admin, then expose metrics only, or expose admin + metrics to localhost only.

benmap-brex avatar Feb 13 '20 19:02 benmap-brex

Splitting the functionality over different ports might be the better way to deal with this. It would certainly make it easier to reason and verify that sensitive endpoints are locked down. It opens the question of how many ports. These are the broad groupings of endpoints:

  • UI/API (currently on the "service" port 443). Authenticated
  • Metrics (currently on the "admin" port 6920). Unauthenticated
  • Read-only dashboard (currently on the "admin" port 6920). Unauthenticated
  • Logs (currently on the "admin" port 6920). Authenticated in default configuration
  • Unsealing (the only true "admin") endpoint (currently on the "admin" port 6920). Authenticated

Perhaps we can move metrics and read-only dashboard to port 6922?

rgooch avatar Feb 13 '20 19:02 rgooch