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

rpc,tenantcapabilities: allow cross tenant reads in shared service #130183

Merged
merged 1 commit into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/tenantrate/limiter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ func (ts *testState) BindReader(tenantcapabilities.Reader) {}

var _ tenantcapabilities.Authorizer = &testState{}

func (ts *testState) HasCrossTenantRead(tenID roachpb.TenantID) bool {
func (ts *testState) HasCrossTenantRead(ctx context.Context, tenID roachpb.TenantID) bool {
return false
}

Expand Down Expand Up @@ -789,7 +789,7 @@ type fakeAuthorizer struct{}

var _ tenantcapabilities.Authorizer = &fakeAuthorizer{}

func (fakeAuthorizer) HasCrossTenantRead(tenID roachpb.TenantID) bool {
func (fakeAuthorizer) HasCrossTenantRead(ctx context.Context, tenID roachpb.TenantID) bool {
return false
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/multitenant/tenantcapabilities/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type Reader interface {
// usage pattern over a timespan.
type Authorizer interface {
// HasCrossTenantRead returns true if a tenant can read other tenant spans.
HasCrossTenantRead(tenID roachpb.TenantID) bool
HasCrossTenantRead(ctx context.Context, tenID roachpb.TenantID) bool

// HasCapabilityForBatch returns an error if a tenant, referenced by its ID,
// is not allowed to execute the supplied batch request given the capabilities
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ func NewAllowEverythingAuthorizer() *AllowEverythingAuthorizer {
}

// HasCrossTenantRead returns true if a tenant can read from other tenants.
func (n *AllowEverythingAuthorizer) HasCrossTenantRead(tenID roachpb.TenantID) bool {
func (n *AllowEverythingAuthorizer) HasCrossTenantRead(
ctx context.Context, tenID roachpb.TenantID,
) bool {
return true
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ func NewAllowNothingAuthorizer() *AllowNothingAuthorizer {
}

// HasCrossTenantRead returns true if a tenant can read from other tenants.
func (n *AllowNothingAuthorizer) HasCrossTenantRead(tenID roachpb.TenantID) bool {
func (n *AllowNothingAuthorizer) HasCrossTenantRead(
ctx context.Context, tenID roachpb.TenantID,
) bool {
return false
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,22 @@ func New(settings *cluster.Settings, knobs *tenantcapabilities.TestingKnobs) *Au
}

// HasCrossTenantRead returns true if a tenant can read from other tenants.
func (a *Authorizer) HasCrossTenantRead(tenID roachpb.TenantID) bool {
return tenID.IsSystem()
func (a *Authorizer) HasCrossTenantRead(ctx context.Context, tenID roachpb.TenantID) bool {
if tenID.IsSystem() {
// The system tenant has access to all request types.
return true
}
_, mode := a.getMode(ctx, tenID)
switch mode {
case authorizerModeOn, authorizerModeV222:
return false
case authorizerModeAllowAll:
return true
default:
err := errors.AssertionFailedf("unknown authorizer mode: %d", mode)
logcrash.ReportOrPanic(ctx, &a.settings.SV, "%v", err)
return false
}
}

// HasCapabilityForBatch implements the tenantcapabilities.Authorizer interface.
Expand Down
54 changes: 30 additions & 24 deletions pkg/rpc/auth_tenant.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (a tenantAuthorizer) authorize(
return a.authBatch(ctx, sv, tenID, req.(*kvpb.BatchRequest))

case "/cockroach.roachpb.Internal/RangeLookup":
return a.authRangeLookup(tenID, req.(*kvpb.RangeLookupRequest))
return a.authRangeLookup(ctx, tenID, req.(*kvpb.RangeLookupRequest))

case "/cockroach.roachpb.Internal/RangeFeed", "/cockroach.roachpb.Internal/MuxRangeFeed":
return a.authRangeFeed(tenID, req.(*kvpb.RangeFeedRequest))
Expand Down Expand Up @@ -123,22 +123,22 @@ func (a tenantAuthorizer) authorize(
return a.authTenant(tenID)

case "/cockroach.server.serverpb.Status/SpanStats":
return a.authSpanStats(tenID, req.(*roachpb.SpanStatsRequest))
return a.authSpanStats(ctx, tenID, req.(*roachpb.SpanStatsRequest))

case "/cockroach.roachpb.Internal/GetSpanConfigs":
return a.authGetSpanConfigs(tenID, req.(*roachpb.GetSpanConfigsRequest))
return a.authGetSpanConfigs(ctx, tenID, req.(*roachpb.GetSpanConfigsRequest))

case "/cockroach.roachpb.Internal/SpanConfigConformance":
return a.authSpanConfigConformance(tenID, req.(*roachpb.SpanConfigConformanceRequest))
return a.authSpanConfigConformance(ctx, tenID, req.(*roachpb.SpanConfigConformanceRequest))

case "/cockroach.roachpb.Internal/GetAllSystemSpanConfigsThatApply":
return a.authGetAllSystemSpanConfigsThatApply(tenID, req.(*roachpb.GetAllSystemSpanConfigsThatApplyRequest))

case "/cockroach.roachpb.Internal/UpdateSpanConfigs":
return a.authUpdateSpanConfigs(tenID, req.(*roachpb.UpdateSpanConfigsRequest))
return a.authUpdateSpanConfigs(ctx, tenID, req.(*roachpb.UpdateSpanConfigsRequest))

case "/cockroach.roachpb.Internal/GetRangeDescriptors":
return a.authGetRangeDescriptors(tenID, req.(*kvpb.GetRangeDescriptorsRequest))
return a.authGetRangeDescriptors(ctx, tenID, req.(*kvpb.GetRangeDescriptorsRequest))

case "/cockroach.server.serverpb.Status/HotRangesV2":
return a.authHotRangesV2(tenID)
Expand Down Expand Up @@ -199,7 +199,7 @@ func (a tenantAuthorizer) authBatch(
tenSpan := tenantPrefix(tenID)

if outsideTenant(rSpan, tenSpan) {
if args.IsReadOnly() && a.capabilitiesAuthorizer.HasCrossTenantRead(tenID) {
if args.IsReadOnly() && a.capabilitiesAuthorizer.HasCrossTenantRead(ctx, tenID) {
return nil
}
return spanErr(rSpan, tenSpan)
Expand All @@ -208,16 +208,16 @@ func (a tenantAuthorizer) authBatch(
}

func (a tenantAuthorizer) authGetRangeDescriptors(
tenID roachpb.TenantID, args *kvpb.GetRangeDescriptorsRequest,
ctx context.Context, tenID roachpb.TenantID, args *kvpb.GetRangeDescriptorsRequest,
) error {
return validateSpan(tenID, args.Span, true, a)
return validateSpan(ctx, tenID, args.Span, true, a)
}

func (a tenantAuthorizer) authSpanStats(
tenID roachpb.TenantID, args *roachpb.SpanStatsRequest,
ctx context.Context, tenID roachpb.TenantID, args *roachpb.SpanStatsRequest,
) error {
for _, span := range args.Spans {
err := validateSpan(tenID, span, true, a)
err := validateSpan(ctx, tenID, span, true, a)
if err != nil {
return err
}
Expand All @@ -228,12 +228,12 @@ func (a tenantAuthorizer) authSpanStats(
// authRangeLookup authorizes the provided tenant to invoke the RangeLookup RPC
// with the provided args.
func (a tenantAuthorizer) authRangeLookup(
tenID roachpb.TenantID, args *kvpb.RangeLookupRequest,
ctx context.Context, tenID roachpb.TenantID, args *kvpb.RangeLookupRequest,
) error {
tenSpan := tenantPrefix(tenID)
if !tenSpan.ContainsKey(args.Key) {
// Allow it anyway if the tenant can read other tenants.
if a.capabilitiesAuthorizer.HasCrossTenantRead(tenID) {
if a.capabilitiesAuthorizer.HasCrossTenantRead(ctx, tenID) {
return nil
}
return authErrorf("requested key %s not fully contained in tenant keyspace %s", args.Key, tenSpan)
Expand Down Expand Up @@ -351,10 +351,10 @@ func (a tenantAuthorizer) authGetAllSystemSpanConfigsThatApply(
// authGetSpanConfigs authorizes the provided tenant to invoke the
// GetSpanConfigs RPC with the provided args.
func (a tenantAuthorizer) authGetSpanConfigs(
tenID roachpb.TenantID, args *roachpb.GetSpanConfigsRequest,
ctx context.Context, tenID roachpb.TenantID, args *roachpb.GetSpanConfigsRequest,
) error {
for _, target := range args.Targets {
if err := validateSpanConfigTarget(tenID, target, true, a); err != nil {
if err := validateSpanConfigTarget(ctx, tenID, target, true, a); err != nil {
return err
}
}
Expand All @@ -364,15 +364,15 @@ func (a tenantAuthorizer) authGetSpanConfigs(
// authUpdateSpanConfigs authorizes the provided tenant to invoke the
// UpdateSpanConfigs RPC with the provided args.
func (a tenantAuthorizer) authUpdateSpanConfigs(
tenID roachpb.TenantID, args *roachpb.UpdateSpanConfigsRequest,
ctx context.Context, tenID roachpb.TenantID, args *roachpb.UpdateSpanConfigsRequest,
) error {
for _, entry := range args.ToUpsert {
if err := validateSpanConfigTarget(tenID, entry.Target, false, a); err != nil {
if err := validateSpanConfigTarget(ctx, tenID, entry.Target, false, a); err != nil {
return err
}
}
for _, target := range args.ToDelete {
if err := validateSpanConfigTarget(tenID, target, false, a); err != nil {
if err := validateSpanConfigTarget(ctx, tenID, target, false, a); err != nil {
return err
}
}
Expand All @@ -393,10 +393,10 @@ func (a tenantAuthorizer) authHotRangesV2(tenID roachpb.TenantID) error {
// authSpanConfigConformance authorizes the provided tenant to invoke the
// SpanConfigConformance RPC with the provided args.
func (a tenantAuthorizer) authSpanConfigConformance(
tenID roachpb.TenantID, args *roachpb.SpanConfigConformanceRequest,
ctx context.Context, tenID roachpb.TenantID, args *roachpb.SpanConfigConformanceRequest,
) error {
for _, sp := range args.Spans {
if err := validateSpan(tenID, sp, false, a); err != nil {
if err := validateSpan(ctx, tenID, sp, false, a); err != nil {
return err
}
}
Expand Down Expand Up @@ -434,7 +434,11 @@ func (a tenantAuthorizer) authTSDBQuery(
// wholly contained within the tenant keyspace and system span config targets
// must be well-formed.
func validateSpanConfigTarget(
tenID roachpb.TenantID, spanConfigTarget roachpb.SpanConfigTarget, read bool, a tenantAuthorizer,
ctx context.Context,
tenID roachpb.TenantID,
spanConfigTarget roachpb.SpanConfigTarget,
read bool,
a tenantAuthorizer,
) error {
validateSystemTarget := func(target roachpb.SystemSpanConfigTarget) error {
if target.SourceTenantID != tenID {
Expand Down Expand Up @@ -463,23 +467,25 @@ func validateSpanConfigTarget(

switch spanConfigTarget.Union.(type) {
case *roachpb.SpanConfigTarget_Span:
return validateSpan(tenID, *spanConfigTarget.GetSpan(), read, a)
return validateSpan(ctx, tenID, *spanConfigTarget.GetSpan(), read, a)
case *roachpb.SpanConfigTarget_SystemSpanConfigTarget:
return validateSystemTarget(*spanConfigTarget.GetSystemSpanConfigTarget())
default:
return errors.AssertionFailedf("unknown span config target type")
}
}

func validateSpan(tenID roachpb.TenantID, sp roachpb.Span, isRead bool, a tenantAuthorizer) error {
func validateSpan(
ctx context.Context, tenID roachpb.TenantID, sp roachpb.Span, isRead bool, a tenantAuthorizer,
) error {
tenSpan := tenantPrefix(tenID)
rSpan, err := keys.SpanAddr(sp)
if err != nil {
return authError(err.Error())
}
if outsideTenant(rSpan, tenSpan) {
// Allow it anyway if the tenant can read other tenants.
if isRead && a.capabilitiesAuthorizer.HasCrossTenantRead(tenID) {
if isRead && a.capabilitiesAuthorizer.HasCrossTenantRead(ctx, tenID) {
return nil
}
return spanErr(rSpan, tenSpan)
Expand Down
2 changes: 1 addition & 1 deletion pkg/rpc/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ func (m mockAuthorizer) HasProcessDebugCapability(
return errors.New("tenant does not have capability")
}

func (m mockAuthorizer) HasCrossTenantRead(tenID roachpb.TenantID) bool {
func (m mockAuthorizer) HasCrossTenantRead(ctx context.Context, tenID roachpb.TenantID) bool {
return m.hasCrossTenantRead
}

Expand Down
Loading