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

Split up SUPER privilege to smaller privileges

Details

    Description

      The SUPER privilege should be split up to multiple admin privileges so that more fine grained tuning of what each user can do could be applied.

      The SUPER privilege as such should still remain as an alias for all these smaller privileges

      Attachments

        Issue Links

          Activity

            SHOW MASTER STATUS and SHOW BINARY LOGS have always been a part of REPLICATION CLIENT.
            I guess this is because of the main purpose of these statements: to allow a human-run client from the slave machine to connect to the master and check the status of the binary logs, to start the automatically-run replication thread properly.

            Sergei proposed to rename REPLICATION CLIENT to BINLOG MONITOR. It much better reflects what this privilege is about.

            Let's rename, but keep also the old syntax for compatibility.

            bar Alexander Barkov added a comment - SHOW MASTER STATUS and SHOW BINARY LOGS have always been a part of REPLICATION CLIENT . I guess this is because of the main purpose of these statements: to allow a human-run client from the slave machine to connect to the master and check the status of the binary logs, to start the automatically-run replication thread properly. Sergei proposed to rename REPLICATION CLIENT to BINLOG MONITOR . It much better reflects what this privilege is about. Let's rename, but keep also the old syntax for compatibility.
            elenst Elena Stepanova added a comment - - edited

            It will provide a better name, but won't solve the problem. Before MDEV-21743, users with SUPER privilege had SHOW MASTER STATUS and SHOW BINARY LOGS capabilities. After upgrade to the new version, they won't, because these capabilities are not given to any new privilege which is assigned to former SUPER users via the means of access/version_id mechanism. It is removed from SUPER and kept only for an existing privilege, which the users might not have, and probably won't have, because who would assign REPLICATION CLIENT privilege to an admin of a master server or a non-replication instance? So, this part will actually break existing access rights upon upgrade.

            elenst Elena Stepanova added a comment - - edited It will provide a better name, but won't solve the problem. Before MDEV-21743 , users with SUPER privilege had SHOW MASTER STATUS and SHOW BINARY LOGS capabilities. After upgrade to the new version, they won't , because these capabilities are not given to any new privilege which is assigned to former SUPER users via the means of access / version_id mechanism. It is removed from SUPER and kept only for an existing privilege, which the users might not have, and probably won't have, because who would assign REPLICATION CLIENT privilege to an admin of a master server or a non-replication instance? So, this part will actually break existing access rights upon upgrade.

            Summary on bb-10.5-bar 7ce6e93fb

            Conformity to requirements

            There were two requirements in the initial description:

            • "The SUPER privilege should be split up to multiple admin privileges" – This is partially done. SUPER still remains a separate privilege with a smaller scope than before; so it hasn't been split up entirely.
            • "The SUPER privilege as such should still remain as an alias for all these smaller privileges" – This is not done.

            In my opinion, we should have done at least one of two things: either keep SUPER as an alias of what it had been before (and start deprecating it), then it's not that important whether we do all splitting in this release or leave something for the next ones; or, much more aggressively, split the privilege entirely into new ones, so that users would have to change their scripts and code only once. We would get a lot of heat with the aggressive approach, but it would still be more user-friendly than splitting it now partially, making users review all their scripts/code and make necessary changes, and then do it again in the next release.

            From a discussion on Slack, the decision was that the implemented solution is acceptable and the result should be documented.

            Problems

            • (mentioned in the previous comment) SUPER privilege has lost SHOW BINLOG STATUS and SHOW BINARY LOGS capabilities. Unlike other ones which were moved to new privileges and are still assigned to SUPER accounts from previous versions via access/version_id magic, these ones are not, they now only belong to an old privilege REPLICATION CLIENT which old SUPER accounts are unlikely to have. So, access will be broken after upgrade. The same consideration applies to REPLICATION CLIENT privilege, which has lost SHOW SLAVE STATUS capability, and to REPLICATION SLAVE privilege, which has lost SHOW RELAYLOG EVENTS capability, but these are probably less important.
            • Replication OM => NM involving user operations will cause discrepancy in privileges. E.g. GRANT SUPER ON *.* TO `u` executed on 10.4 and replicated to 10.5 will end up with the user `u` having less privileges on 10.5 than on 10.4.
            • The check for access value doesn't always detect a wrong format, the result can be dangerous:

              MariaDB [(none)]> update mysql.global_priv set priv = '{"access":111111111111111111111,"version_id":100502,"plugin":"mysql_native_password","authentication_string":"","password_last_changed":1583421540}' where user = 'u';
              Query OK, 1 row affected (0.030 sec)
              Rows matched: 1  Changed: 1  Warnings: 0
               
              MariaDB [(none)]> flush privileges;
              Query OK, 0 rows affected (0.005 sec)
               
              MariaDB [(none)]> show grants for u;
              +----------------------------------------------------------+
              | Grants for u@%                                           |
              +----------------------------------------------------------+
              | GRANT ALL PRIVILEGES ON *.* TO `u`@`%` WITH GRANT OPTION |
              +----------------------------------------------------------+
              1 row in set (0.000 sec)
              

              There are no warnings in the error log, and the account is given maximum permissions. The latter is the reason why it is a problem and not just cosmetics.

            Usability

            • Privilege errors like

              $ client/mysqladmin --protocol=tcp --port=3307 -usuper stop-slave
              client/mysqladmin: Error stopping slave: Access denied for user 'super'@'%' (using password: NO)
              

              are unhelpful. They are not new, it was the same way before, but given the likely misconfiguration which will come after the incompatible change (shrunk SUPER privileges), it would be more user-friendly to return instead a typical message like "requires at least REPLICATION SLAVE ADMIN privilege" etc.

            • Replaying binary logs has become more complicated. Neither SUPER alone nor REPLICATION SLAVE ADMIN alone can do it – SUPER can no longer execute BINLOG records, and REPLICATION SLAVE ADMIN cannot set certain session variables which are a part of the binlog.
            • This task would be a good reason to start explicitly forbidding downgrade to previous major versions. It never works well anyway, and for downgrade from 10.5.2 it is de-facto already rejected on InnoDB level, but it can still be done by disabling InnoDB. Technically it now presents a security risk, since in older versions certain users will have wider permissions than they were configured with in 10.5.
            • (related to MDEV-21704) If possible, it would be good to throw a warning about a wrong access value or version_id upon FLUSH PRIVILEGES – not just in the error log where nobody will see it. The most likely situation for the problem in real life is when somebody tries to (re-)configure a user manually in mysql.global_priv and makes a mistake. So, FLUSH PRIVILEGES after doing so is quite likely, and a warning would be informative.
              Also, it is questionable what should happen upon further actions on such users. MDEV-21704 says "ignore records" (with wrong access and/or version_id values), which is ambiguous. It may mean that the records wouldn't be used at all (as if the user/role doesn't exist), but in fact only global privileges are ignored and considered to be none, while the user/role itself still works, and non-global privileges work as well. And further any GRANT or even ALTER quietly overrides the previous wrong access value, only with a warning in the error log. Again, there should be at least a warning in the client.

            Cosmetics

            • (discussed in previous comments) REPLICATION CLIENT is a misleading name, especially now as it only has two remaining capabilities, SHOW BINLOG STATUS and SHOW BINARY LOGS, none of which has anything to do with replication.
              The suggestion above is to rename it to BINLOG MONITOR while keeping the old alias for compatibility.
            • REPLICATION SLAVE is documented as intended for configuring slave access (not human access). But now after upgrade previous accounts configured as REPLICATION SLAVE also get REPLICATION MASTER ADMIN privilege (I suppose because SHOW SLAVE HOSTS capability is needed for replication?). It looks weird that a technical account for slave replication access gets a "master admin" privilege.
            • It looks like there are no more capabilities which belong to more than one privilege. Possibly it was even a goal in itself. If it is indeed the case, the error message

              Access denied; you need (at least one of) the REPLICATION SLAVE ADMIN privilege(s) for this operation
              

              can be modified to remove the "at least one of" part and plural in privileges, just to make it look cleaner.

            Documentation

            • Documentation effort will require much more than just listing the changes in one place. Casual references to different privileges, especially to SUPER, are spread around documentation like this:

              Description: Maximum simultaneous connections permitted for each user account. When set to 0, there is no per user limit. From MariaDB 5.3, setting it to -1 stops users without the SUPER privilege from connecting to the server.

            • Typos in the comment "The 2020-03-01 edition" (in case it is used for documentation):
              • DES_DESCRIPT => DES_DECRYPT
              • Creating non-deterministic triggers and stored functions when binary log is enabled and log_bin_trust_function_creators=0 => I think non-deterministic doesn't play a role here, it relates to any functions when binary log enabled and log_bin_trust_function_creators=0
              • "Bypass the test for max_connections" => probably use "max_connections + 1" connection, not bypass entirely?
              • "Bypass the test for max_password_errors" – CONNECTION ADMIN doesn't bypass the check (and neither does SUPER in previous versions). Hopefully it's just a typo, I don't think we want an admin account to be easily brute-force-able.
              • "Execution of COM_BINLOG" – I can't find any references to COM_BINLOG. Probably COM_BINLOG_DUMP?
            elenst Elena Stepanova added a comment - Summary on bb-10.5-bar 7ce6e93fb Conformity to requirements There were two requirements in the initial description: "The SUPER privilege should be split up to multiple admin privileges" – This is partially done. SUPER still remains a separate privilege with a smaller scope than before; so it hasn't been split up entirely. "The SUPER privilege as such should still remain as an alias for all these smaller privileges" – This is not done. In my opinion, we should have done at least one of two things: either keep SUPER as an alias of what it had been before (and start deprecating it), then it's not that important whether we do all splitting in this release or leave something for the next ones; or, much more aggressively, split the privilege entirely into new ones, so that users would have to change their scripts and code only once. We would get a lot of heat with the aggressive approach, but it would still be more user-friendly than splitting it now partially, making users review all their scripts/code and make necessary changes, and then do it again in the next release. From a discussion on Slack, the decision was that the implemented solution is acceptable and the result should be documented. Problems (mentioned in the previous comment) SUPER privilege has lost SHOW BINLOG STATUS and SHOW BINARY LOGS capabilities. Unlike other ones which were moved to new privileges and are still assigned to SUPER accounts from previous versions via access/version_id magic, these ones are not, they now only belong to an old privilege REPLICATION CLIENT which old SUPER accounts are unlikely to have. So, access will be broken after upgrade. The same consideration applies to REPLICATION CLIENT privilege, which has lost SHOW SLAVE STATUS capability, and to REPLICATION SLAVE privilege, which has lost SHOW RELAYLOG EVENTS capability, but these are probably less important. Replication OM => NM involving user operations will cause discrepancy in privileges. E.g. GRANT SUPER ON *.* TO `u` executed on 10.4 and replicated to 10.5 will end up with the user `u` having less privileges on 10.5 than on 10.4. The check for access value doesn't always detect a wrong format, the result can be dangerous: MariaDB [(none)]> update mysql.global_priv set priv = '{"access":111111111111111111111,"version_id":100502,"plugin":"mysql_native_password","authentication_string":"","password_last_changed":1583421540}' where user = 'u' ; Query OK, 1 row affected (0.030 sec) Rows matched: 1 Changed: 1 Warnings: 0 MariaDB [(none)]> flush privileges ; Query OK, 0 rows affected (0.005 sec) MariaDB [(none)]> show grants for u; + ----------------------------------------------------------+ | Grants for u@% | + ----------------------------------------------------------+ | GRANT ALL PRIVILEGES ON *.* TO `u`@`%` WITH GRANT OPTION | + ----------------------------------------------------------+ 1 row in set (0.000 sec) There are no warnings in the error log, and the account is given maximum permissions. The latter is the reason why it is a problem and not just cosmetics. Usability Privilege errors like $ client/mysqladmin --protocol=tcp --port=3307 -usuper stop-slave client/mysqladmin: Error stopping slave: Access denied for user 'super'@'%' (using password: NO) are unhelpful. They are not new, it was the same way before, but given the likely misconfiguration which will come after the incompatible change (shrunk SUPER privileges), it would be more user-friendly to return instead a typical message like "requires at least REPLICATION SLAVE ADMIN privilege" etc. Replaying binary logs has become more complicated. Neither SUPER alone nor REPLICATION SLAVE ADMIN alone can do it – SUPER can no longer execute BINLOG records, and REPLICATION SLAVE ADMIN cannot set certain session variables which are a part of the binlog. This task would be a good reason to start explicitly forbidding downgrade to previous major versions. It never works well anyway, and for downgrade from 10.5.2 it is de-facto already rejected on InnoDB level, but it can still be done by disabling InnoDB. Technically it now presents a security risk, since in older versions certain users will have wider permissions than they were configured with in 10.5. (related to MDEV-21704 ) If possible, it would be good to throw a warning about a wrong access value or version_id upon FLUSH PRIVILEGES – not just in the error log where nobody will see it. The most likely situation for the problem in real life is when somebody tries to (re-)configure a user manually in mysql.global_priv and makes a mistake. So, FLUSH PRIVILEGES after doing so is quite likely, and a warning would be informative. Also, it is questionable what should happen upon further actions on such users. MDEV-21704 says "ignore records" (with wrong access and/or version_id values), which is ambiguous. It may mean that the records wouldn't be used at all (as if the user/role doesn't exist), but in fact only global privileges are ignored and considered to be none, while the user/role itself still works, and non-global privileges work as well. And further any GRANT or even ALTER quietly overrides the previous wrong access value, only with a warning in the error log. Again, there should be at least a warning in the client. Cosmetics (discussed in previous comments) REPLICATION CLIENT is a misleading name, especially now as it only has two remaining capabilities, SHOW BINLOG STATUS and SHOW BINARY LOGS , none of which has anything to do with replication. The suggestion above is to rename it to BINLOG MONITOR while keeping the old alias for compatibility. REPLICATION SLAVE is documented as intended for configuring slave access (not human access). But now after upgrade previous accounts configured as REPLICATION SLAVE also get REPLICATION MASTER ADMIN privilege (I suppose because SHOW SLAVE HOSTS capability is needed for replication?). It looks weird that a technical account for slave replication access gets a "master admin" privilege. It looks like there are no more capabilities which belong to more than one privilege. Possibly it was even a goal in itself. If it is indeed the case, the error message Access denied; you need (at least one of) the REPLICATION SLAVE ADMIN privilege(s) for this operation can be modified to remove the "at least one of" part and plural in privileges, just to make it look cleaner. Documentation Documentation effort will require much more than just listing the changes in one place. Casual references to different privileges, especially to SUPER, are spread around documentation like this: Description: Maximum simultaneous connections permitted for each user account. When set to 0, there is no per user limit. From MariaDB 5.3, setting it to -1 stops users without the SUPER privilege from connecting to the server. Typos in the comment "The 2020-03-01 edition" (in case it is used for documentation): DES_DESCRIPT => DES_DECRYPT Creating non-deterministic triggers and stored functions when binary log is enabled and log_bin_trust_function_creators=0 => I think non-deterministic doesn't play a role here, it relates to any functions when binary log enabled and log_bin_trust_function_creators=0 "Bypass the test for max_connections" => probably use "max_connections + 1" connection, not bypass entirely? "Bypass the test for max_password_errors" – CONNECTION ADMIN doesn't bypass the check (and neither does SUPER in previous versions). Hopefully it's just a typo, I don't think we want an admin account to be easily brute-force-able. "Execution of COM_BINLOG " – I can't find any references to COM_BINLOG. Probably COM_BINLOG_DUMP?

            bar,

            Did you happen to fix the privilege bug involving ALTER FUNCTION and log_bin_trust_function_creators=OFF and log_bin=ON (see MDEV-19088) as part of this change?

            Also, would it be appropriate for RocksDB backups (see MDEV-20577) to fall under any of these new privileges?

            GeoffMontee Geoff Montee (Inactive) added a comment - bar , Did you happen to fix the privilege bug involving ALTER FUNCTION and log_bin_trust_function_creators=OFF and log_bin=ON (see MDEV-19088 ) as part of this change? Also, would it be appropriate for RocksDB backups (see MDEV-20577 ) to fall under any of these new privileges?

            GeoffMontee, no I did not fix MDEV-19088, neither did I handle MDEV-20577.

            Note, I'm binding some other variables to smaller-than-SUPER privileges at the moment, so possibly it will be easier to address MDEV-20577 soon.

            bar Alexander Barkov added a comment - GeoffMontee , no I did not fix MDEV-19088 , neither did I handle MDEV-20577 . Note, I'm binding some other variables to smaller-than-SUPER privileges at the moment, so possibly it will be easier to address MDEV-20577 soon.

            People

              bar Alexander Barkov
              maxmether Max Mether
              Votes:
              0 Vote for this issue
              Watchers:
              8 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.