haproxytech / haproxy-consul-connect

HaProxy Connector for Consul Connect. Enables Service Mesh with Consul and HaProxy using TLS and Consul Discovery
Apache License 2.0
95 stars 20 forks source link

Consider removing `-enable-intentions` flag #13

Open banks opened 4 years ago

banks commented 4 years ago

We very purposefully don't have a mode in Connect where you can choose to not enforce intentions. I'd strongly recommend not including one in this integration either. It's understandable if you needed a flag for initial testing or seeing what the overhead may be etc. but I would really prefer to keep away from adding more knobs that affect the security of any integration.

It's always possible to use permissive intentions that allow everything after all (that's even the default if you haven't setup restrictive ACLs). If you feel strongly that this is something you need to keep long term for testing the overhead of intention enforcement, could we at least make it on-by-default and Opt out (i.e. DisableIntentionChecking) rather than opt -in?

Also, from what I can see the Certificate validation is being done in your SPOE handler and so presumably not in HAProxy at all which means that disabling SPOE is more than just disabling intention checks - it's disabling cert validation too. In other words it's a free for all, you don't even need a valid client cert, any old self-signed thing will do. That implies it should be named something more general like -disable-security since it's not only intention enforcement but all of the mutual TLS benefits that go away.

If an option is kept for this I suggest that it needs very clear docs and sufficiently strong warnings about the precise way it weakens the threat model.