Skip to content

Commit c48340e

Browse files
committed
rpc: enable debugzip user for privileged access
This commit introduces support for the debug_user certificate as a privileged user for RPC authentication, similar to the existing root and node users. The `debug_user` is specifically designed for collecting debug information (debug zip) and requires privileged access to `serverpb` admin and status endpoints. Modified `pkg/rpc/auth.go` to allow `debug_user` scope in `checkRootOrNodeInScope()`, treating it as a privileged user alongside root and node. Enhanced `pkg/rpc/auth_test.go` with comprehensive test cases for `debug_user` authentication and authorization across various scenarios. The `debug_user` is not subject to the `disallow-root-login` flag and should always be allowed for debugging purposes unless it contains root in SAN field. Fixes: #150845 Epic: CRDB-49035 Release note (security update): A new `debug_user` certificate can now be used for privileged RPC access to collect debug information. The debug_user must be created manually using the `CREATE USER` command and can be audited using the `SHOW USERS` command. This user has privileged access to `serverpb` admin and status endpoints required for debug zip collection.
1 parent 9a14037 commit c48340e

File tree

6 files changed

+68
-31
lines changed

6 files changed

+68
-31
lines changed

pkg/cli/flags.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -562,8 +562,9 @@ func init() {
562562
// We add the disallow root login flag for disabling the root user from rpc
563563
// and sql access for compliance reasons. We currently mark it as hidden
564564
// since the flag behavior is experimental and subject to change.
565-
// Additionally, a user needs to be configured for collecting debug zips if
566-
// this flag is enabled, which we currently do not validate.
565+
//
566+
// NB: a user needs to be configured for collecting debug zips if this
567+
// flag is enabled, which we currently do not validate.
567568
cliflagcfg.BoolFlag(f, &startCtx.disallowRootLogin, cliflags.DisallowRootLogin)
568569
_ = f.MarkHidden(cliflags.DisallowRootLogin.Name)
569570

@@ -1156,7 +1157,7 @@ func extraServerFlagInit(cmd *cobra.Command) error {
11561157
if err := security.SetNodeSubject(startCtx.serverNodeCertDN); err != nil {
11571158
return err
11581159
}
1159-
security.EnableDisallowRootLogin(startCtx.disallowRootLogin)
1160+
security.SetDisallowRootLogin(startCtx.disallowRootLogin)
11601161
// Currently we don't handle the case where we are setting the --insecure flag
11611162
// as well as providing the --tls-cipher-suites, we should probably error out
11621163
// if both are set, issue: #144935.

pkg/rpc/auth.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ func (a kvAuth) selectAuthzMethod(ar authnResult) (requiredAuthzMethod, error) {
491491
// present in the cert user scopes.
492492
func checkRootOrNodeInScope(clientCert *x509.Certificate, serverTenantID roachpb.TenantID) error {
493493
rootLoginDisabledValidate := false
494+
debugUserScopedCert := false
494495

495496
containsFn := func(scope security.CertificateUserScope) bool {
496497
// Only consider global scopes or scopes that match this server.
@@ -507,12 +508,16 @@ func checkRootOrNodeInScope(clientCert *x509.Certificate, serverTenantID roachpb
507508
return true
508509
}
509510

511+
if scope.Username == username.DebugUser {
512+
debugUserScopedCert = true
513+
}
514+
510515
return false
511516
}
512517
ok, err := security.CertificateUserScopeContainsFunc(clientCert, containsFn)
513-
if ok || err != nil {
518+
if ok || err != nil || debugUserScopedCert {
514519
if rootLoginDisabledValidate {
515-
return authErrorf("failed to perform RPC, as root login has been disallowed")
520+
return authError("failed to perform RPC, as root login has been disallowed")
516521
}
517522

518523
return err

pkg/rpc/auth_test.go

Lines changed: 47 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,15 @@ func testAuthenticateTenant(t *testing.T, enableDRPC bool) {
138138
{systemID: stid, ous: append([]string{"foo"}, correctOU...), commonName: "other", expErr: `could not parse tenant ID from Common Name`},
139139
{systemID: stid, ous: nil, commonName: "root"},
140140
{systemID: stid, ous: nil, commonName: "node"},
141+
{systemID: stid, ous: nil, commonName: "debug_user"},
141142
{systemID: stid, ous: nil, commonName: "root", tenantScope: 10,
142143
expErr: `need root or node client cert to perform RPCs on this server \(this is tenant system; cert is valid for "root" on tenantID 10\)`},
143144
{systemID: tenTen, ous: correctOU, commonName: "10", expTenID: roachpb.TenantID{}},
144145
{systemID: tenTen, ous: correctOU, commonName: "123", expErr: `client tenant identity \(123\) does not match server`},
145146
{systemID: tenTen, ous: correctOU, commonName: "1", expErr: `invalid tenant ID 1 in Common Name \(CN\)`},
146147
{systemID: tenTen, ous: nil, commonName: "root"},
147148
{systemID: tenTen, ous: nil, commonName: "node"},
149+
{systemID: tenTen, ous: nil, commonName: "debug_user"},
148150

149151
// Passing a client ID in metadata instead of relying only on the TLS cert.
150152
{clientTenantInMD: "invalid", expErr: `could not parse tenant ID from (gRPC|drpc) metadata`},
@@ -169,13 +171,17 @@ func testAuthenticateTenant(t *testing.T, enableDRPC bool) {
169171
// Server is KV node: expect tenant authorization.
170172
{clientTenantInMD: "10",
171173
systemID: stid, ous: nil, commonName: "node", expTenID: tenTen},
174+
{clientTenantInMD: "10",
175+
systemID: stid, ous: nil, commonName: "debug_user", expTenID: tenTen},
172176
// tenant ID present in MD, but not in client cert. However,
173177
// client cert is valid. Use MD tenant ID.
174178
// Server is secondary tenant: do not do additional tenant authorization.
175179
{clientTenantInMD: "10",
176180
systemID: tenTen, ous: nil, commonName: "root", expTenID: roachpb.TenantID{}},
177181
{clientTenantInMD: "10",
178182
systemID: tenTen, ous: nil, commonName: "node", expTenID: roachpb.TenantID{}},
183+
{clientTenantInMD: "10",
184+
systemID: tenTen, ous: nil, commonName: "debug_user", expTenID: roachpb.TenantID{}},
179185
// tenant ID present in MD, but not in client cert. Use MD tenant ID.
180186
// Server tenant ID does not match client tenant ID.
181187
{clientTenantInMD: "123",
@@ -209,12 +215,13 @@ func testAuthenticateTenant(t *testing.T, enableDRPC bool) {
209215
rootDNString: "CN=foo", nodeDNString: "CN=bar"},
210216
// Test case for disallow root login functionality
211217
{systemID: stid, ous: nil, commonName: "root", expErr: "root login has been disallowed"},
218+
{systemID: stid, ous: nil, commonName: "foo", certDNSName: "root", expErr: "root login has been disallowed"},
212219
} {
213220
t.Run(fmt.Sprintf("from %v to %v (md %q)", tc.commonName, tc.systemID, tc.clientTenantInMD), func(t *testing.T) {
214221
// Enable root login blocking for the specific test case
215222
if tc.expErr == "root login has been disallowed" {
216-
security.EnableDisallowRootLogin(true)
217-
defer security.EnableDisallowRootLogin(false)
223+
security.SetDisallowRootLogin(true)
224+
defer security.SetDisallowRootLogin(false)
218225
}
219226

220227
err := security.SetCertPrincipalMap(strings.Split(tc.certPrincipalMap, ","))
@@ -1326,8 +1333,8 @@ func TestServerpbEndpointAccess(t *testing.T) {
13261333

13271334
t.Run(testName, func(t *testing.T) {
13281335
if disallowRoot {
1329-
security.EnableDisallowRootLogin(true)
1330-
defer security.EnableDisallowRootLogin(false)
1336+
security.SetDisallowRootLogin(true)
1337+
defer security.SetDisallowRootLogin(false)
13311338
}
13321339

13331340
cert := createCert(t, "root")
@@ -1348,10 +1355,23 @@ func TestServerpbEndpointAccess(t *testing.T) {
13481355
}
13491356
})
13501357
}
1358+
1359+
// Test debug_user authentication - should always succeed
1360+
t.Run("debug_user_authentication", func(t *testing.T) {
1361+
cert := createCert(t, "debug_user")
1362+
ctx := createContextWithCert(cert)
1363+
1364+
sv := &settings.Values{}
1365+
sv.Init(ctx, settings.TestOpaque)
1366+
1367+
// Perform authentication - debug_user should always be allowed
1368+
_, err := rpc.TestingAuthenticateTenant(ctx, systemTenantID, sv, false /* enableDRPC */)
1369+
require.NoError(t, err)
1370+
})
13511371
})
13521372

13531373
t.Run("authorization", func(t *testing.T) {
1354-
// Test root user authorization to access serverpb endpoints
1374+
// Test root and debug_user authorization to access serverpb endpoints
13551375
endpoints := map[string]interface{}{
13561376
"/cockroach.server.serverpb.Admin/Liveness": &serverpb.LivenessRequest{},
13571377
"/cockroach.server.serverpb.Status/Nodes": &serverpb.NodesRequest{},
@@ -1363,24 +1383,29 @@ func TestServerpbEndpointAccess(t *testing.T) {
13631383

13641384
for method, req := range endpoints {
13651385
t.Run(strings.ReplaceAll(method, "/", "_"), func(t *testing.T) {
1366-
cert := createCert(t, "root")
1367-
ctx := createContextWithCert(cert)
1368-
1369-
sv := &settings.Values{}
1370-
sv.Init(ctx, settings.TestOpaque)
1371-
1372-
// Test authorization with mock authorizer
1373-
err := rpc.TestingAuthorizeTenantRequest(ctx, sv, systemTenantID, method, req, mockAuthorizer{
1374-
hasCrossTenantRead: true,
1375-
hasCapabilityForBatch: true,
1376-
hasNodestatusCapability: true,
1377-
hasTSDBQueryCapability: true,
1378-
hasNodelocalStorageCapability: true,
1379-
hasExemptFromRateLimiterCapability: true,
1380-
hasTSDBAllCapability: true,
1381-
})
1386+
// Test both root and debug_user
1387+
for _, user := range []string{"root", "debug_user"} {
1388+
t.Run(user, func(t *testing.T) {
1389+
cert := createCert(t, user)
1390+
ctx := createContextWithCert(cert)
1391+
1392+
sv := &settings.Values{}
1393+
sv.Init(ctx, settings.TestOpaque)
1394+
1395+
// Test authorization with mock authorizer
1396+
err := rpc.TestingAuthorizeTenantRequest(ctx, sv, systemTenantID, method, req, mockAuthorizer{
1397+
hasCrossTenantRead: true,
1398+
hasCapabilityForBatch: true,
1399+
hasNodestatusCapability: true,
1400+
hasTSDBQueryCapability: true,
1401+
hasNodelocalStorageCapability: true,
1402+
hasExemptFromRateLimiterCapability: true,
1403+
hasTSDBAllCapability: true,
1404+
})
13821405

1383-
require.NoError(t, err)
1406+
require.NoError(t, err)
1407+
})
1408+
}
13841409
})
13851410
}
13861411
})

pkg/security/auth.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ func UnsetNodeSubject() {
8989
nodeSubjectMu.unsetDN()
9090
}
9191

92-
// EnableDisallowRootLogin enables or disables root login blocking.
93-
func EnableDisallowRootLogin(disallow bool) {
92+
// SetDisallowRootLogin sets whether root login should be disallowed.
93+
func SetDisallowRootLogin(disallow bool) {
9494
disallowRootLoginMu.Lock()
9595
defer disallowRootLoginMu.Unlock()
9696
disallowRootLoginMu.disallowed = disallow

pkg/security/auth_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,15 +490,15 @@ func TestAuthenticationHook(t *testing.T) {
490490
defer func() {
491491
security.UnsetRootSubject()
492492
security.UnsetNodeSubject()
493-
security.EnableDisallowRootLogin(false)
493+
security.SetDisallowRootLogin(false)
494494
}()
495495
err := security.SetCertPrincipalMap(strings.Split(tc.principalMap, ","))
496496
if err != nil {
497497
t.Fatal(err)
498498
}
499499

500500
// Set the global flag for disallowing root login
501-
security.EnableDisallowRootLogin(tc.disallowRootLogin)
501+
security.SetDisallowRootLogin(tc.disallowRootLogin)
502502

503503
var roleSubject *ldap.DN
504504
if tc.isSubjectRoleOptionOrDNSet {

pkg/security/username/username.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,12 @@ const TestUser = "testuser"
160160
// TestUserName is the SQLUsername for testuser.
161161
func TestUserName() SQLUsername { return SQLUsername{TestUser} }
162162

163+
// DebugUser is used for collecting debug zip.
164+
const DebugUser = "debug_user"
165+
166+
// DebugUserName is the SQLUsername for debug user.
167+
func DebugUserName() SQLUsername { return SQLUsername{DebugUser} }
168+
163169
// MakeSQLUsernameFromUserInput normalizes a username string as
164170
// entered in an ambiguous context into a SQL username (performs case
165171
// folding and unicode normalization form C - NFC).

0 commit comments

Comments
 (0)