Port fixes from v8 (#9397)

* Update oxy
* Do not allow MySQL COM_CHANGE_USER command
* Add support for all MongoDB wire messages
* Drone fix

.drone.yml

@@ -4245,6 +4245,10 @@ steps:
     image: docker
     commands:
       - |
+        if [ "${DRONE_REPO}" != "gravitational/teleport" ]; then
+          echo "---> Not publishing ${DRONE_REPO} packages to repos"
+          exit 78
+        fi
         # length will be 0 after filtering if this is a pre-release, >0 otherwise
         FILTERED_TAG_LENGTH=$(echo ${DRONE_TAG} | egrep -v '(alpha|beta|dev|rc)' | wc -c)
         if [ $$FILTERED_TAG_LENGTH -eq 0 ]; then
@@ -4411,6 +4415,6 @@ volumes:
       name: drone-s3-debrepo-pvc
 ---
 kind: signature
-hmac: 5049d1a961c95216d5131cd9ff35e3f234d74ada94a26dd57dcd8ee36598aeee
+hmac: 49638e91f2ecd4fccf05988f89a2b7515d24863fbb1a3a323a6bed4682b8d3eb
 
 ...

docs/pages/database-access/guides/mysql-self-hosted.mdx

@@ -36,11 +36,16 @@ ssl-key=/path/to/server.key
 ```
 
 Additionally, MySQL database user accounts must be configured to require a
-valid client certificate:
+valid client certificate. If you're creating a new user:
 
 ```sql
-CREATE USER 'alice'@'%' REQUIRE X509;
-ALTER USER 'alice'@'%' REQUIRE X509;
+CREATE USER 'alice'@'%' REQUIRE SUBJECT '/CN=alice';
+```
+
+If you're updating an existing user:
+
+```sql
+ALTER USER 'alice'@'%' REQUIRE SUBJECT '/CN=alice';
 ```
 
 By default the created user may not have access to anything and won't be able

go.mod

@@ -45,7 +45,7 @@ require (
 	github.com/gravitational/form v0.0.0-20151109031454-c4048f792f70
 	github.com/gravitational/kingpin v2.1.11-0.20190130013101-742f2714c145+incompatible
 	github.com/gravitational/license v0.0.0-20210218173955-6d8fb49b117a
-	github.com/gravitational/oxy v0.0.0-20210316180922-c73d80d27348
+	github.com/gravitational/oxy v0.0.0-20211213172937-a1ba0900a4c9
 	github.com/gravitational/reporting v0.0.0-20180907002058-ac7b85c75c4c
 	github.com/gravitational/roundtrip v1.0.1
 	github.com/gravitational/teleport/api v0.0.0

go.sum

@@ -389,8 +389,8 @@ github.com/gravitational/license v0.0.0-20210218173955-6d8fb49b117a h1:PN5vAN1ZA
 github.com/gravitational/license v0.0.0-20210218173955-6d8fb49b117a/go.mod h1:jaxS7X2ouXfNd2Pxpybd01qNQK15UmkixKj4vtpp7f8=
 github.com/gravitational/logrus v1.4.4-0.20210817004754-047e20245621 h1:ivU//THqy3/TMz7Bx6VbLbcVebBJLSOpvfcASbLlT1s=
 github.com/gravitational/logrus v1.4.4-0.20210817004754-047e20245621/go.mod h1:IIxugQsS57BiOTe+8zDv3sfnvM2BQ3smcF1xJdj3Has=
-github.com/gravitational/oxy v0.0.0-20210316180922-c73d80d27348 h1:QFHxHmxOeQT+MwFjNHkTVI7mkgPIabTMqkmHG5SiP2Y=
-github.com/gravitational/oxy v0.0.0-20210316180922-c73d80d27348/go.mod h1:ESOxlf8BB2yG3zJ0SfZe9U6wpYu3YF3znxIICg73FYA=
+github.com/gravitational/oxy v0.0.0-20211213172937-a1ba0900a4c9 h1:7NyppZS8QFt28nn2QjDI44vDTJs0kTRdUS7po/AxWOY=
+github.com/gravitational/oxy v0.0.0-20211213172937-a1ba0900a4c9/go.mod h1:ESOxlf8BB2yG3zJ0SfZe9U6wpYu3YF3znxIICg73FYA=
 github.com/gravitational/protobuf v1.3.2-0.20201123192827-2b9fcfaffcbf h1:MQ4e8XcxvZTeuOmRl7yE519vcWc2h/lyvYzsvt41cdY=
 github.com/gravitational/protobuf v1.3.2-0.20201123192827-2b9fcfaffcbf/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o=
 github.com/gravitational/reporting v0.0.0-20180907002058-ac7b85c75c4c h1:UwN3jo2EfZSGDchLVqH/EJ2A5GWvKROx3NJNUI6/plg=

lib/srv/db/access_test.go

@@ -49,9 +49,12 @@ import (
 	"github.com/jonboulle/clockwork"
 	"github.com/pborman/uuid"
 	"github.com/siddontang/go-mysql/client"
+	mysqllib "github.com/siddontang/go-mysql/mysql"
 	"github.com/stretchr/testify/require"
 	"go.mongodb.org/mongo-driver/bson"
 	"go.mongodb.org/mongo-driver/mongo"
+	"go.mongodb.org/mongo-driver/mongo/options"
+	"go.mongodb.org/mongo-driver/x/mongo/driver/wiremessage"
 )
 
 func TestMain(m *testing.M) {
@@ -297,12 +300,47 @@ func TestMySQLBadHandshake(t *testing.T) {
 	}
 }
 
+// TestAccessMySQLChangeUser verifies that COM_CHANGE_USER command is rejected.
+func TestAccessMySQLChangeUser(t *testing.T) {
+	ctx := context.Background()
+	testCtx := setupTestContext(ctx, t, withSelfHostedMySQL("mysql"))
+	go testCtx.startHandlingConnections()
+
+	// Create user/role with the requested permissions.
+	testCtx.createUserAndRole(ctx, t, "alice", "admin", []string{"alice"}, []string{types.Wildcard})
+
+	// Connect to the database as this user.
+	mysqlConn, err := testCtx.mysqlClient("alice", "mysql", "alice")
+	require.NoError(t, err)
+
+	// Send COM_CHANGE_USER command. The driver doesn't support it natively so
+	// assemble the raw packet and send it which should be enough to test the
+	// rejection logic.
+	packet := []byte{
+		0x05,                     // Payload length.
+		0x00,                     // Payload length cont'd.
+		0x00,                     // Payload length cont'd.
+		0x00,                     // Sequence number.
+		mysqllib.COM_CHANGE_USER, // Command type.
+		'b',                      // Null-terminated string with new user name.
+		'o',
+		'b',
+		0x00,
+		// There would've been other fields in "real" packet but these will
+		// do for the test to detect the command.
+	}
+	err = mysqlConn.WritePacket(packet)
+	require.NoError(t, err)
+
+	// Connection should've been closed so any attempt to use it should fail.
+	_, err = mysqlConn.Execute("select 1")
+	require.Error(t, err)
+}
+
 // TestAccessMongoDB verifies access scenarios to a MongoDB database based
 // on the configured RBAC rules.
 func TestAccessMongoDB(t *testing.T) {
 	ctx := context.Background()
-	testCtx := setupTestContext(ctx, t, withSelfHostedMongo("mongo"))
-	go testCtx.startHandlingConnections()
 
 	tests := []struct {
 		desc         string
@@ -360,7 +398,7 @@ func TestAccessMongoDB(t *testing.T) {
 			queryErr:     "",
 		},
 		{
-			desc:         "access allowed to specific user/database",
+			desc:         "access allowed to specific user and database",
 			user:         "alice",
 			role:         "admin",
 			allowDbNames: []string{"admin"},
@@ -371,7 +409,7 @@ func TestAccessMongoDB(t *testing.T) {
 			queryErr:     "",
 		},
 		{
-			desc:         "access denied to specific user/database",
+			desc:         "access denied to specific user and database",
 			user:         "alice",
 			role:         "admin",
 			allowDbNames: []string{"admin"},
@@ -383,35 +421,76 @@ func TestAccessMongoDB(t *testing.T) {
 		},
 	}
 
-	for _, test := range tests {
-		t.Run(test.desc, func(t *testing.T) {
-			// Create user/role with the requested permissions.
-			testCtx.createUserAndRole(ctx, t, test.user, test.role, test.allowDbUsers, test.allowDbNames)
-
-			// Try to connect to the database as this user.
-			client, err := testCtx.mongoClient(ctx, test.user, "mongo", test.dbUser)
-			if test.connectErr != "" {
-				require.Error(t, err)
-				require.Contains(t, err.Error(), test.connectErr)
-				return
-			}
+	// Each scenario is executed multiple times with different server/client
+	// options to test things like legacy MongoDB servers and clients that
+	// use compression.
+	serverOpts := []struct {
+		name string
+		opts []mongodb.TestServerOption
+	}{
+		{
+			name: "new server",
+			opts: []mongodb.TestServerOption{},
+		},
+		{
+			name: "old server",
+			opts: []mongodb.TestServerOption{
+				mongodb.TestServerWireVersion(wiremessage.OpmsgWireVersion - 1),
+			},
+		},
+	}
 
-			require.NoError(t, err)
+	clientOpts := []struct {
+		name string
+		opts *options.ClientOptions
+	}{
+		{
+			name: "client without compression",
+			opts: options.Client(),
+		},
+		{
+			name: "client with compression",
+			opts: options.Client().SetCompressors([]string{"zlib"}),
+		},
+	}
 
-			// Execute a "find" command. Collection name doesn't matter currently.
-			_, err = client.Database(test.dbName).Collection("test").Find(ctx, bson.M{})
-			if test.queryErr != "" {
-				require.Error(t, err)
-				require.Contains(t, err.Error(), test.queryErr)
-				return
+	// Execute each scenario on both modern and legacy Mongo servers
+	// to make sure legacy messages are also subject to RBAC.
+	for _, test := range tests {
+		for _, serverOpt := range serverOpts {
+			for _, clientOpt := range clientOpts {
+				t.Run(fmt.Sprintf("%v/%v/%v", serverOpt.name, clientOpt.name, test.desc), func(t *testing.T) {
+					testCtx := setupTestContext(ctx, t, withSelfHostedMongo("mongo", serverOpt.opts...))
+					go testCtx.startHandlingConnections()
+
+					// Create user/role with the requested permissions.
+					testCtx.createUserAndRole(ctx, t, test.user, test.role, test.allowDbUsers, test.allowDbNames)
+
+					// Try to connect to the database as this user.
+					client, err := testCtx.mongoClient(ctx, test.user, "mongo", test.dbUser, clientOpt.opts)
+					defer func() {
+						if client != nil {
+							client.Disconnect(ctx)
+						}
+					}()
+					if test.connectErr != "" {
+						require.Error(t, err)
+						require.Contains(t, err.Error(), test.connectErr)
+						return
+					}
+					require.NoError(t, err)
+
+					// Execute a "find" command. Collection name doesn't matter currently.
+					_, err = client.Database(test.dbName).Collection("test").Find(ctx, bson.M{})
+					if test.queryErr != "" {
+						require.Error(t, err)
+						require.Contains(t, err.Error(), test.queryErr)
+						return
+					}
+					require.NoError(t, err)
+				})
 			}
-
-			require.NoError(t, err)
-
-			// Disconnect.
-			err = client.Disconnect(ctx)
-			require.NoError(t, err)
-		})
+		}
 	}
 }
 
@@ -683,12 +762,12 @@ func (c *testContext) mysqlClientWithAddr(address, teleportUser, dbService, dbUs
 
 // mongoClient connects to test MongoDB through database access as a
 // specified Teleport user and database account.
-func (c *testContext) mongoClient(ctx context.Context, teleportUser, dbService, dbUser string) (*mongo.Client, error) {
-	return c.mongoClientWithAddr(ctx, c.webListener.Addr().String(), teleportUser, dbService, dbUser)
+func (c *testContext) mongoClient(ctx context.Context, teleportUser, dbService, dbUser string, opts ...*options.ClientOptions) (*mongo.Client, error) {
+	return c.mongoClientWithAddr(ctx, c.webListener.Addr().String(), teleportUser, dbService, dbUser, opts...)
 }
 
 // mongoClientWithAddr is like mongoClient but allows to override connection address.
-func (c *testContext) mongoClientWithAddr(ctx context.Context, address, teleportUser, dbService, dbUser string) (*mongo.Client, error) {
+func (c *testContext) mongoClientWithAddr(ctx context.Context, address, teleportUser, dbService, dbUser string, opts ...*options.ClientOptions) (*mongo.Client, error) {
 	return mongodb.MakeTestClient(ctx, common.TestClientConfig{
 		AuthClient: c.authClient,
 		AuthServer: c.authServer,
@@ -700,7 +779,7 @@ func (c *testContext) mongoClientWithAddr(ctx context.Context, address, teleport
 			Protocol:    defaults.ProtocolMongoDB,
 			Username:    dbUser,
 		},
-	})
+	}, opts...)
 }
 
 // createUserAndRole creates Teleport user and role with specified names
@@ -1250,12 +1329,12 @@ func withAzureMySQL(name, authUser, authToken string) withDatabaseOption {
 	}
 }
 
-func withSelfHostedMongo(name string) withDatabaseOption {
+func withSelfHostedMongo(name string, opts ...mongodb.TestServerOption) withDatabaseOption {
 	return func(t *testing.T, ctx context.Context, testCtx *testContext) types.Database {
 		mongoServer, err := mongodb.NewTestServer(common.TestServerConfig{
 			Name:       name,
 			AuthClient: testCtx.authClient,
-		})
+		}, opts...)
 		require.NoError(t, err)
 		go mongoServer.Serve()
 		t.Cleanup(func() { mongoServer.Close() })

lib/srv/db/mongodb/engine.go

@@ -19,7 +19,6 @@ package mongodb
 import (
 	"context"
 	"net"
-	"strings"
 
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/services"
@@ -179,36 +178,45 @@ func (e *Engine) authorizeConnection(ctx context.Context, sessionCtx *common.Ses
 // Each MongoDB command contains information about the database it's run in
 // so we check it against allowed databases in the user's role.
 func (e *Engine) authorizeClientMessage(sessionCtx *common.Session, message protocol.Message) error {
-	// Mongo uses OP_MSG for most operations now.
-	msg, ok := message.(*protocol.MessageOpMsg)
-	if !ok {
-		return nil
-	}
-	// Each message has a database information in it.
-	database := msg.GetDatabase()
-	if database == "" {
-		e.Log.Warnf("No database info in message: %v.", message)
-		return nil
+	// Each client message should have database information in it.
+	database, err := message.GetDatabase()
+	if err != nil {
+		return trace.Wrap(err)
 	}
-	dbRoleMatchers := role.DatabaseRoleMatchers(
-		defaults.ProtocolMongoDB,
-		sessionCtx.DatabaseUser,
-		database,
-	)
-	err := sessionCtx.Checker.CheckAccess(
-		sessionCtx.Database,
-		services.AccessMFAParams{Verified: true},
-		dbRoleMatchers...,
-	)
-	e.Audit.OnQuery(e.Context, sessionCtx, common.Query{
-		Database: msg.GetDatabase(),
-		// Commands may consist of multiple bson documents.
-		Query: strings.Join(msg.GetDocumentsAsStrings(), ", "),
-		Error: err,
+	err = e.checkClientMessage(sessionCtx, message, database)
+	defer e.Audit.OnQuery(e.Context, sessionCtx, common.Query{
+		Database: database,
+		Query:    message.String(),
+		Error:    err,
 	})
 	return trace.Wrap(err)
 }
 
+func (e *Engine) checkClientMessage(sessionCtx *common.Session, message protocol.Message, database string) error {
+	// Legacy OP_KILL_CURSORS command doesn't contain database information.
+	if _, ok := message.(*protocol.MessageOpKillCursors); ok {
+		return sessionCtx.Checker.CheckAccess(sessionCtx.Database,
+			services.AccessMFAParams{Verified: true},
+			&services.DatabaseUserMatcher{User: sessionCtx.DatabaseUser})
+	}
+	// Do not allow certain commands that deal with authentication.
+	command, err := message.GetCommand()
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	switch command {
+	case "authenticate", "saslStart", "saslContinue", "logout":
+		return trace.AccessDenied("access denied")
+	}
+	// Otherwise authorize the command against allowed databases.
+	return sessionCtx.Checker.CheckAccess(sessionCtx.Database,
+		services.AccessMFAParams{Verified: true},
+		role.DatabaseRoleMatchers(
+			defaults.ProtocolMongoDB,
+			sessionCtx.DatabaseUser,
+			database)...)
+}
+
 func (e *Engine) replyError(clientConn net.Conn, replyTo protocol.Message, err error) {
 	errSend := protocol.ReplyError(clientConn, replyTo, err)
 	if errSend != nil {

lib/srv/db/mongodb/protocol/errors.go

@@ -27,6 +27,9 @@ import (
 
 // ReplyError sends error wire message to the client.
 func ReplyError(clientConn net.Conn, replyTo Message, clientErr error) (err error) {
+	if msgCompressed, ok := replyTo.(*MessageOpCompressed); ok {
+		replyTo = msgCompressed.GetOriginal()
+	}
 	var errMessage Message
 	switch replyTo.(type) {
 	case *MessageOpMsg: // When client request is OP_MSG, reply should be OP_MSG as well.