From 268b28f52782d20cd3f7bf27ead36438695b786a Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Tue, 28 May 2024 17:15:23 -0700
Subject: [PATCH] Enhance security notes in config description

Some of the configuation knobs require more specific details
regarding security or denial of service related possibilities
if tuned to inappropriate values.

Overall, just some minor improvements for clarity

Change-Id: I008d6e00a528bddba0f843f34968155a9da3ff36
---
 ironic/common/policy.py |  4 ++--
 ironic/conf/agent.py    | 10 +++++++++-
 ironic/conf/api.py      |  3 ++-
 ironic/conf/default.py  |  9 +++++++--
 ironic/conf/ipmi.py     |  4 +++-
 5 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/ironic/common/policy.py b/ironic/common/policy.py
index ad87c8c53b..fc209b754b 100644
--- a/ironic/common/policy.py
+++ b/ironic/common/policy.py
@@ -212,11 +212,11 @@ default_policies = [
     #             the value of this field
     policy.RuleDefault('show_password',
                        '!',
-                       description='Show or mask secrets within node driver information in API responses'),  # noqa
+                       description='Show or mask secrets within node driver information in API responses. This setting should be used with the utmost care as its use can present a security risk.'),  # noqa
     # Generic default to hide instance secrets
     policy.RuleDefault('show_instance_secrets',
                        '!',
-                       description='Show or mask secrets within instance information in API responses'),  # noqa
+                       description='Show or mask secrets within instance information in API responses. This setting should be used with the utmost care as its use can present a security risk.'),  # noqa
     # NOTE(TheJulia): This is a special rule to allow customization of the
     # service role check. The config.service_project_name is a reserved
     # target check field which is loaded from configuration to the
diff --git a/ironic/conf/agent.py b/ironic/conf/agent.py
index 7709d2d1a7..34efb160b0 100644
--- a/ironic/conf/agent.py
+++ b/ironic/conf/agent.py
@@ -114,7 +114,15 @@ opts = [
     cfg.IntOpt('command_timeout',
                default=60,
                mutable=True,
-               help=_('Timeout (in seconds) for IPA commands.')),
+               help=_('Timeout (in seconds) for IPA commands. '
+                      'A large timeout value may result in the conductor '
+                      'free worker pool becoming exhausted should a multi-'
+                      'node network connectivity issue arise during '
+                      'deployment or cleaning operations. These commands '
+                      'also cause the individual node lock to be held while '
+                      'in progress, which prevents new requests from being '
+                      'acted upon for the impacted nodes until the issue '
+                      'has been resolved.')),
     cfg.IntOpt('max_command_attempts',
                default=3,
                help=_('This is the maximum number of attempts that will be '
diff --git a/ironic/conf/api.py b/ironic/conf/api.py
index 7fe4962729..a8989da50b 100644
--- a/ironic/conf/api.py
+++ b/ironic/conf/api.py
@@ -77,7 +77,8 @@ opts = [
                 default=True,
                 mutable=True,
                 help=_('Whether to restrict the lookup API to only nodes '
-                       'in certain states.')),
+                       'in certain states. Setting this to False can be '
+                       'insecure and is not advisable.')),
     cfg.IntOpt('ramdisk_heartbeat_timeout',
                default=300,
                mutable=True,
diff --git a/ironic/conf/default.py b/ironic/conf/default.py
index 74f8f4baa9..74b72f30ee 100644
--- a/ironic/conf/default.py
+++ b/ironic/conf/default.py
@@ -63,7 +63,8 @@ api_opts = [
                  ('http_basic', _('HTTP basic authentication'))],
         help=_('Authentication strategy used by ironic-api. "noauth" should '
                'not be used in a production environment because all '
-               'authentication will be disabled.')),
+               'authentication will be disabled creating insecure '
+               'operating conditions.')),
     cfg.StrOpt('http_basic_auth_user_file',
                default='/etc/ironic/htpasswd',
                help=_('Path to Apache format user authentication file used '
@@ -442,7 +443,11 @@ webserver_opts = [
                default=60,
                help=_('Connection timeout when accessing/interacting with '
                       'remote web servers with images or other artifacts '
-                      'being accessed.')),
+                      'being accessed. An excessive value here is not '
+                      'advisable as excessive requests to an unreachable '
+                      'endpoint can result in Ironic service resources '
+                      'being consumed waiting for the connection to '
+                      'timeout.')),
 ]
 
 rbac_opts = [
diff --git a/ironic/conf/ipmi.py b/ironic/conf/ipmi.py
index 1ea9aa4a6e..fd88c01c84 100644
--- a/ironic/conf/ipmi.py
+++ b/ironic/conf/ipmi.py
@@ -51,7 +51,9 @@ opts = [
                 help=_('Kill `ipmitool` process invoked by ironic to read '
                        'node power state if `ipmitool` process does not exit '
                        'after `command_retry_timeout` timeout expires. '
-                       'Recommended setting is True')),
+                       'Recommended setting is True. Setting to False may '
+                       'present an operational issue and will result in '
+                       'unexpected and undesirable behavior.')),
     cfg.BoolOpt('disable_boot_timeout',
                 default=True,
                 mutable=True,