diff --git a/.github/workflows/aws-e2e-tests-non-root.yaml b/.github/workflows/aws-e2e-tests-non-root.yaml index 0167970c4196f..a2c2c89e271f7 100644 --- a/.github/workflows/aws-e2e-tests-non-root.yaml +++ b/.github/workflows/aws-e2e-tests-non-root.yaml @@ -18,6 +18,7 @@ env: RDS_DISCOVERY_ROLE: arn:aws:iam::307493967395:role/ci-database-e2e-tests-rds-discovery RDS_POSTGRES_INSTANCE_NAME: ci-database-e2e-tests-rds-postgres-instance-us-west-2-307493967395 RDS_MYSQL_INSTANCE_NAME: ci-database-e2e-tests-rds-mysql-instance-us-west-2-307493967395 + RDS_MARIADB_INSTANCE_NAME: ci-database-e2e-tests-rds-mariadb-instance-us-west-2-307493967395 DISCOVERY_MATCHER_LABELS: "*=*" jobs: changes: diff --git a/e2e/aws/main_test.go b/e2e/aws/main_test.go index 03df021073692..9d25b19981225 100644 --- a/e2e/aws/main_test.go +++ b/e2e/aws/main_test.go @@ -49,6 +49,10 @@ const ( // name of the RDS MySQL DB instance that will be created by the Teleport // Discovery Service. rdsMySQLInstanceNameEnv = "RDS_MYSQL_INSTANCE_NAME" + // rdsMariaDBInstanceNameEnv is the environment variable that specifies the + // name of the RDS MariaDB instance that will be created by the Teleport + // Discovery Service. + rdsMariaDBInstanceNameEnv = "RDS_MARIADB_INSTANCE_NAME" // kubeSvcRoleARNEnv is the environment variable that specifies // the IAM role that Teleport Kubernetes Service will assume to access the EKS cluster. // This role needs to have the following permissions: diff --git a/e2e/aws/rds_test.go b/e2e/aws/rds_test.go index 3280603ebc3d4..d884236ffd7cc 100644 --- a/e2e/aws/rds_test.go +++ b/e2e/aws/rds_test.go @@ -370,6 +370,129 @@ func testRDS(t *testing.T) { }) } }) + + t.Run("mariadb", func(t *testing.T) { + t.Parallel() + + // wait for the database to be discovered + mariaDBName := mustGetEnv(t, rdsMariaDBInstanceNameEnv) + waitForDatabases(t, cluster.Process, mariaDBName) + db, err := cluster.Process.GetAuthServer().GetDatabase(ctx, mariaDBName) + require.NoError(t, err) + adminUser := mustGetDBAdmin(t, db) + + // connect as the RDS database admin user - not to be confused + // with Teleport's "db admin user". + conn := connectAsRDSMySQLAdmin(t, ctx, db.GetAWS().RDS.InstanceID) + provisionMariaDBAdminUser(t, ctx, conn, adminUser.Name) + + // create a couple test tables to test role assignment with. + testTable1 := "teleport.test_" + randASCII(t, 4) + _, err = conn.Execute(fmt.Sprintf("CREATE TABLE %s (x int)", testTable1)) + require.NoError(t, err) + t.Cleanup(func() { + _, _ = conn.Execute(fmt.Sprintf("DROP TABLE %s", testTable1)) + }) + testTable2 := "teleport.test_" + randASCII(t, 4) + _, err = conn.Execute(fmt.Sprintf("CREATE TABLE %s (x int)", testTable2)) + require.NoError(t, err) + t.Cleanup(func() { + _, _ = conn.Execute(fmt.Sprintf("DROP TABLE %s", testTable2)) + }) + + // create the roles that Teleport will auto assign. + // role 1 only allows SELECT on test table 1. + // role 2 only allows SELECT on test table 2. + // a user needs to have both roles to select from a join of the tables. + _, err = conn.Execute(fmt.Sprintf("CREATE ROLE %q", autoRole1)) + require.NoError(t, err) + t.Cleanup(func() { + _, _ = conn.Execute(fmt.Sprintf("DROP ROLE %q", autoRole1)) + }) + _, err = conn.Execute(fmt.Sprintf("CREATE ROLE %q", autoRole2)) + require.NoError(t, err) + t.Cleanup(func() { + _, _ = conn.Execute(fmt.Sprintf("DROP ROLE %q", autoRole2)) + }) + _, err = conn.Execute(fmt.Sprintf("GRANT SELECT on %s TO %q", testTable1, autoRole1)) + require.NoError(t, err) + _, err = conn.Execute(fmt.Sprintf("GRANT SELECT on %s TO %q", testTable2, autoRole2)) + require.NoError(t, err) + + // db admin needs the admin option for a role to grant others that role. + _, err = conn.Execute(fmt.Sprintf("GRANT %q TO %q WITH ADMIN OPTION", autoRole1, adminUser.Name)) + require.NoError(t, err) + _, err = conn.Execute(fmt.Sprintf("GRANT %q TO %q WITH ADMIN OPTION", autoRole2, adminUser.Name)) + require.NoError(t, err) + autoRolesQuery := fmt.Sprintf("SELECT 1 FROM %s JOIN %s", testTable1, testTable2) + + t.Cleanup(func() { + // best effort cleanup all the users created by the tests. + // don't cleanup the admin or test runs will interfere with + // each other. + _, _ = conn.Execute(fmt.Sprintf("DROP ROLE %q", "tp-role-"+autoUserKeep)) + _, _ = conn.Execute(fmt.Sprintf("DROP ROLE %q", "tp-role-"+autoUserDrop)) + _, _ = conn.Execute(fmt.Sprintf("DROP USER %q", autoUserKeep)) + _, _ = conn.Execute(fmt.Sprintf("DROP USER %q", autoUserDrop)) + _, _ = conn.Execute("DELETE FROM teleport.user_attributes WHERE USER=?", autoUserKeep) + _, _ = conn.Execute("DELETE FROM teleport.user_attributes WHERE USER=?", autoUserDrop) + }) + + for name, test := range map[string]struct { + user string + dbUser string + query string + afterConnTestFn func(t *testing.T) + }{ + "existing user": { + user: hostUser, + dbUser: adminUser.Name, + query: "select 1", + }, + "auto user keep": { + user: autoUserKeep, + dbUser: autoUserKeep, + query: autoRolesQuery, + afterConnTestFn: func(t *testing.T) { + waitForMariaDBAutoUserDeactivate(t, conn, autoUserKeep) + }, + }, + "auto user drop": { + user: autoUserDrop, + dbUser: autoUserDrop, + query: autoRolesQuery, + afterConnTestFn: func(t *testing.T) { + waitForMariaDBAutoUserDrop(t, conn, autoUserDrop) + }, + }, + } { + test := test + t.Run(name, func(t *testing.T) { + t.Parallel() + route := tlsca.RouteToDatabase{ + ServiceName: mariaDBName, + Protocol: defaults.ProtocolMySQL, + Username: test.dbUser, + Database: "", // not needed + } + t.Run("connect", func(t *testing.T) { + // run multiple conn tests in parallel to test parallel + // auto user connections. + t.Run("via local proxy 1", func(t *testing.T) { + t.Parallel() + mysqlLocalProxyConnTest(t, cluster, test.user, route, test.query) + }) + t.Run("via local proxy 2", func(t *testing.T) { + t.Parallel() + mysqlLocalProxyConnTest(t, cluster, test.user, route, test.query) + }) + }) + if test.afterConnTestFn != nil { + test.afterConnTestFn(t) + } + }) + } + }) } const ( @@ -638,6 +761,11 @@ func getRDSAdminInfo(t *testing.T, ctx context.Context, instanceID string) rdsAd require.NoError(t, err) require.Len(t, result.DBInstances, 1) dbInstance := result.DBInstances[0] + require.NotNil(t, dbInstance.MasterUsername) + require.NotNil(t, dbInstance.MasterUserSecret) + require.NotNil(t, dbInstance.MasterUserSecret.SecretArn) + require.NotEmpty(t, *dbInstance.MasterUsername) + require.NotEmpty(t, *dbInstance.MasterUserSecret.SecretArn) return rdsAdminInfo{ address: *dbInstance.Endpoint.Address, port: int(*dbInstance.Endpoint.Port), @@ -659,6 +787,9 @@ func getRDSMasterUserPassword(t *testing.T, ctx context.Context, secretID string // logs. require.FailNow(t, "error unmarshaling secret string") } + if len(secret.Pass) == 0 { + require.FailNow(t, "empty master user secret string") + } return secret.Pass } @@ -723,6 +854,34 @@ func provisionRDSMySQLAutoUsersAdmin(t *testing.T, ctx context.Context, conn *my require.NoError(t, err) } +// provisionMariaDBAdminUser provisions an admin user suitable for auto-user +// provisioning. +func provisionMariaDBAdminUser(t *testing.T, ctx context.Context, conn *mySQLConn, adminUser string) { + t.Helper() + // provision the IAM user to test with. + // ignore errors from user creation. If the user doesn't exist + // later steps will catch it. The error we might get back when + // another test runner already created the admin is + // unpredictable: all we need to know is the user exists for + // test setup. + _, _ = conn.Execute(fmt.Sprintf("CREATE USER IF NOT EXISTS %q IDENTIFIED WITH AWSAuthenticationPlugin AS 'RDS'", adminUser)) + + // these statements are all idempotent - they should not return + // an error even if run in parallel by many test runners. + _, err := conn.Execute(fmt.Sprintf("GRANT PROCESS, CREATE USER ON *.* TO %q", adminUser)) + require.NoError(t, err) + _, err = conn.Execute(fmt.Sprintf("GRANT SELECT ON mysql.roles_mapping to %q", adminUser)) + require.NoError(t, err) + _, err = conn.Execute(fmt.Sprintf("GRANT UPDATE ON mysql.* TO %q", adminUser)) + require.NoError(t, err) + _, err = conn.Execute(fmt.Sprintf("GRANT SELECT ON *.* TO %q", adminUser)) + require.NoError(t, err) + _, err = conn.Execute("CREATE DATABASE IF NOT EXISTS `teleport`") + require.NoError(t, err) + _, err = conn.Execute(fmt.Sprintf("GRANT ALL ON `teleport`.* TO %q WITH GRANT OPTION", adminUser)) + require.NoError(t, err) +} + // randASCII is a helper func that returns a random string of ascii characters. func randASCII(t *testing.T, length int) string { t.Helper() @@ -829,3 +988,44 @@ func waitForMySQLAutoUserDrop(t *testing.T, conn *mySQLConn, user string) { result.Close() }, autoUserWaitDur, autoUserWaitStep, "waiting for auto user %q to be dropped", user) } + +func waitForMariaDBAutoUserDeactivate(t *testing.T, conn *mySQLConn, user string) { + t.Helper() + require.EventuallyWithT(t, func(c *assert.CollectT) { + result, err := conn.Execute("SELECT 1 FROM mysql.user AS u WHERE u.user = ?", user) + if !assert.NoError(c, err) { + return + } + if !assert.Equal(c, 1, result.RowNumber(), "user %q should not have been dropped after disconnecting", user) { + result.Close() + return + } + result.Close() + + result, err = conn.Execute("SELECT 1 FROM mysql.global_priv AS u WHERE u.user = ? AND JSON_EXTRACT(u.priv, '$.account_locked') = true", user) + if !assert.NoError(c, err) { + return + } + if !assert.Equal(c, 1, result.RowNumber(), "user %q should not be able to login after deactivating", user) { + result.Close() + return + } + result.Close() + + result, err = conn.Execute("SELECT 1 FROM mysql.roles_mapping AS u WHERE u.user = ? AND u.role != 'teleport-auto-user' AND u.ADMIN_OPTION='N'", user) + if !assert.NoError(c, err) { + return + } + if !assert.Equal(c, 0, result.RowNumber(), "user %q should have lost all additional roles after deactivating", user) { + result.Close() + return + } + result.Close() + }, autoUserWaitDur, autoUserWaitStep, "waiting for auto user %q to be deactivated", user) +} + +func waitForMariaDBAutoUserDrop(t *testing.T, conn *mySQLConn, user string) { + t.Helper() + // run the same tests as mysql to check if the user was dropped. + waitForMySQLAutoUserDrop(t, conn, user) +} diff --git a/lib/srv/db/mysql/sql/mariadb_delete_user.sql b/lib/srv/db/mysql/sql/mariadb_delete_user.sql index 20fad5147c213..ce0741389d5f9 100644 --- a/lib/srv/db/mysql/sql/mariadb_delete_user.sql +++ b/lib/srv/db/mysql/sql/mariadb_delete_user.sql @@ -22,12 +22,12 @@ BEGIN CALL teleport_deactivate_user(username); ELSE SET state = 'TP003'; - SET @sql := CONCAT('DROP ROLE ', QUOTE(CONCAT("tp-role-", username))); + SET @sql := CONCAT('DROP ROLE IF EXISTS', QUOTE(CONCAT("tp-role-", username))); PREPARE stmt FROM @sql; EXECUTE stmt; DEALLOCATE PREPARE stmt; - SET @sql := CONCAT('DROP USER ', QUOTE(username)); + SET @sql := CONCAT('DROP USER IF EXISTS', QUOTE(username)); PREPARE stmt FROM @sql; EXECUTE stmt; DEALLOCATE PREPARE stmt; diff --git a/lib/srv/db/mysql/sql/mysql_delete_user.sql b/lib/srv/db/mysql/sql/mysql_delete_user.sql index 4376683730a6d..3d8c6c0ca9fb6 100644 --- a/lib/srv/db/mysql/sql/mysql_delete_user.sql +++ b/lib/srv/db/mysql/sql/mysql_delete_user.sql @@ -16,7 +16,7 @@ BEGIN -- Throw a custom error code when user is still active from other sessions. SIGNAL SQLSTATE 'TP000' SET MESSAGE_TEXT = 'User has active connections'; ELSE - SET @sql := CONCAT('DROP USER ', QUOTE(username)); + SET @sql := CONCAT('DROP USER IF EXISTS', QUOTE(username)); PREPARE stmt FROM @sql; EXECUTE stmt; DEALLOCATE PREPARE stmt; diff --git a/lib/srv/db/postgres/sql/delete-user.sql b/lib/srv/db/postgres/sql/delete-user.sql index 3c537217c0fb4..ff5a63a06b259 100644 --- a/lib/srv/db/postgres/sql/delete-user.sql +++ b/lib/srv/db/postgres/sql/delete-user.sql @@ -9,7 +9,7 @@ BEGIN RAISE NOTICE 'User has active connections'; ELSE BEGIN - EXECUTE FORMAT('DROP USER %I', username); + EXECUTE FORMAT('DROP USER IF EXISTS %I', username); EXCEPTION WHEN SQLSTATE '2BP01' THEN state := 'TP004'; diff --git a/lib/srv/db/postgres/sql/redshift-delete-user.sql b/lib/srv/db/postgres/sql/redshift-delete-user.sql index cadea00d2db57..a225fedc1cf27 100644 --- a/lib/srv/db/postgres/sql/redshift-delete-user.sql +++ b/lib/srv/db/postgres/sql/redshift-delete-user.sql @@ -10,7 +10,7 @@ BEGIN RAISE EXCEPTION 'TP000: User has active connections'; ELSE BEGIN - EXECUTE 'DROP USER ' || QUOTE_IDENT(username); + EXECUTE 'DROP USER IF EXISTS ' || QUOTE_IDENT(username); EXCEPTION WHEN OTHERS THEN -- Redshift only support OTHERS as exception condition, so we handle -- any error that might happen.