MDEV-38601 require FEDERATED ADMIN for SHOW CREATE SERVER#4743
MDEV-38601 require FEDERATED ADMIN for SHOW CREATE SERVER#4743MooSayed1 wants to merge 1 commit intoMariaDB:11.8from
Conversation
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This is a preliminary review.
In addition to the below, please make sure your commit has a commit message complying with CODING_STANDARDS.md.
| OPTIONS (USER 'remote_user', HOST 'localhost', PASSWORD 'secret', DATABASE 'test2'); | ||
| CREATE USER user1@localhost IDENTIFIED BY ''; | ||
| GRANT ALL PRIVILEGES ON *.* TO user1@localhost; | ||
| REVOKE FEDERATED ADMIN, SUPER ON *.* FROM user1@localhost; |
There was a problem hiding this comment.
Why are you revoking SUPER as well? Would SUPER allow SHOW CREATE SERVER? If it would, please document that into the Jira and the commit message.
And also add tests please.
| res= show_create_db(thd, lex); | ||
| break; | ||
| case SQLCOM_SHOW_CREATE_SERVER: | ||
| if (check_global_access(thd, PRIV_STMT_CREATE_SERVER)) |
There was a problem hiding this comment.
I'm not the final reviewer, but here's my two cents (feel free to ignore completely):
It usually is not a good idea to reuse privileges. This turns them into roles (guarding access to multiple operations vs guarding a single operation). I'd advocate to adding a different privilege, to go inline with the already very atomic privileges for CREATE, ALTER and DROP SERVER.
Ideally servers would be like any other SQL object and the privileges to them would not be global. But it's what it is: they are global.
If you absolutely must reuse any of the privileges, I'd suggest reusing ALTER or (DROP + CREATE). As this would be how you can reroute already existing code to new servers. But I'd definitely advise against this.
The Jira issue number for this PR is: MDEV-38601
Description
SHOW CREATE SERVERdid not require any privilege — any authenticated usercould run it and see full server connection details including passwords stored
in
mysql.servers.CREATE SERVER,ALTER SERVER, andDROP SERVERall correctly requireFEDERATED ADMINprivilege, butSHOW CREATE SERVER(introduced in MDEV-15696,MariaDB 11.7) was missing the equivalent check.
The root cause is in
sql/sql_parse.cc: theSQLCOM_SHOW_CREATE_SERVERcasehad no
check_global_access()call before dispatching tomysql_show_create_server().The fix: add
check_global_access(thd, PRIV_STMT_CREATE_SERVER)at the topof the
SQLCOM_SHOW_CREATE_SERVERcase, matching the identical pattern usedby
SQLCOM_CREATE_SERVER,SQLCOM_ALTER_SERVER, andSQLCOM_DROP_SERVER.Release Notes
SHOW CREATE SERVERnow requires theFEDERATED ADMINprivilege.Users without this privilege receive
ERROR 42000: Access denied; you need (at least one of) the FEDERATED ADMIN privilege(s).How can this PR be tested?
The test creates a server as root, then verifies that an unprivileged user
receives
ER_SPECIFIC_ACCESS_DENIED_ERRORonSHOW CREATE SERVER, and thatthe same user succeeds after being granted
FEDERATED ADMIN.Basing the PR against the correct MariaDB version
This is a bug fix.
SHOW CREATE SERVERwas introduced in 11.7, and theearliest affected maintained branch is 11.8, so this PR targets
upstream/11.8.PR quality check