Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

privilege: execute admin command must have Super_priv. #7486

Merged
merged 2 commits into from
Aug 24, 2018
Merged

privilege: execute admin command must have Super_priv. #7486

merged 2 commits into from
Aug 24, 2018

Conversation

winkyao
Copy link
Contributor

@winkyao winkyao commented Aug 24, 2018

What problem does this PR solve?

Fix security bug of admin commands, before this PR, any user can run admin commands. The Schema names, table names are leaked:

mysql> CREATE USER 'testing';
^C
shell> mysql -u testing
mysql> ADMIN SHOW DDL JOBS;
mysql> ADMIN SHOW DDL JOBS 3676;
mysql> ADMIN CANCEL DDL JOBS 3676;

What is changed and how it works?

This PR add privilege checks with admin commands, you can only execute the admin commands with Super_priv.

shell> mysql -h 127.0.0.1 -P4000 -utesting
mysql> ADMIN SHOW DDL JOBS;
ERROR 1105 (HY000): privilege check fail

Check List

Tests

  • Unit test

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zimulala
Copy link
Contributor

/run-all-tests

@zimulala zimulala added the status/LGT2 Indicates that a PR has LGTM 2. label Aug 24, 2018
@winkyao winkyao merged commit 01c6bd8 into pingcap:master Aug 24, 2018
@winkyao winkyao deleted the admin_privilege branch August 24, 2018 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/privilege status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants