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

Stronger TLS validation for Cluster communication #391

Closed
msft-paddy14 opened this issue May 16, 2024 · 0 comments · Fixed by #392
Closed

Stronger TLS validation for Cluster communication #391

msft-paddy14 opened this issue May 16, 2024 · 0 comments · Fixed by #392
Assignees
Labels
enhancement New feature or request

Comments

@msft-paddy14
Copy link
Contributor

Feature request type

enhancement

Is your feature request related to a problem? Please describe

For Garnet Cluster TLS mode, there's no remote cert validation if ClientCert required is set to false. Allowing all connections. However, during cluster gossip, this means a node from ClusterA(3 primaries) can connect to any node of ClusterB( 3 primaries) if a MEET is issued(keeps happening during gossip) using IP. Since IP reuse is common in K8s, it's possible for node to connect to some unwanted Cluster as the stale IP might not have been removed but might be valid as it's part of a different cluster. There seems to be an existing ClusterTlsClientTargetHost parameter in Garnet but it's not being used.
Currently, ClientCertRequired when set to false may be disabled for Clients to connect to server but it also prevents validation of internal nodes participating in gossip.

Describe the solution you'd like

Proposal is to use existing parameter ClusterTlsClientTargetHost to make the check stronger in ClientTlsAuthOptions by validating that remote cert (when Garnet acts as a client during Gossip) and local cert configured have same name along with existing checks. This will prevent nodes meant to be part of different cluster from communicating as part of gossip by failing TLS handshake. So that only MEET happens between nodes that are intended to part of cluster.

Sample error expected when different certs are configured for different nodes:
image

I don't think it should break existing scenarios since this should've always been the intended behavior and seems like we already had parameter but lacking validation.

Describe alternatives you've considered

No response

Additional context

No response

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

Successfully merging a pull request may close this issue.

2 participants