Skip to content

Commit 6ccd99f

Browse files
craig[bot]shubhamdhama
andcommitted
Merge #150225
150225: server: add drpc server authentication interceptors r=shubhamdhama a=shubhamdhama Port existing authorization/authentication interceptors from gRPC to DRPC. There is currently some code duplication between the gRPC and DRPC implementations, which will be refactored in a future commit to extract common functionality. Corresponding PR with the DRPC framework changes: cockroachdb/drpc#11 Epic: CRDB-49359 Release note: none Co-authored-by: Shubham Dhama <[email protected]>
2 parents 34acbb7 + 41bab23 commit 6ccd99f

File tree

12 files changed

+247
-60
lines changed

12 files changed

+247
-60
lines changed

DEPS.bzl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11146,10 +11146,10 @@ def go_deps():
1114611146
name = "io_storj_drpc",
1114711147
build_file_proto_mode = "disable_global",
1114811148
importpath = "storj.io/drpc",
11149-
sha256 = "612016b7a145f386a2163d788e3376cb0f63410b8423f2ef3f723f7aa24c1971",
11150-
strip_prefix = "github.com/cockroachdb/[email protected]20250618091105-e79a954a2193",
11149+
sha256 = "ebb4b3ca9a6c5944255506b4cf5091b86b76851e52211940b609c98e8bcc7aab",
11150+
strip_prefix = "github.com/cockroachdb/[email protected]20250807091527-65dcebaa113e",
1115111151
urls = [
11152-
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/drpc/com_github_cockroachdb_drpc-v0.0.0-20250618091105-e79a954a2193.zip",
11152+
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/drpc/com_github_cockroachdb_drpc-v0.0.0-20250807091527-65dcebaa113e.zip",
1115311153
],
1115411154
)
1115511155
go_repository(

build/bazelutil/distdir_files.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ DISTDIR_FILES = {
351351
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlfmt/com_github_cockroachdb_crlfmt-v0.0.0-20221214225007-b2fc5c302548.zip": "fedc01bdd6d964da0425d5eaac8efadc951e78e13f102292cc0774197f09ab63",
352352
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlib/com_github_cockroachdb_crlib-v0.0.0-20250718215705-7ff5051265b9.zip": "4fb4d32f15fb3c63decb9625406f291874f975518d5937493a7a9d1f3a644f18",
353353
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/datadriven/com_github_cockroachdb_datadriven-v1.0.3-0.20250407164829-2945557346d5.zip": "251593cd9c040fe84a99a3919de7ce6f85030d522159a37d625dc2dea7a4d17f",
354-
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/drpc/com_github_cockroachdb_drpc-v0.0.0-20250618091105-e79a954a2193.zip": "612016b7a145f386a2163d788e3376cb0f63410b8423f2ef3f723f7aa24c1971",
354+
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/drpc/com_github_cockroachdb_drpc-v0.0.0-20250807091527-65dcebaa113e.zip": "ebb4b3ca9a6c5944255506b4cf5091b86b76851e52211940b609c98e8bcc7aab",
355355
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/errors/com_github_cockroachdb_errors-v1.12.0.zip": "f73d8a5f4d8fcbc4ed61db2b47f17e2601d8b32e9a49c0665667489d9d9d6e7c",
356356
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/go-test-teamcity/com_github_cockroachdb_go_test_teamcity-v0.0.0-20191211140407-cff980ad0a55.zip": "bac30148e525b79d004da84d16453ddd2d5cd20528e9187f1d7dac708335674b",
357357
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gogoproto/com_github_cockroachdb_gogoproto-v1.3.3-0.20241216150617-2358cdb156a1.zip": "bf052c9a7f9e23fb3ec7e9f3b7201cfc264c18ed6da0d662952d276dbc339003",

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ replace golang.org/x/time => github.com/cockroachdb/x-time v0.3.1-0.202305251236
524524

525525
replace github.com/gogo/protobuf => github.com/cockroachdb/gogoproto v1.3.3-0.20241216150617-2358cdb156a1
526526

527-
replace storj.io/drpc => github.com/cockroachdb/drpc v0.0.0-20250618091105-e79a954a2193
527+
replace storj.io/drpc => github.com/cockroachdb/drpc v0.0.0-20250807091527-65dcebaa113e
528528

529529
// Note: This forked dependency adds a commit that opens up some
530530
// private APIs to enable us to make some perf improvements to

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,8 @@ github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:z
565565
github.com/cockroachdb/datadriven v1.0.2/go.mod h1:a9RdTaap04u637JoCzcUoIcDmvwSUtcUFtT/C3kJlTU=
566566
github.com/cockroachdb/datadriven v1.0.3-0.20250407164829-2945557346d5 h1:UycK/E0TkisVrQbSoxvU827FwgBBcZ95nRRmpj/12QI=
567567
github.com/cockroachdb/datadriven v1.0.3-0.20250407164829-2945557346d5/go.mod h1:jsaKMvD3RBCATk1/jbUZM8C9idWBJME9+VRZ5+Liq1g=
568-
github.com/cockroachdb/drpc v0.0.0-20250618091105-e79a954a2193 h1:c/lx52g2cnFUVPkHqYX+6Nyh+3L/l5ae0SSrkvrUM4M=
569-
github.com/cockroachdb/drpc v0.0.0-20250618091105-e79a954a2193/go.mod h1:UWP+upGv1Z+4nWxcdwhv3/wQXSOgCZteytaRVez5PDc=
568+
github.com/cockroachdb/drpc v0.0.0-20250807091527-65dcebaa113e h1:Fzex8ZwZ/Exh7ZJf9bTS8BELh93bVFvdzqqKc9jaWUo=
569+
github.com/cockroachdb/drpc v0.0.0-20250807091527-65dcebaa113e/go.mod h1:UWP+upGv1Z+4nWxcdwhv3/wQXSOgCZteytaRVez5PDc=
570570
github.com/cockroachdb/errors v1.9.1/go.mod h1:2sxOtL2WIc096WSZqZ5h8fa17rdDq9HZOZLBCor4mBk=
571571
github.com/cockroachdb/errors v1.12.0 h1:d7oCs6vuIMUQRVbi6jWWWEJZahLCfJpnJSVobd1/sUo=
572572
github.com/cockroachdb/errors v1.12.0/go.mod h1:SvzfYNNBshAVbZ8wzNc/UPK3w1vf0dKDUP41ucAIf7g=

pkg/rpc/BUILD.bazel

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ go_library(
8080
"@io_storj_drpc//:drpc",
8181
"@io_storj_drpc//drpcclient",
8282
"@io_storj_drpc//drpcconn",
83+
"@io_storj_drpc//drpcctx",
8384
"@io_storj_drpc//drpcmanager",
85+
"@io_storj_drpc//drpcmetadata",
8486
"@io_storj_drpc//drpcmigrate",
8587
"@io_storj_drpc//drpcmux",
8688
"@io_storj_drpc//drpcpool",
@@ -177,6 +179,8 @@ go_test(
177179
"@com_github_prometheus_client_model//go",
178180
"@com_github_stretchr_testify//assert",
179181
"@com_github_stretchr_testify//require",
182+
"@io_storj_drpc//drpcctx",
183+
"@io_storj_drpc//drpcmetadata",
180184
"@org_golang_google_grpc//:grpc",
181185
"@org_golang_google_grpc//codes",
182186
"@org_golang_google_grpc//credentials",

pkg/rpc/auth.go

Lines changed: 131 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import (
2323
"google.golang.org/grpc/credentials"
2424
grpcpeer "google.golang.org/grpc/peer"
2525
"google.golang.org/grpc/status"
26+
"storj.io/drpc"
27+
"storj.io/drpc/drpcctx"
28+
"storj.io/drpc/drpcmetadata"
29+
"storj.io/drpc/drpcmux"
2630
)
2731

2832
var errTLSInfoMissing = authError("TLSInfo is not available in request context")
@@ -41,11 +45,14 @@ func authErrorf(format string, a ...interface{}) error {
4145
type kvAuth struct {
4246
sv *settings.Values
4347
tenant tenantAuthorizer
48+
isDRPC bool
4449
}
4550

4651
// kvAuth implements the auth interface.
47-
func (a kvAuth) AuthUnary() grpc.UnaryServerInterceptor { return a.unaryInterceptor }
48-
func (a kvAuth) AuthStream() grpc.StreamServerInterceptor { return a.streamInterceptor }
52+
func (a kvAuth) AuthUnary() grpc.UnaryServerInterceptor { return a.unaryInterceptor }
53+
func (a kvAuth) AuthDRPCUnary() drpcmux.UnaryServerInterceptor { return a.unaryDRPCInterceptor }
54+
func (a kvAuth) AuthDRPCStream() drpcmux.StreamServerInterceptor { return a.streamDRPCInterceptor }
55+
func (a kvAuth) AuthStream() grpc.StreamServerInterceptor { return a.streamInterceptor }
4956

5057
func (a kvAuth) unaryInterceptor(
5158
ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler,
@@ -81,7 +88,41 @@ func (a kvAuth) unaryInterceptor(
8188
return nil, err
8289
}
8390
case authzTenantServerToTenantServer:
84-
// Tenant servers can see all of each other's RPCs.
91+
// Tenant servers can see all of each other's RPCs.
92+
case authzPrivilegedPeerToServer:
93+
// Privileged clients (root/node) can see all RPCs.
94+
default:
95+
return nil, errors.AssertionFailedf("unhandled case: %T", err)
96+
}
97+
return handler(ctx, req)
98+
}
99+
100+
func (a kvAuth) unaryDRPCInterceptor(
101+
ctx context.Context, req interface{}, rpc string, handler drpcmux.UnaryHandler,
102+
) (out interface{}, err error) {
103+
// Perform authentication and authz selection.
104+
authnRes, authz, err := a.authenticateAndSelectAuthzRule(ctx)
105+
if err != nil {
106+
return nil, err
107+
}
108+
109+
// Enhance the context to ensure the API handler only sees a client tenant ID
110+
// via roachpb.ClientTenantFromContext when relevant.
111+
ctx = contextForRequest(ctx, authnRes)
112+
113+
// Handle authorization according to the selected authz method.
114+
switch ar := authz.(type) {
115+
case authzTenantServerToKVServer:
116+
// Clear any leftover incoming DRPC metadata, if this call is
117+
// originating from a RPC handler function called as a result of a
118+
// tenant call. See unaryInterceptor for more details.
119+
ctx = drpcmetadata.ClearContext(ctx)
120+
121+
if err := a.tenant.authorize(ctx, a.sv, roachpb.TenantID(ar), rpc, req); err != nil {
122+
return nil, err
123+
}
124+
case authzTenantServerToTenantServer:
125+
// Tenant servers can see all of each other's RPCs.
85126
case authzPrivilegedPeerToServer:
86127
// Privileged clients (root/node) can see all RPCs.
87128
default:
@@ -143,7 +184,7 @@ func (a kvAuth) streamInterceptor(
143184
},
144185
}
145186
case authzTenantServerToTenantServer:
146-
// Tenant servers can see all of each other's RPCs.
187+
// Tenant servers can see all of each other's RPCs.
147188
case authzPrivilegedPeerToServer:
148189
// Privileged clients (root/node) can see all RPCs.
149190
default:
@@ -152,6 +193,55 @@ func (a kvAuth) streamInterceptor(
152193
return handler(srv, ss)
153194
}
154195

196+
func (a kvAuth) streamDRPCInterceptor(
197+
stream drpc.Stream, rpc string, handler drpcmux.StreamHandler,
198+
) (out interface{}, err error) {
199+
ctx := stream.Context()
200+
// Perform authentication and authz selection.
201+
authnRes, authz, err := a.authenticateAndSelectAuthzRule(ctx)
202+
if err != nil {
203+
return nil, err
204+
}
205+
206+
// Enhance the context to ensure the API handler only sees a client tenant ID
207+
// via roachpb.ClientTenantFromContext when relevant.
208+
ctx = contextForRequest(ctx, authnRes)
209+
210+
// Handle authorization according to the selected authz method.
211+
switch ar := authz.(type) {
212+
case authzTenantServerToKVServer:
213+
// Clear any leftover incoming DRPC metadata, if this call is
214+
// originating from a RPC handler function called as a result of a
215+
// tenant call. See streamInterceptor for more details.
216+
217+
if rpc == "/cockroach.blobs.Blob/PutStream" {
218+
ctx = drpcmetadata.ClearContextExcept(ctx, "filename")
219+
} else {
220+
ctx = drpcmetadata.ClearContext(ctx)
221+
}
222+
223+
originalStream := stream
224+
stream = &wrappedDRPCServerStream{
225+
Stream: originalStream,
226+
ctx: ctx,
227+
recv: func(m drpc.Message, enc drpc.Encoding) error {
228+
if err := originalStream.MsgRecv(m, enc); err != nil {
229+
return err
230+
}
231+
// 'm' is now populated and contains the request from the client.
232+
return a.tenant.authorize(ctx, a.sv, roachpb.TenantID(ar), rpc, m)
233+
},
234+
}
235+
case authzTenantServerToTenantServer:
236+
// Tenant servers can see all of each other's RPCs.
237+
case authzPrivilegedPeerToServer:
238+
// Privileged clients (root/node) can see all RPCs.
239+
default:
240+
return nil, errors.AssertionFailedf("unhandled case: %T", err)
241+
}
242+
return handler(stream)
243+
}
244+
155245
func (a kvAuth) authenticateAndSelectAuthzRule(
156246
ctx context.Context,
157247
) (authnResult, requiredAuthzMethod, error) {
@@ -162,7 +252,7 @@ func (a kvAuth) authenticateAndSelectAuthzRule(
162252
}
163253

164254
// Select authorization rules suitable for the peer.
165-
authz, err := a.selectAuthzMethod(ctx, authnRes)
255+
authz, err := a.selectAuthzMethod(authnRes)
166256
if err != nil {
167257
return nil, nil, err
168258
}
@@ -176,19 +266,30 @@ type authnResult interface {
176266
authnResult()
177267
}
178268

179-
func getClientCert(ctx context.Context) (*x509.Certificate, error) {
180-
p, ok := grpcpeer.FromContext(ctx)
181-
if !ok {
182-
return nil, errTLSInfoMissing
269+
func (a kvAuth) getClientCert(ctx context.Context) (*x509.Certificate, error) {
270+
var certs []*x509.Certificate
271+
if a.isDRPC {
272+
info, ok := drpcctx.GetPeerConnectionInfo(ctx)
273+
if !ok {
274+
return nil, errTLSInfoMissing
275+
}
276+
certs = info.Certificates
277+
} else {
278+
p, ok := grpcpeer.FromContext(ctx)
279+
if !ok {
280+
return nil, errTLSInfoMissing
281+
}
282+
tlsInfo, ok := p.AuthInfo.(credentials.TLSInfo)
283+
if !ok {
284+
return nil, errTLSInfoMissing
285+
}
286+
certs = tlsInfo.State.PeerCertificates
183287
}
184288

185-
tlsInfo, ok := p.AuthInfo.(credentials.TLSInfo)
186-
if !ok || len(tlsInfo.State.PeerCertificates) == 0 {
289+
if len(certs) == 0 {
187290
return nil, errTLSInfoMissing
188291
}
189-
190-
clientCert := tlsInfo.State.PeerCertificates[0]
191-
return clientCert, nil
292+
return certs[0], nil
192293
}
193294

194295
// authnSuccessPeerIsTenantServer indicates authentication has
@@ -247,7 +348,7 @@ func (a kvAuth) authenticateLocalRequest(
247348
) (authnResult, error) {
248349
// Sanity check: verify that we do not also have gRPC network credentials
249350
// in the context. This would indicate that metadata was improperly propagated.
250-
maybeTid, err := tenantIDFromRPCMetadata(ctx)
351+
maybeTid, err := a.tenantIDFromRPCMetadata(ctx)
251352
if err != nil || maybeTid.IsSet() {
252353
logcrash.ReportOrPanic(ctx, a.sv, "programming error: network credentials in internal adapter request (%v, %v)", maybeTid, err)
253354
return nil, authErrorf("programming error")
@@ -268,12 +369,12 @@ func (a kvAuth) authenticateLocalRequest(
268369
func (a kvAuth) authenticateNetworkRequest(ctx context.Context) (authnResult, error) {
269370
// We will need to look at the TLS cert in any case, so extract it
270371
// first.
271-
clientCert, err := getClientCert(ctx)
372+
clientCert, err := a.getClientCert(ctx)
272373
if err != nil {
273374
return nil, err
274375
}
275376

276-
tenantIDFromMetadata, err := tenantIDFromRPCMetadata(ctx)
377+
tenantIDFromMetadata, err := a.tenantIDFromRPCMetadata(ctx)
277378
if err != nil {
278379
return nil, authErrorf("client provided invalid tenant ID: %v", err)
279380
}
@@ -357,9 +458,7 @@ func (authzPrivilegedPeerToServer) rpcAuthzMethod() {}
357458

358459
// selectAuthzMethod selects the authorization rule to use for the
359460
// given authentication event.
360-
func (a kvAuth) selectAuthzMethod(
361-
ctx context.Context, ar authnResult,
362-
) (requiredAuthzMethod, error) {
461+
func (a kvAuth) selectAuthzMethod(ar authnResult) (requiredAuthzMethod, error) {
363462
switch res := ar.(type) {
364463
case authnSuccessPeerIsTenantServer:
365464
// The client is a tenant server. We have two possible cases:
@@ -488,9 +587,20 @@ func newTenantClientCreds(tid roachpb.TenantID) credentials.PerRPCCredentials {
488587
}
489588
}
490589

590+
func newPerRPCTIDMetdata(tid roachpb.TenantID) (string, string) {
591+
return clientTIDMetadataHeaderKey, fmt.Sprint(tid)
592+
}
593+
491594
// tenantIDFromRPCMetadata checks if there is a tenant ID in
492595
// the incoming gRPC metadata.
493-
func tenantIDFromRPCMetadata(ctx context.Context) (roachpb.TenantID, error) {
596+
func (a kvAuth) tenantIDFromRPCMetadata(ctx context.Context) (roachpb.TenantID, error) {
597+
if a.isDRPC {
598+
val, ok := drpcmetadata.GetValue(ctx, clientTIDMetadataHeaderKey)
599+
if !ok {
600+
return roachpb.TenantID{}, nil
601+
}
602+
return tenantIDFromString(val, "drpc metadata")
603+
}
494604
val, ok := grpcutil.FastFirstValueFromIncomingContext(ctx, clientTIDMetadataHeaderKey)
495605
if !ok {
496606
return roachpb.TenantID{}, nil

pkg/rpc/auth_tenant.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/cockroachdb/errors"
2020
"github.com/cockroachdb/logtags"
2121
"google.golang.org/grpc"
22+
"storj.io/drpc"
2223
)
2324

2425
// tenantAuthorizer authorizes RPCs sent by tenants to a node's tenant RPC
@@ -573,3 +574,19 @@ func (ss *wrappedServerStream) Context() context.Context {
573574
func (ss *wrappedServerStream) RecvMsg(m interface{}) error {
574575
return ss.recv(m)
575576
}
577+
578+
type wrappedDRPCServerStream struct {
579+
drpc.Stream
580+
ctx context.Context
581+
recv func(m drpc.Message, enc drpc.Encoding) error
582+
}
583+
584+
// Context overrides the nested stream.Context().
585+
func (s *wrappedDRPCServerStream) Context() context.Context {
586+
return s.ctx
587+
}
588+
589+
// MsgRecv overrides the nested stream.MsgRecv().
590+
func (s *wrappedDRPCServerStream) MsgRecv(m drpc.Message, enc drpc.Encoding) error {
591+
return s.recv(m, enc)
592+
}

0 commit comments

Comments
 (0)