packaging: setup: Filter from logs secrets from otopi answer files

When running engine-setup with an answer file generated by a previous
interactive engine-setup, without this patch, we log some secrets
unfiltered. Fix this:

- Require a new otopi that provides LOG_FILTER_QUESTIONS.

- Add an attribute 'asked_on' for constants. If a constant is_secret,
require setting asked_on, to a list of question names that might
change/set it.

- Add a field CREDS_Q_NAME_FUNC to the various *DB_ENV_KEYS. This field
should point at a function that should return the question name for a
particular field.

- Change getCredentials to not get a parameter queryprefix for
constructing the question names, instead relying on CREDS_Q_NAME_FUNC.

- Add functions *question_name for both passing as CREDS_Q_NAME_FUNC and
for asked_on.

- And finally: Patch filter_secrets.py to also loop over all the
constants that set is_secret, and add their asked_on to
env[LOG_FILTER_QUESTIONS]. This makes otopi filter out all the answers
provided for these questions in answer files.

Change-Id: Ibaca2a03f2020750f96ae30a3448ea2ad17fe43c
Signed-off-by: Yedidyah Bar David <didi@redhat.com>

ovirt-engine.spec.in

@@ -529,7 +529,7 @@ Requires:	bind-utils
 Requires:	iproute
 Requires:	python3-libxml2
 Requires:	logrotate
-Requires:	python3-otopi >= 1.10.0
+Requires:	python3-otopi >= 1.10.3
 Requires:	python3-paramiko
 Requires:	python3-distro
 Requires(pre):	shadow-utils
@@ -686,7 +686,7 @@ Requires:	%{name}-setup-plugin-ovirt-engine-common >= %{version}-%{release}
 Requires:	tar
 Requires:	bzip2
 Requires:	xz
-Requires:	python3-otopi >= 1.10.0
+Requires:	python3-otopi >= 1.10.3
 Requires:	python3-distro
 Requires:	postgresql >= 12.0
 Requires:	sqlite

packaging/setup/ovirt_engine_setup/cinderlib/constants.py

@@ -94,6 +94,7 @@ def CINDERLIB_DB_ENV_KEYS(self):
             DEK.DUMPER: CinderlibDBEnv.DUMPER,
             DEK.FILTER: CinderlibDBEnv.FILTER,
             DEK.RESTORE_JOBS: CinderlibDBEnv.RESTORE_JOBS,
+            DEK.CREDS_Q_NAME_FUNC: cinderlib_question_name,
         }
 
     @classproperty
@@ -113,6 +114,10 @@ def DEFAULT_CINDERLIB_DB_ENV_KEYS(self):
         }
 
 
+def cinderlib_question_name(what):
+    return f'OVESETUP_CINDERLIB_DB_{what.upper()}'
+
+
 @util.export
 @util.codegen
 @osetupattrsclass
@@ -172,6 +177,7 @@ def USER(self):
             ProvisioningEnv.POSTGRES_PROVISIONING_ENABLED
         ),
         is_secret=True,
+        asked_on=(cinderlib_question_name(DEK.PASSWORD),),
     )
     def PASSWORD(self):
         return 'OVESETUP_CL_DB/password'

packaging/setup/ovirt_engine_setup/constants.py

@@ -44,6 +44,7 @@ def osetupattrs(
     answerfile_condition=lambda env: True,
     summary_condition=lambda env: True,
     is_secret=False,
+    asked_on=None,
     doc_text=None,
 ):
     class decorator(classproperty):
@@ -58,8 +59,11 @@ def __init__(self, o):
                 answerfile_condition=answerfile_condition,
                 summary_condition=summary_condition,
                 is_secret=is_secret,
+                asked_on=asked_on,
                 doc_text=doc_text,
             )
+    if is_secret and asked_on is None:
+        raise RuntimeError('asked_on must be set when is_secret is set')
     return decorator
 
 
@@ -477,6 +481,7 @@ def REMOTE_ENGINE_HOST_SSH_PORT(self):
     @osetupattrs(
         answerfile=True,
         is_secret=True,
+        asked_on=('SSH_ACCESS_REMOTE_ENGINE_PASSWORD',),
     )
     def REMOTE_ENGINE_HOST_ROOT_PASSWORD(self):
         return 'OVESETUP_CONFIG/remoteEngineHostRootPassword'

packaging/setup/ovirt_engine_setup/engine/constants.py

@@ -449,6 +449,7 @@ def ENGINE_DB_ENV_KEYS(self):
             DEK.FILTER: EngineDBEnv.FILTER,
             DEK.RESTORE_JOBS: EngineDBEnv.RESTORE_JOBS,
             DEK.INVALID_CONFIG_ITEMS: EngineDBEnv.INVALID_CONFIG_ITEMS,
+            DEK.CREDS_Q_NAME_FUNC: engine_question_name,
         }
 
     @classproperty
@@ -467,6 +468,10 @@ def DEFAULT_ENGINE_DB_ENV_KEYS(self):
         }
 
 
+def engine_question_name(what):
+    return f'OVESETUP_ENGINE_DB_{what.upper()}'
+
+
 @util.export
 @util.codegen
 @osetupattrsclass
@@ -526,6 +531,7 @@ def USER(self):
             oengcommcons.ProvisioningEnv.POSTGRES_PROVISIONING_ENABLED
         ),
         is_secret=True,
+        asked_on=(engine_question_name(DEK.PASSWORD),),
     )
     def PASSWORD(self):
         return 'OVESETUP_DB/password'
@@ -623,6 +629,7 @@ def MEMCHECK_ENABLED(self):
 class PKIEnv(object):
     @osetupattrs(
         is_secret=True,
+        asked_on=(),
     )
     def STORE_PASS(self):
         return 'OVESETUP_PKI/storePassword'
@@ -723,6 +730,7 @@ def ADMIN_USER_ID(self):
     @osetupattrs(
         answerfile=True,
         is_secret=True,
+        asked_on=('OVESETUP_CONFIG_ADMIN_SETUP',),
     )
     def ADMIN_PASSWORD(self):
         return 'OVESETUP_CONFIG/adminPassword'
@@ -822,6 +830,7 @@ def OVIRT_PROVIDER_OVN_USER(self):
         answerfile=True,
         description=_('oVirt OVN provider password'),
         is_secret=True,
+        asked_on=('ovirt-provider-ovn-password',),
     )
     def OVIRT_PROVIDER_OVN_PASSWORD(self):
         return 'OVESETUP_OVN/ovirtProviderOvnPassword'
@@ -842,6 +851,7 @@ def OVIRT_PROVIDER_OVN_PASSWORD(self):
 
     @osetupattrs(
         is_secret=True,
+        asked_on=(),
         answerfile=True,
         postinstallfile=True,
     )

packaging/setup/ovirt_engine_setup/engine_common/constants.py

@@ -220,6 +220,10 @@ class DBEnvKeysConst(object):
     FILTER = 'filter'
     RESTORE_JOBS = 'restoreJobs'
     INVALID_CONFIG_ITEMS = 'invalidConfigItems'
+    # Abusing this for the following item, which is not related to the
+    # environment, but I do need it with each DB and this looks like
+    # a good place.
+    CREDS_Q_NAME_FUNC = 'credsQNameFunc'
 
     REQUIRED_KEYS = (
         HOST,
@@ -236,6 +240,7 @@ class DBEnvKeysConst(object):
         DUMPER,
         FILTER,
         RESTORE_JOBS,
+        CREDS_Q_NAME_FUNC,
     )
 
     DEFAULTS_KEYS = (
@@ -409,6 +414,7 @@ def ENABLE(self):
 
     @osetupattrs(
         is_secret=True,
+        asked_on=(),
         answerfile=True,
         postinstallfile=True,
     )
@@ -423,6 +429,10 @@ def KEYCLOAK_OVIRT_ADMIN_USER_WITH_PROFILE(self):
 
     @osetupattrs(
         is_secret=True,
+        # This is the name used by ovirt-setup-lib's dialog.queryPassword.
+        # TODO: Consider doing something to not hard-code this here.
+        asked_on=('queryEnvKey_input_OVESETUP_CONFIG/keycloakAdminPasswd',),
+
     )
     def ADMIN_PASSWORD(self):
         return 'OVESETUP_CONFIG/keycloakAdminPasswd'

packaging/setup/ovirt_engine_setup/engine_common/database.py

@@ -1175,7 +1175,6 @@ def getUpdatedPGConf(self, content):
     def getCredentials(
         self,
         name,
-        queryprefix,
         defaultdbenvkeys,
         show_create_msg=False,
         note=None,
@@ -1241,17 +1240,17 @@ def getCredentials(
             ):
                 dbenv[self._dbenvkeys[k]] = _ind_env(self, k)
 
+            def question_name(what):
+                return self._dbenvkeys[DEK.CREDS_Q_NAME_FUNC](what.upper())
+
             def query_dbenv(
                 what,
                 note,
                 tests=None,
                 **kwargs
             ):
                 dialog.queryEnvKey(
-                    name='{qpref}{what}'.format(
-                        qpref=queryprefix,
-                        what=what.upper(),
-                    ),
+                    name=question_name(what.upper()),
                     dialog=self.dialog,
                     logger=self.logger,
                     env=dbenv,
@@ -1292,7 +1291,7 @@ def query_dbenv(
             if dbenv[self._dbenvkeys[DEK.SECURED]] is None:
                 dbenv[self._dbenvkeys[DEK.SECURED]] = dialog.queryBoolean(
                     dialog=self.dialog,
-                    name='{qpref}SECURED'.format(qpref=queryprefix),
+                    name=question_name('SECURED'),
                     note=_(
                         '{name} database secured connection (@VALUES@) '
                         '[@DEFAULT@]: '
@@ -1311,9 +1310,7 @@ def query_dbenv(
                     self._dbenvkeys[DEK.HOST_VALIDATION]
                 ] = dialog.queryBoolean(
                     dialog=self.dialog,
-                    name='{qpref}SECURED_HOST_VALIDATION'.format(
-                        qpref=queryprefix
-                    ),
+                    name=question_name('SECURED_HOST_VALIDATION'),
                     note=_(
                         '{name} database host name validation in secured '
                         'connection (@VALUES@) [@DEFAULT@]: '

packaging/setup/ovirt_engine_setup/provisiondb/constants.py

@@ -62,6 +62,9 @@ def PROVISION_DB_ENV_KEYS(self):
             DEK.DUMPER: ProvDBEnv.DUMPER,
             DEK.FILTER: ProvDBEnv.FILTER,
             DEK.RESTORE_JOBS: ProvDBEnv.RESTORE_JOBS,
+            # Not used.
+            # See also: https://bugzilla.redhat.com/show_bug.cgi?id=1636907
+            DEK.CREDS_Q_NAME_FUNC: None,
         }
 
     @classproperty
@@ -95,6 +98,7 @@ class ProvDBEnv(object):
 
     @osetupattrs(
         is_secret=True,
+        asked_on=(),
     )
     def PASSWORD(self):
         return 'OVESETUP_PROVISION_DB/password'

packaging/setup/plugins/ovirt-engine-common/base/core/filter_secrets.py

@@ -9,7 +9,6 @@
 
 """Filter secrets plugin."""
 
-
 from otopi import constants as otopicons
 from otopi import plugin
 from otopi import util
@@ -33,6 +32,32 @@ def __init__(self, context):
     )
     def _boot(self):
         secret_keys = []
+        secret_question_names = []
+        # ovirt-setup-lib's queryPassword has a custom question name for the
+        # first question, based on the key, which each caller has to add (using
+        # asked_on), but the second (verification) question is hard-coded to
+        # pass a fake key 'second_password' which results in a fixed question
+        # name. Add that here.
+        #
+        # Why is this important? Consider the following flow:
+        # User wants to use password 'topsec1' for e.g. grafana admin,
+        # and also has a password 'topsec2' for some other service.
+        # User runs engine-setup interactively, and when asked about
+        # the password:
+        # 1. On first prompt, provides topsecc1
+        # 2. On second prompt, provides topsec2
+        # 3. They mismatch, so user is asked again. On third and fourth
+        # prompts, user provides topsec1.
+        # Then user uses the generated answer file to run engine-setup
+        # unattended.
+        # We want to filter out all of topsecc1, topsec1, topsec2 - all of
+        # them were provided as 'passwords', so in principle might be -
+        # even if we didn't accept them because of a mismatch.
+        #
+        # Without the mismatch, it's not important - the password will be
+        # filtered out correctly due to being provided for the first
+        # question.
+        secret_question_names.append('queryEnvKey_input_second_password')
         consts = []
         for constobj in self.environment[
             osetupcons.CoreEnv.SETUP_ATTRS_MODULES
@@ -44,14 +69,26 @@ def _boot(self):
                     hasattr(k, '__osetup_attrs__') and
                     k.__osetup_attrs__['is_secret']
                 ):
-                    k = k.fget(None)
-                    secret_keys.append(k)
+                    secret_keys.append(k.fget(None))
+                    # If is_secret is set, we now require also passing
+                    # asked_on. This should be a list of question names.
+                    # We then also filter these out as well if found in
+                    # the env, as it means they were passed via an answer
+                    # file.
+                    for question_name in k.__osetup_attrs__['asked_on']:
+                        # E.g.: OVESETUP_CONFIG_ADMIN_SETUP
+                        secret_question_names.append(question_name)
 
         self.environment[
             otopicons.CoreEnv.LOG_FILTER_KEYS
         ].extend(
             secret_keys
         )
+        self.environment[
+            otopicons.CoreEnv.LOG_FILTER_QUESTIONS
+        ].extend(
+            secret_question_names
+        )
 
 
 # vim: expandtab tabstop=4 shiftwidth=4

packaging/setup/plugins/ovirt-engine-common/base/core/show_doc_text.py

@@ -66,6 +66,7 @@ def _show_doc_text(self):
             '- reconfigurable: Can it be set using '
             '--reconfigure-optional-components.\n'
             '- is_secret: Should it be filtered out in the log.\n'
+            '- asked_on: If key is secret: Question names setting it.\n'
             '- doc_text: Documentation for this key.\n\n'
         ))
         attrs_defaults = {

packaging/setup/plugins/ovirt-engine-setup/cinderlib/db/connection.py

@@ -105,7 +105,6 @@ def _customization(self):
             dbenvkeys=oclcons.Const.CINDERLIB_DB_ENV_KEYS,
         ).getCredentials(
             name='Cinderlib',
-            queryprefix='OVESETUP_CINDERLIB_DB_',
             defaultdbenvkeys=oclcons.Const.DEFAULT_CINDERLIB_DB_ENV_KEYS,
             show_create_msg=True,
         )

packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/db/connection.py

@@ -103,7 +103,6 @@ def _customization(self):
             dbenvkeys=oenginecons.Const.ENGINE_DB_ENV_KEYS,
         ).getCredentials(
             name='Engine',
-            queryprefix='OVESETUP_ENGINE_DB_',
             defaultdbenvkeys=oenginecons.Const.DEFAULT_ENGINE_DB_ENV_KEYS,
             show_create_msg=True,
         )