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

Only require verify_incoming_rpc for auto_encrypt.allow_tls #6376

Merged
merged 1 commit into from
Aug 27, 2019

Conversation

hanshasselberg
Copy link
Member

Otherwise it is impossible to host the UI on a server node while using auto_encrypt. Fixes #6338.

@hanshasselberg hanshasselberg added the theme/tls Using TLS (Transport Layer Security) or mTLS (mutual TLS) to secure communication label Aug 22, 2019
Copy link
Contributor

@mikemorris mikemorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm a little confused (perhaps it's just the naming?) as to why this is necessary. Intentionally disabling verification (is this verifying that requests are coming from a shared CA in Consul cluster?) of all incoming HTTPS requests just to serve the UI seems undesirable and maybe opening a vulnerability?

Would it be possible/desirable instead to only allow incoming requests to the UI to ignore verification instead of disabling it entirely? Very possible I'm not fully understanding what's going on here though 😄

I'm trying to configure the UI for HTTPS which requires setting verify_incoming to false and verify_incoming_rpc to true.
#6127 (comment)

@hanshasselberg
Copy link
Member Author

@mikemorris good questions! AutoEncrypt works via RPC and technically it doesn't require verify_incoming_https to be secured since it doesn't use that to ship the certs.

is this verifying that requests are coming from a shared CA in Consul cluster?

Yes.

Intentionally disabling verification of all incoming HTTPS requests just to serve the UI seems undesirable and maybe opening a vulnerability?

Yes. It means that HTTPS requests to that particular agent's HTTP API are not required to provide a client cert that was signed by the shared CA.

Would it be possible/desirable instead to only allow incoming requests to the UI to ignore verification instead of disabling it entirely?

It would be better indeed if that would be possible, but it is not at this point. And in general I am more inclined to remove knobs that tune security and go to secure by default. I thought about the UI before and in my mind the problem is that we share the same CA/certs for RPC, HTTPS, and serving the UI. If these things would be separate it would be easier to reason about. If there would be a separate CA/cert for the UI we could even allow letsencrypt possibly. But thats not an option if the same cert is used for RPC as well.

If you want to read more about how to enable the HTTPS UI, you can go here: https://learn.hashicorp.com/consul/security-networking/certificates#configuring-the-consul-ui-for-https. I think it explains pretty well how the system works right now. Not saying this is a great way by any means though. :)

Does that clarify things?

Copy link
Contributor

@mikemorris mikemorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about the UI before and in my mind the problem is that we share the same CA/certs for RPC, HTTPS, and serving the UI.

Separating the UI to be able to use its' own cert (including something like letsencrypt) rather than being linked to internal cluster settings feels like a good direction for future work. 👍

If you want to read more about how to enable the HTTPS UI, you can go here: https://learn.hashicorp.com/consul/security-networking/certificates#configuring-the-consul-ui-for-https.

I think the details I was missing were the suggested config below to be used in conjunction with "verify_incoming": false and the note that this is to be configured manually for 1-2 agents, not everywhere in something like central config... (although this feels potentially too easy for an operator to misconfigure)

"enable_script_checks": false,
"disable_remote_exec": true

@hanshasselberg hanshasselberg added this to the 1.6.1 milestone Aug 27, 2019
@hanshasselberg hanshasselberg merged commit faa54ab into master Aug 27, 2019
@hanshasselberg hanshasselberg deleted the auto_encrypt_verify_rpc branch August 27, 2019 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/tls Using TLS (Transport Layer Security) or mTLS (mutual TLS) to secure communication
Projects
None yet
Development

Successfully merging this pull request may close these issues.

auto_encrypt: check verify_incoming_rpc instead of verify_incoming on the server to make hosting UI possible.
2 participants