diff --git a/go/cmd/vtctldclient/command/permissions.go b/go/cmd/vtctldclient/command/permissions.go index 25f0357582e..d9276a7a829 100644 --- a/go/cmd/vtctldclient/command/permissions.go +++ b/go/cmd/vtctldclient/command/permissions.go @@ -24,6 +24,7 @@ import ( "vitess.io/vitess/go/cmd/vtctldclient/cli" "vitess.io/vitess/go/vt/topo/topoproto" + tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata" ) @@ -56,6 +57,26 @@ var ( } ) +// redactUserPermissions ensures sensitive info is redacted from a +// *tabletmanagerdatapb.Permissions response. +func redactUserPermissions(perms *tabletmanagerdatapb.Permissions) { + if perms == nil { + return + } + for _, up := range perms.UserPermissions { + if up == nil { + continue + } + if up.Privileges != nil { + // Remove the "authentication_string" field, which is a + // sensitive field from the mysql.users table. This is + // redacted server-side in v23+ so this line can be + // removed in the future. + delete(up.Privileges, "authentication_string") + } + } +} + func commandGetPermissions(cmd *cobra.Command, args []string) error { alias, err := topoproto.ParseTabletAlias(cmd.Flags().Arg(0)) if err != nil { @@ -70,13 +91,9 @@ func commandGetPermissions(cmd *cobra.Command, args []string) error { if err != nil { return err } - // Obfuscate the checksum so as not to potentially display sensitive info. + // Obfuscate the secrets so as not to potentially display sensitive info. if resp != nil && resp.Permissions != nil { - for _, up := range resp.Permissions.UserPermissions { - if up != nil { - up.PasswordChecksum = 0 - } - } + redactUserPermissions(resp.Permissions) } cli.DefaultMarshalOptions.EmitUnpopulated = false p, err := cli.MarshalJSON(resp.Permissions) diff --git a/go/cmd/vtctldclient/command/permissions_test.go b/go/cmd/vtctldclient/command/permissions_test.go index a9ddba36f7f..1ed47f25fb4 100644 --- a/go/cmd/vtctldclient/command/permissions_test.go +++ b/go/cmd/vtctldclient/command/permissions_test.go @@ -33,6 +33,7 @@ import ( "vitess.io/vitess/go/vt/topo/memorytopo" querypb "vitess.io/vitess/go/vt/proto/query" + tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" topodatapb "vitess.io/vitess/go/vt/proto/topodata" vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata" ) @@ -606,3 +607,27 @@ func TestPermissions(t *testing.T) { }) require.ErrorContains(t, err, "has an extra user") } + +func TestRedactUserPermissions(t *testing.T) { + perms := &tabletmanagerdatapb.Permissions{ + UserPermissions: []*tabletmanagerdatapb.UserPermission{ + { + Host: "%", + User: "vt", + Privileges: map[string]string{ + "authentication_string": "this should be removed from the response", + }, + }, + }, + } + redactUserPermissions(perms) + require.EqualValues(t, &tabletmanagerdatapb.Permissions{ + UserPermissions: []*tabletmanagerdatapb.UserPermission{ + { + Host: "%", + User: "vt", + Privileges: map[string]string{}, + }, + }, + }, perms) +} diff --git a/go/vt/mysqlctl/tmutils/permissions.go b/go/vt/mysqlctl/tmutils/permissions.go index 57c8922adfa..74d31d4dde9 100644 --- a/go/vt/mysqlctl/tmutils/permissions.go +++ b/go/vt/mysqlctl/tmutils/permissions.go @@ -73,6 +73,9 @@ func NewUserPermission(fields []*querypb.Field, values []sqltypes.Value) *tablet case "password_last_changed": // we skip this one, as the value may be // different on primary and replicas. + case "authentication_string": + // skip authentication_string as it + // may contain a password hash. default: up.Privileges[field.Name] = values[i].ToString() } diff --git a/go/vt/mysqlctl/tmutils/permissions_test.go b/go/vt/mysqlctl/tmutils/permissions_test.go index 6305772147a..25fe1b58d8d 100644 --- a/go/vt/mysqlctl/tmutils/permissions_test.go +++ b/go/vt/mysqlctl/tmutils/permissions_test.go @@ -19,9 +19,10 @@ package tmutils import ( "testing" + "github.com/stretchr/testify/require" + "vitess.io/vitess/go/sqltypes" querypb "vitess.io/vitess/go/vt/proto/query" - tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" ) @@ -60,6 +61,29 @@ func testPermissionsDiff(t *testing.T, left, right *tabletmanagerdatapb.Permissi } } +func TestNewUserPermission(t *testing.T) { + up := NewUserPermission(mapToSQLResults(map[string]string{ + "Host": "%", + "User": "vt", + "Password": "correct horse battery staple", + "Select_priv": "Y", + "Insert_priv": "N", + // Test the next field is skipped (to avoid date drifts). + "password_last_changed": "2016-11-08 02:56:23", + // Test the next field is filtered. + "authentication_string": "this should be filtered out", + })) + require.EqualValues(t, &tabletmanagerdatapb.UserPermission{ + Host: "%", + User: "vt", + PasswordChecksum: 17759204488013904955, + Privileges: map[string]string{ + "Insert_priv": "N", + "Select_priv": "Y", + }, + }, up) +} + func TestPermissionsDiff(t *testing.T) { p1 := &tabletmanagerdatapb.Permissions{}