From eaa2fcc73fbda0caa402ad6233434f981baccab6 Mon Sep 17 00:00:00 2001 From: Ganesh Nalawade Date: Fri, 3 Nov 2017 21:57:14 +0530 Subject: Fix password leak in logs for provider argument (#32215) * Fix password leak in logs for provider argument Since provider argument is not validated against a spec the `no_log` arguments are not handled leading to password leaking to syslogs. To fix this: * Mask password and other `no_log` provider arguments in action plugin * In case of eapi and nxapi as the password is used in module code, * copy the provider password to top-level password argument which * handles `no_log` correctly. This will, however, throw a deprecation * warning message for password arg even if it is not given as a * top-level argument. * Remove auth details from provider args in action plugin * Update CHANGELOG --- CHANGELOG.md | 2 +- lib/ansible/plugins/action/dellos10.py | 4 ++++ lib/ansible/plugins/action/dellos6.py | 4 ++++ lib/ansible/plugins/action/dellos9.py | 4 ++++ lib/ansible/plugins/action/eos.py | 16 ++++++++++++---- lib/ansible/plugins/action/ios.py | 4 ++++ lib/ansible/plugins/action/iosxr.py | 3 +++ lib/ansible/plugins/action/junos.py | 3 +++ lib/ansible/plugins/action/nxos.py | 11 +++++++++-- lib/ansible/plugins/action/sros.py | 3 +++ lib/ansible/plugins/action/vyos.py | 3 +++ 11 files changed, 50 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdcb37b294..f4b582b45d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,7 +56,7 @@ Ansible Changes By Release * Include_role now complains about invalid arguments * Added socket conditions to ignore for wait_for, no need to error for closing already closed connection * Updated hostname module to work on newer RHEL7 releases - +* Security fix to avoid provider password leaking in logs for network modules diff --git a/lib/ansible/plugins/action/dellos10.py b/lib/ansible/plugins/action/dellos10.py index 171a917bee..2f62e60930 100644 --- a/lib/ansible/plugins/action/dellos10.py +++ b/lib/ansible/plugins/action/dellos10.py @@ -64,6 +64,10 @@ class ActionModule(_ActionModule): pc.become = provider['authorize'] or False pc.become_pass = provider['auth_pass'] + # remove auth from provider arguments + provider.pop('password', None) + provider.pop('auth_pass', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) diff --git a/lib/ansible/plugins/action/dellos6.py b/lib/ansible/plugins/action/dellos6.py index 944e6b0a0b..4d2a4f13a1 100644 --- a/lib/ansible/plugins/action/dellos6.py +++ b/lib/ansible/plugins/action/dellos6.py @@ -60,6 +60,10 @@ class ActionModule(_ActionModule): pc.become = provider['authorize'] or False pc.become_pass = provider['auth_pass'] + # remove auth from provider arguments + provider.pop('password', None) + provider.pop('auth_pass', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) diff --git a/lib/ansible/plugins/action/dellos9.py b/lib/ansible/plugins/action/dellos9.py index d5ecdb161e..1e393c039e 100644 --- a/lib/ansible/plugins/action/dellos9.py +++ b/lib/ansible/plugins/action/dellos9.py @@ -64,6 +64,10 @@ class ActionModule(_ActionModule): pc.become = provider['authorize'] or False pc.become_pass = provider['auth_pass'] + # remove auth from provider arguments + provider.pop('password', None) + provider.pop('auth_pass', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) diff --git a/lib/ansible/plugins/action/eos.py b/lib/ansible/plugins/action/eos.py index 2f0c7fd2bb..85e43ee385 100644 --- a/lib/ansible/plugins/action/eos.py +++ b/lib/ansible/plugins/action/eos.py @@ -65,6 +65,10 @@ class ActionModule(_ActionModule): pc.become = provider['authorize'] or False pc.become_pass = provider['auth_pass'] + # remove auth from provider arguments + provider.pop('password', None) + provider.pop('auth_pass', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) @@ -109,18 +113,22 @@ class ActionModule(_ActionModule): if provider.get('username') is None: provider['username'] = self._play_context.connection_user - if provider.get('password') is None: - provider['password'] = self._play_context.password - if provider.get('authorize') is None: provider['authorize'] = False if provider.get('validate_certs') is None: provider['validate_certs'] = ARGS_DEFAULT_VALUE['validate_certs'] - self._task.args['provider'] = provider + # copy auth to top level module arguments to correctly handle `no_log`. + if self._task.args.get('password') is None: + self._task.args['password'] = provider['password'] or self._play_context.password + # remove auth from provider arguments + provider.pop('password', None) + + self._task.args['provider'] = provider result = super(ActionModule, self).run(tmp, task_vars) + return result def _get_socket_path(self, play_context): diff --git a/lib/ansible/plugins/action/ios.py b/lib/ansible/plugins/action/ios.py index 13c0c7d9ec..0074ee12bf 100644 --- a/lib/ansible/plugins/action/ios.py +++ b/lib/ansible/plugins/action/ios.py @@ -62,6 +62,10 @@ class ActionModule(_ActionModule): pc.become = provider['authorize'] or False pc.become_pass = provider['auth_pass'] + # remove auth from provider arguments + provider.pop('password', None) + provider.pop('auth_pass', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) diff --git a/lib/ansible/plugins/action/iosxr.py b/lib/ansible/plugins/action/iosxr.py index 2b0f4e8e12..46091f1d14 100644 --- a/lib/ansible/plugins/action/iosxr.py +++ b/lib/ansible/plugins/action/iosxr.py @@ -60,6 +60,9 @@ class ActionModule(_ActionModule): pc.password = provider['password'] or self._play_context.password pc.timeout = provider['timeout'] or self._play_context.timeout + # remove auth from provider arguments + provider.pop('password', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) diff --git a/lib/ansible/plugins/action/junos.py b/lib/ansible/plugins/action/junos.py index f637146714..88bbac871b 100644 --- a/lib/ansible/plugins/action/junos.py +++ b/lib/ansible/plugins/action/junos.py @@ -71,6 +71,9 @@ class ActionModule(_ActionModule): pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file pc.timeout = provider['timeout'] or self._play_context.timeout + # remove auth from provider arguments + provider.pop('password', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) diff --git a/lib/ansible/plugins/action/nxos.py b/lib/ansible/plugins/action/nxos.py index 8967afc86a..2f01abd40b 100644 --- a/lib/ansible/plugins/action/nxos.py +++ b/lib/ansible/plugins/action/nxos.py @@ -63,6 +63,9 @@ class ActionModule(_ActionModule): pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file pc.timeout = provider['timeout'] or self._play_context.timeout + # remove auth from provider arguments + provider.pop('password', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) @@ -105,8 +108,12 @@ class ActionModule(_ActionModule): if provider.get('username') is None: provider['username'] = self._play_context.connection_user - if provider.get('password') is None: - provider['password'] = self._play_context.password + # copy auth to top level module arguments to correctly handle `no_log`. + if self._task.args.get('password') is None: + self._task.args['password'] = provider['password'] or self._play_context.password + + # remove auth from provider arguments + provider.pop('password', None) if provider.get('use_ssl') is None: provider['use_ssl'] = False diff --git a/lib/ansible/plugins/action/sros.py b/lib/ansible/plugins/action/sros.py index d510a773af..71de9e22de 100644 --- a/lib/ansible/plugins/action/sros.py +++ b/lib/ansible/plugins/action/sros.py @@ -61,6 +61,9 @@ class ActionModule(_ActionModule): pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file pc.timeout = provider['timeout'] or self._play_context.timeout + # remove auth from provider arguments + provider.pop('password', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) diff --git a/lib/ansible/plugins/action/vyos.py b/lib/ansible/plugins/action/vyos.py index c5cd2c017b..ad39f3551c 100644 --- a/lib/ansible/plugins/action/vyos.py +++ b/lib/ansible/plugins/action/vyos.py @@ -59,6 +59,9 @@ class ActionModule(_ActionModule): pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file pc.timeout = provider['timeout'] or self._play_context.timeout + # remove auth from provider arguments + provider.pop('password', None) + display.vvv('using connection plugin %s' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) -- cgit v1.2.1