openstack
/
trove
Code Issues Proposed changes

Fix PostgreSQL root functions 

The default PostgreSQL administration account is 'postgres'.

This account always exists and cannot be removed.
Its attributes and access can however be altered.

Clients can connect from the localhost or remotely via TCP/IP:

Local clients (e.g. psql) can connect from a preset *system* account
called 'postgres'.
This system account has no password and is *locked* by default,
so that it can be used by *local* users only.
It should *never* be enabled (or it's password set)!!!
That would just open up a new attack vector on the system account.

Remote clients should use a build-in *database* account of the same
name. It's password can be changed using the "ALTER USER" statement.

Access to this account is disabled by Trove exposed only once the
superuser access is requested.

In the current implementation Trove uses the 'postgres' account and
return a new superuser called 'root' when the root access is requested.
The user 'root' has however no special meaning in PostgreSQL and the
existing applications may rely on the default superuser name 'postgres'.

This patch includes the following fixes:

 - Make Trove create and use its own administrative account (os_admin).
 - Disable (disable logins) the built-in 'postgres' account by default.
 - Enable and return the 'postgres' user when the root access is
   requested.
 - Fix failure on repeated 'root-enable' calls. The new implementation
   just regenerates the user's password.
 - Prevent users from hijacking the 'os_admin' account by allowing only
   local access.
 - Use the existing framework to generate secure passwords.
 - Stop using the system 'postgres' user to execute command-line client
   calls. Communicate with database via a native Python interface
   (like other datastores) instead.
 - Remove unnecessary complex result-set caching on the guest.
 - Simplify the status-checking code using the native exceptions.
   Avoid performing complex checks with uncertain benefits
   (i.e. attempt to guess the state of the process).

Notes:

 The current implementation is broken for variaous reasons:

 - It uses UUIDs in place of 'secure' password.
 - It creates a 'root' user, but no database for it.
   The clients won't be able to authenticate without explicitly
   providing an existing database name.
 - The created 'root' user has no 'SUPERUSER' attribute and
   hence is not a real superuser (cannot perform certain tasks)...
 - The implementation suffers a defect that allows a non-root user
   gain root access to an instance without marking is as 'root-enabled'
   A similar defect exists in other datastores (MySQL) too:

     1. Create an instance.
     2. Enable root.
     3. Use your root access to change the password of the built-in
        'postgres' account (Trove will still work because it uses the
        'peer' authentication method - the UNIX account).
     4. Login as 'postgres' using the changed password and drop the
        created 'root' account.
     5. Backup & restore the instance.
     6. Trove reports the root has never been enabled (it checks for
        existence of superuser accounts other than the built-in
        'postgres').
     7. You enjoy the root access of the 'postgres' user
        (the password is not reset on restore).

Depends-On: I9d6b3df5bebc3c499cea8306f5a1a6bab18adef6
Change-Id: Ie56ac0850bf4de742ce7841195ce29ca3b30c9ef
Closes-Bug: 1514569
This commit is contained in:
Petr Malik 2015-09-21 14:19:57 -04:00
parent 89e22f030c
commit 84e9018524
7 changed files with 269 additions and 203 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
test-requirements.txt
View File

@ -23,3 +23,4 @@ testtools>=1.4.0
testrepository>=0.0.18
pymongo>=3.0.2
redis>=2.10.0
psycopg2>=2.5

20
trove/guestagent/datastore/experimental/postgresql/manager.py
View File

@ -23,7 +23,8 @@ from .service.database import PgSqlDatabase
from .service.install import PgSqlInstall
from .service.root import PgSqlRoot
from .service.status import PgSqlAppStatus
from .service.users import PgSqlUsers
import pgutil
from trove.common import utils
from trove.guestagent import backup
from trove.guestagent.datastore import manager
from trove.guestagent import volume
@ -34,13 +35,14 @@ LOG = logging.getLogger(__name__)
class Manager(
manager.Manager,
PgSqlUsers,
PgSqlDatabase,
PgSqlRoot,
PgSqlConfig,
PgSqlInstall,
):
PG_BUILTIN_ADMIN = 'postgres'
def __init__(self):
super(Manager, self).__init__()
@ -51,6 +53,7 @@ class Manager(
def do_prepare(self, context, packages, databases, memory_mb, users,
device_path, mount_point, backup_info, config_contents,
root_password, overrides, cluster_config, snapshot):
pgutil.PG_ADMIN = self.PG_BUILTIN_ADMIN
self.install(context, packages)
self.stop_db(context)
if device_path:
@ -65,10 +68,23 @@ class Manager(
if backup_info:
backup.restore(context, backup_info, '/tmp')
pgutil.PG_ADMIN = self.ADMIN_USER
else:
self._secure(context)
if root_password and not backup_info:
self.enable_root(context, root_password)
def _secure(self, context):
# Create a new administrative user for Trove and also
# disable the built-in superuser.
self.create_database(context, [{'_name': self.ADMIN_USER}])
self._create_admin_user(context)
pgutil.PG_ADMIN = self.ADMIN_USER
postgres = {'_name': self.PG_BUILTIN_ADMIN,
'_password': utils.generate_random_password()}
self.alter_user(context, postgres, 'NOSUPERUSER', 'NOLOGIN')
def create_backup(self, context, backup_info):
backup.backup(context, backup_info)

155
trove/guestagent/datastore/experimental/postgresql/pgutil.py
View File

@ -13,78 +13,77 @@
# License for the specific language governing permissions and limitations
# under the License.
import os
import tempfile
import uuid
from oslo_log import log as logging
import psycopg2
from trove.common import utils
from trove.guestagent.common import operating_system
from trove.guestagent.common.operating_system import FileMode
from trove.common import exception
LOG = logging.getLogger(__name__)
def execute(*command, **kwargs):
"""Execute a command as the 'postgres' user."""
LOG.debug('Running as postgres: {0}'.format(command))
return utils.execute_with_timeout(
"sudo", "-u", "postgres", *command, **kwargs
)
PG_ADMIN = 'os_admin'
def result(filename):
"""A generator representing the results of a query.
class PostgresConnection(object):
This generator produces result records of a query by iterating over a
CSV file created by the query. When the file is out of records it is
removed.
def __init__(self, autocommit=False, **connection_args):
self._autocommit = autocommit
self._connection_args = connection_args
The purpose behind this abstraction is to provide a record set interface
with minimal memory consumption without requiring an active DB connection.
This makes it possible to iterate over any sized record set without
allocating memory for the entire record set and without using a DB cursor.
def execute(self, statement, identifiers=None, data_values=None):
"""Execute a non-returning statement.
"""
self._execute_stmt(statement, identifiers, data_values, False)
Each row is returned as an iterable of column values. The order of these
values is determined by the query.
"""
def query(self, query, identifiers=None, data_values=None):
"""Execute a query and return the result set.
"""
return self._execute_stmt(query, identifiers, data_values, True)
operating_system.chmod(filename, FileMode.SET_FULL, as_root=True)
with open(filename, 'r+') as file_handle:
for line in file_handle:
if line != "":
yield line.split(',')
operating_system.remove(filename, as_root=True)
raise StopIteration()
def _execute_stmt(self, statement, identifiers, data_values, fetch):
if statement:
with psycopg2.connect(**self._connection_args) as connection:
connection.autocommit = self._autocommit
with connection.cursor() as cursor:
cursor.execute(
self._bind(statement, identifiers), data_values)
if fetch:
return cursor.fetchall()
else:
raise exception.UnprocessableEntity(_("Invalid SQL statement: %s")
% statement)
def _bind(self, statement, identifiers):
if identifiers:
return statement.format(*identifiers)
return statement
class PostgresLocalhostConnection(PostgresConnection):
HOST = 'localhost'
def __init__(self, user, password=None, port=5432, autocommit=False):
super(PostgresLocalhostConnection, self).__init__(
autocommit=autocommit, user=user, password=password,
host=self.HOST, port=port)
# TODO(pmalik): No need to recreate the connection every time.
def psql(statement, timeout=30):
"""Execute a statement using the psql client."""
LOG.debug('Sending to local db: {0}'.format(statement))
return execute('psql', '-c', statement, timeout=timeout)
def query(statement, timeout=30):
"""Execute a pgsql query and get a generator of results.
This method will pipe a CSV format of the query results into a temporary
file. The return value is a generator object that feeds from this file.
"""Execute a non-returning statement (usually DDL);
Turn autocommit ON (this is necessary for statements that cannot run
within an implicit transaction, like CREATE DATABASE).
"""
return PostgresLocalhostConnection(
PG_ADMIN, autocommit=True).execute(statement)
filename = os.path.join(tempfile.gettempdir(), str(uuid.uuid4()))
LOG.debug('Querying: {0}'.format(statement))
psql(
"Copy ({statement}) To '{filename}' With CSV".format(
statement=statement,
filename=filename,
),
timeout=timeout,
)
return result(filename)
# TODO(pmalik): No need to recreate the connection every time.
def query(query, timeout=30):
"""Execute a query and return the result set.
"""
return PostgresLocalhostConnection(
PG_ADMIN, autocommit=False).query(query)
class DatabaseQuery(object):
@ -166,22 +165,48 @@ class UserQuery(object):
)
@classmethod
def create(cls, name, password):
def create(cls, name, password, encrypt_password=None, *options):
"""Query to create a user with a password."""
return "CREATE USER \"{name}\" WITH PASSWORD '{password}'".format(
name=name,
password=password,
)
create_clause = "CREATE USER \"{name}\"".format(name=name)
with_clause = cls._build_with_clause(
password, encrypt_password, *options)
return ''.join([create_clause, with_clause])
@classmethod
def update_password(cls, name, password):
def _build_with_clause(cls, password, encrypt_password=None, *options):
tokens = ['WITH']
if password:
# Do not specify the encryption option if 'encrypt_password'
# is None. PostgreSQL will use the configuration default.
if encrypt_password is True:
tokens.append('ENCRYPTED')
elif encrypt_password is False:
tokens.append('UNENCRYPTED')
tokens.append('PASSWORD')
tokens.append("'{password}'".format(password=password))
if options:
tokens.extend(options)
if len(tokens) > 1:
return ' '.join(tokens)
return ''
@classmethod
def update_password(cls, name, password, encrypt_password=None):
"""Query to update the password for a user."""
return "ALTER USER \"{name}\" WITH PASSWORD '{password}'".format(
name=name,
password=password,
)
return cls.alter_user(name, password, encrypt_password)
@classmethod
def alter_user(cls, name, password, encrypt_password=None, *options):
"""Query to alter a user."""
alter_clause = "ALTER USER \"{name}\"".format(name=name)
with_clause = cls._build_with_clause(
password, encrypt_password, *options)
return ''.join([alter_clause, with_clause])
@classmethod
def update_name(cls, old, new):

38
trove/guestagent/datastore/experimental/postgresql/service/config.py
View File

@ -21,7 +21,6 @@ from trove.common import cfg
from trove.common.i18n import _
from trove.common import utils
from trove.guestagent.common import operating_system
from trove.guestagent.datastore.experimental.postgresql import pgutil
from trove.guestagent.datastore.experimental.postgresql.service.process import(
PgSqlProcess)
from trove.guestagent.datastore.experimental.postgresql.service.status import(
@ -50,7 +49,7 @@ class PgSqlConfig(PgSqlProcess):
guest_id=CONF.guest_id,
)
)
out, err = pgutil.execute('psql', '--version', timeout=30)
out, err = utils.execute('psql', '--version')
pattern = re.compile('\d\.\d')
return pattern.search(out).group(0)
@ -78,22 +77,37 @@ class PgSqlConfig(PgSqlProcess):
def set_db_to_listen(self, context):
"""Allow remote connections with encrypted passwords."""
# Using cat to read file due to read permissions issues.
out, err = utils.execute_with_timeout(
'sudo', 'cat',
PGSQL_HBA_CONFIG.format(
version=self._get_psql_version(),
),
timeout=30,
)
LOG.debug(
"{guest_id}: Writing hba file to /tmp/pgsql_hba_config.".format(
guest_id=CONF.guest_id,
)
)
# Local access from administrative users is implicitly trusted.
#
# Remote access from the Trove's account is always rejected as
# it is not needed and could be used by malicious users to hijack the
# instance.
#
# Connections from other accounts always require a hashed password.
with open('/tmp/pgsql_hba_config', 'w+') as config_file:
config_file.write(out)
config_file.write("host all all 0.0.0.0/0 md5\n")
config_file.write(
"local all postgres,os_admin trust\n")
config_file.write(
"local all all md5\n")
config_file.write(
"host all postgres,os_admin 127.0.0.1/32 trust\n")
config_file.write(
"host all postgres,os_admin ::1/128 trust\n")
config_file.write(
"host all postgres,os_admin localhost trust\n")
config_file.write(
"host all os_admin 0.0.0.0/0 reject\n")
config_file.write(
"host all os_admin ::/0 reject\n")
config_file.write(
"host all all 0.0.0.0/0 md5\n")
config_file.write(
"host all all ::/0 md5\n")
operating_system.chown('/tmp/pgsql_hba_config',
'postgres', None, recursive=False, as_root=True)

77
trove/guestagent/datastore/experimental/postgresql/service/root.py
View File

@ -13,67 +13,74 @@
# License for the specific language governing permissions and limitations
# under the License.
import uuid
from oslo_log import log as logging
from trove.common import cfg
from trove.common import utils
from trove.guestagent.datastore.experimental.postgresql import pgutil
from trove.guestagent.datastore.experimental.postgresql.service.users import (
PgSqlUsers)
LOG = logging.getLogger(__name__)
CONF = cfg.CONF
class PgSqlRoot(object):
class PgSqlRoot(PgSqlUsers):
"""Mixin that provides the root-enable API."""
def is_root_enabled(self, context):
"""Return True if there is a superuser account enabled.
This ignores the built-in superuser of postgres and the potential
system administration superuser of os_admin.
"""
results = pgutil.query(
pgutil.UserQuery.list_root(ignore=cfg.get_ignored_users(
manager='postgresql')),
pgutil.UserQuery.list_root(),
timeout=30,
)
# Reduce iter of iters to iter of single values.
results = (r[0] for r in results)
return len(tuple(results)) > 0
# There should be only one superuser (Trove's administrative account).
return len(results) > 1 or (results[0] != self.ADMIN_USER)
# TODO(pmalik): For future use by 'root-disable'.
# def disable_root(self, context):
# """Generate a new random password for the public superuser account.
# Do not disable its access rights. Once enabled the account should
# stay that way.
# """
# self.enable_root(context)
def enable_root(self, context, root_password=None):
"""Create a root user or reset the root user password.
"""Create a superuser user or reset the superuser password.
The default superuser for PgSql is postgres, but that account is used
for administration. Instead, this method will create a new user called
root that also has superuser privileges.
The default PostgreSQL administration account is 'postgres'.
This account always exists and cannot be removed.
Its attributes and access can however be altered.
If no root_password is given then a random UUID will be used for the
superuser password.
Clients can connect from the localhost or remotely via TCP/IP:
Return value is a dictionary in the following form:
Local clients (e.g. psql) can connect from a preset *system* account
called 'postgres'.
This system account has no password and is *locked* by default,
so that it can be used by *local* users only.
It should *never* be enabled (or it's password set)!!!
That would just open up a new attack vector on the system account.
{"_name": "root", "_password": ""}
Remote clients should use a build-in *database* account of the same
name. It's password can be changed using the "ALTER USER" statement.
Access to this account is disabled by Trove exposed only once the
superuser access is requested.
Trove itself creates its own administrative account.
{"_name": "postgres", "_password": "<secret>"}
"""
user = {
"_name": "root",
"_password": root_password or str(uuid.uuid4()),
"_name": "postgres",
"_password": root_password or utils.generate_random_password(),
}
LOG.debug(
"{guest_id}: Creating root user with password {password}.".format(
guest_id=CONF.guest_id,
password=user['_password'],
)
query = pgutil.UserQuery.alter_user(
user['_name'],
user['_password'],
None,
*self.ADMIN_OPTIONS
)
query = pgutil.UserQuery.create(
name=user['_name'],
password=user['_password'],
)
if self.is_root_enabled(context):
query = pgutil.UserQuery.update_password(
name=user['_name'],
password=user['_password'],
)
pgutil.psql(query, timeout=30)
return user

54
trove/guestagent/datastore/experimental/postgresql/service/status.py
View File

@ -13,11 +13,9 @@
# License for the specific language governing permissions and limitations
# under the License.
import os
from oslo_log import log as logging
import psycopg2
from trove.common import exception
from trove.common import instance
from trove.common import utils
from trove.guestagent.datastore.experimental.postgresql import pgutil
@ -25,10 +23,9 @@ from trove.guestagent.datastore import service
LOG = logging.getLogger(__name__)
PGSQL_PID = "'/var/run/postgresql/postgresql.pid'"
class PgSqlAppStatus(service.BaseDbStatus):
@classmethod
def get(cls):
if not cls._instance:
@ -36,43 +33,16 @@ class PgSqlAppStatus(service.BaseDbStatus):
return cls._instance
def _get_actual_db_status(self):
"""Checks the acutal PgSql process to determine status.
Status will be one of the following:
- RUNNING
The process is running and responsive.
- BLOCKED
The process is running but unresponsive.
- CRASHED
The process is not running, but should be or the process
is running and should not be.
- SHUTDOWN
The process was gracefully shut down.
"""
# Run a simple scalar query to make sure the process is responsive.
try:
pgutil.execute('psql', '-c', 'SELECT 1')
# Any query will initiate a new database connection.
pgutil.psql("SELECT 1")
return instance.ServiceStatuses.RUNNING
except psycopg2.OperationalError:
return instance.ServiceStatuses.SHUTDOWN
except utils.Timeout:
return instance.ServiceStatuses.BLOCKED
except exception.ProcessExecutionError:
try:
utils.execute_with_timeout(
"/bin/ps", "-C", "postgres", "h"
)
except exception.ProcessExecutionError:
if os.path.exists(PGSQL_PID):
return instance.ServiceStatuses.CRASHED
return instance.ServiceStatuses.SHUTDOWN
else:
return instance.ServiceStatuses.BLOCKED
else:
return instance.ServiceStatuses.RUNNING
except Exception:
LOG.exception(_("Error getting Postgres status."))
return instance.ServiceStatuses.CRASHED
return instance.ServiceStatuses.SHUTDOWN

127
trove/guestagent/datastore/experimental/postgresql/service/users.py
View File

@ -19,6 +19,7 @@ from oslo_log import log as logging
from trove.common import cfg
from trove.common.i18n import _
from trove.common import utils
from trove.guestagent.datastore.experimental.postgresql import pgutil
from trove.guestagent.datastore.experimental.postgresql.service.access import (
PgSqlAccess)
@ -33,6 +34,31 @@ class PgSqlUsers(PgSqlAccess):
This mixin has a dependency on the PgSqlAccess mixin.
"""
@property
def ADMIN_USER(self):
"""Trove's administrative user."""
return 'os_admin'
@property
def ADMIN_OPTIONS(self):
"""Default set of options of an administrative account."""
return [
'SUPERUSER',
'CREATEDB',
'CREATEROLE',
'INHERIT',
'REPLICATION',
'LOGIN']
def _create_admin_user(self, context):
"""Create an administrative user for Trove.
Force password encryption.
"""
password = utils.generate_random_password()
os_admin = {'_name': self.ADMIN_USER, '_password': password,
'_databases': [{'_name': self.ADMIN_USER}]}
self._create_user(context, os_admin, True, *self.ADMIN_OPTIONS)
def create_user(self, context, users):
"""Create users and grant privileges for the specified databases.
@ -41,35 +67,36 @@ class PgSqlUsers(PgSqlAccess):
{"_name": "", "_password": "", "_databases": [{"_name": ""}, ...]}
"""
for user in users:
LOG.debug(
"{guest_id}: Creating user {name} with password {password}."
.format(
guest_id=CONF.guest_id,
name=user['_name'],
password=user['_password'],
)
)
LOG.info(
_("{guest_id}: Creating user {name} with password {password}.")
.format(
guest_id=CONF.guest_id,
name=user['_name'],
password="<SANITIZED>",
)
)
pgutil.psql(
pgutil.UserQuery.create(
name=user['_name'],
password=user['_password'],
self._create_user(context, user, None)
def _create_user(self, context, user, encrypt_password=None, *options):
LOG.info(
_("{guest_id}: Creating user {user} {with_clause}.")
.format(
guest_id=CONF.guest_id,
user=user['_name'],
with_clause=pgutil.UserQuery._build_with_clause(
'<SANITIZED>',
encrypt_password,
*options
),
timeout=30,
)
self.grant_access(
context,
)
pgutil.psql(
pgutil.UserQuery.create(
user['_name'],
None,
[d['_name'] for d in user['_databases']],
)
user['_password'],
encrypt_password,
*options
),
timeout=30,
)
self.grant_access(
context,
user['_name'],
None,
[d['_name'] for d in user['_databases']],
)
def list_users(
self,
@ -179,29 +206,35 @@ class PgSqlUsers(PgSqlAccess):
{"name": "", "password": ""}
"""
for user in users:
LOG.debug(
"{guest_id}: Changing password for {user} to {password}."
.format(
guest_id=CONF.guest_id,
user=user['name'],
password=user['password'],
)
)
LOG.info(
_("{guest_id}: Changing password for {user} to {password}.")
.format(
guest_id=CONF.guest_id,
user=user['name'],
password="<SANITIZED>",
)
)
pgutil.psql(
pgutil.UserQuery.update_password(
user=user['name'],
password=user['password'],
self.alter_user(context, user, None)
def alter_user(self, context, user, encrypt_password=None, *options):
"""Change the password and options of an existing users.
The user parameter is a dictionary of the following form:
{"name": "", "password": ""}
"""
LOG.info(
_("{guest_id}: Altering user {user} {with_clause}.")
.format(
guest_id=CONF.guest_id,
user=user['_name'],
with_clause=pgutil.UserQuery._build_with_clause(
'<SANITIZED>',
encrypt_password,
*options
),
timeout=30,
)
)
pgutil.psql(
pgutil.UserQuery.alter_user(
user['_name'],
user['_password'],
encrypt_password,
*options),
timeout=30,
)
def update_attributes(self, context, username, hostname, user_attrs):
"""Change the attributes of one existing user.