Skip to content

Commit 804fa04

Browse files
craig[bot]souravcrl
andcommitted
Merge #155216
155216: cli, security: disable root user and auth changes for debug zip user r=souravcrl a=souravcrl **cli, security: add --disallow-root-login flag to disable root user** We introduce a `--disallow-root-login` flag which now disables root user from access to both sql and rpc apis. **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. Release note (security update): We will be adding a new flag `--disallow-root-login` to the cockroach start command to explicitly allowcrestricting the root user from logging into the system. This change affects the [unstated, unchangeable root access rule](https://www.cockroachlabs.com/docs/stable/security-reference/authentication#the-unstated-unchangeable-root-access-rule) as part of compliance requirements. This flag is currently experimental and also needs an additional user setup for debug zip collection as disabling the root user affects the debug zip service. We currently do not validate if this user is set up or not. Note: Care must be taken to ensure none of the certificates that are in use by the cluster or the SQL/RPC clients have a root in the SAN fields since the flag will block access to that client. 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. Fixes: #150845, #152817 Epic: CRDB-49035 Co-authored-by: souravcrl <[email protected]>
2 parents 5e92542 + c48340e commit 804fa04

File tree

10 files changed

+268
-29
lines changed

10 files changed

+268
-29
lines changed

pkg/cli/cliflags/flags.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,22 @@ provided for node user if this flag is set.
950950
`,
951951
}
952952

953+
DisallowRootLogin = FlagInfo{
954+
Name: "disallow-root-login",
955+
Description: `
956+
When set, prevents authentication attempts by clients presenting certificates
957+
with "root" as one of the principals (CommonName or SubjectAlternativeName).
958+
This applies to both SQL client connections and RPC connections. Authentication
959+
attempts by root will be rejected with an error.
960+
<PRE>
961+
962+
</PRE>
963+
Note: Please ensure none of the certificates that are in use by the cluster or
964+
the SQL/RPC clients have a root in the SAN fields since the flag will block
965+
access to that client.
966+
`,
967+
}
968+
953969
TLSCipherSuites = FlagInfo{
954970
Name: "tls-cipher-suites",
955971
Description: `

pkg/cli/context.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@ var startCtx struct {
488488
serverRootCertDN string
489489
serverNodeCertDN string
490490
serverTLSCipherSuites []string
491+
disallowRootLogin bool
491492

492493
// The TLS auto-handshake parameters.
493494
initToken string
@@ -543,6 +544,7 @@ func setStartContextDefaults() {
543544
startCtx.serverCertPrincipalMap = nil
544545
startCtx.serverRootCertDN = ""
545546
startCtx.serverNodeCertDN = ""
547+
startCtx.disallowRootLogin = false
546548
startCtx.serverListenAddr = ""
547549
startCtx.unencryptedLocalhostHTTP = false
548550
startCtx.tempDir = ""

pkg/cli/flags.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,15 @@ func init() {
562562
// Node cert distinguished name
563563
cliflagcfg.StringFlag(f, &startCtx.serverNodeCertDN, cliflags.NodeCertDistinguishedName)
564564

565+
// We add the disallow root login flag for disabling the root user from rpc
566+
// and sql access for compliance reasons. We currently mark it as hidden
567+
// since the flag behavior is experimental and subject to change.
568+
//
569+
// NB: a user needs to be configured for collecting debug zips if this
570+
// flag is enabled, which we currently do not validate.
571+
cliflagcfg.BoolFlag(f, &startCtx.disallowRootLogin, cliflags.DisallowRootLogin)
572+
_ = f.MarkHidden(cliflags.DisallowRootLogin.Name)
573+
565574
// TLS Cipher Suites configured
566575
cliflagcfg.StringSliceFlag(f, &startCtx.serverTLSCipherSuites, cliflags.TLSCipherSuites)
567576

@@ -1151,6 +1160,7 @@ func extraServerFlagInit(cmd *cobra.Command) error {
11511160
if err := security.SetNodeSubject(startCtx.serverNodeCertDN); err != nil {
11521161
return err
11531162
}
1163+
security.SetDisallowRootLogin(startCtx.disallowRootLogin)
11541164
// Currently we don't handle the case where we are setting the --insecure flag
11551165
// as well as providing the --tls-cipher-suites, we should probably error out
11561166
// if both are set, issue: #144935.
@@ -1160,6 +1170,7 @@ func extraServerFlagInit(cmd *cobra.Command) error {
11601170
serverCfg.User = username.NodeUserName()
11611171
serverCfg.Insecure = startCtx.serverInsecure
11621172
serverCfg.SSLCertsDir = startCtx.serverSSLCertsDir
1173+
serverCfg.DisallowRootLogin = startCtx.disallowRootLogin
11631174

11641175
fs := cliflagcfg.FlagSetForCmd(cmd)
11651176

pkg/rpc/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ go_test(
148148
"//pkg/security/securityassets",
149149
"//pkg/security/securitytest",
150150
"//pkg/security/username",
151+
"//pkg/server/serverpb",
151152
"//pkg/settings",
152153
"//pkg/settings/cluster",
153154
"//pkg/spanconfig",

pkg/rpc/auth.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,21 +490,36 @@ func (a kvAuth) selectAuthzMethod(ar authnResult) (requiredAuthzMethod, error) {
490490
// checkRootOrNodeInScope checks that the root or node principals are
491491
// present in the cert user scopes.
492492
func checkRootOrNodeInScope(clientCert *x509.Certificate, serverTenantID roachpb.TenantID) error {
493+
rootLoginDisabledValidate := false
494+
debugUserScopedCert := false
495+
493496
containsFn := func(scope security.CertificateUserScope) bool {
494497
// Only consider global scopes or scopes that match this server.
495498
if !(scope.Global || scope.TenantID == serverTenantID) {
496499
return false
497500
}
498501

502+
if security.CheckRootLoginDisallowed() && scope.Username == username.RootUser {
503+
rootLoginDisabledValidate = true
504+
}
505+
499506
// If we get a scope that matches the Node user, immediately return.
500507
if scope.Username == username.NodeUser || scope.Username == username.RootUser {
501508
return true
502509
}
503510

511+
if scope.Username == username.DebugUser {
512+
debugUserScopedCert = true
513+
}
514+
504515
return false
505516
}
506517
ok, err := security.CertificateUserScopeContainsFunc(clientCert, containsFn)
507-
if ok || err != nil {
518+
if ok || err != nil || debugUserScopedCert {
519+
if rootLoginDisabledValidate {
520+
return authError("failed to perform RPC, as root login has been disallowed")
521+
}
522+
508523
return err
509524
}
510525
certUserScope, err := security.GetCertificateUserScope(clientCert)

pkg/rpc/auth_test.go

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/cockroachdb/cockroach/pkg/rpc"
2626
"github.com/cockroachdb/cockroach/pkg/security"
2727
"github.com/cockroachdb/cockroach/pkg/security/username"
28+
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
2829
"github.com/cockroachdb/cockroach/pkg/settings"
2930
"github.com/cockroachdb/cockroach/pkg/spanconfig"
3031
"github.com/cockroachdb/cockroach/pkg/testutils"
@@ -137,13 +138,15 @@ func testAuthenticateTenant(t *testing.T, enableDRPC bool) {
137138
{systemID: stid, ous: append([]string{"foo"}, correctOU...), commonName: "other", expErr: `could not parse tenant ID from Common Name`},
138139
{systemID: stid, ous: nil, commonName: "root"},
139140
{systemID: stid, ous: nil, commonName: "node"},
141+
{systemID: stid, ous: nil, commonName: "debug_user"},
140142
{systemID: stid, ous: nil, commonName: "root", tenantScope: 10,
141143
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\)`},
142144
{systemID: tenTen, ous: correctOU, commonName: "10", expTenID: roachpb.TenantID{}},
143145
{systemID: tenTen, ous: correctOU, commonName: "123", expErr: `client tenant identity \(123\) does not match server`},
144146
{systemID: tenTen, ous: correctOU, commonName: "1", expErr: `invalid tenant ID 1 in Common Name \(CN\)`},
145147
{systemID: tenTen, ous: nil, commonName: "root"},
146148
{systemID: tenTen, ous: nil, commonName: "node"},
149+
{systemID: tenTen, ous: nil, commonName: "debug_user"},
147150

148151
// Passing a client ID in metadata instead of relying only on the TLS cert.
149152
{clientTenantInMD: "invalid", expErr: `could not parse tenant ID from (gRPC|drpc) metadata`},
@@ -168,13 +171,17 @@ func testAuthenticateTenant(t *testing.T, enableDRPC bool) {
168171
// Server is KV node: expect tenant authorization.
169172
{clientTenantInMD: "10",
170173
systemID: stid, ous: nil, commonName: "node", expTenID: tenTen},
174+
{clientTenantInMD: "10",
175+
systemID: stid, ous: nil, commonName: "debug_user", expTenID: tenTen},
171176
// tenant ID present in MD, but not in client cert. However,
172177
// client cert is valid. Use MD tenant ID.
173178
// Server is secondary tenant: do not do additional tenant authorization.
174179
{clientTenantInMD: "10",
175180
systemID: tenTen, ous: nil, commonName: "root", expTenID: roachpb.TenantID{}},
176181
{clientTenantInMD: "10",
177182
systemID: tenTen, ous: nil, commonName: "node", expTenID: roachpb.TenantID{}},
183+
{clientTenantInMD: "10",
184+
systemID: tenTen, ous: nil, commonName: "debug_user", expTenID: roachpb.TenantID{}},
178185
// tenant ID present in MD, but not in client cert. Use MD tenant ID.
179186
// Server tenant ID does not match client tenant ID.
180187
{clientTenantInMD: "123",
@@ -206,8 +213,17 @@ func testAuthenticateTenant(t *testing.T, enableDRPC bool) {
206213
{systemID: stid, ous: nil, commonName: "foo", subjectRequired: true, nodeDNString: "CN=foo"},
207214
{systemID: stid, ous: nil, commonName: "foo", subjectRequired: true,
208215
rootDNString: "CN=foo", nodeDNString: "CN=bar"},
216+
// Test case for disallow root login functionality
217+
{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"},
209219
} {
210220
t.Run(fmt.Sprintf("from %v to %v (md %q)", tc.commonName, tc.systemID, tc.clientTenantInMD), func(t *testing.T) {
221+
// Enable root login blocking for the specific test case
222+
if tc.expErr == "root login has been disallowed" {
223+
security.SetDisallowRootLogin(true)
224+
defer security.SetDisallowRootLogin(false)
225+
}
226+
211227
err := security.SetCertPrincipalMap(strings.Split(tc.certPrincipalMap, ","))
212228
if err != nil {
213229
t.Fatal(err)
@@ -1272,3 +1288,125 @@ func (m mockAuthorizer) IsExemptFromRateLimiting(context.Context, roachpb.Tenant
12721288
}
12731289

12741290
type contextKey struct{}
1291+
1292+
// TestServerpbEndpointAccess ensures root user access to serverpb admin and status endpoints
1293+
// works correctly with authentication (considering disallow-root-login flag) and authorization.
1294+
func TestServerpbEndpointAccess(t *testing.T) {
1295+
defer leaktest.AfterTest(t)()
1296+
1297+
systemTenantID := roachpb.SystemTenantID
1298+
const noError = ""
1299+
1300+
// Helper to create a certificate with a given common name
1301+
createCert := func(t *testing.T, commonName string) *x509.Certificate {
1302+
cert := &x509.Certificate{
1303+
Subject: pkix.Name{
1304+
CommonName: commonName,
1305+
},
1306+
}
1307+
var err error
1308+
cert.RawSubject, err = asn1.Marshal(cert.Subject.ToRDNSequence())
1309+
require.NoError(t, err)
1310+
return cert
1311+
}
1312+
1313+
// Helper to create a context with peer certificate
1314+
createContextWithCert := func(cert *x509.Certificate) context.Context {
1315+
tlsInfo := credentials.TLSInfo{
1316+
State: tls.ConnectionState{
1317+
PeerCertificates: []*x509.Certificate{cert},
1318+
},
1319+
}
1320+
p := peer.Peer{AuthInfo: tlsInfo}
1321+
return peer.NewContext(context.Background(), &p)
1322+
}
1323+
1324+
t.Run("authentication", func(t *testing.T) {
1325+
// Test root user authentication with disallow-root-login flag
1326+
for _, disallowRoot := range []bool{false, true} {
1327+
testName := "disallow_root_login_disabled"
1328+
expectedError := noError
1329+
if disallowRoot {
1330+
testName = "disallow_root_login_enabled"
1331+
expectedError = "failed to perform RPC, as root login has been disallowed"
1332+
}
1333+
1334+
t.Run(testName, func(t *testing.T) {
1335+
if disallowRoot {
1336+
security.SetDisallowRootLogin(true)
1337+
defer security.SetDisallowRootLogin(false)
1338+
}
1339+
1340+
cert := createCert(t, "root")
1341+
ctx := createContextWithCert(cert)
1342+
1343+
sv := &settings.Values{}
1344+
sv.Init(ctx, settings.TestOpaque)
1345+
1346+
// Perform authentication - this is where the root login check happens
1347+
_, err := rpc.TestingAuthenticateTenant(ctx, systemTenantID, sv, false /* enableDRPC */)
1348+
1349+
if expectedError == noError {
1350+
require.NoError(t, err)
1351+
} else {
1352+
require.Error(t, err)
1353+
require.Equal(t, codes.Unauthenticated, status.Code(err))
1354+
require.Regexp(t, expectedError, err)
1355+
}
1356+
})
1357+
}
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+
})
1371+
})
1372+
1373+
t.Run("authorization", func(t *testing.T) {
1374+
// Test root and debug_user authorization to access serverpb endpoints
1375+
endpoints := map[string]interface{}{
1376+
"/cockroach.server.serverpb.Admin/Liveness": &serverpb.LivenessRequest{},
1377+
"/cockroach.server.serverpb.Status/Nodes": &serverpb.NodesRequest{},
1378+
"/cockroach.server.serverpb.Status/TenantRanges": &serverpb.TenantRangesRequest{},
1379+
"/cockroach.server.serverpb.Status/Gossip": &serverpb.GossipRequest{},
1380+
"/cockroach.server.serverpb.Status/Ranges": &serverpb.RangesRequest{},
1381+
"/cockroach.server.serverpb.Status/EngineStats": &serverpb.EngineStatsRequest{},
1382+
}
1383+
1384+
for method, req := range endpoints {
1385+
t.Run(strings.ReplaceAll(method, "/", "_"), func(t *testing.T) {
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+
})
1405+
1406+
require.NoError(t, err)
1407+
})
1408+
}
1409+
})
1410+
}
1411+
})
1412+
}

pkg/security/auth.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ func (c *userCertDistinguishedNameMu) setDNWithString(dnString string) error {
6767

6868
var rootSubjectMu, nodeSubjectMu userCertDistinguishedNameMu
6969

70+
// disallowRootLoginMu controls whether root login should be disallowed.
71+
var disallowRootLoginMu struct {
72+
syncutil.RWMutex
73+
disallowed bool
74+
}
75+
7076
func SetRootSubject(rootDNString string) error {
7177
return rootSubjectMu.setDNWithString(rootDNString)
7278
}
@@ -83,6 +89,20 @@ func UnsetNodeSubject() {
8389
nodeSubjectMu.unsetDN()
8490
}
8591

92+
// SetDisallowRootLogin sets whether root login should be disallowed.
93+
func SetDisallowRootLogin(disallow bool) {
94+
disallowRootLoginMu.Lock()
95+
defer disallowRootLoginMu.Unlock()
96+
disallowRootLoginMu.disallowed = disallow
97+
}
98+
99+
// CheckRootLoginDisallowed returns whether root login is currently disallowed.
100+
func CheckRootLoginDisallowed() bool {
101+
disallowRootLoginMu.RLock()
102+
defer disallowRootLoginMu.RUnlock()
103+
return disallowRootLoginMu.disallowed
104+
}
105+
86106
// CertificateUserScope indicates the scope of a user certificate i.e. which
87107
// tenant the user is allowed to authenticate on. Older client certificates
88108
// without a tenant scope are treated as global certificates which can
@@ -490,6 +510,11 @@ func ValidateUserScope(
490510
return roleSubject.Equal(certSubject)
491511
}
492512
for _, scope := range certUserScope {
513+
// Check if root login is disallowed and certificate contains "root" as a principal
514+
if CheckRootLoginDisallowed() && scope.Username == username.RootUser {
515+
return false
516+
}
517+
493518
if scope.Username == user {
494519
// If username matches, allow authentication to succeed if
495520
// the tenantID is a match or if the certificate scope is global.

0 commit comments

Comments
 (0)