Flexible IPMI credential persistence method configuration

Instead of only file-based persistence which leaves files
with credentials on the conductor disk for the duration of
the session.

User can now pass ``True`` to the ``store_cred_in_env`` parameter
which instead stores IPMI password as an environment variable, still
for the duration of the session, but limiting exposure to just the
user session of ironic and anyone that has access to it.

Defaults to ``False``.

Closes-Bug: #2058749

Change-Id: Icd91e969e5c58bf42fc50958c3cd1acabd36ccdf

doc/source/admin/drivers/ipmitool.rst

@@ -82,6 +82,28 @@ with an IPMItool-based driver. For example::
         --driver-info ipmi_username=<username> \
         --driver-info ipmi_password=<password>
 
+
+Changing The Default IPMI Credential Persistence Method
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- ``store_cred_in_env``: :oslo.config:option:`ipmi.store_cred_in_env`.
+
+The `store_cred_in_env` configuration option allow users to switch
+between file-based and environment variable persistence methods for
+IPMI password.
+
+For the temporary file option, long lived IPMI sessions, such as those for
+console support, leave files with credentials on the conductor disk for the
+duration of the session.
+
+To switch to environment variable persistence, set the
+``store_cred_in_env`` parameter to ``True`` in the configuration file:
+
+.. code-block:: ini
+
+  [ipmi]
+  store_cred_in_env = True
+
 Advanced configuration
 ======================
 

ironic/conf/ipmi.py

@@ -73,6 +73,11 @@ opts = [
                        'additional debugging output. This is a separate '
                        'option as ipmitool can log a substantial amount '
                        'of misleading text when in this mode.')),
+    cfg.BoolOpt('store_cred_in_env',
+                default=False,
+                help=_('Boolean flag to determine IPMI password persistence '
+                       'method. Defaults to False (file-based persistence). '
+                       )),
     cfg.ListOpt('cipher_suite_versions',
                 default=[],
                 help=_('List of possible cipher suites versions that can '

ironic/drivers/modules/ipmitool.py

@@ -71,9 +71,9 @@ REQUIRED_PROPERTIES = {
 }
 OPTIONAL_PROPERTIES = {
     'ipmi_password': _("password. Optional."),
-    'ipmi_hex_kg_key': _('Kg key for IPMIv2 authentication. '
+    'ipmi_hex_kg_key': _("Kg key for IPMIv2 authentication. "
-                         'The key is expected in hexadecimal format. '
+                         "The key is expected in hexadecimal format. "
-                         'Optional.'),
+                         "Optional."),
     'ipmi_port': _("remote IPMI RMCP port. Optional."),
     'ipmi_priv_level': _("privilege level; default is ADMINISTRATOR. One of "
                          "%s. Optional.") % ', '.join(VALID_PRIV_LEVELS),
@@ -280,18 +280,23 @@ def _console_pwfile_path(uuid):
 
 
 @contextlib.contextmanager
-def _make_password_file(password):
+def _prepare_ipmi_password(driver_info):
-    """Makes a temporary file that contains the password.
+    """Prepares the IPMI password by either setting it in the environment
 
-    :param password: the password
+    or creating a temporary file.
-    :returns: the absolute pathname of the temporary file
+
+    :param driver_info: the ipmitool parameters for accessing a node.
+    :returns: the absolute pathname of the password
     :raises: PasswordFileFailedToCreate from creating or writing to the
              temporary file
     """
+    if CONF.ipmi.store_cred_in_env:
+        yield _persist_ipmi_password(driver_info)
+    else:
         f = None
         try:
             f = tempfile.NamedTemporaryFile(mode='w', dir=CONF.tempdir)
-        f.write(str(password))
+            f.write(str(driver_info['password'] or '\0'))
             f.flush()
         except (IOError, OSError) as exc:
             if f is not None:
@@ -303,11 +308,11 @@ def _make_password_file(password):
                     f.close()
 
         try:
-        # NOTE(jlvillal): This yield can not be in the try/except block above
+            # NOTE(jlvillal): This yield can not be in the try/except block
-        # because an exception by the caller of this function would then get
+            # above because an exception by the caller of this function would
-        # changed to a PasswordFileFailedToCreate exception which would mislead
+            # then get changed to a PasswordFileFailedToCreate exception which
-        # about the problem and its cause.
+            # would mislead about the problem and its cause.
-        yield f.name
+            yield ('-f', f.name)
         finally:
             if f is not None:
                 f.close()
@@ -495,6 +500,25 @@ def _get_ipmitool_args(driver_info, pw_file=None):
     return args
 
 
+def _persist_ipmi_password(driver_info):
+    """Persists IPMI password for passing to the ipmitool
+
+    :param driver_info: driver info with the ipmitool parameters
+    """
+    env = {}
+    if CONF.ipmi.store_cred_in_env:
+        password = driver_info.get('password')
+        if password:
+            env = {'IPMI_PASSWORD': password}
+        return '-E', env
+
+    path = _console_pwfile_path(driver_info['uuid'])
+    pw_file = console_utils.make_persistent_password_file(
+        path, driver_info['password'] or '\0')
+
+    return '-f', pw_file
+
+
 def _ipmitool_timing_args():
     if not _is_option_supported('timing'):
         return []
@@ -644,10 +668,15 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
         # 'ipmitool' command will prompt password if there is no '-f'
         # option, we set it to '\0' to write a password file to support
         # empty password
-        with _make_password_file(driver_info['password'] or '\0') as pw_file:
+
-            cmd_args.append('-f')
+        with _prepare_ipmi_password(driver_info) as (flag, env_path):
-            cmd_args.append(pw_file)
+            cmd_args.append(flag)
+            if CONF.ipmi.store_cred_in_env:
+                extra_args['env_variables'] = env_path
+            else:
+                cmd_args.append(env_path)
             cmd_args.extend(command.split(" "))
+
             try:
                 out, err = utils.execute(*cmd_args, **extra_args)
                 return out, err
@@ -679,6 +708,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
                                     'Error: %(error)s',
                                     {'node': driver_info['uuid'],
                                      'cmd': e.cmd, 'error': e})
+
             finally:
                 LAST_CMD_TIME[driver_info['address']] = time.time()
 
@@ -1543,17 +1573,17 @@ class IPMIConsole(base.ConsoleInterface):
                  created
         :raises: ConsoleSubprocessFailed when invoking the subprocess failed
         """
-        path = _console_pwfile_path(driver_info['uuid'])
-        pw_file = console_utils.make_persistent_password_file(
-            path, driver_info['password'] or '\0')
-        ipmi_cmd = self._get_ipmi_cmd(driver_info, pw_file)
-        ipmi_cmd += ' sol activate'
-
         try:
-            start_method(driver_info['uuid'], driver_info['port'], ipmi_cmd)
+            cmd = self._get_ipmi_cmd(driver_info, pw_file=None)
-        except (exception.ConsoleError, exception.ConsoleSubprocessFailed):
+            cmd += ' sol activate'
-            with excutils.save_and_reraise_exception():
+            start_method(driver_info['uuid'], driver_info['port'], cmd)
-                ironic_utils.unlink_without_raise(path)
+        except (exception.ConsoleError,
+                exception.ConsoleSubprocessFailed) as e:
+            LOG.exception('IPMI Error while attempting "%(cmd)s" '
+                          'for node %(node)s. Error: %(error)s',
+                          {'node': driver_info['uuid'],
+                           'cmd': cmd, 'error': e})
+            raise
 
 
 class IPMIShellinaboxConsole(IPMIConsole):

releasenotes/notes/flexible_ipmi_credential_persistence_method_configuration-e5ed052576576d71.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Adds a new configuration option ``store_cred_in_env`` to allow
+    switching between file-based and environment variable persistence for
+    IPMI credentials. Defaults to ``False``.