Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-23610

Slave user can't run "SHOW SLAVE STATUS" anymore after upgrade to 10.5, mysql_upgrade should take of that

Details

    Description

      A user that had REPLICATION SLAVE, REPLICATION CLIENT privileges before upgrade to 10.5, has REPLICATION SLAVE, BINLOG MONITOR after upgrade, but is not able to run SHOW SLAVE STATUS anymore, as it is missing the new REPLICATION SLAVE ADMIN privilege now which is necessary for this in 10.5.

      IMHO mysql_upgrade should take care of replacing REPLICATION CLIENT with BINLOG MONITOR, REPLICATION SLAVE ADMIN when doing a pre-10.5 to 10.5 or later upgrade.

      Same for REPLICATION SLAVE, that should become REPLICATION SLAVE, REPLICATION MASTER ADMIN, REPLICATION SLAVE ADMIN instead on upgrades to 10.5., so that existing users can still continue to use SHOW SLAVE HOSTS and SHOW RELAYLOG EVENTS.

      Attachments

        Issue Links

          Activity

            bar Alexander Barkov added a comment - - edited

            I agree with serg, this is a balance, so we tried not to introduce too many new privileges. But if the choice is between:

            • (a) binding a feature to multiple privileges, or
            • (b) adding a new dedicated privilege for this feature

            then I'd probably go with the latter (b).

            I think (b) should be easier for DBAs to remember, when every protected feature is consistently controlled by a single privilege, and only by this privilege.

            (with SUPER being the only exception)

            bar Alexander Barkov added a comment - - edited I agree with serg , this is a balance, so we tried not to introduce too many new privileges. But if the choice is between: (a) binding a feature to multiple privileges, or (b) adding a new dedicated privilege for this feature then I'd probably go with the latter (b). I think (b) should be easier for DBAs to remember, when every protected feature is consistently controlled by a single privilege, and only by this privilege. (with SUPER being the only exception)
            maxmether Max Mether added a comment -

            I think they key here is that functionality should not be different after an upgrade. If a user had REPLICATION SLAVE and REPLICATION CLIENT in 10.4 he should be able to do the same functions after an upgrade to 10.5. I think that is the baseline.

            So either 1) we do what Geoff's table suggests above (ie replace the old REPLICATION SLAVE with REPLICATION SLAVE, REPLICATION MASTER ADMIN, REPLICATION SLAVE ADMIN and replace the old REPLICATION CLIENT with BINLOG MONITOR, REPLICATION SLAVE ADMIN) or 2) we introduce a new privilege that allows him to just issue SHOW SLAVE STATUS

            I think 1) is easier and less intrusive but I would be fine with 2) as well. We just need to make sure that people can upgrade without it affecting their application.

            maxmether Max Mether added a comment - I think they key here is that functionality should not be different after an upgrade. If a user had REPLICATION SLAVE and REPLICATION CLIENT in 10.4 he should be able to do the same functions after an upgrade to 10.5. I think that is the baseline. So either 1) we do what Geoff's table suggests above (ie replace the old REPLICATION SLAVE with REPLICATION SLAVE, REPLICATION MASTER ADMIN, REPLICATION SLAVE ADMIN and replace the old REPLICATION CLIENT with BINLOG MONITOR, REPLICATION SLAVE ADMIN) or 2) we introduce a new privilege that allows him to just issue SHOW SLAVE STATUS I think 1) is easier and less intrusive but I would be fine with 2) as well. We just need to make sure that people can upgrade without it affecting their application.

            MDEV-23899 was closed as a duplicate of this one.

            elenst Elena Stepanova added a comment - MDEV-23899 was closed as a duplicate of this one.

            Hello Sergie,

            Can you please review the fix.

            Patch: https://github.com/MariaDB/server/commit/d66d5457589dba526b358183ea795ddd860f45e6

            Buildbot testing: http://buildbot.askmonty.org/buildbot/grid?category=main&branch=bb-10.5-sujatha

            Most of the fix is implemented as an extension to MDEV-21743 except the following change.

            diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
            index ef96e8f2484..8a3551c14f5 100644
            --- a/sql/sql_acl.cc
            +++ b/sql/sql_acl.cc
            @@ -1057,6 +1057,9 @@ class User_table_tabular: public User_table
                 if (access & REPL_SLAVE_ACL)
                   access|= REPL_MASTER_ADMIN_ACL;
             
            +    if (access & FILE_ACL)
            +      access|= REPLICA_MONITOR_ACL;
            +
                 return access & GLOBAL_ACLS;
               }
             

            Initially I added the changes as part of

            diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
            index 8a3551c14f5..a0118a1fd13 100644
            --- a/sql/sql_acl.cc
            +++ b/sql/sql_acl.cc
            @@ -1015,7 +1015,7 @@ class User_table_tabular: public User_table
              if (num_fields() <= 18)
                {
                  access|= LOCK_TABLES_ACL | CREATE_TMP_ACL | SHOW_DB_ACL;
                   if (access & FILE_ACL)
                     access|= BINLOG_MONITOR_ACL | REPL_SLAVE_ACL | BINLOG_ADMIN_ACL |
            -                 BINLOG_REPLAY_ACL;
            +                BINLOG_REPLAY_ACL | REPLICA_MONITOR_ACL;
                   if (access & PROCESS_ACL)
                     access|= SUPER_ACL | EXECUTE_ACL;
                 }
            

            But it breaks main.grant.test

            -GRANT ALL PRIVILEGES ON *.* TO `ten2`@`%`
            +GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, RELOAD, SHUTDOWN, PROCESS, FILE, REFERENCES, INDEX, ALTER, SHOW DATABASES, SUPER, CREATE TEMPORARY TABLES, LOCK TABLES, EXECUTE, REPLICATION SLAVE, BINLOG MONITOR, CREATE VIEW, SHOW VIEW, CREATE ROUTINE, ALTER ROUTINE, CREATE USER, EVENT, TRIGGER, CREATE TABLESPACE, DELETE HISTORY, SET USER, FEDERATED ADMIN, CONNECTION ADMIN, READ_ONLY ADMIN, REPLICATION SLAVE ADMIN, REPLICATION MASTER ADMIN, BINLOG ADMIN, BINLOG REPLAY ON *.* TO `ten2`@`%`
            

            bar suggested that "The new bit should probably also go inside some of if (num_fields() <= XXX) code branches."

            Hence I added the new bit setting part to generic code. Please suggest if it fine or needs refinement.

            Thank you.

            sujatha.sivakumar Sujatha Sivakumar (Inactive) added a comment - Hello Sergie, Can you please review the fix. Patch: https://github.com/MariaDB/server/commit/d66d5457589dba526b358183ea795ddd860f45e6 Buildbot testing: http://buildbot.askmonty.org/buildbot/grid?category=main&branch=bb-10.5-sujatha Most of the fix is implemented as an extension to MDEV-21743 except the following change. diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index ef96e8f2484..8a3551c14f5 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -1057,6 +1057,9 @@ class User_table_tabular: public User_table if (access & REPL_SLAVE_ACL) access|= REPL_MASTER_ADMIN_ACL; + if (access & FILE_ACL) + access|= REPLICA_MONITOR_ACL; + return access & GLOBAL_ACLS; } Initially I added the changes as part of diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 8a3551c14f5..a0118a1fd13 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -1015,7 +1015,7 @@ class User_table_tabular: public User_table if (num_fields() <= 18) { access|= LOCK_TABLES_ACL | CREATE_TMP_ACL | SHOW_DB_ACL; if (access & FILE_ACL) access|= BINLOG_MONITOR_ACL | REPL_SLAVE_ACL | BINLOG_ADMIN_ACL | - BINLOG_REPLAY_ACL; + BINLOG_REPLAY_ACL | REPLICA_MONITOR_ACL; if (access & PROCESS_ACL) access|= SUPER_ACL | EXECUTE_ACL; } But it breaks main.grant.test -GRANT ALL PRIVILEGES ON *.* TO `ten2`@`%` +GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, RELOAD, SHUTDOWN, PROCESS, FILE, REFERENCES, INDEX, ALTER, SHOW DATABASES, SUPER, CREATE TEMPORARY TABLES, LOCK TABLES, EXECUTE, REPLICATION SLAVE, BINLOG MONITOR, CREATE VIEW, SHOW VIEW, CREATE ROUTINE, ALTER ROUTINE, CREATE USER, EVENT, TRIGGER, CREATE TABLESPACE, DELETE HISTORY, SET USER, FEDERATED ADMIN, CONNECTION ADMIN, READ_ONLY ADMIN, REPLICATION SLAVE ADMIN, REPLICATION MASTER ADMIN, BINLOG ADMIN, BINLOG REPLAY ON *.* TO `ten2`@`%` bar suggested that "The new bit should probably also go inside some of if (num_fields() <= XXX) code branches." Hence I added the new bit setting part to generic code. Please suggest if it fine or needs refinement. Thank you.
            Elkin Andrei Elkin added a comment - - edited

            nicklamb: The summary of the fixes

            • a new (10.5.9 birthversion) SLAVE MONITOR privilege allows SHOW SLAVE STATUS, SHOW RELAYLOG EVENTS
            • 10.5.1 and earlier REPLICATION CLIENT and REPLICATION SLAVE privileges are upgraded in 10.5.9 (and up) so that their users automatically (no mysql_upgrade is required that is mere server restart suffices) receive SLAVE MONITOR
            • REPLICATION SLAVE ADMIN of 10.5.2-8 is reduced to disallow SHOW SLAVE STATUS, SHOW RELAYLOG EVENTS starting from 10.5.9 in favor of the new SLAVE MONITOR
            • nothing extra is done at upgrading from 10.5.2-8 to 10.5.9 as its impossible to distinguish
              the version in which a privilege was granted.
            Elkin Andrei Elkin added a comment - - edited nicklamb : The summary of the fixes a new (10.5.9 birthversion) SLAVE MONITOR privilege allows SHOW SLAVE STATUS , SHOW RELAYLOG EVENTS 10.5.1 and earlier REPLICATION CLIENT and REPLICATION SLAVE privileges are upgraded in 10.5.9 (and up) so that their users automatically (no mysql_upgrade is required that is mere server restart suffices) receive SLAVE MONITOR REPLICATION SLAVE ADMIN of 10.5.2-8 is reduced to disallow SHOW SLAVE STATUS , SHOW RELAYLOG EVENTS starting from 10.5.9 in favor of the new SLAVE MONITOR nothing extra is done at upgrading from 10.5.2-8 to 10.5.9 as its impossible to distinguish the version in which a privilege was granted.

            People

              sujatha.sivakumar Sujatha Sivakumar (Inactive)
              hholzgra Hartmut Holzgraefe
              Votes:
              0 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.