From 48505af9d263411fb14060e9e55bc82b95d4bfe3 Mon Sep 17 00:00:00 2001 From: Lihu Ben-Ezri-Ravin Date: Wed, 5 Feb 2020 14:57:55 -0500 Subject: [PATCH] Remove filtering from edgeos_config module (#63362) The edgeos_config module had a list of commands to filter out to avoid load failures. This list had a single regular expression which caught commands that attempted to set pre-encrypted passwords. This behavior is undesirable for a few reasons. * It's poorly documented. The documentation makes cryptic mention of a return value that some commands might be filtered out, but offers no explanation as to what they are or why. * It's hard-coded. There's no way for the user to change or disable this functionality, rendering the commands caught by that expression completely unusable with the edgeos_config module. * The obvious workaround is unsafe. The filter catches passwords that are already encrypted, but is perfectly fine letting the user set plain-text passwords. EdgeOS will encrypt them upon commit, but this module encourages unsafe handling of secrets up to that point. * It's a security vulnerability if the user doesn't know about this behavior. While the module will warn if commands are filtered, the user won't know what got filtered out until after the fact, and may easily miss that warning if they are not vigilant. For something as sensitive as setting a password, it's not hard to imagine naive use of this module resulting in incorrect credentials being deployed. * It provides no discernible benefit. Using the module without filtering does not result in load failures. If those commands are indeed harmful for some reason on (old?) versions of EdgeOS, it should be incumbent upon the user to be scrupulous in what commands they issue, rather than the module maintaining a blacklist of possible ways the user might misuse their own system. --- .../63362-remove-edgeos-filtering.yaml | 2 ++ .../modules/network/edgeos/edgeos_config.py | 23 ------------------- 2 files changed, 2 insertions(+), 23 deletions(-) create mode 100644 changelogs/fragments/63362-remove-edgeos-filtering.yaml diff --git a/changelogs/fragments/63362-remove-edgeos-filtering.yaml b/changelogs/fragments/63362-remove-edgeos-filtering.yaml new file mode 100644 index 00000000000..9ed91ff4666 --- /dev/null +++ b/changelogs/fragments/63362-remove-edgeos-filtering.yaml @@ -0,0 +1,2 @@ +bugfixes: + - edgeos_config - fix issue where module would silently filter out encrypted passwords diff --git a/lib/ansible/modules/network/edgeos/edgeos_config.py b/lib/ansible/modules/network/edgeos/edgeos_config.py index 8aabeaf621a..0a44a54ae35 100644 --- a/lib/ansible/modules/network/edgeos/edgeos_config.py +++ b/lib/ansible/modules/network/edgeos/edgeos_config.py @@ -137,11 +137,6 @@ commands: returned: always type: list sample: ['...', '...'] -filtered: - description: The list of configuration commands removed to avoid a load failure - returned: always - type: list - sample: ['...', '...'] backup_path: description: The full path to the backup file returned: when backup is yes @@ -159,10 +154,6 @@ from ansible.module_utils.network.edgeos.edgeos import load_config, get_config, DEFAULT_COMMENT = 'configured by edgeos_config' -CONFIG_FILTERS = [ - re.compile(r'set system login user \S+ authentication encrypted-password') -] - def config_to_commands(config): set_format = config.startswith('set') or config.startswith('delete') @@ -234,15 +225,6 @@ def diff_config(commands, config): return list(updates) -def sanitize_config(config, result): - result['filtered'] = list() - for regex in CONFIG_FILTERS: - for index, line in reversed(list(enumerate(config))): - if regex.search(line): - result['filtered'].append(line) - del config[index] - - def run(module, result): # get the current active config from the node or passed in via # the config param @@ -253,7 +235,6 @@ def run(module, result): # create loadable config that includes only the configuration updates commands = diff_config(candidate, config) - sanitize_config(commands, result) result['commands'] = commands @@ -263,10 +244,6 @@ def run(module, result): if commands: load_config(module, commands, commit=commit, comment=comment) - if result.get('filtered'): - result['warnings'].append('Some configuration commands were ' - 'removed, please see the filtered key') - result['changed'] = True