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

            hholzgra Hartmut Holzgraefe created issue -
            GeoffMontee Geoff Montee (Inactive) made changes -
            Field Original Value New Value
            GeoffMontee Geoff Montee (Inactive) made changes -
            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 HOTSTS }} and {{ SHOW RELAYLOG EVENTS }}.
            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 HOTSTS}} and {{SHOW RELAYLOG EVENTS}}.
            GeoffMontee Geoff Montee (Inactive) made changes -
            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 HOTSTS}} and {{SHOW RELAYLOG EVENTS}}.
            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}}.

            Should users with SUPER privileges also be granted the other new privileges after upgrading to 10.5? e.g. a mapping like this:

            Pre-Upgrade Privileges Post-Upgrade Privileges
            REPLICATION CLIENT BINLOG MONITOR, REPLICATION SLAVE ADMIN
            REPLICATION SLAVE REPLICATION SLAVE, REPLICATION MASTER ADMIN, REPLICATION SLAVE ADMIN
            SUPER SUPER, BINLOG ADMIN, BINLOG REPLAY, CONNECTION ADMIN, FEDERATED ADMIN, READ ONLY ADMIN, REPLICATION MASTER ADMIN, REPLICATION SLAVE ADMIN, SET USER
            GeoffMontee Geoff Montee (Inactive) added a comment - Should users with SUPER privileges also be granted the other new privileges after upgrading to 10.5? e.g. a mapping like this: Pre-Upgrade Privileges Post-Upgrade Privileges REPLICATION CLIENT BINLOG MONITOR, REPLICATION SLAVE ADMIN REPLICATION SLAVE REPLICATION SLAVE, REPLICATION MASTER ADMIN, REPLICATION SLAVE ADMIN SUPER SUPER, BINLOG ADMIN, BINLOG REPLAY, CONNECTION ADMIN, FEDERATED ADMIN, READ ONLY ADMIN, REPLICATION MASTER ADMIN, REPLICATION SLAVE ADMIN, SET USER
            serg Sergei Golubchik made changes -
            serg Sergei Golubchik made changes -

            This is intentional and needs to be documented.

            REPLICATION SLAVE ADMIN allows START SLAVE, STOP SLAVE, and CHANGE MASTER statements. This is not the kind of power you want to grant automatically to everyone who used to do SHOW SLAVE STATUS.

            serg Sergei Golubchik added a comment - This is intentional and needs to be documented. REPLICATION SLAVE ADMIN allows START SLAVE , STOP SLAVE , and CHANGE MASTER statements. This is not the kind of power you want to grant automatically to everyone who used to do SHOW SLAVE STATUS .
            serg Sergei Golubchik made changes -
            Component/s Documentation [ 10903 ]
            elenst Elena Stepanova made changes -
            Fix Version/s N/A [ 14700 ]
            Assignee Geoff Montee [ geoffmontee ]

            The core issue is that SHOW SLAVE STATUS worked for a user before upgrade and no longer works (without manual intervention afterwards). This is an issue for customers as noted in the original report who expect previously working commands to function after upgrade.

            Is there a less permissive option that allows just SHOW SLAVE STATUS or does that now give more privileges than it did previously?

            nicklamb Nick (Inactive) added a comment - The core issue is that SHOW SLAVE STATUS worked for a user before upgrade and no longer works (without manual intervention afterwards). This is an issue for customers as noted in the original report who expect previously working commands to function after upgrade. Is there a less permissive option that allows just SHOW SLAVE STATUS or does that now give more privileges than it did previously?

            Unfortunately, there is no less permissive option that allows just SHOW SLAVE STATUS at the moment.

            serg Sergei Golubchik added a comment - Unfortunately, there is no less permissive option that allows just SHOW SLAVE STATUS at the moment.

            In order to make upgrades work, should we give SHOW SLAVE STATUS to an existing privilege (besides REPLICATION SLAVE ADMIN) or add a new privilege and give that to users on upgrade, or something else?

            nicklamb Nick (Inactive) added a comment - In order to make upgrades work, should we give SHOW SLAVE STATUS to an existing privilege (besides REPLICATION SLAVE ADMIN) or add a new privilege and give that to users on upgrade, or something else?

            Generally we try to avoid having more than one privilege to allow a specific action. There are few historical instances of that, but we'd rather not to add more.

            Adding a new privilege could be possible. We tried to strike the balance between having a fine-grained privilege system and swamping users with hundreds of privileges to learn. It could as well be that we've misjudged this one.

            bar, what do you think?

            serg Sergei Golubchik added a comment - Generally we try to avoid having more than one privilege to allow a specific action. There are few historical instances of that, but we'd rather not to add more. Adding a new privilege could be possible. We tried to strike the balance between having a fine-grained privilege system and swamping users with hundreds of privileges to learn. It could as well be that we've misjudged this one. bar , what do you think?
            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.
            serg Sergei Golubchik made changes -
            Fix Version/s N/A [ 14700 ]
            serg Sergei Golubchik made changes -
            Assignee Geoff Montee [ geoffmontee ] Ian Gilfillan [ greenman ]
            serg Sergei Golubchik made changes -
            Component/s Documentation [ 10903 ]
            serg Sergei Golubchik made changes -
            Assignee Ian Gilfillan [ greenman ]
            julien.fritsch Julien Fritsch made changes -
            Assignee Oleksandr Byelkin [ sanja ]
            julien.fritsch Julien Fritsch made changes -
            Fix Version/s 10.5 [ 23123 ]
            elenst Elena Stepanova made changes -

            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.
            julien.fritsch Julien Fritsch made changes -
            Priority Major [ 3 ] Critical [ 2 ]
            sujatha.sivakumar Sujatha Sivakumar (Inactive) made changes -
            Elkin Andrei Elkin made changes -
            Assignee Oleksandr Byelkin [ sanja ] Sujatha Sivakumar [ sujatha.sivakumar ]
            julien.fritsch Julien Fritsch made changes -
            serg Sergei Golubchik made changes -
            sujatha.sivakumar Sujatha Sivakumar (Inactive) made changes -
            Status Open [ 1 ] Confirmed [ 10101 ]
            sujatha.sivakumar Sujatha Sivakumar (Inactive) made changes -
            Status Confirmed [ 10101 ] In Progress [ 3 ]
            maxmether Max Mether made changes -

            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.
            sujatha.sivakumar Sujatha Sivakumar (Inactive) made changes -
            Assignee Sujatha Sivakumar [ sujatha.sivakumar ] Sergei Golubchik [ serg ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            serg Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Sujatha Sivakumar [ sujatha.sivakumar ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            ralf.gebhardt Ralf Gebhardt made changes -
            ralf.gebhardt Ralf Gebhardt made changes -
            sujatha.sivakumar Sujatha Sivakumar (Inactive) made changes -
            Fix Version/s 10.5.9 [ 25109 ]
            Fix Version/s 10.5 [ 23123 ]
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]
            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.
            Elkin Andrei Elkin made changes -
            Labels privileges slave
            serg Sergei Golubchik made changes -
            Workflow MariaDB v3 [ 113011 ] MariaDB v4 [ 158315 ]
            mariadb-jira-automation Jira Automation (IT) made changes -
            Zendesk Related Tickets 136169

            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.