From eb674313fbedd5dab66adff4ba89dcfb330a2e5e Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Wed, 18 Sep 2024 18:18:37 -0400 Subject: [PATCH] sql/pgwire: treat system identity as string, not SQLUsername The external system identity may not be a SQLUsername -- that mapping is performed at a different phase of authentication. To make this more clear, we stop using the SQLUsername type for system identity. Release note: None --- pkg/ccl/gssapiccl/gssapi.go | 25 ++-- pkg/ccl/ldapccl/BUILD.bazel | 1 + pkg/ccl/ldapccl/authentication_ldap.go | 3 +- pkg/security/auth.go | 33 ++--- pkg/security/auth_test.go | 28 ++--- pkg/security/certificate_manager.go | 4 +- pkg/sql/conn_executor.go | 2 +- pkg/sql/exec_util.go | 2 +- pkg/sql/pgwire/BUILD.bazel | 1 + pkg/sql/pgwire/auth.go | 42 +++---- pkg/sql/pgwire/auth_behaviors.go | 14 +-- pkg/sql/pgwire/auth_methods.go | 113 +++++++++++------- pkg/sql/pgwire/authenticator.go | 3 +- pkg/sql/pgwire/authorizer.go | 10 +- pkg/sql/pgwire/authpipe_test.go | 3 +- pkg/sql/pgwire/pre_serve_options.go | 2 +- pkg/sql/pgwire/role_mapper.go | 25 ++-- pkg/sql/pgwire/server.go | 4 +- .../local_only_session_data.proto | 2 +- pkg/sql/sessiondatapb/session_data.go | 4 +- pkg/sql/vars.go | 4 +- 21 files changed, 177 insertions(+), 148 deletions(-) diff --git a/pkg/ccl/gssapiccl/gssapi.go b/pkg/ccl/gssapiccl/gssapi.go index 9ab969282a95..44f4ce10129a 100644 --- a/pkg/ccl/gssapiccl/gssapi.go +++ b/pkg/ccl/gssapiccl/gssapi.go @@ -57,11 +57,7 @@ func authGSS( // Update the incoming connection with the GSS username. We'll expect // to see this value come back to the mapper function below. - if u, err := username.MakeSQLUsernameFromUserInput(gssUser, username.PurposeValidation); err != nil { - return nil, err - } else { - behaviors.SetReplacementIdentity(u) - } + behaviors.SetReplacementIdentity(gssUser) // We enforce that the "map" and/or "include_realm=0" options are set // in the HBA validation function below. @@ -84,7 +80,7 @@ func authGSS( } behaviors.SetAuthenticator(func( - _ context.Context, _ username.SQLUsername, _ bool, _ pgwire.PasswordRetrievalFn, _ *ldap.DN, + _ context.Context, _ string, _ bool, _ pgwire.PasswordRetrievalFn, _ *ldap.DN, ) error { // Enforce krb_realm option, if any. if realms := entry.GetOptions("krb_realm"); len(realms) > 0 { @@ -142,19 +138,16 @@ func checkEntry(_ *settings.Values, entry hba.Entry) error { } // stripRealm removes the realm data, if any, from the provided username. -func stripRealm(u username.SQLUsername) (username.SQLUsername, error) { - norm := u.Normalized() - if idx := strings.Index(norm, "@"); idx != -1 { - norm = norm[:idx] +func stripRealm(u string) (username.SQLUsername, error) { + if idx := strings.Index(u, "@"); idx != -1 { + u = u[:idx] } - return username.MakeSQLUsernameFromUserInput(norm, username.PurposeValidation) + return username.MakeSQLUsernameFromUserInput(u, username.PurposeValidation) } // stripRealmMapper is a pgwire.RoleMapper that just strips the trailing // realm information, if any, from the gssapi username. -func stripRealmMapper( - _ context.Context, systemIdentity username.SQLUsername, -) ([]username.SQLUsername, error) { +func stripRealmMapper(_ context.Context, systemIdentity string) ([]username.SQLUsername, error) { ret, err := stripRealm(systemIdentity) return []username.SQLUsername{ret}, err } @@ -163,12 +156,12 @@ func stripRealmMapper( // the incoming identity has its realm information stripped before the // next mapping is applied. func stripAndDelegateMapper(delegate pgwire.RoleMapper) pgwire.RoleMapper { - return func(ctx context.Context, systemIdentity username.SQLUsername) ([]username.SQLUsername, error) { + return func(ctx context.Context, systemIdentity string) ([]username.SQLUsername, error) { next, err := stripRealm(systemIdentity) if err != nil { return nil, err } - return delegate(ctx, next) + return delegate(ctx, next.Normalized()) } } diff --git a/pkg/ccl/ldapccl/BUILD.bazel b/pkg/ccl/ldapccl/BUILD.bazel index 1d0f456bfc12..ca6cc522db9f 100644 --- a/pkg/ccl/ldapccl/BUILD.bazel +++ b/pkg/ccl/ldapccl/BUILD.bazel @@ -21,6 +21,7 @@ go_library( "//pkg/server/telemetry", "//pkg/settings", "//pkg/settings/cluster", + "//pkg/sql/lexbase", "//pkg/sql/pgwire", "//pkg/sql/pgwire/hba", "//pkg/sql/pgwire/identmap", diff --git a/pkg/ccl/ldapccl/authentication_ldap.go b/pkg/ccl/ldapccl/authentication_ldap.go index a93ecd90f598..3c4c8c2af1f9 100644 --- a/pkg/ccl/ldapccl/authentication_ldap.go +++ b/pkg/ccl/ldapccl/authentication_ldap.go @@ -17,6 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/server/telemetry" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/sql/lexbase" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/hba" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/identmap" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -109,7 +110,7 @@ func (authManager *ldapAuthManager) FetchLDAPUserDN( "cannot find provided user %s on LDAP server", user.Normalized()) } - retrievedUserDN, err = distinguishedname.ParseDN(userDN) + retrievedUserDN, err = distinguishedname.ParseDN(lexbase.NormalizeName(userDN)) if err != nil { return nil, redact.Sprintf("error parsing user DN %s obtained from LDAP server: %v", userDN, err), errors.WithDetailf( diff --git a/pkg/security/auth.go b/pkg/security/auth.go index f3c12a1bf506..c0aefb7f0abd 100644 --- a/pkg/security/auth.go +++ b/pkg/security/auth.go @@ -116,7 +116,7 @@ type CertificateUserScope struct { // that may have been applied to the given connection. type UserAuthHook func( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, clientConnection bool, ) error @@ -125,14 +125,12 @@ type UserAuthHook func( // respectively if systemIdentity conforms to one of these 2 users. It may also // return previously set subject option and systemIdentity is not root or node. // Root and Node roles cannot have subject role option set for them. -func applyRootOrNodeDNFlag( - previouslySetRoleSubject *ldap.DN, systemIdentity username.SQLUsername, -) (dn *ldap.DN) { +func applyRootOrNodeDNFlag(previouslySetRoleSubject *ldap.DN, systemIdentity string) (dn *ldap.DN) { dn = previouslySetRoleSubject switch { - case systemIdentity.IsRootUser(): + case systemIdentity == username.RootUser: dn = rootSubjectMu.getDN() - case systemIdentity.IsNodeUser(): + case systemIdentity == username.NodeUser: dn = nodeSubjectMu.getDN() } return dn @@ -287,13 +285,12 @@ func UserAuthCertHook( } } - return func(ctx context.Context, systemIdentity username.SQLUsername, clientConnection bool) error { - // TODO(marc): we may eventually need stricter user syntax rules. - if systemIdentity.Undefined() { + return func(ctx context.Context, systemIdentity string, clientConnection bool) error { + if systemIdentity == "" { return errors.New("user is missing") } - if !clientConnection && !systemIdentity.IsNodeUser() { + if !clientConnection && systemIdentity != username.NodeUser { return errors.Errorf("user %q is not allowed", systemIdentity) } @@ -315,7 +312,7 @@ func UserAuthCertHook( if subjectRequired && roleSubject == nil { return errors.Newf( "user %q does not have a distinguished name set which subject_required cluster setting mandates", - systemIdentity.Normalized(), + systemIdentity, ) } @@ -327,7 +324,7 @@ func UserAuthCertHook( } } - if ValidateUserScope(certUserScope, systemIdentity.Normalized(), tenantID, roleSubject, certSubject) { + if ValidateUserScope(certUserScope, systemIdentity, tenantID, roleSubject, certSubject) { if certManager != nil { certManager.MaybeUpsertClientExpiration( ctx, @@ -375,8 +372,12 @@ func IsTenantCertificate(cert *x509.Certificate) bool { func UserAuthPasswordHook( insecureMode bool, passwordStr string, hashedPassword password.PasswordHash, gauge *metric.Gauge, ) UserAuthHook { - return func(ctx context.Context, systemIdentity username.SQLUsername, clientConnection bool) error { - if systemIdentity.Undefined() { + return func(ctx context.Context, systemIdentity string, clientConnection bool) error { + u, err := username.MakeSQLUsernameFromUserInput(systemIdentity, username.PurposeValidation) + if err != nil { + return err + } + if u.Undefined() { return errors.New("user is missing") } @@ -390,7 +391,7 @@ func UserAuthPasswordHook( // If the requested user has an empty password, disallow authentication. if len(passwordStr) == 0 { - return NewErrPasswordUserAuthFailed(systemIdentity) + return NewErrPasswordUserAuthFailed(u) } ok, err := password.CompareHashAndCleartextPassword(ctx, hashedPassword, passwordStr, GetExpensiveHashComputeSemWithGauge(ctx, gauge)) @@ -398,7 +399,7 @@ func UserAuthPasswordHook( return err } if !ok { - return NewErrPasswordUserAuthFailed(systemIdentity) + return NewErrPasswordUserAuthFailed(u) } return nil diff --git a/pkg/security/auth_test.go b/pkg/security/auth_test.go index a702e5de9b04..a9ad97b50a42 100644 --- a/pkg/security/auth_test.go +++ b/pkg/security/auth_test.go @@ -375,9 +375,9 @@ func TestAuthenticationHook(t *testing.T) { defer leaktest.AfterTest(t)() defer func() { _ = security.SetCertPrincipalMap(nil) }() - fooUser := username.MakeSQLUsernameFromPreNormalizedString("foo") - barUser := username.MakeSQLUsernameFromPreNormalizedString("bar") - blahUser := username.MakeSQLUsernameFromPreNormalizedString("blah") + fooUser := "foo" + barUser := "bar" + blahUser := "blah" subjectDNString := "O=Cockroach,OU=Order Processing Team,UID=b8b40653-7f74-4f14-8a61-59f7f3b18184,CN=foo" fieldMismatchSubjectDNString := "O=Cockroach,OU=Marketing Team,UID=b8b40653-7f74-4f14-8a61-59f7f3b18184,CN=foo" subsetSubjectDNString := "O=Cockroach,OU=Order Processing Team,CN=foo" @@ -388,7 +388,7 @@ func TestAuthenticationHook(t *testing.T) { testCases := []struct { insecure bool tlsSpec string - username username.SQLUsername + username string distinguishedNameString string principalMap string buildHookSuccess bool @@ -400,21 +400,21 @@ func TestAuthenticationHook(t *testing.T) { expectedErr string }{ // Insecure mode, empty username. - {true, "", username.SQLUsername{}, "", "", true, false, false, roachpb.SystemTenantID, false, false, `user is missing`}, + {true, "", username.EmptyRole, "", "", true, false, false, roachpb.SystemTenantID, false, false, `user is missing`}, // Insecure mode, non-empty username. {true, "", fooUser, "", "", true, true, false, roachpb.SystemTenantID, false, false, `user "foo" is not allowed`}, // Secure mode, no TLS state. - {false, "", username.SQLUsername{}, "", "", false, false, false, roachpb.SystemTenantID, false, false, `no client certificates in request`}, + {false, "", username.EmptyRole, "", "", false, false, false, roachpb.SystemTenantID, false, false, `no client certificates in request`}, // Secure mode, bad user. - {false, "(CN=foo)", username.NodeUserName(), "", "", true, false, false, roachpb.SystemTenantID, + {false, "(CN=foo)", username.NodeUser, "", "", true, false, false, roachpb.SystemTenantID, false, false, `certificate authentication failed for user "node"`}, // Secure mode, node user. - {false, "(CN=node)", username.NodeUserName(), "", "", true, true, true, roachpb.SystemTenantID, false, false, ``}, + {false, "(CN=node)", username.NodeUser, "", "", true, true, true, roachpb.SystemTenantID, false, false, ``}, // Secure mode, node cert and unrelated user. {false, "(CN=node)", fooUser, "", "", true, false, false, roachpb.SystemTenantID, false, false, `certificate authentication failed for user "foo"`}, // Secure mode, root user. - {false, "(CN=root)", username.NodeUserName(), "", "", true, false, false, roachpb.SystemTenantID, + {false, "(CN=root)", username.NodeUser, "", "", true, false, false, roachpb.SystemTenantID, false, false, `certificate authentication failed for user "node"`}, // Secure mode, tenant cert, foo user. {false, "(OU=Tenants,CN=foo)", fooUser, "", "", true, false, false, roachpb.SystemTenantID, @@ -460,12 +460,12 @@ func TestAuthenticationHook(t *testing.T) { // matching) having DNS as foo. {false, "(" + fieldMismatchOnlyOnCommonNameString + ")dns:foo", fooUser, subjectDNString, "", true, false, false, roachpb.MustMakeTenantID(123), true, false, `certificate authentication failed for user "foo"`}, - {false, "(" + rootDNString + ")", username.RootUserName(), rootDNString, "", true, true, false, roachpb.MustMakeTenantID(123), + {false, "(" + rootDNString + ")", username.RootUser, rootDNString, "", true, true, false, roachpb.MustMakeTenantID(123), true, false, `user "root" is not allowed`}, - {false, "(" + nodeDNString + ")", username.NodeUserName(), nodeDNString, "", true, true, true, roachpb.MustMakeTenantID(123), + {false, "(" + nodeDNString + ")", username.NodeUser, nodeDNString, "", true, true, true, roachpb.MustMakeTenantID(123), true, false, ""}, // tls cert dn matching root dn set, where CN != root - {false, "(" + subjectDNString + ")", username.RootUserName(), subjectDNString, "", true, true, false, roachpb.MustMakeTenantID(123), + {false, "(" + subjectDNString + ")", username.RootUser, subjectDNString, "", true, true, false, roachpb.MustMakeTenantID(123), true, false, `user "root" is not allowed`}, {false, "(" + subjectDNString + ")", fooUser, "", "", true, false, false, roachpb.MustMakeTenantID(123), false, true, `user "foo" does not have a distinguished name set which subject_required cluster setting mandates`}, @@ -489,12 +489,12 @@ func TestAuthenticationHook(t *testing.T) { var roleSubject *ldap.DN if tc.isSubjectRoleOptionOrDNSet { switch tc.username { - case username.RootUserName(): + case username.RootUser: err = security.SetRootSubject(tc.distinguishedNameString) if err != nil { t.Fatalf("could not set root subject DN, err: %v", err) } - case username.NodeUserName(): + case username.NodeUser: err = security.SetNodeSubject(tc.distinguishedNameString) if err != nil { t.Fatalf("could not set node subject DN, err: %v", err) diff --git a/pkg/security/certificate_manager.go b/pkg/security/certificate_manager.go index 21ee3d77ef75..dc9f94565f97 100644 --- a/pkg/security/certificate_manager.go +++ b/pkg/security/certificate_manager.go @@ -207,11 +207,11 @@ func (cm *CertificateManager) RegisterExpirationCache(cache *ClientCertExpiratio // given client certificate. An update is contingent on whether the old // expiration is after the new expiration. func (cm *CertificateManager) MaybeUpsertClientExpiration( - ctx context.Context, identity username.SQLUsername, expiration int64, + ctx context.Context, identity string, expiration int64, ) { if cache := cm.clientCertExpirationCache; cache != nil { cache.MaybeUpsert(ctx, - identity.Normalized(), + identity, expiration, cm.certMetrics.ClientExpiration, cm.certMetrics.ClientTTL, diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go index a525a74e3df2..fd97d4e30159 100644 --- a/pkg/sql/conn_executor.go +++ b/pkg/sql/conn_executor.go @@ -983,7 +983,7 @@ func newSessionData(args SessionArgs) *sessiondata.SessionData { LocalOnlySessionData: sessiondatapb.LocalOnlySessionData{ ResultsBufferSize: args.ConnResultsBufferSize, IsSuperuser: args.IsSuperuser, - SystemIdentityProto: args.SystemIdentity.EncodeProto(), + SystemIdentityProto: args.SystemIdentity, }, } if len(args.CustomOptionSessionDefaults) > 0 { diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index a1b90e201594..4e7dc998d8c7 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -2266,7 +2266,7 @@ type SessionArgs struct { IsSuperuser bool IsSSL bool ReplicationMode sessiondatapb.ReplicationMode - SystemIdentity username.SQLUsername + SystemIdentity string SessionDefaults SessionDefaults CustomOptionSessionDefaults SessionDefaults // RemoteAddr is the client's address. This is nil iff this is an internal diff --git a/pkg/sql/pgwire/BUILD.bazel b/pkg/sql/pgwire/BUILD.bazel index 18fc37daed22..0b7c9064ee24 100644 --- a/pkg/sql/pgwire/BUILD.bazel +++ b/pkg/sql/pgwire/BUILD.bazel @@ -42,6 +42,7 @@ go_library( "//pkg/sql/catalog/colinfo", "//pkg/sql/clusterunique", "//pkg/sql/lex", + "//pkg/sql/lexbase", "//pkg/sql/parser", "//pkg/sql/parser/statements", "//pkg/sql/pgrepl/pgreplparser", diff --git a/pkg/sql/pgwire/auth.go b/pkg/sql/pgwire/auth.go index c5a2e5f7a99c..884dec6bfecd 100644 --- a/pkg/sql/pgwire/auth.go +++ b/pkg/sql/pgwire/auth.go @@ -21,6 +21,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql" + "github.com/cockroachdb/cockroach/pkg/sql/lexbase" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/hba" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/identmap" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -120,18 +121,19 @@ func (c *conn) handleAuthentication( } // Choose the system identity that we'll use below for mapping - // externally-provisioned principals to database users. - var systemIdentity username.SQLUsername + // externally-provisioned principals to database users. The system identity + // always is normalized to lower case. + var systemIdentity string if found, ok := behaviors.ReplacementIdentity(); ok { - systemIdentity = found + systemIdentity = lexbase.NormalizeName(found) ac.SetSystemIdentity(systemIdentity) } else { - if !c.sessionArgs.SystemIdentity.Undefined() { + if c.sessionArgs.SystemIdentity != "" { // This case is used in tests, which pass a system_identity // option directly. - systemIdentity = c.sessionArgs.SystemIdentity + systemIdentity = lexbase.NormalizeName(c.sessionArgs.SystemIdentity) } else { - systemIdentity = c.sessionArgs.User + systemIdentity = c.sessionArgs.User.Normalized() } } c.sessionArgs.SystemIdentity = systemIdentity @@ -179,10 +181,9 @@ func (c *conn) handleAuthentication( // At this point, we know that the requested user exists and is allowed to log // in. Now we can delegate to the selected AuthMethod implementation to - // complete the authentication. - // TODO(souravcrl): Verify whether to use systemIdentity or dbUser here since - // systemIdentity refers to external system name, which is same as dbUser name - // incase ReplacementIdentity is not set. + // complete the authentication. We must pass in the systemIdentity here, + // since the authenticator may use an external source to verify the + // user and its credentials. if err := behaviors.Authenticate(ctx, systemIdentity, true /* public */, pwRetrievalFn, roleSubject); err != nil { ac.LogAuthFailed(ctx, eventpb.AuthFailReason_UNKNOWN, err) if pErr := (*security.PasswordUserAuthError)(nil); errors.As(err, &pErr) { @@ -195,7 +196,8 @@ func (c *conn) handleAuthentication( // Since authentication was successful for the user session, we try to perform // additional authorization for the user. This delegates to the selected - // AuthMethod implementation to complete the authorization. + // AuthMethod implementation to complete the authorization. Only certain + // methods, like LDAP, will actually perform authorization here. if err := behaviors.MaybeAuthorize(ctx, systemIdentity, true /* public */); err != nil { ac.LogAuthFailed(ctx, eventpb.AuthFailReason_UNKNOWN, err) err = pgerror.WithCandidateCode(err, pgcode.InvalidAuthorizationSpecification) @@ -262,14 +264,14 @@ func (c *conn) authOKMessage() error { // from the external authentication system with the database user name // that the user has requested to connect as." func (c *conn) checkClientUsernameMatchesMapping( - ctx context.Context, ac AuthConn, mapper RoleMapper, systemIdentity username.SQLUsername, + ctx context.Context, ac AuthConn, mapper RoleMapper, systemIdentity string, ) error { mapped, err := mapper(ctx, systemIdentity) if err != nil { return err } if len(mapped) == 0 { - return errors.Newf("system identity %q did not map to a database role", systemIdentity.Normalized()) + return errors.Newf("system identity %q did not map to a database role", systemIdentity) } for _, m := range mapped { if m == c.sessionArgs.User { @@ -278,7 +280,7 @@ func (c *conn) checkClientUsernameMatchesMapping( } } return errors.Newf("requested user identity %q does not correspond to any mapping for system identity %q", - c.sessionArgs.User, systemIdentity.Normalized()) + c.sessionArgs.User, systemIdentity) } func (c *conn) findAuthenticationMethod( @@ -423,7 +425,7 @@ type AuthConn interface { // SetSystemIdentity updates the AuthConn with an externally-defined // identity for the connection. This is useful for "ambient" // authentication mechanisms, such as GSSAPI. - SetSystemIdentity(systemIdentity username.SQLUsername) + SetSystemIdentity(systemIdentity string) // LogAuthInfof logs details about the progress of the // authentication. LogAuthInfof(ctx context.Context, msg redact.RedactableString) @@ -463,15 +465,13 @@ type authRes struct { err error } -func newAuthPipe( - c *conn, logAuthn bool, authOpt authOptions, systemIdentity username.SQLUsername, -) *authPipe { +func newAuthPipe(c *conn, logAuthn bool, authOpt authOptions, systemIdentity string) *authPipe { ap := &authPipe{ c: c, log: logAuthn, connDetails: authOpt.connDetails, authDetails: eventpb.CommonSessionDetails{ - SystemIdentity: systemIdentity.Normalized(), + SystemIdentity: systemIdentity, Transport: authOpt.connType.String(), }, ch: make(chan []byte), @@ -529,8 +529,8 @@ func (p *authPipe) SetDbUser(dbUser username.SQLUsername) { p.authDetails.User = dbUser.Normalized() } -func (p *authPipe) SetSystemIdentity(systemIdentity username.SQLUsername) { - p.authDetails.SystemIdentity = systemIdentity.Normalized() +func (p *authPipe) SetSystemIdentity(systemIdentity string) { + p.authDetails.SystemIdentity = systemIdentity } func (p *authPipe) LogAuthOK(ctx context.Context) { diff --git a/pkg/sql/pgwire/auth_behaviors.go b/pkg/sql/pgwire/auth_behaviors.go index ad46a1390be1..246ecfc72d6f 100644 --- a/pkg/sql/pgwire/auth_behaviors.go +++ b/pkg/sql/pgwire/auth_behaviors.go @@ -27,7 +27,7 @@ import ( type AuthBehaviors struct { authenticator Authenticator connClose func() - replacementIdentity username.SQLUsername + replacementIdentity string replacedIdentity bool roleMapper RoleMapper authorizer Authorizer @@ -51,7 +51,7 @@ var _ = (*AuthBehaviors)(nil).SetReplacementIdentity // returns an error if SetAuthenticator has not been called. func (b *AuthBehaviors) Authenticate( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, clientConnection bool, pwRetrieveFn PasswordRetrievalFn, roleSubject *ldap.DN, @@ -86,13 +86,13 @@ func (b *AuthBehaviors) SetConnClose(fn func()) { // This allows "ambient" authentication mechanisms, such as GSSAPI, to // provide replacement values. This method will return ok==false if // SetReplacementIdentity has not been called. -func (b *AuthBehaviors) ReplacementIdentity() (_ username.SQLUsername, ok bool) { +func (b *AuthBehaviors) ReplacementIdentity() (_ string, ok bool) { return b.replacementIdentity, b.replacedIdentity } // SetReplacementIdentity allows the AuthMethod to override the // client-reported system identity. -func (b *AuthBehaviors) SetReplacementIdentity(id username.SQLUsername) { +func (b *AuthBehaviors) SetReplacementIdentity(id string) { b.replacementIdentity = id b.replacedIdentity = true } @@ -100,7 +100,7 @@ func (b *AuthBehaviors) SetReplacementIdentity(id username.SQLUsername) { // MapRole delegates to the RoleMapper passed to SetRoleMapper or // returns an error if SetRoleMapper has not been called. func (b *AuthBehaviors) MapRole( - ctx context.Context, systemIdentity username.SQLUsername, + ctx context.Context, systemIdentity string, ) ([]username.SQLUsername, error) { if found := b.roleMapper; found != nil { return found(ctx, systemIdentity) @@ -121,7 +121,7 @@ func (b *AuthBehaviors) SetAuthorizer(a Authorizer) { // MaybeAuthorize delegates to the Authorizer passed to SetAuthorizer and if // successful obtains the grants using RoleGranter passed to SetRoleGranter. func (b *AuthBehaviors) MaybeAuthorize( - ctx context.Context, systemIdentity username.SQLUsername, clientConnection bool, + ctx context.Context, systemIdentity string, clientConnection bool, ) error { if b.authorizer != nil { sqlGroups, err := b.authorizer(ctx, systemIdentity, clientConnection) @@ -136,7 +136,7 @@ func (b *AuthBehaviors) MaybeAuthorize( // GrantRole delegates to the RoleGranter passed to SetRoleGranter or // returns an error if SetRoleGranter has not been called. func (b *AuthBehaviors) GrantRole( - ctx context.Context, systemIdentity username.SQLUsername, sqlGroups []username.SQLUsername, + ctx context.Context, systemIdentity string, sqlGroups []username.SQLUsername, ) error { if found := b.roleGranter; found != nil { return found(ctx, systemIdentity, sqlGroups) diff --git a/pkg/sql/pgwire/auth_methods.go b/pkg/sql/pgwire/auth_methods.go index 8b6726faa107..9ca91c5d310f 100644 --- a/pkg/sql/pgwire/auth_methods.go +++ b/pkg/sql/pgwire/auth_methods.go @@ -25,6 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql" + "github.com/cockroachdb/cockroach/pkg/sql/lexbase" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/hba" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/identmap" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirebase" @@ -123,7 +124,7 @@ var _ AuthMethod = authLDAP func authPassword( _ context.Context, c AuthConn, - _ username.SQLUsername, + user username.SQLUsername, _ tls.ConnectionState, execCfg *sql.ExecutorConfig, _ *hba.Entry, @@ -133,12 +134,12 @@ func authPassword( b.SetRoleMapper(UseProvidedIdentity) b.SetAuthenticator(func( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, clientConnection bool, pwRetrieveFn PasswordRetrievalFn, _ *ldap.DN, ) error { - return passwordAuthenticator(ctx, systemIdentity, clientConnection, pwRetrieveFn, c, execCfg) + return passwordAuthenticator(ctx, user, clientConnection, pwRetrieveFn, c, execCfg) }) return b, nil } @@ -149,7 +150,7 @@ var errExpiredPassword = errors.New("password is expired") // behavior constructed by authPassword(). func passwordAuthenticator( ctx context.Context, - systemIdentity username.SQLUsername, + user username.SQLUsername, clientConnection bool, pwRetrieveFn PasswordRetrievalFn, c AuthConn, @@ -208,7 +209,7 @@ func passwordAuthenticator( // Now check the cleartext password against the retrieved credentials. if err := security.UserAuthPasswordHook( false, passwordStr, hashedPassword, metrics.ConnsWaitingToHash, - )(ctx, systemIdentity, clientConnection); err != nil { + )(ctx, user.Normalized(), clientConnection); err != nil { if errors.HasType(err, &security.PasswordUserAuthError{}) { c.LogAuthFailed(ctx, eventpb.AuthFailReason_CREDENTIALS_INVALID, err) } @@ -225,7 +226,7 @@ func passwordAuthenticator( // makes it easy to rollback from SCRAM-SHA-256 if there are issues. sql.MaybeConvertStoredPasswordHash(ctx, execCfg, - systemIdentity, + user, passwordStr, hashedPassword) return nil @@ -248,7 +249,7 @@ func passwordString(pwdData []byte) (string, error) { func authScram( ctx context.Context, c AuthConn, - _ username.SQLUsername, + user username.SQLUsername, _ tls.ConnectionState, execCfg *sql.ExecutorConfig, _ *hba.Entry, @@ -258,12 +259,12 @@ func authScram( b.SetRoleMapper(UseProvidedIdentity) b.SetAuthenticator(func( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, clientConnection bool, pwRetrieveFn PasswordRetrievalFn, _ *ldap.DN, ) error { - return scramAuthenticator(ctx, systemIdentity, clientConnection, pwRetrieveFn, c, execCfg) + return scramAuthenticator(ctx, user, clientConnection, pwRetrieveFn, c, execCfg) }) return b, nil } @@ -437,7 +438,7 @@ func authCert( b.SetRoleMapper(HbaMapper(hbaEntry, identMap)) b.SetAuthenticator(func( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, clientConnection bool, pwRetrieveFn PasswordRetrievalFn, roleSubject *ldap.DN, @@ -470,11 +471,9 @@ func authCert( }) if len(tlsState.PeerCertificates) > 0 && hbaEntry.GetOption("map") != "" { // The common name in the certificate is set as the system identity in case we have an HBAEntry for db user. - commonName, err := username.MakeSQLUsernameFromUserInput(tlsState.PeerCertificates[0].Subject.CommonName, username.PurposeValidation) - if err != nil { - return nil, err - } - b.SetReplacementIdentity(commonName) + b.SetReplacementIdentity( + lexbase.NormalizeName(tlsState.PeerCertificates[0].Subject.CommonName), + ) } return b, nil } @@ -537,7 +536,7 @@ var AutoSelectPasswordAuth = settings.RegisterBoolSetting( func authAutoSelectPasswordProtocol( _ context.Context, c AuthConn, - _ username.SQLUsername, + user username.SQLUsername, _ tls.ConnectionState, execCfg *sql.ExecutorConfig, _ *hba.Entry, @@ -547,7 +546,7 @@ func authAutoSelectPasswordProtocol( b.SetRoleMapper(UseProvidedIdentity) b.SetAuthenticator(func( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, clientConnection bool, pwRetrieveFn PasswordRetrievalFn, _ *ldap.DN, @@ -567,7 +566,7 @@ func authAutoSelectPasswordProtocol( if pwRetrieveErr == nil && hashedPassword.Method() == password.HashBCrypt { // Yes: we have no choice but to request a cleartext password. c.LogAuthInfof(ctx, "found stored crdb-bcrypt credentials; requesting cleartext password") - return passwordAuthenticator(ctx, systemIdentity, clientConnection, newpwfn, c, execCfg) + return passwordAuthenticator(ctx, user, clientConnection, newpwfn, c, execCfg) } if pwRetrieveErr == nil && hashedPassword.Method() == password.HashSCRAMSHA256 { @@ -580,7 +579,7 @@ func authAutoSelectPasswordProtocol( // If the cluster is configured to automatically downgrade from SCRAM to // bcrypt, then we also request the cleartext password. c.LogAuthInfof(ctx, "found stored SCRAM-SHA-256 credentials but cluster is configured to downgrade to bcrypt; requesting cleartext password") - return passwordAuthenticator(ctx, systemIdentity, clientConnection, newpwfn, c, execCfg) + return passwordAuthenticator(ctx, user, clientConnection, newpwfn, c, execCfg) } if autoRehashOnCostChangeBool && configuredHashMethod == password.HashSCRAMSHA256 { @@ -593,7 +592,7 @@ func authAutoSelectPasswordProtocol( // password when the default cost is changed, then we also request the // cleartext password. c.LogAuthInfof(ctx, "found stored SCRAM-SHA-256 credentials but cluster is configured to re-hash after SCRAM cost change; requesting cleartext password") - return passwordAuthenticator(ctx, systemIdentity, clientConnection, newpwfn, c, execCfg) + return passwordAuthenticator(ctx, user, clientConnection, newpwfn, c, execCfg) } } } @@ -606,7 +605,7 @@ func authAutoSelectPasswordProtocol( // error, we don't want the fallback to force the client to // transmit a password in clear. c.LogAuthInfof(ctx, "no crdb-bcrypt credentials found; proceeding with SCRAM-SHA-256") - return scramAuthenticator(ctx, systemIdentity, clientConnection, newpwfn, c, execCfg) + return scramAuthenticator(ctx, user, clientConnection, newpwfn, c, execCfg) }) return b, nil } @@ -648,7 +647,7 @@ func authTrust( ) (*AuthBehaviors, error) { b := &AuthBehaviors{} b.SetRoleMapper(UseProvidedIdentity) - b.SetAuthenticator(func(_ context.Context, _ username.SQLUsername, _ bool, _ PasswordRetrievalFn, _ *ldap.DN) error { + b.SetAuthenticator(func(_ context.Context, _ string, _ bool, _ PasswordRetrievalFn, _ *ldap.DN) error { return nil }) return b, nil @@ -667,7 +666,9 @@ func authReject( ) (*AuthBehaviors, error) { b := &AuthBehaviors{} b.SetRoleMapper(UseProvidedIdentity) - b.SetAuthenticator(func(ctx context.Context, _ username.SQLUsername, _ bool, _ PasswordRetrievalFn, _ *ldap.DN) error { + b.SetAuthenticator(func( + ctx context.Context, _ string, _ bool, _ PasswordRetrievalFn, _ *ldap.DN, + ) error { err := errors.New("authentication rejected by configuration") c.LogAuthFailed(ctx, eventpb.AuthFailReason_LOGIN_DISABLED, err) return err @@ -691,7 +692,7 @@ func authSessionRevivalToken(token []byte) AuthMethod { return func( _ context.Context, c AuthConn, - _ username.SQLUsername, + user username.SQLUsername, _ tls.ConnectionState, execCfg *sql.ExecutorConfig, _ *hba.Entry, @@ -699,7 +700,9 @@ func authSessionRevivalToken(token []byte) AuthMethod { ) (*AuthBehaviors, error) { b := &AuthBehaviors{} b.SetRoleMapper(UseProvidedIdentity) - b.SetAuthenticator(func(ctx context.Context, user username.SQLUsername, _ bool, _ PasswordRetrievalFn, _ *ldap.DN) error { + b.SetAuthenticator(func( + ctx context.Context, systemIdentity string, _ bool, _ PasswordRetrievalFn, _ *ldap.DN, + ) error { c.LogAuthInfof(ctx, "session revival token detected; attempting to use it") if !sql.AllowSessionRevival.Get(&execCfg.Settings.SV) || execCfg.Codec.ForSystemTenant() { return errors.New("session revival tokens are not supported on this cluster") @@ -760,7 +763,7 @@ var ConfigureJWTAuth = func( func authJwtToken( sctx context.Context, c AuthConn, - _ username.SQLUsername, + user username.SQLUsername, _ tls.ConnectionState, execCfg *sql.ExecutorConfig, _ *hba.Entry, @@ -772,7 +775,9 @@ func authJwtToken( } b := &AuthBehaviors{} b.SetRoleMapper(UseProvidedIdentity) - b.SetAuthenticator(func(ctx context.Context, user username.SQLUsername, clientConnection bool, pwRetrieveFn PasswordRetrievalFn, _ *ldap.DN) error { + b.SetAuthenticator(func( + ctx context.Context, systemIdentity string, clientConnection bool, pwRetrieveFn PasswordRetrievalFn, _ *ldap.DN, + ) error { c.LogAuthInfof(ctx, "JWT token detected; attempting to use it") if !clientConnection { err := errors.New("JWT token authentication is only available for client connections") @@ -915,7 +920,7 @@ func authLDAP( } }) b := &AuthBehaviors{} - b.SetRoleMapper(UseProvidedIdentity) + b.SetRoleMapper(UseSpecifiedIdentity(sessionUser)) ldapUserDN, detailedErrors, authError := ldapManager.m.FetchLDAPUserDN(sCtx, execCfg.Settings, sessionUser, entry, identMap) if authError != nil { @@ -928,15 +933,21 @@ func authLDAP( } else { // The DN of user from LDAP server is set as the system identity DN which // can then be used for authenticator & authorizer AuthBehaviors fn. - externalUserDN, err := username.MakeSQLUsernameFromUserInput(ldapUserDN.String(), username.PurposeValidation) - if err != nil { - log.Warningf(sCtx, "cannot create sql user for retrieved DN from LDAP server: %+v", err) - } - c.SetSystemIdentity(externalUserDN) + b.SetReplacementIdentity(ldapUserDN.String()) } - b.SetAuthenticator(func(ctx context.Context, user username.SQLUsername, clientConnection bool, _ PasswordRetrievalFn, _ *ldap.DN) error { + b.SetAuthenticator(func( + ctx context.Context, systemIdentity string, clientConnection bool, _ PasswordRetrievalFn, _ *ldap.DN, + ) error { c.LogAuthInfof(ctx, "LDAP password provided; attempting to bind to domain") + + // Verify that the systemIdentity is what we expect. + if ldapUserDN.String() != systemIdentity { + err := errors.Newf("LDAP user DN mismatch, expected user DN: %s, obtained systemIdentity: %s", ldapUserDN.String(), systemIdentity) + c.LogAuthFailed(ctx, eventpb.AuthFailReason_PRE_HOOK_ERROR, err) + return err + } + if !clientConnection { err := errors.New("LDAP authentication is only available for client connections") c.LogAuthFailed(ctx, eventpb.AuthFailReason_PRE_HOOK_ERROR, err) @@ -963,9 +974,11 @@ func authLDAP( // If there is no ldap pwd, send the Password Auth Failed error to make the // client prompt for a password. if len(ldapPwd) == 0 { - return security.NewErrPasswordUserAuthFailed(user) + return security.NewErrPasswordUserAuthFailed(sessionUser) } - if detailedErrors, authError := ldapManager.m.ValidateLDAPLogin(ctx, execCfg.Settings, ldapUserDN, user, ldapPwd, entry, identMap); authError != nil { + if detailedErrors, authError := ldapManager.m.ValidateLDAPLogin( + ctx, execCfg.Settings, ldapUserDN, sessionUser, ldapPwd, entry, identMap, + ); authError != nil { errForLog := authError if detailedErrors != "" { errForLog = errors.Join(errForLog, errors.Newf("%s", detailedErrors)) @@ -976,29 +989,41 @@ func authLDAP( return nil }) - b.SetAuthorizer(func(ctx context.Context, user username.SQLUsername, clientConnection bool) ([]username.SQLUsername, error) { + b.SetAuthorizer(func(ctx context.Context, systemIdentity string, clientConnection bool) ([]username.SQLUsername, error) { c.LogAuthInfof(ctx, "LDAP authentication succeeded; attempting authorization") - if ldapGroups, detailedErrors, authError := ldapManager.m.FetchLDAPGroups(ctx, execCfg.Settings, ldapUserDN, user, entry, identMap); authError != nil { - errForLog := authError + + // Verify that the systemIdentity is what we expect. + if ldapUserDN.String() != systemIdentity { + return nil, errors.Newf("LDAP user DN mismatch, expected user DN: %s, obtained systemIdentity: %s", ldapUserDN.String(), systemIdentity) + } + + if ldapGroups, detailedErrors, authError := ldapManager.m.FetchLDAPGroups( + ctx, execCfg.Settings, ldapUserDN, sessionUser, entry, identMap, + ); authError != nil { + errForLog := errors.Wrapf(authError, "LDAP authorization: error retrieving ldap groups for authorization") if detailedErrors != "" { errForLog = errors.Join(errForLog, errors.Newf("%s", detailedErrors)) } - log.Warningf(ctx, "LDAP authorization: error retrieving ldap groups for authorization: %+v", errForLog) + c.LogAuthFailed(ctx, eventpb.AuthFailReason_USER_RETRIEVAL_ERROR, errForLog) return nil, authError } else { - log.Infof(ctx, "LDAP authorization sync succeeded; attempting to assign roles") - // parse and apply transformation to LDAP group DNs for roles granter. + c.LogAuthInfof(ctx, "LDAP authorization sync succeeded; attempting to assign roles") + // Parse and apply transformation to LDAP group DNs for roles granter. sqlGroupRoles := make([]username.SQLUsername, len(ldapGroups)) for idx := range ldapGroups { var err error - if sqlGroupRoles[idx], err = username.MakeSQLUsernameFromUserInput(ldapGroups[idx].String(), username.PurposeValidation); err != nil { + // TODO: instead of using the DN as the SQLUsername, we should extract + // the CN and use that as the SQLUsername + if sqlGroupRoles[idx], err = username.MakeSQLUsernameFromUserInput( + ldapGroups[idx].String(), username.PurposeValidation, + ); err != nil { return nil, errors.Wrapf(err, "LDAP authorization: error creating group role for DN %s", ldapGroups[idx].String()) } } return sqlGroupRoles, nil } }) - b.SetRoleGranter(func(ctx context.Context, user username.SQLUsername, sqlGroups []username.SQLUsername) error { + b.SetRoleGranter(func(ctx context.Context, systemIdentity string, sqlGroups []username.SQLUsername) error { return nil }) diff --git a/pkg/sql/pgwire/authenticator.go b/pkg/sql/pgwire/authenticator.go index 631043846e64..189f5bcbb439 100644 --- a/pkg/sql/pgwire/authenticator.go +++ b/pkg/sql/pgwire/authenticator.go @@ -14,7 +14,6 @@ import ( "context" "github.com/cockroachdb/cockroach/pkg/security/password" - "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/go-ldap/ldap/v3" ) @@ -23,7 +22,7 @@ import ( // username, etc) is who it claims to be. type Authenticator = func( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, clientConnection bool, pwRetrieveFn PasswordRetrievalFn, roleSubject *ldap.DN, diff --git a/pkg/sql/pgwire/authorizer.go b/pkg/sql/pgwire/authorizer.go index b32e19c48036..3acc19b635c5 100644 --- a/pkg/sql/pgwire/authorizer.go +++ b/pkg/sql/pgwire/authorizer.go @@ -21,12 +21,12 @@ import ( // synchronize this information from some external authorization system (e.g.: // LDAP groups, JWT claims or X.509 SAN or other fields, etc). It returns list // of system identities which map to roles created specifically to assign the -// privileges to the session and could be either the subject distinguished names -// defined in role options or the role itself. Authorizer is intended to be used -// with GrantRolesFn which assigns it to intended groups(roles). +// privileges to the session and must be valid SQL users/roles. Authorizer is +// intended to be used with GrantRolesFn which assigns it to intended +// groups(roles). type Authorizer = func( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, clientConnection bool, ) ([]username.SQLUsername, error) @@ -40,6 +40,6 @@ type Authorizer = func( // (*AuthBehaviors).MaybeAuthorize(). type RoleGranter = func( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, sqlGroups []username.SQLUsername, ) error diff --git a/pkg/sql/pgwire/authpipe_test.go b/pkg/sql/pgwire/authpipe_test.go index 43d69fe40ca4..4edbeedc92b6 100644 --- a/pkg/sql/pgwire/authpipe_test.go +++ b/pkg/sql/pgwire/authpipe_test.go @@ -13,7 +13,6 @@ package pgwire import ( "testing" - "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/hba" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" @@ -27,7 +26,7 @@ func createAuthPipe() *authPipe { authOptions{ connType: hba.ConnLocal, }, - username.SQLUsername{}, + "", ) } diff --git a/pkg/sql/pgwire/pre_serve_options.go b/pkg/sql/pgwire/pre_serve_options.go index 30cb294e037a..963470062cfc 100644 --- a/pkg/sql/pgwire/pre_serve_options.go +++ b/pkg/sql/pgwire/pre_serve_options.go @@ -180,7 +180,7 @@ func parseClientProvidedSessionParameters( return args, pgerror.Newf(pgcode.InvalidParameterValue, "cannot specify system identity via options") } - args.SystemIdentity, _ = username.MakeSQLUsernameFromUserInput(optvalue, username.PurposeValidation) + args.SystemIdentity = optvalue continue case "cluster": diff --git a/pkg/sql/pgwire/role_mapper.go b/pkg/sql/pgwire/role_mapper.go index 39d71f2c512c..490a911e35c2 100644 --- a/pkg/sql/pgwire/role_mapper.go +++ b/pkg/sql/pgwire/role_mapper.go @@ -30,19 +30,28 @@ import ( // maps groups of users onto database roles. type RoleMapper = func( ctx context.Context, - systemIdentity username.SQLUsername, + systemIdentity string, ) ([]username.SQLUsername, error) // UseProvidedIdentity is a trivial implementation of RoleMapper which always // returns its input. -func UseProvidedIdentity( - _ context.Context, id username.SQLUsername, -) ([]username.SQLUsername, error) { - return []username.SQLUsername{id}, nil +func UseProvidedIdentity(_ context.Context, id string) ([]username.SQLUsername, error) { + u, err := username.MakeSQLUsernameFromUserInput(id, username.PurposeValidation) + if err != nil { + return nil, err + } + return []username.SQLUsername{u}, nil } var _ RoleMapper = UseProvidedIdentity +// UseSpecifiedIdentity is a RoleMapper that always returns a fixed user. +func UseSpecifiedIdentity(user username.SQLUsername) RoleMapper { + return func(_ context.Context, _ string) ([]username.SQLUsername, error) { + return []username.SQLUsername{user}, nil + } +} + // HbaMapper implements the "map" option that may be defined in a // host-based authentication rule. If the HBA entry does not define a // "map" option, this function will return UseProvidedIdentity. @@ -55,15 +64,15 @@ func HbaMapper(hbaEntry *hba.Entry, identMap *identmap.Conf) RoleMapper { if mapName == "" { return UseProvidedIdentity } - return func(_ context.Context, id username.SQLUsername) ([]username.SQLUsername, error) { - users, _, err := identMap.Map(mapName, id.Normalized()) + return func(_ context.Context, id string) ([]username.SQLUsername, error) { + users, _, err := identMap.Map(mapName, id) if err != nil { return nil, err } for _, user := range users { if user.IsRootUser() || user.IsReserved() { return nil, errors.Newf("system identity %q mapped to reserved database role %q", - id.Normalized(), user.Normalized()) + id, user.Normalized()) } } return users, nil diff --git a/pkg/sql/pgwire/server.go b/pkg/sql/pgwire/server.go index e33b15c7164d..a62b933c4c31 100644 --- a/pkg/sql/pgwire/server.go +++ b/pkg/sql/pgwire/server.go @@ -1100,8 +1100,8 @@ func (s *Server) serveImpl( // We'll build an authPipe to communicate with the authentication process. systemIdentity := c.sessionArgs.SystemIdentity - if systemIdentity.Undefined() { - systemIdentity = c.sessionArgs.User + if systemIdentity == "" { + systemIdentity = c.sessionArgs.User.Normalized() } logVerboseAuthn := !inTestWithoutSQL && c.verboseAuthLogEnabled() authPipe := newAuthPipe(c, logVerboseAuthn, authOpt, systemIdentity) diff --git a/pkg/sql/sessiondatapb/local_only_session_data.proto b/pkg/sql/sessiondatapb/local_only_session_data.proto index ddea6abd47e3..fa8f38090998 100644 --- a/pkg/sql/sessiondatapb/local_only_session_data.proto +++ b/pkg/sql/sessiondatapb/local_only_session_data.proto @@ -305,7 +305,7 @@ message LocalOnlySessionData { int64 transaction_timeout = 81 [(gogoproto.casttype) = "time.Duration"]; // SystemIdentityProto is the original name of the client presented to pgwire // before it was mapped to a SQL identifier. - string system_identity_proto = 82 [(gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/security/username.SQLUsernameProto"]; + string system_identity_proto = 82; // DescriptorValidationMode indicates whether to validate the descriptors at // read and write time, at read time only, or never. int64 descriptor_validation_mode = 83 [(gogoproto.casttype) = "DescriptorValidationMode"]; diff --git a/pkg/sql/sessiondatapb/session_data.go b/pkg/sql/sessiondatapb/session_data.go index cf37f2546220..52ba0646a304 100644 --- a/pkg/sql/sessiondatapb/session_data.go +++ b/pkg/sql/sessiondatapb/session_data.go @@ -106,6 +106,6 @@ func (s *SessionData) User() username.SQLUsername { // SystemIdentity retrieves the session's system identity. // (Identity presented by the client prior to identity mapping.) -func (s *LocalOnlySessionData) SystemIdentity() username.SQLUsername { - return s.SystemIdentityProto.Decode() +func (s *LocalOnlySessionData) SystemIdentity() string { + return s.SystemIdentityProto } diff --git a/pkg/sql/vars.go b/pkg/sql/vars.go index 8ebaf4829f42..1e24ff285de3 100644 --- a/pkg/sql/vars.go +++ b/pkg/sql/vars.go @@ -1097,10 +1097,10 @@ var varGen = map[string]sessionVar{ // CockroachDB extension. `system_identity`: { Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) { - return evalCtx.SessionData().SystemIdentity().Normalized(), nil + return evalCtx.SessionData().SystemIdentity(), nil }, GetFromSessionData: func(sd *sessiondata.SessionData) string { - return sd.SystemIdentity().Normalized() + return sd.SystemIdentity() }, GlobalDefault: func(_ *settings.Values) string { return "" }, },