From 2e654c8bbedc911bf6354af57a18d42b5a9e9ef2 Mon Sep 17 00:00:00 2001 From: Matt Lord Date: Wed, 11 Sep 2024 19:09:10 -0400 Subject: [PATCH] Limit and specialize priv checks Signed-off-by: Matt Lord --- go/vt/vtctl/workflow/utils.go | 14 +++++----- .../tabletmanager/rpc_vreplication.go | 27 ++++++++++++++++--- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/go/vt/vtctl/workflow/utils.go b/go/vt/vtctl/workflow/utils.go index 5229816293e..6299d71369b 100644 --- a/go/vt/vtctl/workflow/utils.go +++ b/go/vt/vtctl/workflow/utils.go @@ -30,6 +30,7 @@ import ( "google.golang.org/protobuf/encoding/prototext" + "vitess.io/vitess/go/constants/sidecar" "vitess.io/vitess/go/mysql/sqlerror" "vitess.io/vitess/go/sets" "vitess.io/vitess/go/sqltypes" @@ -665,21 +666,20 @@ func areTabletsAvailableToStreamFrom(ctx context.Context, req *vtctldatapb.Workf allErrors.RecordError(fmt.Errorf("no tablet found to source data in keyspace %s, shard %s", keyspace, shard.ShardName())) return } - // Ensure the tablet has the expected privileges for a dry run. We don't do this - // for an actual execution as the privileges likely needed to perform the necessary - // work are a subset and the user may have locked things down as much as possible. - if req.GetDryRun() { + if req.GetEnableReverseReplication() { + // Ensure the tablet has the minimum privileges required on the sidecar database + // table in order to create the reverse workflow as part of the traffic switch. for _, tablet := range tablets { wg.Add(1) go func() { defer wg.Done() res, err := ts.ws.tmc.ValidateVReplicationPermissions(ctx, tablet.Tablet, nil) if err != nil { - allErrors.RecordError(vterrors.Wrapf(err, "failed to validate vreplication user permissions on tablet %s", topoproto.TabletAliasString(tablet.Alias))) + allErrors.RecordError(vterrors.Wrapf(err, "failed to validate required vreplication metadata permissions on tablet %s", topoproto.TabletAliasString(tablet.Alias))) } if !res.GetOk() { - allErrors.RecordError(fmt.Errorf("WARNING: vreplication user %s does not have the expected full set of permissions on tablet %s and may not be able to perform some actions", - res.GetUser(), topoproto.TabletAliasString(tablet.Alias))) + allErrors.RecordError(fmt.Errorf("user %s does not have the required set of permissions (insert,update,delete) on the %s.vreplication table on tablet %s", + res.GetUser(), sidecar.GetIdentifier(), topoproto.TabletAliasString(tablet.Alias))) } }() } diff --git a/go/vt/vttablet/tabletmanager/rpc_vreplication.go b/go/vt/vttablet/tabletmanager/rpc_vreplication.go index 4cbf0f59cde..764f690617c 100644 --- a/go/vt/vttablet/tabletmanager/rpc_vreplication.go +++ b/go/vt/vttablet/tabletmanager/rpc_vreplication.go @@ -61,8 +61,22 @@ const ( sqlUpdateVReplicationWorkflows = "update /*vt+ ALLOW_UNSAFE_VREPLICATION_WRITE */ %s.vreplication set%s where db_name = '%s'%s" // Check if workflow is still copying. sqlGetVReplicationCopyStatus = "select distinct vrepl_id from %s.copy_state where vrepl_id = %d" - // Validate the full set of permissions needed to execute any vreplication related work. - sqlValidateVReplicationPermissions = "select if(count(*)>0, 1, 0) as good from mysql.user where user = %a and instr(concat(select_priv, insert_priv, update_priv, delete_priv, create_priv, drop_priv, reload_priv, process_priv, file_priv, references_priv, index_priv, alter_priv, show_db_priv, create_tmp_table_priv, lock_tables_priv, execute_priv, repl_slave_priv, repl_client_priv, create_view_priv, show_view_priv, create_routine_priv, alter_routine_priv, create_user_priv, event_priv, trigger_priv), 'N') = 0" + // Validate the minimum set of permissions needed to manage vreplication metadata. + sqlValidateVReplicationPermissions = ` +select if(count(*)>0, 1, 0) as good from mysql.user as u + left join mysql.db as d on (u.user = d.user) + left join mysql.tables_priv as t on (u.user = t.user) +where u.user = %a + and ( + (u.insert_priv = 'y' and u.update_priv = 'y' and u.delete_priv = 'y') + or (d.db = %a and u.insert_priv = 'y' and u.update_priv = 'y' and u.delete_priv = 'y') + or (t.db = %a and t.table_name = 'vreplication' + and find_in_set('insert', t.table_priv) + and find_in_set('update', t.table_priv) + and find_in_set('delete', t.table_priv) + ) + ) +` ) var ( @@ -534,9 +548,14 @@ func (tm *TabletManager) UpdateVReplicationWorkflows(ctx context.Context, req *t } // ValidateVReplicationPermissions validates that the --db_filtered_user has -// the full expected set of global permissions. +// the minimum permissions required on the sidecardb vreplication table +// needed in order to manager vreplication metatdata. func (tm *TabletManager) ValidateVReplicationPermissions(ctx context.Context, req *tabletmanagerdatapb.ValidateVReplicationPermissionsRequest) (*tabletmanagerdatapb.ValidateVReplicationPermissionsResponse, error) { - query, err := sqlparser.ParseAndBind(sqlValidateVReplicationPermissions, sqltypes.StringBindVariable(tm.DBConfigs.Filtered.User)) + query, err := sqlparser.ParseAndBind(sqlValidateVReplicationPermissions, + sqltypes.StringBindVariable(tm.DBConfigs.Filtered.User), + sqltypes.StringBindVariable(sidecar.GetName()), + sqltypes.StringBindVariable(sidecar.GetName()), + ) if err != nil { return nil, err }