From 360b32ff062be60120c3eac108d29409bc26d1eb Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Wed, 14 Feb 2024 16:04:53 -0500 Subject: [PATCH 1/9] GCP MySQL IAM Auth support --- lib/cloud/gcp/sql.go | 13 ++++ lib/srv/db/common/auth.go | 7 +- lib/srv/db/mysql/engine.go | 28 ++------ lib/srv/db/mysql/gcp.go | 133 +++++++++++++++++++++++++++++++++++++ 4 files changed, 157 insertions(+), 24 deletions(-) create mode 100644 lib/srv/db/mysql/gcp.go diff --git a/lib/cloud/gcp/sql.go b/lib/cloud/gcp/sql.go index 05148bcd61efd..18104913d58d0 100644 --- a/lib/cloud/gcp/sql.go +++ b/lib/cloud/gcp/sql.go @@ -38,6 +38,8 @@ import ( // SQLAdminClient defines an interface providing access to the GCP Cloud SQL API. type SQLAdminClient interface { + // GetUser retrieves a resource containing information about a user. + GetUser(ctx context.Context, db types.Database, dbUser string) (*sqladmin.User, error) // UpdateUser updates an existing user for the project/instance configured in a session. UpdateUser(ctx context.Context, db types.Database, dbUser string, user *sqladmin.User) error // GetDatabaseInstance returns database instance details for the project/instance @@ -63,6 +65,17 @@ type gcpSQLAdminClient struct { service *sqladmin.Service } +// GetUser retrieves a resource containing information about a user. +func (g *gcpSQLAdminClient) GetUser(ctx context.Context, db types.Database, dbUser string) (*sqladmin.User, error) { + user, err := g.service.Users.Get( + db.GetGCP().ProjectID, + db.GetGCP().InstanceID, + dbUser, + ).Host("%").Context(ctx).Do() + // TODO convert + return user, trace.Wrap(err) +} + // UpdateUser updates an existing user in a Cloud SQL for the project/instance // configured in a session. func (g *gcpSQLAdminClient) UpdateUser(ctx context.Context, db types.Database, dbUser string, user *sqladmin.User) error { diff --git a/lib/srv/db/common/auth.go b/lib/srv/db/common/auth.go index b5969eed69ec4..05a150e4e96bf 100644 --- a/lib/srv/db/common/auth.go +++ b/lib/srv/db/common/auth.go @@ -375,6 +375,11 @@ func (a *dbAuth) GetCloudSQLAuthToken(ctx context.Context, sessionCtx *Session) return "", trace.Wrap(err) } a.cfg.Log.Debugf("Generating GCP auth token for %s.", sessionCtx) + + serviceAccountName := sessionCtx.DatabaseUser + if !strings.HasSuffix(serviceAccountName, ".gserviceaccount.com") { + serviceAccountName = serviceAccountName + ".gserviceaccount.com" + } resp, err := gcpIAM.GenerateAccessToken(ctx, &gcpcredentialspb.GenerateAccessTokenRequest{ // From GenerateAccessToken docs: @@ -382,7 +387,7 @@ func (a *dbAuth) GetCloudSQLAuthToken(ctx context.Context, sessionCtx *Session) // The resource name of the service account for which the credentials // are requested, in the following format: // projects/-/serviceAccounts/{ACCOUNT_EMAIL_OR_UNIQUEID} - Name: fmt.Sprintf("projects/-/serviceAccounts/%v.gserviceaccount.com", sessionCtx.DatabaseUser), + Name: fmt.Sprintf("projects/-/serviceAccounts/%v", serviceAccountName), // From GenerateAccessToken docs: // // Code to identify the scopes to be included in the OAuth 2.0 access diff --git a/lib/srv/db/mysql/engine.go b/lib/srv/db/mysql/engine.go index d0dfc0341704f..646d8fc4b1926 100644 --- a/lib/srv/db/mysql/engine.go +++ b/lib/srv/db/mysql/engine.go @@ -35,7 +35,6 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/utils/retryutils" - "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/srv/db/cloud" "github.com/gravitational/teleport/lib/srv/db/common" @@ -222,34 +221,17 @@ func (e *Engine) connect(ctx context.Context, sessionCtx *common.Session) (*clie return nil, trace.Wrap(err) } case sessionCtx.Database.IsCloudSQL(): - // For Cloud SQL MySQL there is no IAM auth, so we use one-time passwords - // by resetting the database user password for each connection. Thus, - // acquire a lock to make sure all connection attempts to the same - // database and user are serialized. - retryCtx, cancel := context.WithTimeout(ctx, defaults.DatabaseConnectTimeout) - defer cancel() - lease, err := services.AcquireSemaphoreWithRetry(retryCtx, e.makeAcquireSemaphoreConfig(sessionCtx)) - if err != nil { - return nil, trace.Wrap(err) - } - // Only release the semaphore after the connection has been established - // below. If the semaphore fails to release for some reason, it will - // expire in a minute on its own. - defer func() { - err := e.AuthClient.CancelSemaphoreLease(ctx, *lease) - if err != nil { - e.Log.WithError(err).Errorf("Failed to cancel lease: %v.", lease) - } - }() - password, err = e.Auth.GetCloudSQLPassword(ctx, sessionCtx) + // Get the client once for subsequent calls (it acquires a read lock). + gcpClient, err := e.CloudClients.GetGCPSQLAdminClient(ctx) if err != nil { return nil, trace.Wrap(err) } - // Get the client once for subsequent calls (it acquires a read lock). - gcpClient, err := e.CloudClients.GetGCPSQLAdminClient(ctx) + + user, password, err = e.getGCPUserAndPassword(ctx, sessionCtx, gcpClient) if err != nil { return nil, trace.Wrap(err) } + // Detect whether the instance is set to require SSL. // Fallback to not requiring SSL for access denied errors. requireSSL, err := cloud.GetGCPRequireSSL(ctx, sessionCtx, gcpClient) diff --git a/lib/srv/db/mysql/gcp.go b/lib/srv/db/mysql/gcp.go new file mode 100644 index 0000000000000..7031bcb392d58 --- /dev/null +++ b/lib/srv/db/mysql/gcp.go @@ -0,0 +1,133 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package mysql + +import ( + "context" + "fmt" + "strings" + + "github.com/gravitational/trace" + + "github.com/gravitational/teleport/lib/cloud/gcp" + "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/services" + "github.com/gravitational/teleport/lib/srv/db/common" +) + +// TODO +func isDBUserGCPServiceAccount(dbUser string) bool { + if strings.Contains(dbUser, "@") { + if strings.HasSuffix(dbUser, ".iam") || strings.HasSuffix(dbUser, ".iam.gserviceaccount.com") { + return true + } + } + return false +} + +// TODO +func getInDatabaseUserFromGCPServiceAccount(serviceAccountName string) string { + user, _, _ := strings.Cut(serviceAccountName, "@") + return user +} + +func makeGCPServiceAccountFromInDatabaseUser(sessionCtx *common.Session) string { + return fmt.Sprintf("%s@%s.iam.gserviceaccount.com", sessionCtx.DatabaseUser, sessionCtx.Database.GetGCP().ProjectID) +} + +func (e *Engine) getGCPUserAndPassword(ctx context.Context, sessionCtx *common.Session, gcpClient gcp.SQLAdminClient) (string, string, error) { + // If `--db-user` is an service account email, use IAM Auth. + if isDBUserGCPServiceAccount(sessionCtx.DatabaseUser) { + return e.getGCPIAMUserAndPassword(ctx, sessionCtx) + } + + // Get user info to decide how to authenticate. + user, err := gcpClient.GetUser(ctx, sessionCtx.Database, sessionCtx.DatabaseUser) + if err != nil { + // GetUser permission is new for IAM auth. If no permission, assume legacy password user. + if trace.IsAccessDenied(err) { + return e.getGCPPasswordUserAndPassword(ctx, sessionCtx) + } + return "", "", trace.Wrap(err) + } + + // Possible values (copied from SDK): + // "BUILT_IN" - The database's built-in user type. + // "CLOUD_IAM_USER" - Cloud IAM user. + // "CLOUD_IAM_SERVICE_ACCOUNT" - Cloud IAM service account. + // "CLOUD_IAM_GROUP" - Cloud IAM group non-login user. + // "CLOUD_IAM_GROUP_USER" - Cloud IAM group login user. + // "CLOUD_IAM_GROUP_SERVICE_ACCOUNT" - Cloud IAM group service account. + // + // In practice, type can also be empty for built-in user. + switch user.Type { + case "", + "BUILT_IN": + return e.getGCPPasswordUserAndPassword(ctx, sessionCtx) + + case "CLOUD_IAM_SERVICE_ACCOUNT", + "CLOUD_IAM_GROUP_SERVICE_ACCOUNT": + return e.getGCPIAMUserAndPassword( + ctx, + sessionCtx.WithUser(makeGCPServiceAccountFromInDatabaseUser(sessionCtx)), + ) + + default: + return "", "", trace.BadParameter("GCP MySQL user type %q not supported") + } +} + +func (e *Engine) getGCPIAMUserAndPassword(ctx context.Context, sessionCtx *common.Session) (string, string, error) { + e.Log.WithField("session", sessionCtx).Debug("Authenticating GCP MySQL with IAM auth.") + + password, err := e.Auth.GetCloudSQLAuthToken(ctx, sessionCtx) + if err != nil { + return "", "", trace.Wrap(err) + } + return getInDatabaseUserFromGCPServiceAccount(sessionCtx.DatabaseUser), password, nil +} + +func (e *Engine) getGCPPasswordUserAndPassword(ctx context.Context, sessionCtx *common.Session) (string, string, error) { + e.Log.WithField("session", sessionCtx).Debug("Authenticating GCP MySQL with password auth.") + + // For Cloud SQL MySQL legacy auth, we use one-time passwords by resetting + // the database user password for each connection. Thus, acquire a lock to + // make sure all connection attempts to the same database and user are + // serialized. + retryCtx, cancel := context.WithTimeout(ctx, defaults.DatabaseConnectTimeout) + defer cancel() + lease, err := services.AcquireSemaphoreWithRetry(retryCtx, e.makeAcquireSemaphoreConfig(sessionCtx)) + if err != nil { + return "", "", trace.Wrap(err) + } + // Only release the semaphore after the connection has been established + // below. If the semaphore fails to release for some reason, it will + // expire in a minute on its own. + defer func() { + err := e.AuthClient.CancelSemaphoreLease(ctx, *lease) + if err != nil { + e.Log.WithError(err).Errorf("Failed to cancel lease: %v.", lease) + } + }() + password, err := e.Auth.GetCloudSQLPassword(ctx, sessionCtx) + if err != nil { + return "", "", trace.Wrap(err) + } + return sessionCtx.DatabaseUser, password, nil +} From a735143de1bf3dc0bc369a959f2d6deef6487b65 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Thu, 15 Feb 2024 10:37:27 -0500 Subject: [PATCH 2/9] convert no permission --- lib/cloud/gcp/sql.go | 9 ++++----- lib/srv/db/common/auth.go | 11 +++++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/cloud/gcp/sql.go b/lib/cloud/gcp/sql.go index 18104913d58d0..594d19eacb2d7 100644 --- a/lib/cloud/gcp/sql.go +++ b/lib/cloud/gcp/sql.go @@ -72,8 +72,7 @@ func (g *gcpSQLAdminClient) GetUser(ctx context.Context, db types.Database, dbUs db.GetGCP().InstanceID, dbUser, ).Host("%").Context(ctx).Do() - // TODO convert - return user, trace.Wrap(err) + return user, trace.Wrap(convertAPIError(err)) } // UpdateUser updates an existing user in a Cloud SQL for the project/instance @@ -84,7 +83,7 @@ func (g *gcpSQLAdminClient) UpdateUser(ctx context.Context, db types.Database, d db.GetGCP().InstanceID, user).Name(dbUser).Host("%").Context(ctx).Do() if err != nil { - return trace.Wrap(err) + return trace.Wrap(convertAPIError(err)) } return nil } @@ -95,7 +94,7 @@ func (g *gcpSQLAdminClient) GetDatabaseInstance(ctx context.Context, db types.Da gcp := db.GetGCP() dbi, err := g.service.Instances.Get(gcp.ProjectID, gcp.InstanceID).Context(ctx).Do() if err != nil { - return nil, trace.Wrap(err) + return nil, trace.Wrap(convertAPIError(err)) } return dbi, nil } @@ -125,7 +124,7 @@ func (g *gcpSQLAdminClient) GenerateEphemeralCert(ctx context.Context, db types. }) resp, err := req.Context(ctx).Do() if err != nil { - return nil, trace.Wrap(err) + return nil, trace.Wrap(convertAPIError(err)) } // Create TLS certificate from returned ephemeral certificate and private key. diff --git a/lib/srv/db/common/auth.go b/lib/srv/db/common/auth.go index 05a150e4e96bf..b45370d22d8e9 100644 --- a/lib/srv/db/common/auth.go +++ b/lib/srv/db/common/auth.go @@ -453,12 +453,19 @@ func (a *dbAuth) GetCloudSQLPassword(ctx context.Context, sessionCtx *Session) ( func (a *dbAuth) updateCloudSQLUser(ctx context.Context, sessionCtx *Session, gcpCloudSQL gcp.SQLAdminClient, user *sqladmin.User) error { err := gcpCloudSQL.UpdateUser(ctx, sessionCtx.Database, sessionCtx.DatabaseUser, user) if err != nil { + // Note that mysql client has a 1024 char limit for displaying errors + // so we need to keep the message short when possible. This message + // does get cut off when sessionCtx.DatabaseUser or err is long. return trace.AccessDenied(`Could not update Cloud SQL user %q password: %v -Make sure Teleport db service has "Cloud SQL Admin" GCP IAM role, or -"cloudsql.users.update" IAM permission. +If the db user uses IAM authentication, please use the full service account email +ID as "--db-user", or grant the Teleport Database Service the +"cloudsql.users.get" IAM permission so it can discover the user type. + +If the db user uses passwords, make sure Teleport Database Service has "Cloud +SQL Admin" GCP IAM role, or "cloudsql.users.update" IAM permission. `, sessionCtx.DatabaseUser, err) } return nil From 177d7cae8432738f63e36ea67016b786377d973b Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Fri, 16 Feb 2024 10:59:38 -0500 Subject: [PATCH 3/9] fix mock --- lib/cloud/mocks/gcp.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/cloud/mocks/gcp.go b/lib/cloud/mocks/gcp.go index cda4c426f5744..7e65d77f036c9 100644 --- a/lib/cloud/mocks/gcp.go +++ b/lib/cloud/mocks/gcp.go @@ -41,6 +41,15 @@ type GCPSQLAdminClientMock struct { DatabaseInstance *sqladmin.DatabaseInstance // EphemeralCert is returned from GenerateEphemeralCert. EphemeralCert *tls.Certificate + // DatabaseUser is returned from GetUser. + DatabaseUser *sqladmin.User +} + +func (g *GCPSQLAdminClientMock) GetUser(ctx context.Context, db types.Database, dbUser string) (*sqladmin.User, error) { + if g.DatabaseUser == nil { + return nil, trace.AccessDenied("unauthorized") + } + return g.DatabaseUser, nil } func (g *GCPSQLAdminClientMock) UpdateUser(ctx context.Context, db types.Database, dbUser string, user *sqladmin.User) error { From a9d0d464e72b079ac1800b73934e9e17c738e6d1 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Fri, 16 Feb 2024 13:23:49 -0500 Subject: [PATCH 4/9] add UT --- lib/srv/db/mysql/gcp.go | 21 ++-- lib/srv/db/mysql/gcp_test.go | 213 +++++++++++++++++++++++++++++++++++ 2 files changed, 226 insertions(+), 8 deletions(-) create mode 100644 lib/srv/db/mysql/gcp_test.go diff --git a/lib/srv/db/mysql/gcp.go b/lib/srv/db/mysql/gcp.go index 7031bcb392d58..50ef553ab18b8 100644 --- a/lib/srv/db/mysql/gcp.go +++ b/lib/srv/db/mysql/gcp.go @@ -31,23 +31,27 @@ import ( "github.com/gravitational/teleport/lib/srv/db/common" ) -// TODO func isDBUserGCPServiceAccount(dbUser string) bool { if strings.Contains(dbUser, "@") { - if strings.HasSuffix(dbUser, ".iam") || strings.HasSuffix(dbUser, ".iam.gserviceaccount.com") { + switch { + // Example: mysql-iam-user@my-project-id.iam + // This format is used to align with PostgreSQL. + case strings.HasSuffix(dbUser, ".iam"): + return true + // Example: mysql-iam-user@my-project-id..gserviceaccount.com + case strings.HasSuffix(dbUser, ".iam.gserviceaccount.com"): return true } } return false } -// TODO -func getInDatabaseUserFromGCPServiceAccount(serviceAccountName string) string { +func gcpServiceAccountToDatabaseUser(serviceAccountName string) string { user, _, _ := strings.Cut(serviceAccountName, "@") return user } -func makeGCPServiceAccountFromInDatabaseUser(sessionCtx *common.Session) string { +func databaseUserToGCPServiceAccount(sessionCtx *common.Session) string { return fmt.Sprintf("%s@%s.iam.gserviceaccount.com", sessionCtx.DatabaseUser, sessionCtx.Database.GetGCP().ProjectID) } @@ -85,22 +89,23 @@ func (e *Engine) getGCPUserAndPassword(ctx context.Context, sessionCtx *common.S "CLOUD_IAM_GROUP_SERVICE_ACCOUNT": return e.getGCPIAMUserAndPassword( ctx, - sessionCtx.WithUser(makeGCPServiceAccountFromInDatabaseUser(sessionCtx)), + sessionCtx.WithUser(databaseUserToGCPServiceAccount(sessionCtx)), ) default: - return "", "", trace.BadParameter("GCP MySQL user type %q not supported") + return "", "", trace.BadParameter("GCP MySQL user type %q not supported", user.Type) } } func (e *Engine) getGCPIAMUserAndPassword(ctx context.Context, sessionCtx *common.Session) (string, string, error) { e.Log.WithField("session", sessionCtx).Debug("Authenticating GCP MySQL with IAM auth.") + // Note that sessionCtx.DatabaseUser is the service account. password, err := e.Auth.GetCloudSQLAuthToken(ctx, sessionCtx) if err != nil { return "", "", trace.Wrap(err) } - return getInDatabaseUserFromGCPServiceAccount(sessionCtx.DatabaseUser), password, nil + return gcpServiceAccountToDatabaseUser(sessionCtx.DatabaseUser), password, nil } func (e *Engine) getGCPPasswordUserAndPassword(ctx context.Context, sessionCtx *common.Session) (string, string, error) { diff --git a/lib/srv/db/mysql/gcp_test.go b/lib/srv/db/mysql/gcp_test.go new file mode 100644 index 0000000000000..23760d01dd27c --- /dev/null +++ b/lib/srv/db/mysql/gcp_test.go @@ -0,0 +1,213 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package mysql + +import ( + "context" + "testing" + + "github.com/jonboulle/clockwork" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" + sqladmin "google.golang.org/api/sqladmin/v1beta4" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/cloud/gcp" + "github.com/gravitational/teleport/lib/cloud/mocks" + "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/srv/db/common" +) + +type fakeAuth struct { + common.Auth +} + +func (a fakeAuth) GetCloudSQLAuthToken(ctx context.Context, sessionCtx *common.Session) (string, error) { + return "iam-auth-token", nil +} + +func (a fakeAuth) GetCloudSQLPassword(ctx context.Context, sessionCtx *common.Session) (string, error) { + return "one-time-password", nil +} + +func Test_getGCPUserAndPassowrd(t *testing.T) { + ctx := context.Background() + authClient := makeAuthClient(t) + db := makeGCPMySQLDatabase(t) + dbAuth := &fakeAuth{} + + tests := []struct { + name string + inputDatabaseUser string + mockDBAuth common.Auth + mockGCPClient gcp.SQLAdminClient + wantDatabaseUser string + wantPassword string + wantError bool + }{ + { + name: "iam auth with full service account", + inputDatabaseUser: "iam-auth-user@project-id.iam.gserviceaccount.com", + mockDBAuth: dbAuth, + wantDatabaseUser: "iam-auth-user", + wantPassword: "iam-auth-token", + }, + { + name: "iam auth with short service account", + inputDatabaseUser: "iam-auth-user@project-id.iam", + mockDBAuth: dbAuth, + wantDatabaseUser: "iam-auth-user", + wantPassword: "iam-auth-token", + }, + { + name: "iam auth with CLOUD_IAM_SERVICE_ACCOUNT user", + inputDatabaseUser: "iam-auth-user", + mockDBAuth: dbAuth, + mockGCPClient: &mocks.GCPSQLAdminClientMock{ + DatabaseUser: makeGCPDatabaseUser("iam-auth-user", "CLOUD_IAM_SERVICE_ACCOUNT"), + }, + wantDatabaseUser: "iam-auth-user", + wantPassword: "iam-auth-token", + }, + { + name: "iam auth with CLOUD_IAM_GROUP_SERVICE_ACCOUNT user", + inputDatabaseUser: "iam-auth-user", + mockDBAuth: dbAuth, + mockGCPClient: &mocks.GCPSQLAdminClientMock{ + DatabaseUser: makeGCPDatabaseUser("iam-auth-user", "CLOUD_IAM_GROUP_SERVICE_ACCOUNT"), + }, + wantDatabaseUser: "iam-auth-user", + wantPassword: "iam-auth-token", + }, + { + name: "password auth without GetUser permission", + inputDatabaseUser: "some-user", + mockDBAuth: dbAuth, + mockGCPClient: &mocks.GCPSQLAdminClientMock{ + // Default no permission to GetUser, + }, + wantDatabaseUser: "some-user", + wantPassword: "one-time-password", + }, + { + name: "password auth with BUILT_IN user", + inputDatabaseUser: "password-user", + mockDBAuth: dbAuth, + mockGCPClient: &mocks.GCPSQLAdminClientMock{ + DatabaseUser: makeGCPDatabaseUser("password-user", "BUILT_IN"), + }, + wantDatabaseUser: "password-user", + wantPassword: "one-time-password", + }, + { + name: "password auth with empty user type", + inputDatabaseUser: "password-user", + mockDBAuth: dbAuth, + mockGCPClient: &mocks.GCPSQLAdminClientMock{ + DatabaseUser: makeGCPDatabaseUser("password-user", ""), + }, + wantDatabaseUser: "password-user", + wantPassword: "one-time-password", + }, + { + name: "unsupported user type", + inputDatabaseUser: "some-user", + mockDBAuth: dbAuth, + mockGCPClient: &mocks.GCPSQLAdminClientMock{ + DatabaseUser: makeGCPDatabaseUser("some-user", "CLOUD_IAM_USER"), + }, + wantError: true, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + engine := NewEngine(common.EngineConfig{ + Auth: test.mockDBAuth, + AuthClient: authClient, + Context: ctx, + Clock: clockwork.NewRealClock(), + Log: logrus.StandardLogger(), + }).(*Engine) + + sessionCtx := &common.Session{ + Database: db, + DatabaseUser: test.inputDatabaseUser, + } + + databaseUser, password, err := engine.getGCPUserAndPassword(ctx, sessionCtx, test.mockGCPClient) + if test.wantError { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, test.wantDatabaseUser, databaseUser) + require.Equal(t, test.wantPassword, password) + } + }) + } +} + +func makeAuthClient(t *testing.T) *auth.Client { + t.Helper() + + authServer, err := auth.NewTestAuthServer(auth.TestAuthServerConfig{ + ClusterName: "mysql-test", + Dir: t.TempDir(), + }) + require.NoError(t, err) + t.Cleanup(func() { authServer.Close() }) + + tlsServer, err := authServer.NewTestTLSServer() + require.NoError(t, err) + t.Cleanup(func() { tlsServer.Close() }) + + authClient, err := tlsServer.NewClient(auth.TestServerID(types.RoleDatabase, "mysql-test")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, authClient.Close()) }) + + return authClient +} + +func makeGCPMySQLDatabase(t *testing.T) types.Database { + t.Helper() + + database, err := types.NewDatabaseV3(types.Metadata{ + Name: "gcp-mysql", + }, types.DatabaseSpecV3{ + Protocol: defaults.ProtocolMySQL, + URI: "localhost:3306", + GCP: types.GCPCloudSQL{ + ProjectID: "project-1", + InstanceID: "instance-1", + }, + }) + require.NoError(t, err) + return database +} + +func makeGCPDatabaseUser(name, userType string) *sqladmin.User { + return &sqladmin.User{ + Name: name, + Host: "%", + Type: userType, + Instance: "instance-1", + } +} From e3cf6378f9602c2ae775033aae458aef8d338b24 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Mon, 26 Feb 2024 12:53:14 -0500 Subject: [PATCH 5/9] minor refactoring --- lib/srv/db/mysql/gcp.go | 91 ++++++++++++++++++++++-------------- lib/srv/db/mysql/gcp_test.go | 7 +++ 2 files changed, 63 insertions(+), 35 deletions(-) diff --git a/lib/srv/db/mysql/gcp.go b/lib/srv/db/mysql/gcp.go index 50ef553ab18b8..c2040986b6bbd 100644 --- a/lib/srv/db/mysql/gcp.go +++ b/lib/srv/db/mysql/gcp.go @@ -38,7 +38,7 @@ func isDBUserGCPServiceAccount(dbUser string) bool { // This format is used to align with PostgreSQL. case strings.HasSuffix(dbUser, ".iam"): return true - // Example: mysql-iam-user@my-project-id..gserviceaccount.com + // Example: mysql-iam-user@my-project-id.iam.gserviceaccount.com case strings.HasSuffix(dbUser, ".iam.gserviceaccount.com"): return true } @@ -58,57 +58,65 @@ func databaseUserToGCPServiceAccount(sessionCtx *common.Session) string { func (e *Engine) getGCPUserAndPassword(ctx context.Context, sessionCtx *common.Session, gcpClient gcp.SQLAdminClient) (string, string, error) { // If `--db-user` is an service account email, use IAM Auth. if isDBUserGCPServiceAccount(sessionCtx.DatabaseUser) { - return e.getGCPIAMUserAndPassword(ctx, sessionCtx) + user := gcpServiceAccountToDatabaseUser(sessionCtx.DatabaseUser) + password, err := e.getGCPIAMAuthToken(ctx, sessionCtx) + if err != nil { + return "", "", trace.Wrap(err) + } + return user, password, nil } // Get user info to decide how to authenticate. - user, err := gcpClient.GetUser(ctx, sessionCtx.Database, sessionCtx.DatabaseUser) - if err != nil { - // GetUser permission is new for IAM auth. If no permission, assume legacy password user. - if trace.IsAccessDenied(err) { - return e.getGCPPasswordUserAndPassword(ctx, sessionCtx) + user := sessionCtx.DatabaseUser + dbUserInfo, err := gcpClient.GetUser(ctx, sessionCtx.Database, sessionCtx.DatabaseUser) + switch { + // GetUser permission is new for IAM auth. If no permission, assume legacy password user. + case trace.IsAccessDenied(err): + password, err := e.getGCPOneTimePassword(ctx, sessionCtx) + if err != nil { + return "", "", trace.Wrap(err) } + return user, password, nil + + // Report any other error. + case err != nil: return "", "", trace.Wrap(err) } - // Possible values (copied from SDK): - // "BUILT_IN" - The database's built-in user type. - // "CLOUD_IAM_USER" - Cloud IAM user. - // "CLOUD_IAM_SERVICE_ACCOUNT" - Cloud IAM service account. - // "CLOUD_IAM_GROUP" - Cloud IAM group non-login user. - // "CLOUD_IAM_GROUP_USER" - Cloud IAM group login user. - // "CLOUD_IAM_GROUP_SERVICE_ACCOUNT" - Cloud IAM group service account. - // - // In practice, type can also be empty for built-in user. - switch user.Type { + // The user type constants are documented in their SDK. However, in + // practice, type can also be empty for built-in user. + switch dbUserInfo.Type { case "", - "BUILT_IN": - return e.getGCPPasswordUserAndPassword(ctx, sessionCtx) + gcpMySQLDBUserTypeBuiltIn: + password, err := e.getGCPOneTimePassword(ctx, sessionCtx) + if err != nil { + return "", "", trace.Wrap(err) + } + return user, password, nil - case "CLOUD_IAM_SERVICE_ACCOUNT", - "CLOUD_IAM_GROUP_SERVICE_ACCOUNT": - return e.getGCPIAMUserAndPassword( - ctx, - sessionCtx.WithUser(databaseUserToGCPServiceAccount(sessionCtx)), - ) + case gcpMySQLDBUserTypeServiceAccount, + gcpMySQLDBUserTypeGroupServiceAccount: + serviceAccountName := databaseUserToGCPServiceAccount(sessionCtx) + password, err := e.getGCPIAMAuthToken(ctx, sessionCtx.WithUser(serviceAccountName)) + if err != nil { + return "", "", trace.Wrap(err) + } + return user, password, nil default: - return "", "", trace.BadParameter("GCP MySQL user type %q not supported", user.Type) + return "", "", trace.BadParameter("GCP MySQL user type %q not supported", dbUserInfo.Type) } } -func (e *Engine) getGCPIAMUserAndPassword(ctx context.Context, sessionCtx *common.Session) (string, string, error) { +func (e *Engine) getGCPIAMAuthToken(ctx context.Context, sessionCtx *common.Session) (string, error) { e.Log.WithField("session", sessionCtx).Debug("Authenticating GCP MySQL with IAM auth.") // Note that sessionCtx.DatabaseUser is the service account. password, err := e.Auth.GetCloudSQLAuthToken(ctx, sessionCtx) - if err != nil { - return "", "", trace.Wrap(err) - } - return gcpServiceAccountToDatabaseUser(sessionCtx.DatabaseUser), password, nil + return password, trace.Wrap(err) } -func (e *Engine) getGCPPasswordUserAndPassword(ctx context.Context, sessionCtx *common.Session) (string, string, error) { +func (e *Engine) getGCPOneTimePassword(ctx context.Context, sessionCtx *common.Session) (string, error) { e.Log.WithField("session", sessionCtx).Debug("Authenticating GCP MySQL with password auth.") // For Cloud SQL MySQL legacy auth, we use one-time passwords by resetting @@ -119,7 +127,7 @@ func (e *Engine) getGCPPasswordUserAndPassword(ctx context.Context, sessionCtx * defer cancel() lease, err := services.AcquireSemaphoreWithRetry(retryCtx, e.makeAcquireSemaphoreConfig(sessionCtx)) if err != nil { - return "", "", trace.Wrap(err) + return "", trace.Wrap(err) } // Only release the semaphore after the connection has been established // below. If the semaphore fails to release for some reason, it will @@ -132,7 +140,20 @@ func (e *Engine) getGCPPasswordUserAndPassword(ctx context.Context, sessionCtx * }() password, err := e.Auth.GetCloudSQLPassword(ctx, sessionCtx) if err != nil { - return "", "", trace.Wrap(err) + return "", trace.Wrap(err) } - return sessionCtx.DatabaseUser, password, nil + return password, nil } + +const ( + // gcpMySQLDBUserTypeBuiltIn indicates the database's built-in user type. + gcpMySQLDBUserTypeBuiltIn = "BUILT_IN" + // gcpMySQLDBUserTypeServiceAccount indicates a Cloud IAM service account. + gcpMySQLDBUserTypeServiceAccount = "CLOUD_IAM_SERVICE_ACCOUNT" + // gcpMySQLDBUserTypeGroupServiceAccount indicates a Cloud IAM group service account. + gcpMySQLDBUserTypeGroupServiceAccount = "CLOUD_IAM_GROUP_SERVICE_ACCOUNT" + // gcpMySQLDBUserTypeUser indicates a Cloud IAM user. + gcpMySQLDBUserTypeUser = "CLOUD_IAM_USER" + // gcpMySQLDBUserTypeGroupUser indicates a Cloud IAM group login user. + gcpMySQLDBUserTypeGroupUser = "CLOUD_IAM_GROUP_USER" +) diff --git a/lib/srv/db/mysql/gcp_test.go b/lib/srv/db/mysql/gcp_test.go index 23760d01dd27c..36d2e2cc371e1 100644 --- a/lib/srv/db/mysql/gcp_test.go +++ b/lib/srv/db/mysql/gcp_test.go @@ -22,6 +22,7 @@ import ( "context" "testing" + "github.com/gravitational/trace" "github.com/jonboulle/clockwork" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" @@ -40,10 +41,16 @@ type fakeAuth struct { } func (a fakeAuth) GetCloudSQLAuthToken(ctx context.Context, sessionCtx *common.Session) (string, error) { + if !isDBUserGCPServiceAccount(sessionCtx.DatabaseUser) { + return "", trace.BadParameter("database user must be a service account") + } return "iam-auth-token", nil } func (a fakeAuth) GetCloudSQLPassword(ctx context.Context, sessionCtx *common.Session) (string, error) { + if isDBUserGCPServiceAccount(sessionCtx.DatabaseUser) { + return "", trace.BadParameter("database user must not be a service account") + } return "one-time-password", nil } From b8781acba64c3032ba4a00b9be5c455100b6ca90 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Mon, 26 Feb 2024 13:17:50 -0500 Subject: [PATCH 6/9] fix lint --- lib/srv/db/mysql/gcp.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/srv/db/mysql/gcp.go b/lib/srv/db/mysql/gcp.go index c2040986b6bbd..4047538c046b3 100644 --- a/lib/srv/db/mysql/gcp.go +++ b/lib/srv/db/mysql/gcp.go @@ -103,8 +103,12 @@ func (e *Engine) getGCPUserAndPassword(ctx context.Context, sessionCtx *common.S } return user, password, nil - default: + case gcpMySQLDBUserTypeUser, + gcpMySQLDBUserTypeGroupUser: return "", "", trace.BadParameter("GCP MySQL user type %q not supported", dbUserInfo.Type) + + default: + return "", "", trace.BadParameter("unknown GCP MySQL user type %q", dbUserInfo.Type) } } From fd64366a733a486261356d60d6338d046127ba4d Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Mon, 4 Mar 2024 09:49:05 -0500 Subject: [PATCH 7/9] return an error for postgres username format --- lib/srv/db/mysql/gcp.go | 35 ++++++++++++++++++++--------------- lib/srv/db/mysql/gcp_test.go | 7 +++---- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/srv/db/mysql/gcp.go b/lib/srv/db/mysql/gcp.go index 4047538c046b3..dccd4d8b6f76b 100644 --- a/lib/srv/db/mysql/gcp.go +++ b/lib/srv/db/mysql/gcp.go @@ -31,19 +31,16 @@ import ( "github.com/gravitational/teleport/lib/srv/db/common" ) -func isDBUserGCPServiceAccount(dbUser string) bool { - if strings.Contains(dbUser, "@") { - switch { - // Example: mysql-iam-user@my-project-id.iam - // This format is used to align with PostgreSQL. - case strings.HasSuffix(dbUser, ".iam"): - return true - // Example: mysql-iam-user@my-project-id.iam.gserviceaccount.com - case strings.HasSuffix(dbUser, ".iam.gserviceaccount.com"): - return true - } - } - return false +func isDBUserFullGCPServerAccountID(dbUser string) bool { + // Example: mysql-iam-user@my-project-id.iam.gserviceaccount.com + return strings.Contains(dbUser, "@") && + strings.HasSuffix(dbUser, ".iam.gserviceaccount.com") +} + +func isDBUserShortGCPServiceAccountID(dbUser string) bool { + // Example: mysql-iam-user@my-project-id.iam + return strings.Contains(dbUser, "@") && + strings.HasSuffix(dbUser, ".iam") } func gcpServiceAccountToDatabaseUser(serviceAccountName string) string { @@ -56,8 +53,8 @@ func databaseUserToGCPServiceAccount(sessionCtx *common.Session) string { } func (e *Engine) getGCPUserAndPassword(ctx context.Context, sessionCtx *common.Session, gcpClient gcp.SQLAdminClient) (string, string, error) { - // If `--db-user` is an service account email, use IAM Auth. - if isDBUserGCPServiceAccount(sessionCtx.DatabaseUser) { + // If `--db-user` is the full service account email ID, use IAM Auth. + if isDBUserFullGCPServerAccountID(sessionCtx.DatabaseUser) { user := gcpServiceAccountToDatabaseUser(sessionCtx.DatabaseUser) password, err := e.getGCPIAMAuthToken(ctx, sessionCtx) if err != nil { @@ -66,6 +63,14 @@ func (e *Engine) getGCPUserAndPassword(ctx context.Context, sessionCtx *common.S return user, password, nil } + // Note that GCP Postgres' format "user@my-project-id.iam" is not accepted + // for GCP MySQL. For GCP Postgres, "user@my-project-id.iam" is the actual + // mapped in-database username. However, the mapped in-database username + // for GCP MySQL does not have the "@my-project-id.iam" part. + if isDBUserShortGCPServiceAccountID(sessionCtx.DatabaseUser) { + return "", "", trace.BadParameter("username %q is not accepted for GCP MySQL. Please use the in-database username or the full service account Email ID.", sessionCtx.DatabaseUser) + } + // Get user info to decide how to authenticate. user := sessionCtx.DatabaseUser dbUserInfo, err := gcpClient.GetUser(ctx, sessionCtx.Database, sessionCtx.DatabaseUser) diff --git a/lib/srv/db/mysql/gcp_test.go b/lib/srv/db/mysql/gcp_test.go index 36d2e2cc371e1..04a6f6384a78f 100644 --- a/lib/srv/db/mysql/gcp_test.go +++ b/lib/srv/db/mysql/gcp_test.go @@ -41,14 +41,14 @@ type fakeAuth struct { } func (a fakeAuth) GetCloudSQLAuthToken(ctx context.Context, sessionCtx *common.Session) (string, error) { - if !isDBUserGCPServiceAccount(sessionCtx.DatabaseUser) { + if !isDBUserFullGCPServerAccountID(sessionCtx.DatabaseUser) { return "", trace.BadParameter("database user must be a service account") } return "iam-auth-token", nil } func (a fakeAuth) GetCloudSQLPassword(ctx context.Context, sessionCtx *common.Session) (string, error) { - if isDBUserGCPServiceAccount(sessionCtx.DatabaseUser) { + if isDBUserFullGCPServerAccountID(sessionCtx.DatabaseUser) { return "", trace.BadParameter("database user must not be a service account") } return "one-time-password", nil @@ -80,8 +80,7 @@ func Test_getGCPUserAndPassowrd(t *testing.T) { name: "iam auth with short service account", inputDatabaseUser: "iam-auth-user@project-id.iam", mockDBAuth: dbAuth, - wantDatabaseUser: "iam-auth-user", - wantPassword: "iam-auth-token", + wantError: true, }, { name: "iam auth with CLOUD_IAM_SERVICE_ACCOUNT user", From 851170c95ec30c2b52a0785bdb5926e26dc67c8c Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Mon, 4 Mar 2024 13:33:26 -0500 Subject: [PATCH 8/9] make user not found error more readable --- lib/srv/db/mysql/gcp.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/srv/db/mysql/gcp.go b/lib/srv/db/mysql/gcp.go index dccd4d8b6f76b..08503ee6b9d8b 100644 --- a/lib/srv/db/mysql/gcp.go +++ b/lib/srv/db/mysql/gcp.go @@ -83,6 +83,12 @@ func (e *Engine) getGCPUserAndPassword(ctx context.Context, sessionCtx *common.S } return user, password, nil + // Make the original error message "object not found" more readable. Note + // that catching not found here also prevents Google creating a new + // database user during OTP generation. + case trace.IsNotFound(err): + return "", "", trace.NotFound("database user %q does not exist in database %q", sessionCtx.DatabaseUser, sessionCtx.Database.GetName()) + // Report any other error. case err != nil: return "", "", trace.Wrap(err) From b91384b0d36aa5816cb059a0548136164b0bc1c4 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Wed, 6 Mar 2024 10:27:02 -0500 Subject: [PATCH 9/9] add a debug log when falling back to password auth --- lib/srv/db/mysql/gcp.go | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/srv/db/mysql/gcp.go b/lib/srv/db/mysql/gcp.go index 08503ee6b9d8b..4b424f062f00e 100644 --- a/lib/srv/db/mysql/gcp.go +++ b/lib/srv/db/mysql/gcp.go @@ -77,6 +77,7 @@ func (e *Engine) getGCPUserAndPassword(ctx context.Context, sessionCtx *common.S switch { // GetUser permission is new for IAM auth. If no permission, assume legacy password user. case trace.IsAccessDenied(err): + e.Log.WithField("user", sessionCtx.DatabaseUser).Debug("Access denied to get GCP MySQL database user info. Continue with password auth.") password, err := e.getGCPOneTimePassword(ctx, sessionCtx) if err != nil { return "", "", trace.Wrap(err)