802.1x: Hide the password from output by default

Introduced new argument `include_secret=False|True` to:
    * `libnmstate.show_running_config()`
    * `libnmstate.show()`

This will mask the passwords with `<_password_hid_by_nmstate>` by
default(`include_secret=False`).

The passwords are also masked in logging and error message.

Added new argument:
    * `nmstatectl show -s `

Without `-s` switch, nmstatectl output will use the masked password.

When applying with above reserved password, nmstate will ignore it and
use current password.

Integration and unit test cases included.

Signed-off-by: Gris Ge <fge@redhat.com>

doc/nmstatectl.8.in

@@ -124,6 +124,12 @@ change the output format to \fIJSON\fR.
 Showing the running network configuration.
 .RE
 
+.B -s, --show-secrets
+.RS
+Showing with the secrets. By default, nmstate is masking the passwords by
+\fI<_password_hid_by_nmstate>\fR.
+.RE
+
 .IP \fB--no-verify
 skip the desired network state verification.
 .IP \fB--no-commit

libnmstate/ifaces/base_iface.py

@@ -36,8 +36,9 @@ from libnmstate.schema import InterfaceType
 from libnmstate.schema import LLDP
 from libnmstate.schema import OvsDB
 
-from ..state import state_match
+from ..state import hide_the_secrets
 from ..state import merge_dict
+from ..state import state_match
 from .ethtool import IfaceEthtool
 
 
@@ -210,7 +211,9 @@ class BaseIface:
         self._origin_info[Interface.STATE] = InterfaceState.ABSENT
 
     def to_dict(self):
-        return deepcopy(self._info)
+        info = deepcopy(self._info)
+        hide_the_secrets(info)
+        return info
 
     @property
     def original_desire_dict(self):

libnmstate/netapplier.py

@@ -26,6 +26,7 @@ from libnmstate import validator
 from libnmstate.error import NmstateVerificationError
 from libnmstate.schema import InterfaceType
 
+from .net_state import NetState
 from .nmstate import create_checkpoints
 from .nmstate import destroy_checkpoints
 from .nmstate import plugin_context
@@ -33,7 +34,8 @@ from .nmstate import plugins_capabilities
 from .nmstate import remove_metadata_leftover
 from .nmstate import rollback_checkpoints
 from .nmstate import show_with_plugins
-from .net_state import NetState
+from .state import hide_the_secrets
+from .state import remove_the_reserved_secrets
 from .version import get_version
 
 MAINLOOP_TIMEOUT = 35
@@ -65,12 +67,18 @@ def apply(
     :rtype: str
     """
     logging.debug(f"Nmstate version: {get_version()}")
-    logging.debug(f"Applying desire state: {desired_state}")
+
+    desired_state_without_secrets = copy.deepcopy(desired_state)
+    hide_the_secrets(desired_state_without_secrets)
+    logging.debug(f"Applying desire state: {desired_state_without_secrets}")
 
     desired_state = copy.deepcopy(desired_state)
+    remove_the_reserved_secrets(desired_state)
     with plugin_context() as plugins:
         validator.schema_validate(desired_state)
-        current_state = show_with_plugins(plugins, include_status_data=True)
+        current_state = show_with_plugins(
+            plugins, include_status_data=True, include_secrets=True
+        )
         validator.validate_capabilities(
             desired_state, plugins_capabilities(plugins)
         )
@@ -142,7 +150,9 @@ def _net_state_contains_sriov_interface(net_state):
 
 
 def _verify_change(plugins, net_state):
-    current_state = remove_metadata_leftover(show_with_plugins(plugins))
+    current_state = remove_metadata_leftover(
+        show_with_plugins(plugins, include_secrets=True)
+    )
     net_state.verify(current_state)
 
 

libnmstate/netinfo.py

@@ -23,7 +23,7 @@ from .nmstate import show_with_plugins
 from .nmstate import show_running_config_with_plugins
 
 
-def show(*, include_status_data=False):
+def show(*, include_status_data=False, include_secrets=False):
     """
     Reports configuration and status data on the system.
     Configuration data is the set of writable data which can change the system
@@ -35,10 +35,14 @@ def show(*, include_status_data=False):
     """
     with plugin_context() as plugins:
         return remove_metadata_leftover(
-            show_with_plugins(plugins, include_status_data)
+            show_with_plugins(
+                plugins,
+                include_status_data=include_status_data,
+                include_secrets=include_secrets,
+            )
         )
 
 
-def show_running_config():
+def show_running_config(include_secrets=False):
     with plugin_context() as plugins:
-        return show_running_config_with_plugins(plugins)
+        return show_running_config_with_plugins(plugins, include_secrets)

libnmstate/nmstate.py

@@ -18,10 +18,10 @@
 #
 
 from contextlib import contextmanager
+from operator import attrgetter
+from operator import itemgetter
 import importlib
 import logging
-from operator import itemgetter
-from operator import attrgetter
 import os
 import pkgutil
 
@@ -36,10 +36,11 @@ from libnmstate.schema import InterfaceType
 from libnmstate.schema import Route
 from libnmstate.schema import RouteRule
 
+from .net_state import NetState
 from .nispor.plugin import NisporPlugin
 from .plugin import NmstatePlugin
+from .state import hide_the_secrets
 from .state import merge_dict
-from .net_state import NetState
 
 _INFO_TYPE_RUNNING = 1
 _INFO_TYPE_RUNNING_CONFIG = 2
@@ -68,7 +69,10 @@ def plugin_context():
 
 
 def show_with_plugins(
-    plugins, include_status_data=None, info_type=_INFO_TYPE_RUNNING
+    plugins,
+    include_status_data=None,
+    info_type=_INFO_TYPE_RUNNING,
+    include_secrets=False,
 ):
     for plugin in plugins:
         plugin.refresh_content()
@@ -96,6 +100,9 @@ def show_with_plugins(
         report.update(plugin.get_global_state())
 
     validator.schema_validate(report)
+
+    if not include_secrets:
+        hide_the_secrets(report)
     return report
 
 
@@ -392,8 +399,12 @@ def _get_iface_types_by_name(iface_infos, name):
     return iface_types
 
 
-def show_running_config_with_plugins(plugins):
-    return show_with_plugins(plugins, info_type=_INFO_TYPE_RUNNING_CONFIG)
+def show_running_config_with_plugins(plugins, include_secrets):
+    return show_with_plugins(
+        plugins,
+        info_type=_INFO_TYPE_RUNNING_CONFIG,
+        include_secrets=include_secrets,
+    )
 
 
 def generate_configurations(desire_state):

libnmstate/state.py

@@ -19,9 +19,13 @@
 
 from abc import ABCMeta
 from abc import abstractmethod
-from collections.abc import Sequence
 from collections.abc import Mapping
+from collections.abc import Sequence
 from functools import total_ordering
+import logging
+
+
+PASSWORD_HID_BY_NMSTATE = "<_password_hid_by_nmstate>"
 
 
 @total_ordering
@@ -102,3 +106,36 @@ def merge_dict(dict_to, dict_from):
             dict_to[key] = from_value
         elif isinstance(dict_to[key], Mapping):
             merge_dict(dict_to[key], from_value)
+
+
+def hide_the_secrets(state):
+    if isinstance(state, Mapping):
+        for key, value in state.items():
+            if isinstance(value, Mapping) or isinstance(value, list):
+                hide_the_secrets(value)
+            elif key.endswith("password") and isinstance(value, str):
+                state[key] = PASSWORD_HID_BY_NMSTATE
+    elif isinstance(state, list):
+        for value in state:
+            hide_the_secrets(value)
+
+
+def remove_the_reserved_secrets(state):
+    if isinstance(state, Mapping):
+        keys_to_remove = []
+        for key, value in state.items():
+            if isinstance(value, Mapping) or isinstance(value, list):
+                remove_the_reserved_secrets(value)
+            elif key.endswith("password") and value == PASSWORD_HID_BY_NMSTATE:
+                logging.warn(
+                    "Got nmstate reserved password"
+                    f"({PASSWORD_HID_BY_NMSTATE}) for key: {key}, "
+                    "will use current password if available."
+                )
+                keys_to_remove.append(key)
+        for key in keys_to_remove:
+            state.pop(key)
+
+    elif isinstance(state, list):
+        for value in state:
+            remove_the_reserved_secrets(value)

nmstatectl/nmstatectl.py

@@ -38,6 +38,7 @@ from libnmstate.schema import Interface
 from libnmstate.schema import InterfaceIP
 from libnmstate.schema import Route
 from libnmstate.schema import RouteRule
+from libnmstate.state import hide_the_secrets
 
 
 def main():
@@ -226,6 +227,13 @@ def setup_subcommand_show(subparsers):
         action="store_true",
         dest="running_config",
     )
+    parser_show.add_argument(
+        "-s, --show-secrets",
+        help="Show secrets also",
+        default=False,
+        action="store_true",
+        dest="include_secrets",
+    )
     parser_show.add_argument(
         "only",
         default="*",
@@ -302,9 +310,11 @@ def rollback(args):
 
 def show(args):
     if args.running_config:
-        full_state = libnmstate.show_running_config()
+        full_state = libnmstate.show_running_config(
+            include_secrets=args.include_secrets
+        )
     else:
-        full_state = libnmstate.show()
+        full_state = libnmstate.show(include_secrets=args.include_secrets)
     state = _filter_state(full_state, args.only)
     print_state(state, use_yaml=args.yaml)
 
@@ -397,6 +407,7 @@ def apply_state(statedata, verify_change, commit, timeout, save_to_disk):
         )
         return os.EX_UNAVAILABLE
 
+    hide_the_secrets(state)
     print("Desired state applied: ")
     print_state(state, use_yaml=use_yaml)
     if checkpoint:

tests/ctl/nmstatectl_test.py

@@ -96,16 +96,23 @@ def test_run_ctl_directly_set():
 
 
 @mock.patch("sys.argv", ["nmstatectl", "show"])
-@mock.patch.object(nmstatectl.libnmstate, "show", lambda: {})
+@mock.patch.object(nmstatectl.libnmstate, "show", lambda *args, **kwargs: {})
 def test_run_ctl_directly_show_empty():
     nmstatectl.main()
 
 
 @mock.patch("sys.argv", ["nmstatectl", "show", "non_existing_interface"])
 @mock.patch.object(
-    nmstatectl.libnmstate, "show", lambda: json.loads(LO_JSON_STATE)
+    nmstatectl.libnmstate,
+    "show",
+    lambda *args, **kwargs: json.loads(LO_JSON_STATE),
 )
 @mock.patch("nmstatectl.nmstatectl.sys.stdout", new_callable=io.StringIO)
+@mock.patch.object(
+    nmstatectl.libnmstate,
+    "show",
+    lambda *args, **kwargs: json.loads(LO_JSON_STATE),
+)
 def test_run_ctl_directly_show_only_empty(mock_stdout):
     nmstatectl.main()
     assert mock_stdout.getvalue() == EMPTY_YAML_STATE
@@ -113,7 +120,9 @@ def test_run_ctl_directly_show_only_empty(mock_stdout):
 
 @mock.patch("sys.argv", ["nmstatectl", "show", "lo"])
 @mock.patch.object(
-    nmstatectl.libnmstate, "show", lambda: json.loads(LO_JSON_STATE)
+    nmstatectl.libnmstate,
+    "show",
+    lambda *args, **kwargs: json.loads(LO_JSON_STATE),
 )
 @mock.patch("nmstatectl.nmstatectl.sys.stdout", new_callable=io.StringIO)
 def test_run_ctl_directly_show_only(mock_stdout):
@@ -125,7 +134,9 @@ def test_run_ctl_directly_show_only(mock_stdout):
     "sys.argv", ["nmstatectl", "show", "--json", "non_existing_interface"]
 )
 @mock.patch.object(
-    nmstatectl.libnmstate, "show", lambda: json.loads(LO_JSON_STATE)
+    nmstatectl.libnmstate,
+    "show",
+    lambda *args, **kwargs: json.loads(EMPTY_JSON_STATE),
 )
 @mock.patch("nmstatectl.nmstatectl.sys.stdout", new_callable=io.StringIO)
 def test_run_ctl_directly_show_json_only_empty(mock_stdout):
@@ -135,7 +146,9 @@ def test_run_ctl_directly_show_json_only_empty(mock_stdout):
 
 @mock.patch("sys.argv", ["nmstatectl", "show", "--json", "lo"])
 @mock.patch.object(
-    nmstatectl.libnmstate, "show", lambda: json.loads(LO_JSON_STATE)
+    nmstatectl.libnmstate,
+    "show",
+    lambda *args, **kwargs: json.loads(LO_JSON_STATE),
 )
 @mock.patch("nmstatectl.nmstatectl.sys.stdout", new_callable=io.StringIO)
 def test_run_ctl_directly_show_json_only(mock_stdout):

tests/integration/nm/ieee802_1x_test.py

@@ -31,6 +31,7 @@ from ..testlib import assertlib
 from ..testlib.env import is_k8s
 from ..testlib.veth import create_veth_pair
 from ..testlib.veth import remove_veth_pair
+from ..testlib.statelib import show_only
 
 
 TEST_1X_CLI_NIC = "1x_cli"
@@ -134,11 +135,82 @@ def test_eth_with_802_1x(ieee_802_1x_env):
     }
 
     libnmstate.apply(desire_state)
+
     # Even without 802.1x authenticated, the veth peer is still pingable.
     # So we just check NetworkManager has the 802.1x config
     assertlib.assert_state_match(desire_state)
+
+    current_iface_state = show_only((TEST_1X_CLI_NIC,), include_secrets=True)[
+        Interface.KEY
+    ][0]
+
+    assert (
+        TEST_PRIVATE_KEY_PASSWORD
+        == current_iface_state[Ieee8021X.CONFIG_SUBTREE][
+            Ieee8021X.PRIVATE_KEY_PASSWORD
+        ]
+    )
+
+    current_iface_state = show_only((TEST_1X_CLI_NIC,), include_secrets=False)[
+        Interface.KEY
+    ][0]
+    assert (
+        TEST_PRIVATE_KEY_PASSWORD
+        != current_iface_state[Ieee8021X.CONFIG_SUBTREE][
+            Ieee8021X.PRIVATE_KEY_PASSWORD
+        ]
+    )
+
     assert len(
         cmdlib.exec_cmd(
             f"nmcli -g 802-1x c  show {TEST_1X_CLI_NIC}".split(), check=True
         )[1]
     )
+
+
+@pytest.fixture
+def ieee_1x_cli_up(ieee_802_1x_env):
+    desire_state = {
+        Interface.KEY: [
+            {
+                Interface.NAME: TEST_1X_CLI_NIC,
+                Ieee8021X.CONFIG_SUBTREE: {
+                    Ieee8021X.IDENTITY: TEST_IDENTITY,
+                    Ieee8021X.EAP_METHODS: ["tls"],
+                    Ieee8021X.PRIVATE_KEY: TEST_PRIVATE_KEY,
+                    Ieee8021X.PRIVATE_KEY_PASSWORD: TEST_PRIVATE_KEY_PASSWORD,
+                    Ieee8021X.CLIENT_CERT: TEST_CLIENT_CERT,
+                    Ieee8021X.CA_CERT: TEST_CA_CERT,
+                },
+            }
+        ]
+    }
+
+    libnmstate.apply(desire_state)
+
+
+@pytest.mark.tier1
+@pytest.mark.xfail(
+    is_k8s(),
+    reason=(
+        "Requires adjusts for k8s. Ref:"
+        "https://github.com/nmstate/nmstate/issues/1579"
+    ),
+    raises=NmstateLibnmError,
+    strict=False,
+)
+def test_apply_ieee_802_1x_with_reserved_password(ieee_1x_cli_up):
+    desire_state = show_only((TEST_1X_CLI_NIC,), include_secrets=False)
+
+    libnmstate.apply(desire_state)
+
+    current_iface_state = show_only((TEST_1X_CLI_NIC,), include_secrets=True)[
+        Interface.KEY
+    ][0]
+
+    assert (
+        TEST_PRIVATE_KEY_PASSWORD
+        == current_iface_state[Ieee8021X.CONFIG_SUBTREE][
+            Ieee8021X.PRIVATE_KEY_PASSWORD
+        ]
+    )

tests/integration/testlib/assertlib.py

@@ -75,7 +75,7 @@ def _iface_macs(state):
 
 
 def _prepare_state_for_verify(desired_state_data):
-    current_state_data = libnmstate.show()
+    current_state_data = libnmstate.show(include_secrets=True)
     # Ignore route and dns for assert check as the check are done in the test
     # case code.
     current_state_data.pop(Route.KEY, None)

tests/integration/testlib/statelib.py

@@ -35,14 +35,17 @@ from libnmstate.schema import LinuxBridge
 from libnmstate.schema import OVSBridge
 
 
-def show_only(ifnames):
+def show_only(ifnames, include_secrets=False):
     """
     Report the current state, filtering based on the given interface names.
     """
     base_filter_state = {
         Interface.KEY: [{Interface.NAME: ifname} for ifname in ifnames]
     }
-    current_state = State(libnmstate.show())
+    if include_secrets:
+        current_state = State(libnmstate.show(include_secrets=True))
+    else:
+        current_state = State(libnmstate.show())
     current_state.filter(base_filter_state)
     return current_state.state
 

tests/lib/ifaces/base_iface_test.py

@@ -202,3 +202,10 @@ class TestBaseIface:
             BaseIface.PERMANENT_MAC_ADDRESS_METADATA
             not in iface.state_for_verify()
         )
+
+    def test_to_dict_hide_the_password(self):
+        iface_info = gen_foo_iface_info()
+        iface_info["password"] = "foo"
+        iface = BaseIface(iface_info)
+
+        assert iface.to_dict()["password"] != "foo"