diff --git a/test-requirements.txt b/test-requirements.txt index 1811e88cdf..15b861f92f 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -23,3 +23,4 @@ testtools>=1.4.0 testrepository>=0.0.18 pymongo>=3.0.2 redis>=2.10.0 +psycopg2>=2.5 diff --git a/trove/guestagent/datastore/experimental/postgresql/manager.py b/trove/guestagent/datastore/experimental/postgresql/manager.py index 2b9f689037..edced88c1e 100644 --- a/trove/guestagent/datastore/experimental/postgresql/manager.py +++ b/trove/guestagent/datastore/experimental/postgresql/manager.py @@ -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__('postgresql') @@ -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,9 +68,22 @@ 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) diff --git a/trove/guestagent/datastore/experimental/postgresql/pgutil.py b/trove/guestagent/datastore/experimental/postgresql/pgutil.py index 3695bf1017..f32c414b6d 100644 --- a/trove/guestagent/datastore/experimental/postgresql/pgutil.py +++ b/trove/guestagent/datastore/experimental/postgresql/pgutil.py @@ -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): diff --git a/trove/guestagent/datastore/experimental/postgresql/service/config.py b/trove/guestagent/datastore/experimental/postgresql/service/config.py index 15864f3e21..adad86ada1 100644 --- a/trove/guestagent/datastore/experimental/postgresql/service/config.py +++ b/trove/guestagent/datastore/experimental/postgresql/service/config.py @@ -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) diff --git a/trove/guestagent/datastore/experimental/postgresql/service/root.py b/trove/guestagent/datastore/experimental/postgresql/service/root.py index d4016d15d6..f5a13a4812 100644 --- a/trove/guestagent/datastore/experimental/postgresql/service/root.py +++ b/trove/guestagent/datastore/experimental/postgresql/service/root.py @@ -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": ""} """ 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 diff --git a/trove/guestagent/datastore/experimental/postgresql/service/status.py b/trove/guestagent/datastore/experimental/postgresql/service/status.py index 987d0625b4..aee4d37eea 100644 --- a/trove/guestagent/datastore/experimental/postgresql/service/status.py +++ b/trove/guestagent/datastore/experimental/postgresql/service/status.py @@ -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 diff --git a/trove/guestagent/datastore/experimental/postgresql/service/users.py b/trove/guestagent/datastore/experimental/postgresql/service/users.py index 19b8726a9e..4829111787 100644 --- a/trove/guestagent/datastore/experimental/postgresql/service/users.py +++ b/trove/guestagent/datastore/experimental/postgresql/service/users.py @@ -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="", - ) - ) - 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( + '', + 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="", - ) - ) - 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( + '', + 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.