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

kv: lock down manual splits (and maybe all admin KV requests) on sql tenants #52360

Closed
nvanbenschoten opened this issue Aug 4, 2020 · 1 comment · Fixed by #52592
Closed

kv: lock down manual splits (and maybe all admin KV requests) on sql tenants #52360

nvanbenschoten opened this issue Aug 4, 2020 · 1 comment · Fixed by #52592
Assignees
@nvanbenschoten
Labels
A-multitenancy Related to multi-tenancy C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@nvanbenschoten
Copy link
Member

SQL tenants must not be allowed to perform manual range splits. This would allow them to create arbitrarily many ranges, instead of the maximum range count of a tenant being a function of the maximum data size offered to it.

The best way to do this might be to lock down all Admin KV requests in a follow-up change to #52094. I need to explore that and see if it's possible or if we need to support other admin kv requests.
@nvanbenschoten nvanbenschoten self-assigned this Aug 4, 2020
@blathers-crl
Copy link

blathers-crl bot commented Aug 4, 2020

Hi @nvanbenschoten, I've guessed the C-ategory of your issue and suitably labeled it. Please re-label if inaccurate.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Aug 4, 2020
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Aug 10, 2020
@nvanbenschoten
kv: reject admin KV requests from tenant SQL processes
730a44e 
Fixes cockroachdb#52360.
First two commits from cockroachdb#52589.

This commit adds a new restriction to the tenantAuth policy that it
accepts no "Admin" KV requests. This prevents tenants from splitting
ranges, merging range, rebalancing ranges, or issuing any other KV
requests with the `isAdmin` flag that could dictate KV-level
distribution decisions.

This further mitigates the impact that a compromised tenant SQL process
could have on the rest of the cluster.
@nvanbenschoten nvanbenschoten mentioned this issue Aug 10, 2020
Merged
craig bot pushed a commit that referenced this issue Aug 11, 2020
@nvanbenschoten
Merge #52592
2b85581 
52592: kv: reject admin KV requests from tenant SQL processes r=nvanbenschoten a=nvanbenschoten

Fixes #52360.

This commit adds a new restriction to the tenantAuth policy that it accepts no "Admin" KV requests. This prevents tenants from splitting ranges, merging range, rebalancing ranges, or issuing any other KV requests with the `isAdmin` flag that could dictate KV-level distribution decisions.

This further mitigates the impact that a compromised tenant SQL process could have on the rest of the cluster.

Co-authored-by: Nathan VanBenschoten <[email protected]>
@craig craig bot closed this as completed in 3018397 Aug 11, 2020
madelineliao pushed a commit to madelineliao/cockroach that referenced this issue Aug 12, 2020
@nvanbenschoten @madelineliao
kv: reject admin KV requests from tenant SQL processes
4407045 
Fixes cockroachdb#52360.

This commit adds a new restriction to the tenantAuth policy that it
accepts no "Admin" KV requests. This prevents tenants from splitting
ranges, merging range, rebalancing ranges, or issuing any other KV
requests with the `isAdmin` flag that could dictate KV-level
distribution decisions.

This further mitigates the impact that a compromised tenant SQL process
could have on the rest of the cluster.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nvanbenschoten nvanbenschoten

Labels
A-multitenancy Related to multi-tenancy C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kv: reject admin KV requests from tenant SQL processes nvanbenschoten/cockroach
2 participants
@tbg @nvanbenschoten