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.

Sign up for GitHub

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

Jump to bottom

rpc: we're not enforcing one tenant not talking gRPC to another tenant #95465

Closed
andreimatei opened this issue Jan 18, 2023 · 1 comment
Closed

rpc: we're not enforcing one tenant not talking gRPC to another tenant #95465

andreimatei opened this issue Jan 18, 2023 · 1 comment
Assignees
@knz
Labels
A-multitenancy Related to multi-tenancy A-security C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-multitenant Issues owned by the multi-tenant virtual team

Comments

@andreimatei
Copy link
Contributor

andreimatei commented Jan 18, 2023 •
edited by cockroach-jira-scripts
Loading

A tenant is supposed to only be able to make RPCs to servers from the same tenant, and to KV. But we're missing checks for the first part - as far as CRDB is concerned, I think one tenant can call into any other. In CC we have network-level protection against this.

I've briefly looked into adding the missing checks, but I failed to do something good with the current code structure. There's this code which deals with authenticating the caller, but as written it doesn't even return a tenant ID when the receiver is a tenant server, so the authorization code is inhibited.

cc @ajstorm

Jira issue: CRDB-23525
@andreimatei andreimatei added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-security A-multitenancy Related to multi-tenancy T-multitenant Issues owned by the multi-tenant virtual team labels Jan 18, 2023
@andreimatei andreimatei self-assigned this Jan 18, 2023
@knz knz mentioned this issue Jan 30, 2023
Merged
@knz
Copy link
Contributor

knz commented Jan 30, 2023

fixed by #96152

@knz knz closed this as completed Jan 30, 2023
@knz knz assigned knz and unassigned andreimatei Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

Labels
A-multitenancy Related to multi-tenancy A-security C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-multitenant Issues owned by the multi-tenant virtual team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@andreimatei @knz