Page MenuHomePhabricator

Create maintain-views user for labsdb1001 and labsdb1003
Closed, ResolvedPublic

Description

As part of T138450 testing a user was created on labsdb1008 specifically to manage view maintenance. That user/pass combo is is defined in the password module on the puppet master.

It is referenced here publicly:

https://phabricator.wikimedia.org/diffusion/OPUP/browse/production/modules/role/manifests/labsdb/views.pp;70506065e90f5081e2768924d345f740aadbc57c$16-18

I think T138450: maintain-replicas.pl unmaintained, unmaintainable is in a decent state and so want to attempt T147302 but I would like to carry over the service user model who can only access locally via the fqdn for the specific host.

Event Timeline

(a script run would also handle wikimania2017wiki_p and tcywiki_p which are currently missing)

(a script run would also handle wikimania2017wiki_p and tcywiki_p which are currently missing)

Good point, I plan on doing per DB or per table runs for the moment with scrutiny on schema outcomes and I will be sure to include this in the plan.

chasemp removed chasemp as the assignee of this task.Oct 18 2016, 7:15 PM

I'm not sure what the right steps are to do this through Puppet so I'm hoping to connect with one of the DBA folks to knock it out soon-ish

Amire80 removed a subscriber: Amire80.Oct 19 2016, 1:09 AM

@chasemp are we talking about this user from labsdb1008 that you want to get replicated to the other labs hosts?
If so, I will get @jcrespo to teach me how to create new users in production and I will get it done.

----------------------------------------------------------------------------------------------------------------------------------+
| Grants for maintain-views@10.64.37.15                                                                                            |
+----------------------------------------------------------------------------------------------------------------------------------+
| GRANT ALL PRIVILEGES ON *.* TO 'maintain-views'@'10.64.37.15' IDENTIFIED BY PASSWORD '*xxx' |
+----------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)
Marostegui moved this task from Triage to Next on the DBA board.Oct 19 2016, 8:52 AM

A couple of things- I created a new pasword for something similar to this, which is currently living on palladium secrets git store. If this is blocking something and has to be done soon, Manuel, you can create the users directly; but it would be better if it was puppetized.

Also I would like to suggest to avoid special characters like '-' on usernames; there is no problem with mysql with those (you can probably create users with emojis :-)), but we recently had issues with scripts and its escape characters such as \_ \_\_ and others. Better prevent those issues from reappearing.

@jcrespo thanks - I see it now.
You suggest using the same password for this new user? or even using the same user?

@chasemp do you have any objection in either using maintainviews instead of maintain-views? Or maybe even reuse that same user @jcrespo created?

@chasemp are we talking about this user from labsdb1008 that you want to get replicated to the other labs hosts?
If so, I will get @jcrespo to teach me how to create new users in production and I will get it done.

----------------------------------------------------------------------------------------------------------------------------------+
| Grants for maintain-views@10.64.37.15                                                                                            |
+----------------------------------------------------------------------------------------------------------------------------------+
| GRANT ALL PRIVILEGES ON *.* TO 'maintain-views'@'10.64.37.15' IDENTIFIED BY PASSWORD '*xxx' |
+----------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

yep, exactly :)

@chasemp do you have any objection in either using maintainviews instead of maintain-views? Or maybe even reuse that same user @jcrespo created?

I have no bias on name, whatever is easiest for you guys to reason about. maintainviews or viewmanager etc it's all good to me. My only qualifier is I'm hoping to limit this user to maintaining views only, and restricted to only accessing via fqdn on localhost :)

Thanks @Marostegui

@chasemp are we talking about this user from labsdb1008 that you want to get replicated to the other labs hosts?
If so, I will get @jcrespo to teach me how to create new users in production and I will get it done.

----------------------------------------------------------------------------------------------------------------------------------+
| Grants for maintain-views@10.64.37.15                                                                                            |
+----------------------------------------------------------------------------------------------------------------------------------+
| GRANT ALL PRIVILEGES ON *.* TO 'maintain-views'@'10.64.37.15' IDENTIFIED BY PASSWORD '*xxx' |
+----------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

yep, exactly :)

@chasemp do you have any objection in either using maintainviews instead of maintain-views? Or maybe even reuse that same user @jcrespo created?

I have no bias on name, whatever is easiest for you guys to reason about. maintainviews or viewmanager etc it's all good to me. My only qualifier is I'm hoping to limit this user to maintaining views only, and restricted to only accessing via fqdn on localhost :)

I can limit it to *_p databases (which are the ones with the views). Would that be enough?

I think it needs to be able to at least read non-_p databases to be allowed to create views selecting them?

At the moment the user has full permissions iirc. @Marostegui for my part I meant more like "Let's not use this user for anything else" rather than a prescriptive permissions set. I'm for it but am not sure what would work out. As @Krenair noted it needs (read) access to the underlying tables to validate. If you want to tweak the user on labsdb1008 we can test out an ACL set?

As @chasemp and myself agreed on IRC, we have decided to do some tweaking with a new user (to avoid touching maintain-views one) .
The idea is to test and modify this user until we find a proper setup.
For now we have created the following user in labsdb1008

MariaDB MARIADB labsdb1008 (none) > show grants for 'maintain-views2'@'10.64.37.15';
+---------------------------------------------------------------------------------------------------------------------------+
| Grants for maintain-views2@10.64.37.15                                                                                    |
+---------------------------------------------------------------------------------------------------------------------------+
| GRANT SELECT ON *.* TO 'maintain-views2'@'10.64.37.15' IDENTIFIED BY PASSWORD 'xxx' |
| GRANT ALL PRIVILEGES ON `%\_p`.* TO 'maintain-views2'@'10.64.37.15'                                                       |
+---------------------------------------------------------------------------------------------------------------------------+
2 rows in set (0.00 sec)

All this testing users will be deleted once we are done with the testing and we are sure everything works fine.

With maintain-views2 error:

pymysql.err.InternalError: (1227, 'Access denied; you need (at least one of) the SUPER privilege(s) for this operation')

Attempted operation:

    CREATE OR REPLACE
    DEFINER={0}
    VIEW `{1}`.`{2}`
    AS SELECT * FROM `{3}`.`{2}`;
""".format(self.definer, self.db_p, view, self.db))

    CREATE OR REPLACE
    DEFINER=viewmaster
    VIEW `adywiki_p`.`sites`
    AS SELECT * FROM `adywiki`.`sites`;

Note the view definer is not the maintain-views user

So far I have only been able to create it with either SUPER or ALL PRIVILEGES ON: GRANT SELECT ON *.* TO 'maintain-views2'@'10.64.37.15'
Which is not what we want.

I will keep checking

I suggest we should avoid providing wildcard grants. Those have created issues in the past, and we want to move away from them. When grants are created in the new system, I envision to create a whitelist, from a single puppet place.

Right now we maintain a blacklist on realm.pp. We should transform that into a white list- it is technically easy, but it requires some checking with the mediawiki private lists and some human checking. Then we will use the whitelist to generate the triggers, the role grants and we could use it for this user, too. I would also like to suggest to create this user with localhost host, so it doesn't have to change in the future because the place we run it from changes. Not the original one that creates the view, but the one used for the DEFINER clause, as we do with the triggers. So one user to create the views, another to own them. The ones that owns them should not have access to the underlying tables.

There is right now a labsadmin user that currently is used to admin users- we can reuse it or at least be aware of its automations. This was created with limited/special grants so it cannot by mistake do things we do now want.

This is getting a bit complex for me to type clearly, hit me on IRC and I will be able to explain it better, then decide.

Right now we maintain a blacklist on realm.pp. We should transform that into a white list- it is technically easy, but it requires some checking with the mediawiki private lists and some human checking.

So we gain yet another list to update when creating a new wiki?

I suppose we go from one list that needs updating when private wikis are created to one that needs updating when non-private (the majority) wikis are created, in a repository we need to update anyway. Slightly irritating but I can live with that.

Dzahn removed a subscriber: Dzahn.Oct 20 2016, 5:59 PM

I had a chat with Jaime yesterday about the past issues with the wildcard-based privileges and it is certainily worrying.
Probably having another list to maintain isn't the best thing ever, but it will work out and will prevent future leaks.

So far I will try to work out the combination of privileges that we need for those users and the _p databases plus the normal ones. And once Chase can confirm it works, we can move forward and see how we can implement that whitelist.

So the tricky part is this one:

DEFINER=viewmaster

As per MySQL documentation:

If you specify the DEFINER clause, you cannot set the value to any user but your own unless you have the SUPER privilege. These rules determine the legal DEFINER user values:

*If you do not have the SUPER privilege, the only legal user value is your own account, either specified literally or by using CURRENT_USER. You cannot set the definer to some other account.*
If you have the SUPER privilege, you can specify any syntactically legal account name. If the account does not actually exist, a warning is generated.

So there are two solutions here:

  1. Create the view without the definer clause so every user would need to have privileges to create its own.
  1. We'd need to give SUPER privileges to maintain-views if we want it to be able to create all the views.

Giving SUPER privileges isn't ideal, but we can try to limit it to connect from localhost or another host in order to minimize risks.

I guess solution #1 needs a lot of changes in the script, right?.

With these current grants:

MariaDB MARIADB labsdb1008 (none) > show grants for 'maintain-views2'@'10.64.37.15';
+---------------------------------------------------------------------------------------------------------------------------+
| Grants for maintain-views2@10.64.37.15                                                                                    |
+---------------------------------------------------------------------------------------------------------------------------+
| GRANT SELECT ON *.* TO 'maintain-views2'@'10.64.37.15' IDENTIFIED BY PASSWORD '*xxx' |
| GRANT ALL PRIVILEGES ON `%\_p`.* TO 'maintain-views2'@'10.64.37.15'                                                       |
+---------------------------------------------------------------------------------------------------------------------------+
2 rows in set (0.00 sec)

I am able to create a view without the definer clause.

mysql:maintain-views2@labsdb1008 [(none)]> CREATE OR REPLACE        VIEW `adywiki_p`.`sites`     AS SELECT * FROM `adywiki`.`sites`;
Query OK, 0 rows affected (0.00 sec)

mysql:maintain-views2@labsdb1008 [(none)]> CREATE OR REPLACE     DEFINER=viewmaster     VIEW `adywiki_p`.`sites`     AS SELECT * FROM `adywiki`.`sites`;
ERROR 1227 (42000): Access denied; you need (at least one of) the SUPER privilege(s) for this operation

@Marostegui I'm of the opinion at the moment that reworking the definer could be a more nuanced bit of work itself. Honestly, no idea what the fallout there would be but I would want to carry it forward to make all the things consistent. Not opposed but let's look at that separately?

Then the path forward I think is to keep both the VIEWMASTER and MAINTAINVIEWS users with SUPER privs and to use the MAINTAINVEWS users to create the views only. We also definitely limit this to connections from FQDN. The script itself has been refactored with this model in mind already. This is already a big jump forward as all the things have been done w/ root previously and puts us in a place to make consistent things like T147302 and then revisit the perms model separately.

Option 2 for now?

Then the path forward I think is to keep both the VIEWMASTER and MAINTAINVIEWS users with SUPER privs and to use the MAINTAINVEWS users to create the views only.

No, I very much against that- the owner of the views cannot have SUPER privileges, or it would have access to read and write everywhere (e.g. to the original tables, even in read only mode). The creator of the views can have it (that would be the solution), the owner of the views has to only have select privileges on the original tables.

Also, there is a labsdbadmin user already existing, which was tasked to do maintenance (create users), maybe that should be the one using for management?

Then the path forward I think is to keep both the VIEWMASTER and MAINTAINVIEWS users with SUPER privs and to use the MAINTAINVEWS users to create the views only.

No, I very much against that- the owner of the views cannot have SUPER privileges, or it would have access to read and write everywhere (e.g. to the original tables, even in read only mode). The creator of the views can have it (that would be the solution), the owner of the views has to only have select privileges on the original tables.

Isn't this status quo?

I'm confused a bit here, one angle on this task is to say we should stop doing view creation as root and transition to a designated user (or my thinking on the ask). Reviewing and changing of usage for viewmaster seems like another line of reasoning to me. I'm really open to what works for you guys here. I'm unsure on the scope of this task atm.

The only thing I think is definitely bad news is we change how things are done and don't backport it for consistency.

Also, there is a labsdbadmin user already existing, which was tasked to do maintenance (create users), maybe that should be the one using for management?

AFAICT this would be the user that create-dbusers has for this but that seems like a bad candidate for reuse and a bad name for what it's used for now.

jcrespo claimed this task.Oct 31 2016, 3:24 PM

So, 'maintainviews' will be the user used to create the view (you will connect to mysql using that user).

viewmaster will be the users used on "DEFINER='viewmaster@localhost'".

I will create the users on on all replica servers, old and new, and puppetize them.

I feel there is another missunderstanding, there is $::passwords::mysql::maintain_views and $::passwords::labsdb::maintainviews. I will try to clean up that.

ok thanks, $::passwords::labsdb::maintainviews works for me

jcrespo removed jcrespo as the assignee of this task.Oct 31 2016, 7:21 PM
jcrespo moved this task from Next to Done on the DBA board.

So this are the privileges created on all labsdbs (not yet on 9/10/11), but on 8 and the existing labs dbs:

{P4338}

And this is how I checked the permissions were good:

I connect as root:

$ mysql -u root bla bla bla

MariaDB MARIADB labsdb1008 (none) > create database nonexistentwiki;
Query OK, 1 row affected (0.00 sec)

MariaDB MARIADB labsdb1008 (none) > create table nonexistentwiki.revision (i int);
Query OK, 0 rows affected (0.00 sec)

MariaDB MARIADB labsdb1008 (none) > create table nonexistentwiki.page (i int);
Query OK, 0 rows affected (0.00 sec)

Now as $user, from localhost:

$ mysql -h localhost -p$PASS -u $USER
Welcome to the MariaDB monitor.  Commands end with ; or \g.
Your MariaDB connection id is 409170
Server version: 10.0.23-MariaDB MariaDB Server

Copyright (c) 2000, 2015, Oracle, MariaDB Corporation Ab and others.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

MariaDB [(none)]> use nonexistentwiki_p
Database changed
MariaDB [nonexistentwiki_p]> CREATE DEFINER='viewmaster'@'localhost' VIEW revision AS SELECT * FROM nonexistentwiki.revision;
Query OK, 0 rows affected (0.00 sec)

MariaDB [nonexistentwiki_p]> CREATE DEFINER='viewmaster'@'localhost' VIEW page AS SELECT * FROM nonexistentwiki.page;
Query OK, 0 rows affected (0.00 sec)

MariaDB [nonexistentwiki_p]> DROP VIEW revision;
Query OK, 0 rows affected (0.00 sec)

MariaDB [nonexistentwiki_p]> DROP VIEW page;
Query OK, 0 rows affected (0.00 sec)

Credentials are handled on $::passwords::labsdb::maintainviews.

If you need more grants, please tell.

jcrespo closed this task as Resolved.Nov 2 2016, 5:55 PM
jcrespo claimed this task.

I am assuming this as resolved because I think it is done and saw no complain it is not working from Chase.

Actually there was one issue with entry point (using the socket instead of the hostname), and another one with a check which required extra grants:

GRANT SELECT (host, user) ON `mysql`.`user` TO 'maintainviews'@'localhost';
Meno25 removed a subscriber: Meno25.Nov 4 2016, 7:04 AM