From 6b28ef3c17449fdd34d0b51c3846278db80239c7 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Sat, 11 Jul 2020 08:50:15 +0530 Subject: [PATCH] kubectl: redacted token and password from console log (#159) ** SECURITY_FIX ** for CVE-2020-1753 kubectl connection plugin now redact kubectl_token and kubectl_password from console log. Fixes: #65 Signed-off-by: Abhijeet Kasurde --- changelogs/fragments/65_kubectl.yml | 2 ++ plugins/connection/kubectl.py | 28 +++++++++++++++++++--------- 2 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 changelogs/fragments/65_kubectl.yml diff --git a/changelogs/fragments/65_kubectl.yml b/changelogs/fragments/65_kubectl.yml new file mode 100644 index 00000000..0c30a61b --- /dev/null +++ b/changelogs/fragments/65_kubectl.yml @@ -0,0 +1,2 @@ +security_fixes: +- kubectl - connection plugin now redact kubectl_token and kubectl_password in console log (https://github.com/ansible-collections/community.kubernetes/issues/65). diff --git a/plugins/connection/kubectl.py b/plugins/connection/kubectl.py index 71f21e2f..f3015e2e 100644 --- a/plugins/connection/kubectl.py +++ b/plugins/connection/kubectl.py @@ -20,7 +20,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -DOCUMENTATION = """ +DOCUMENTATION = r""" author: - xuxinkun @@ -38,7 +38,8 @@ options: kubectl_pod: description: - - Pod name. Required when the host name does not match pod name. + - Pod name. + - Required when the host name does not match pod name. default: '' vars: - name: ansible_kubectl_pod @@ -46,7 +47,8 @@ - name: K8S_AUTH_POD kubectl_container: description: - - Container name. Required when a pod contains more than one container. + - Container name. + - Required when a pod contains more than one container. default: '' vars: - name: ansible_kubectl_container @@ -226,7 +228,7 @@ def __init__(self, play_context, new_stdin, *args, **kwargs): def _build_exec_cmd(self, cmd): """ Build the local kubectl exec command to run cmd on remote_host """ - local_cmd = [self.transport_cmd] + local_cmd = censored_local_cmd = [self.transport_cmd] # Build command options based on doc string doc_yaml = AnsibleLoader(self.documentation).get_single_data() @@ -235,28 +237,36 @@ def _build_exec_cmd(self, cmd): # Translate verify_ssl to skip_verify_ssl, and output as string skip_verify_ssl = not self.get_option(key) local_cmd.append(u'{0}={1}'.format(self.connection_options[key], str(skip_verify_ssl).lower())) + censored_local_cmd.append(u'{0}={1}'.format(self.connection_options[key], str(skip_verify_ssl).lower())) elif not key.endswith('container') and self.get_option(key) and self.connection_options.get(key): cmd_arg = self.connection_options[key] local_cmd += [cmd_arg, self.get_option(key)] + # Redact password and token from console log + if key.endswith(('_token', '_password')): + censored_local_cmd += [cmd_arg, '********'] extra_args_name = u'{0}_extra_args'.format(self.transport) if self.get_option(extra_args_name): local_cmd += self.get_option(extra_args_name).split(' ') + censored_local_cmd += self.get_option(extra_args_name).split(' ') pod = self.get_option(u'{0}_pod'.format(self.transport)) if not pod: pod = self._play_context.remote_addr # -i is needed to keep stdin open which allows pipelining to work local_cmd += ['exec', '-i', pod] + censored_local_cmd += ['exec', '-i', pod] # if the pod has more than one container, then container is required container_arg_name = u'{0}_container'.format(self.transport) if self.get_option(container_arg_name): local_cmd += ['-c', self.get_option(container_arg_name)] + censored_local_cmd += ['-c', self.get_option(container_arg_name)] local_cmd += ['--'] + cmd + censored_local_cmd += ['--'] + cmd - return local_cmd + return local_cmd, censored_local_cmd def _connect(self, port=None): """ Connect to the container. Nothing to do """ @@ -269,9 +279,9 @@ def exec_command(self, cmd, in_data=None, sudoable=False): """ Run a command in the container """ super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable) - local_cmd = self._build_exec_cmd([self._play_context.executable, '-c', cmd]) + local_cmd, censored_local_cmd = self._build_exec_cmd([self._play_context.executable, '-c', cmd]) - display.vvv("EXEC %s" % (local_cmd,), host=self._play_context.remote_addr) + display.vvv("EXEC %s" % (censored_local_cmd,), host=self._play_context.remote_addr) local_cmd = [to_bytes(i, errors='surrogate_or_strict') for i in local_cmd] p = subprocess.Popen(local_cmd, shell=False, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -311,7 +321,7 @@ def put_file(self, in_path, out_path): count = ' count=0' else: count = '' - args = self._build_exec_cmd([self._play_context.executable, "-c", "dd of=%s bs=%s%s" % (out_path, BUFSIZE, count)]) + args, dummy = self._build_exec_cmd([self._play_context.executable, "-c", "dd of=%s bs=%s%s" % (out_path, BUFSIZE, count)]) args = [to_bytes(i, errors='surrogate_or_strict') for i in args] try: p = subprocess.Popen(args, stdin=in_file, @@ -333,7 +343,7 @@ def fetch_file(self, in_path, out_path): # kubectl doesn't have native support for fetching files from # running containers, so we use kubectl exec to implement this - args = self._build_exec_cmd([self._play_context.executable, "-c", "dd if=%s bs=%s" % (in_path, BUFSIZE)]) + args, dummy = self._build_exec_cmd([self._play_context.executable, "-c", "dd if=%s bs=%s" % (in_path, BUFSIZE)]) args = [to_bytes(i, errors='surrogate_or_strict') for i in args] actual_out_path = os.path.join(out_dir, os.path.basename(in_path)) with open(to_bytes(actual_out_path, errors='surrogate_or_strict'), 'wb') as out_file: