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

server: pod-to-pod communication should use same TLS cert as pod-kv comms, not node.crt #71106

Closed
knz opened this issue Oct 4, 2021 · 2 comments · Fixed by #71248 · May be fixed by #71190
Closed

server: pod-to-pod communication should use same TLS cert as pod-kv comms, not node.crt #71106

knz opened this issue Oct 4, 2021 · 2 comments · Fixed by #71248 · May be fixed by #71190
Labels
A-security A-server-networking Pertains to network addressing,routing,initialization C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-server-and-security DB Server & Security

Comments

@knz
Copy link
Contributor

knz commented Oct 4, 2021

Describe the problem

Currently pod-pod communication load and use node.crt.
It only requires the cert to have CN=node and does not perform checks further.

This is problematic in two ways:

  • it does not protect against a mistaken pod-pod connection from one tenant to another (i.e. there won't be an error if pod 1 for tenant A connects to pod 1 for tenant B)
  • it creates a file with CN=node on the filesystem which, if leaked, can be used to establish SQL connections with node-level privilege

Expected outcome

We should use the same TLS cert for pod-pod connections as used for pod-kv connections.
And also we should verify upon incoming pod-pod connections that the tenant ID in the cert matches the local tenant ID.

@knz knz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-security A-server-networking Pertains to network addressing,routing,initialization labels Oct 4, 2021
@knz
Copy link
Contributor Author

knz commented Oct 4, 2021

cc @dhartunian @catj-cockroach @aaron-crl @piyush-singh for triage

@knz
Copy link
Contributor Author

knz commented Oct 6, 2021

Discussed with @catj-cockroach - we should use a separate node cert, not the tenant client cert. I'll take this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-security A-server-networking Pertains to network addressing,routing,initialization C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-server-and-security DB Server & Security
Projects
None yet
1 participant