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

Grant fails if symbol privilege contains an underscore #243

Open
dud225 opened this issue Mar 22, 2019 · 9 comments
Open

Grant fails if symbol privilege contains an underscore #243

dud225 opened this issue Mar 22, 2019 · 9 comments
Assignees
Labels
Bug Something isn't working Waiting on Contributor Awaiting on the person who raised this to update
Projects

Comments

@dud225
Copy link

dud225 commented Mar 22, 2019

Hello

the manual implies that we can express grant privileges with symbols :

# Grant SELECT, UPDATE, and INSERT privileges to all tables in foo db from all hosts
mariadb_user 'foo_user' do
  password 'super_secret'
  database_name 'foo'
  host '%'
  privileges [:select,:update,:insert]
  action :grant
end

the full list can be found here

However MariaDB doesn't allow granting privileges containing an undersocre :

MariaDB [(none)]> GRANT select ON *.* TO 'root'@'localhost';
Query OK, 0 rows affected (0.00 sec)

MariaDB [(none)]> GRANT show_view ON *.* TO 'root'@'localhost';
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'ON *.* TO 'root'@'localhost'' at line 1

So using :

  privileges [:show_view]

makes the cookbook fail.

To grant that permission, one has to write :

  privileges ["SHOW VIEW"]

I suggest the manual be updated to explicitely use a string representing the usual uppercase form.

@xorima
Copy link
Contributor

xorima commented Mar 22, 2019

Hi @dud225

Would you be willing to create a PR for this issue, then we can get released quickly 👍

@sinfomicien
Copy link
Contributor

I don't initally wrote the part of the cookbook... But what i can tell is that underscore ARE supported by MariaDB. It came from Mysql, please see here for details.

So yes, show_view is not supported as is... But Show_view_priv is, and the cookbook normally change show_view to Show_view_priv here

But seems there's a bug in the grant function, as we don't use the good variable here

So i'd prefer to correct the bug in grant method that convert the whole cookbook to upper case privilege.

@sinfomicien sinfomicien self-assigned this Apr 4, 2019
@dud225
Copy link
Author

dud225 commented Apr 4, 2019

From my reading of the doc, it is not clear to me that the "_priv" syntax is supported.

I nevertheless tested your suggestion but was not successful :

MariaDB [(none)]> GRANT select_priv ON *.* TO 'root'@'localhost';
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'ON *.* TO 'root'@'localhost'' at line 1
MariaDB [(none)]> GRANT Select_priv ON *.* TO 'root'@'localhost';
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'ON *.* TO 'root'@'localhost'' at line 1
MariaDB [(none)]> GRANT show_view_priv ON *.* TO 'root'@'localhost';
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'ON *.* TO 'root'@'localhost'' at line 1
MariaDB [(none)]> GRANT Show_view_priv ON *.* TO 'root'@'localhost';
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'ON *.* TO 'root'@'localhost'' at line 1

Are you able to make it work on your side ?

@sinfomicien
Copy link
Contributor

Ok, after reading my post, i see i'm far from having said, what i had to. I won't edit it, to let your question understandable. I wrote english, but think in french, and so the result is wrong.

What i REALLY mean (i hope to be clear this time, but i can't give warranty ;-) ):

show_view is not supported in the GRANT request... But we always compare the result of the column Show_view_priv (which exist for real) to the fact that we wanna have the Show view privilege. In the case of you remove the '_', it will be difficult to compare the actual privilege, to the one you want to have (as we don't want to parse the result of the SHOW GRANT request).

So the misunderstanding is on the way we try to find if we need to really use the GRANT request, or if all grants are ok.

If you see the revoke function, we use the method revokify_key to grab the good GRANT name from the column name of the good right we want.

To my opinion, we just need to change the variable new_resource.privileges by a one which is cleaned via the revokify_key method (by the way also changing the revokify_key name to something clearer like get_clean_grant_right_name)

Sorry for my bad post before. I'm trying to improve myself :)

@dud225
Copy link
Author

dud225 commented Apr 10, 2019

Thanks for your detailed message I think I've got your point.
There is just a last thing that I'm missing : why do we bother checking the current permission ? Why not just executing the GRANT/REVOKE request ?

@sinfomicien
Copy link
Contributor

This time, i have a clear answer ! In many audit system, all grant request are logged with a special importance. If you play it as it's not needed, it generate noises on your audit system.

The original author of these code is @stissot, perhaps he can add some more insight on his own needs.

@dud225
Copy link
Author

dud225 commented Apr 11, 2019

Hello @sinfomicien

I feel that adding this kind of overhead to the whole community for constraints you have in a specific environment is quite awkward. It makes the resource code more complex that it needs to be and even though it let your audit system be happy it makes the server executes twice as much queries.

Yet this has a nice side-effect : it probably makes the cookbook work on AWS RDS (I haven't tested for now) as the user we get can't get "ALL PRIVILEGES". See this article for more details.

Anyway I did a new PR with your suggestion, let me know what you think about this one.

@sinfomicien
Copy link
Contributor

Hello @dud225 ,

I just push a PR i prepared yesterday, and did not see you also changed yours. I will see if your is better than mine, i will just add the bug correct of the issue #236 in mine and merge yours. Will do it tomorrow for sure.

sinfomicien added a commit to sinfomicien/mariadb that referenced this issue Apr 11, 2019
@damacus damacus added this to Need Triage in Project H-N May 11, 2019
@xorima
Copy link
Contributor

xorima commented Feb 21, 2020

Hey @sinfomicien what was the outcome of checking on this one?

@ramereth ramereth added Bug Something isn't working Waiting on Contributor Awaiting on the person who raised this to update labels Oct 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Waiting on Contributor Awaiting on the person who raised this to update
Projects
Project H-N
  
Need Triage
Development

Successfully merging a pull request may close this issue.

4 participants