User Grant does not validate databases

[Dan Nguyen]

There was a bug reported on our end where a MySQL user was able to change an admin's password.
The recommended fix is to validate databases on grant.

[Dan Nguyen (daniel-a-nguyen)]

trove/guestagent/datastore/mysql/service.py

def create_user(self, users):
        """Create users and grant them privileges for the
           specified databases"""
        with LocalSqlClient(get_engine()) as client:
            for item in users:
                user = models.MySQLUser()
                user.deserialize(item)
                # TODO(cp16net):Should users be allowed to create users
                # 'os_admin' or 'debian-sys-maint'
                g = sql_query.Grant(user=user.name, host=user.host,
                                    clear=user.password)
                t = text(str(g))
                client.execute(t)
                for database in user.databases:
                    mydb = models.ValidatedMySQLDatabase()
                    mydb.deserialize(database)
                    g = sql_query.Grant(permissions='ALL', database=mydb.name,
                                        user=user.name, host=user.host,
                                        clear=user.password)
                    t = text(str(g))
                    client.execute(t)

[Dan Nguyen (daniel-a-nguyen)]

Looks like the fix is to just validate on grant to ignore particular databases we have set in config.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/59529

[Sushil Kumar (sushil-kumar2)]

changing the admins password is a big security threat

[Sushil Kumar (sushil-kumar2)]

It could be simply fixed by using the method available MySQLDatabase._is_valid

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/59529
Committed: https://git.openstack.org/cgit/openstack/trove/commit/?id=e47be9c5cc9ab4e42e21dfa39886bfdbba686621
Submitter: Jenkins
Branch: master

commit e47be9c5cc9ab4e42e21dfa39886bfdbba686621
Author: daniel-a-nguyen <email address hidden>
Date: Mon Dec 2 12:05:35 2013 -0800

    Validate databases for user grants

    Change-Id: I397f18322774772d1c15653a4b815abf528399ad
    Closes-Bug: #1257021

[Sushil Kumar (sushil-kumar2)]

This bug is still on.

[Sushil Kumar (sushil-kumar2)]

Since https://github.com/openstack/trove/blob/master/trove/guestagent/datastore/mysql/service.py#L387 only logs an informative log in guestagent log, this is not doing any good to resolve the problem.
After handling the exception code shifts to https://github.com/openstack/trove/blob/master/trove/guestagent/datastore/mysql/service.py#L390 and permissions for mysql or rather "ignore_dbs" are still granted.

The fix would be to raise the exception instead of an informative logging.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/72595

[Sushil Kumar (sushil-kumar2)]

This is a critical bug, which is not yet fixed ....

[Dan Nguyen (daniel-a-nguyen)]

If you go with the fix you have now you will have follow up with a fix to the python-troveclient to handle the error response.

ubuntu@ubuntu:~/trove-integration/scripts$ trove user-grant-access 3603282e-6788-403b-b2a7-f0534c404852 loco mysql

ERROR: An error occurred communicating with the guest: Remote error: BadRequest Grant access to mysql is not allowed
[u'Traceback (most recent call last):\n', u' File "/home/ubuntu/trove/trove/openstack/common/rpc/amqp.py", line 440, in _process_data\n **args)\n', u' File "/home/ubuntu/trove/trove/openstack/common/rpc/dispatcher.py", line 172, in dispatch\n result = getattr(proxyobj, method)(ctxt, **kwargs)\n', u' File "/home/ubuntu/trove/trove/guestagent/datastore/mysql/manager.py", line 70, in grant_access\n return MySqlAdmin().grant_access(username, hostname, databases)\n', u' File "/home/ubuntu/trove/trove/guestagent/datastore/mysql/service.py", line 388, in grant_access\n "Grant access to %s is not allowed") % database)\n', u'BadRequest: Grant access to mysql is not allowed\n']..

Alternatively you could just "ignore" the dbs by moving the code into the try/except block.

...

 def grant_access(self, username, hostname, databases):
        """Grant a user permission to use a given database."""
        user = self._get_user(username, hostname)
        mydb = models.ValidatedMySQLDatabase()
        with LocalSqlClient(get_engine()) as client:
            for database in databases:
                    try:
                        mydb.name = database

                        g = sql_query.Grant(permissions='ALL', database=mydb.name,
                                        user=user.name, host=user.host,
                                        hashed=user.password)
                        t = text(str(g))
                       client.execute(t)

                       except ValueError:
                        LOG.info(_(
                            "Grant access to %s is not allowed") % database)

   ...

This might have the advantage of allowing you to skip over bad databases if databases contains more than 1 database rather than stopping at the first error. (not a big deal)

[Sushil Kumar (sushil-kumar2)]

I would say to treat troveclient message handling bug separately, as if we modiy the code to ignore db, we may have a bug on our hand for someone saying "when we request mysql schema access it says, thats done, but we do not recieve that".

So, troveclient should be treated separately.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/72595
Committed: https://git.openstack.org/cgit/openstack/trove/commit/?id=4c8ef841d3580c9bb9a97f50f36a42686c3de8ab
Submitter: Jenkins
Branch: master

commit 4c8ef841d3580c9bb9a97f50f36a42686c3de8ab
Author: Sushil Kumar <email address hidden>
Date: Tue Feb 11 08:45:58 2014 +0000

    Raises BadRequest for grant_access on ignore_dbs

    Reasons:
    - Currently, it just logs an informative log in guestagent log, and then
      control shifts to next line, which provides the requested grant.
    - This brings the need to raise the exception instead of informative
      logging.

    Changes:
    - Raises BadRequest if ignore_dbs are requested access instead of
      informative logging into guest-agent logs.

    Change-Id: I9df08d04cd42916f3d1f91e05cd4799bb9041eae
    Closes-Bug: #1257021