[1.8.x] rpc: authorize raft requests #10933
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
By checking the names in the verified certificate chain
In preparation for fixing Raft RPC authorization this change is made to the tlsutil.Configurator to store a CertPool which will omit the Connect CA certificate. This commit also removes the need to append slices of PEMs together. The append function can modify the backing array of the first slice passed to append, and that can often lead to difficult to understand bugs. It's not clear to me if the use of append here would actually cause such bugs, but it was easier to remove the use of append than to prove the bug existed. Removing the use of append should also prevent any such bugs from happening in the future.
Connect CA certs could potentially contain DNSNames that match Consul servers. So to prevent those certificates from being used to gain unauthorized access we verify that the connection performing the raft RPC has a certificate signed by the agent TLS CA. Also add tests cases for ConnectCA leaf cert, which fail without this change.
This test started to fail because the CertFile used is no longer allowed to perform Raft RPC (it must not have a valid DNSName). From my reading of this test, it is intended to limit client connections, so we can probably test with a different byte that would be used by clients. There's no special handling for the RPCRaft byte, so I believe the test is still valid after this change.
The requester is a "Server" in Consul's architecture, but is a Client from the perspective of this RPC request. Co-authored-by: Paul Banks <banks@banksco.de>
eculver
added
theme/tls
|
This pull request is being automatically deployed with Vercel (learn more). consul – ./website🔍 Inspect: https://vercel.com/hashicorp/consul/Hr15WZmF9FWHJQ9miC4TAhNZbiCS [Deployment for 8f8bede failed] consul-ui-staging – ./ui🔍 Inspect: https://vercel.com/hashicorp/consul-ui-staging/CsLUyXt64wibgojHya7fhET7JYW9 [Deployment for 8f8bede failed] |
mikemorris
removed
the
pr/no-changelog
picatz
reviewed
Aug 26, 2021
Co-authored-by: Kent 'picat' Gruber <kent@hashicorp.com>
…icorp/consul into dnephin/backport-1.8-raft-authz-fix
mikemorris
approved these changes
Aug 26, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #10925