From 6a6c31ca9d6892c9bd90827474bd96016efac7e2 Mon Sep 17 00:00:00 2001 From: Pedro Escaleira Date: Tue, 27 Jun 2023 16:42:41 -0300 Subject: [PATCH] Fix bug 2088 by quoting inputs for commands CVE-2022-35503 Change-Id: I924c8a4125b7506f62c6af008ba8ff725b1ca2eb Signed-off-by: Pedro Escaleira --- n2vc/k8s_helm3_conn.py | 46 +++++++++++++++++--------- n2vc/k8s_helm_base_conn.py | 47 +++++++++++++------------- n2vc/k8s_helm_conn.py | 67 +++++++++++++++++++++----------------- n2vc/n2vc_conn.py | 3 +- 4 files changed, 92 insertions(+), 71 deletions(-) diff --git a/n2vc/k8s_helm3_conn.py b/n2vc/k8s_helm3_conn.py index 777fa38..a142a8c 100644 --- a/n2vc/k8s_helm3_conn.py +++ b/n2vc/k8s_helm3_conn.py @@ -20,6 +20,7 @@ # contact with: nfvlabs@tid.es ## from typing import Union +from shlex import quote import os import yaml @@ -257,7 +258,7 @@ class K8sHelm3Connector(K8sHelmBaseConnector): ) command = "{} --kubeconfig={} get namespaces -o=yaml".format( - self.kubectl_command, paths["kube_config"] + self.kubectl_command, quote(paths["kube_config"]) ) output, _rc = await self._local_async_exec( command=command, raise_exception_on_error=True, env=env @@ -278,7 +279,7 @@ class K8sHelm3Connector(K8sHelmBaseConnector): ) command = "{} --kubeconfig={} create namespace {}".format( - self.kubectl_command, paths["kube_config"], namespace + self.kubectl_command, quote(paths["kube_config"]), quote(namespace) ) _, _rc = await self._local_async_exec( command=command, raise_exception_on_error=True, env=env @@ -296,9 +297,11 @@ class K8sHelm3Connector(K8sHelmBaseConnector): ) command1 = "env KUBECONFIG={} {} get manifest {} --namespace={}".format( - kubeconfig, self._helm_command, kdu_instance, namespace + kubeconfig, self._helm_command, quote(kdu_instance), quote(namespace) + ) + command2 = "{} get --namespace={} -f -".format( + self.kubectl_command, quote(namespace) ) - command2 = "{} get --namespace={} -f -".format(self.kubectl_command, namespace) output, _rc = await self._local_async_exec_pipe( command1, command2, env=env, raise_exception_on_error=True ) @@ -362,7 +365,7 @@ class K8sHelm3Connector(K8sHelmBaseConnector): """ inspect_command = "{} show {} {}{} {}".format( - self._helm_command, show_command, kdu_model, repo_str, version + self._helm_command, show_command, quote(kdu_model), repo_str, version ) return inspect_command @@ -371,7 +374,11 @@ class K8sHelm3Connector(K8sHelmBaseConnector): ): get_command = ( "env KUBECONFIG={} {} get {} {} --namespace={} --output yaml".format( - kubeconfig, self._helm_command, get_command, kdu_instance, namespace + kubeconfig, + self._helm_command, + get_command, + quote(kdu_instance), + quote(namespace), ) ) return get_command @@ -396,7 +403,10 @@ class K8sHelm3Connector(K8sHelmBaseConnector): cluster_name=cluster_id, create_if_not_exist=True ) command = "env KUBECONFIG={} {} status {} --namespace={} --output yaml".format( - paths["kube_config"], self._helm_command, kdu_instance, namespace + paths["kube_config"], + self._helm_command, + quote(kdu_instance), + quote(namespace), ) output, rc = await self._local_async_exec( @@ -453,7 +463,7 @@ class K8sHelm3Connector(K8sHelmBaseConnector): # namespace namespace_str = "" if namespace: - namespace_str = "--namespace {}".format(namespace) + namespace_str = "--namespace {}".format(quote(namespace)) # version version_str = "" @@ -465,12 +475,12 @@ class K8sHelm3Connector(K8sHelmBaseConnector): "{params} {timeout} {ns} {model} {ver}".format( kubeconfig=kubeconfig, helm=self._helm_command, - name=kdu_instance, + name=quote(kdu_instance), atomic=atomic_str, params=params_str, timeout=timeout_str, ns=namespace_str, - model=kdu_model, + model=quote(kdu_model), ver=version_str, ) ) @@ -567,12 +577,12 @@ class K8sHelm3Connector(K8sHelmBaseConnector): # version version_str = "" if version: - version_str = "--version {}".format(version) + version_str = "--version {}".format(quote(version)) # namespace namespace_str = "" if namespace: - namespace_str = "--namespace {}".format(namespace) + namespace_str = "--namespace {}".format(quote(namespace)) command = ( "env KUBECONFIG={kubeconfig} {helm} upgrade {name} {model} {namespace} {atomic} " @@ -580,12 +590,12 @@ class K8sHelm3Connector(K8sHelmBaseConnector): ).format( kubeconfig=kubeconfig, helm=self._helm_command, - name=kdu_instance, + name=quote(kdu_instance), namespace=namespace_str, atomic=atomic_str, params=params_str, timeout=timeout_str, - model=kdu_model, + model=quote(kdu_model), ver=version_str, ) return command @@ -594,14 +604,18 @@ class K8sHelm3Connector(K8sHelmBaseConnector): self, kdu_instance: str, namespace: str, revision: float, kubeconfig: str ) -> str: return "env KUBECONFIG={} {} rollback {} {} --namespace={} --wait".format( - kubeconfig, self._helm_command, kdu_instance, revision, namespace + kubeconfig, + self._helm_command, + quote(kdu_instance), + revision, + quote(namespace), ) def _get_uninstall_command( self, kdu_instance: str, namespace: str, kubeconfig: str ) -> str: return "env KUBECONFIG={} {} uninstall {} --namespace={}".format( - kubeconfig, self._helm_command, kdu_instance, namespace + kubeconfig, self._helm_command, quote(kdu_instance), quote(namespace) ) def _get_helm_chart_repos_ids(self, cluster_uuid) -> list: diff --git a/n2vc/k8s_helm_base_conn.py b/n2vc/k8s_helm_base_conn.py index 5985066..16ebbaa 100644 --- a/n2vc/k8s_helm_base_conn.py +++ b/n2vc/k8s_helm_base_conn.py @@ -22,6 +22,7 @@ import abc import asyncio from typing import Union +from shlex import quote import random import time import shlex @@ -180,7 +181,7 @@ class K8sHelmBaseConnector(K8sConnector): # helm repo add name url command = ("env KUBECONFIG={} {} repo add {} {}").format( - paths["kube_config"], self._helm_command, name, url + paths["kube_config"], self._helm_command, quote(name), quote(url) ) if cert: @@ -190,13 +191,13 @@ class K8sHelmBaseConnector(K8sConnector): os.makedirs(os.path.dirname(temp_cert_file), exist_ok=True) with open(temp_cert_file, "w") as the_cert: the_cert.write(cert) - command += " --ca-file {}".format(temp_cert_file) + command += " --ca-file {}".format(quote(temp_cert_file)) if user: - command += " --username={}".format(user) + command += " --username={}".format(quote(user)) if password: - command += " --password={}".format(password) + command += " --password={}".format(quote(password)) self.log.debug("adding repo: {}".format(command)) await self._local_async_exec( @@ -205,7 +206,7 @@ class K8sHelmBaseConnector(K8sConnector): # helm repo update command = "env KUBECONFIG={} {} repo update {}".format( - paths["kube_config"], self._helm_command, name + paths["kube_config"], self._helm_command, quote(name) ) self.log.debug("updating repo: {}".format(command)) await self._local_async_exec( @@ -231,7 +232,7 @@ class K8sHelmBaseConnector(K8sConnector): self.fs.sync(from_path=cluster_uuid) # helm repo update - command = "{} repo update {}".format(self._helm_command, name) + command = "{} repo update {}".format(self._helm_command, quote(name)) self.log.debug("updating repo: {}".format(command)) await self._local_async_exec( command=command, raise_exception_on_error=False, env=env @@ -293,7 +294,7 @@ class K8sHelmBaseConnector(K8sConnector): self.fs.sync(from_path=cluster_uuid) command = "env KUBECONFIG={} {} repo remove {}".format( - paths["kube_config"], self._helm_command, name + paths["kube_config"], self._helm_command, quote(name) ) await self._local_async_exec( command=command, raise_exception_on_error=True, env=env @@ -1688,7 +1689,10 @@ class K8sHelmBaseConnector(K8sConnector): ) command = "{} --kubeconfig={} --namespace={} get service {} -o=yaml".format( - self.kubectl_command, paths["kube_config"], namespace, service_name + self.kubectl_command, + paths["kube_config"], + quote(namespace), + quote(service_name), ) output, _rc = await self._local_async_exec( @@ -1739,20 +1743,20 @@ class K8sHelmBaseConnector(K8sConnector): repo_str = "" if repo_url: - repo_str = " --repo {}".format(repo_url) + repo_str = " --repo {}".format(quote(repo_url)) # Obtain the Chart's name and store it in the var kdu_model kdu_model, _ = self._split_repo(kdu_model=kdu_model) kdu_model, version = self._split_version(kdu_model) if version: - version_str = "--version {}".format(version) + version_str = "--version {}".format(quote(version)) else: version_str = "" full_command = self._get_inspect_command( show_command=inspect_command, - kdu_model=kdu_model, + kdu_model=quote(kdu_model), repo_str=repo_str, version=version_str, ) @@ -1970,19 +1974,14 @@ class K8sHelmBaseConnector(K8sConnector): # params for use in --set option @staticmethod def _params_to_set_option(params: dict) -> str: - params_str = "" - if params and len(params) > 0: - start = True - for key in params: - value = params.get(key, None) - if value is not None: - if start: - params_str += "--set " - start = False - else: - params_str += "," - params_str += "{}={}".format(key, value) - return params_str + pairs = [ + f"{quote(str(key))}={quote(str(value))}" + for key, value in params.items() + if value is not None + ] + if not pairs: + return "" + return "--set " + ",".join(pairs) @staticmethod def generate_kdu_instance_name(**kwargs): diff --git a/n2vc/k8s_helm_conn.py b/n2vc/k8s_helm_conn.py index a3644c8..824ea6a 100644 --- a/n2vc/k8s_helm_conn.py +++ b/n2vc/k8s_helm_conn.py @@ -21,6 +21,7 @@ ## import asyncio from typing import Union +from shlex import quote import os import yaml @@ -73,7 +74,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): self.log.debug("Initializing helm client-only...") command = "{} init --client-only {} ".format( self._helm_command, - "--stable-repo-url {}".format(self._stable_repo_url) + "--stable-repo-url {}".format(quote(self._stable_repo_url)) if self._stable_repo_url else "--skip-repos", ) @@ -240,9 +241,11 @@ class K8sHelmConnector(K8sHelmBaseConnector): ) command1 = "env KUBECONFIG={} {} get manifest {} ".format( - kubeconfig, self._helm_command, kdu_instance + kubeconfig, self._helm_command, quote(kdu_instance) + ) + command2 = "{} get --namespace={} -f -".format( + self.kubectl_command, quote(namespace) ) - command2 = "{} get --namespace={} -f -".format(self.kubectl_command, namespace) output, _rc = await self._local_async_exec_pipe( command1, command2, env=env, raise_exception_on_error=True ) @@ -260,7 +263,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): # check if tiller pod is up in cluster command = "{} --kubeconfig={} --namespace={} get deployments".format( - self.kubectl_command, paths["kube_config"], namespace + self.kubectl_command, paths["kube_config"], quote(namespace) ) output, _rc = await self._local_async_exec( command=command, raise_exception_on_error=True, env=env @@ -285,7 +288,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): "Initializing helm in client and server: {}".format(cluster_id) ) command = "{} --kubeconfig={} --namespace kube-system create serviceaccount {}".format( - self.kubectl_command, paths["kube_config"], self.service_account + self.kubectl_command, paths["kube_config"], quote(self.service_account) ) _, _rc = await self._local_async_exec( command=command, raise_exception_on_error=False, env=env @@ -294,7 +297,9 @@ class K8sHelmConnector(K8sHelmBaseConnector): command = ( "{} --kubeconfig={} create clusterrolebinding osm-tiller-cluster-rule " "--clusterrole=cluster-admin --serviceaccount=kube-system:{}" - ).format(self.kubectl_command, paths["kube_config"], self.service_account) + ).format( + self.kubectl_command, paths["kube_config"], quote(self.service_account) + ) _, _rc = await self._local_async_exec( command=command, raise_exception_on_error=False, env=env ) @@ -305,10 +310,10 @@ class K8sHelmConnector(K8sHelmBaseConnector): ).format( self._helm_command, paths["kube_config"], - namespace, - paths["helm_dir"], - self.service_account, - "--stable-repo-url {}".format(self._stable_repo_url) + quote(namespace), + quote(paths["helm_dir"]), + quote(self.service_account), + "--stable-repo-url {}".format(quote(self._stable_repo_url)) if self._stable_repo_url else "--skip-repos", ) @@ -329,9 +334,9 @@ class K8sHelmConnector(K8sHelmBaseConnector): ).format( self._helm_command, paths["kube_config"], - namespace, - paths["helm_dir"], - "--stable-repo-url {}".format(self._stable_repo_url) + quote(namespace), + quote(paths["helm_dir"]), + "--stable-repo-url {}".format(quote(self._stable_repo_url)) if self._stable_repo_url else "--skip-repos", ) @@ -365,7 +370,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): if not namespace: # find namespace for tiller pod command = "{} --kubeconfig={} get deployments --all-namespaces".format( - self.kubectl_command, paths["kube_config"] + self.kubectl_command, quote(paths["kube_config"]) ) output, _rc = await self._local_async_exec( command=command, raise_exception_on_error=False, env=env @@ -389,7 +394,9 @@ class K8sHelmConnector(K8sHelmBaseConnector): # uninstall tiller from cluster self.log.debug("Uninstalling tiller from cluster {}".format(cluster_id)) command = "{} --kubeconfig={} --home={} reset".format( - self._helm_command, paths["kube_config"], paths["helm_dir"] + self._helm_command, + quote(paths["kube_config"]), + quote(paths["helm_dir"]), ) self.log.debug("resetting: {}".format(command)) output, _rc = await self._local_async_exec( @@ -400,16 +407,16 @@ class K8sHelmConnector(K8sHelmBaseConnector): command = ( "{} --kubeconfig={} delete clusterrolebinding.rbac.authorization.k8s." "io/osm-tiller-cluster-rule" - ).format(self.kubectl_command, paths["kube_config"]) + ).format(self.kubectl_command, quote(paths["kube_config"])) output, _rc = await self._local_async_exec( command=command, raise_exception_on_error=False, env=env ) command = ( "{} --kubeconfig={} --namespace {} delete serviceaccount/{}".format( self.kubectl_command, - paths["kube_config"], - namespace, - self.service_account, + quote(paths["kube_config"]), + quote(namespace), + quote(self.service_account), ) ) output, _rc = await self._local_async_exec( @@ -446,7 +453,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): self, show_command: str, kdu_model: str, repo_str: str, version: str ): inspect_command = "{} inspect {} {}{} {}".format( - self._helm_command, show_command, kdu_model, repo_str, version + self._helm_command, show_command, quote(kdu_model), repo_str, version ) return inspect_command @@ -454,7 +461,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): self, get_command: str, kdu_instance: str, namespace: str, kubeconfig: str ): get_command = "env KUBECONFIG={} {} get {} {} --output yaml".format( - kubeconfig, self._helm_command, get_command, kdu_instance + kubeconfig, self._helm_command, get_command, quote(kdu_instance) ) return get_command @@ -475,7 +482,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): cluster_name=cluster_id, create_if_not_exist=True ) command = ("env KUBECONFIG={} {} status {} --output yaml").format( - paths["kube_config"], self._helm_command, kdu_instance + paths["kube_config"], self._helm_command, quote(kdu_instance) ) output, rc = await self._local_async_exec( command=command, @@ -613,7 +620,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): # namespace namespace_str = "" if namespace: - namespace_str = "--namespace {}".format(namespace) + namespace_str = "--namespace {}".format(quote(namespace)) # version version_str = "" @@ -628,9 +635,9 @@ class K8sHelmConnector(K8sHelmBaseConnector): atomic=atomic_str, params=params_str, timeout=timeout_str, - name=kdu_instance, + name=quote(kdu_instance), ns=namespace_str, - model=kdu_model, + model=quote(kdu_model), ver=version_str, ) ) @@ -727,7 +734,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): # version version_str = "" if version: - version_str = "--version {}".format(version) + version_str = "--version {}".format(quote(version)) command = ( "env KUBECONFIG={kubeconfig} {helm} upgrade {atomic} --output yaml {params} {timeout} " @@ -738,8 +745,8 @@ class K8sHelmConnector(K8sHelmBaseConnector): atomic=atomic_str, params=params_str, timeout=timeout_str, - name=kdu_instance, - model=kdu_model, + name=quote(kdu_instance), + model=quote(kdu_model), ver=version_str, ) return command @@ -748,12 +755,12 @@ class K8sHelmConnector(K8sHelmBaseConnector): self, kdu_instance, namespace, revision, kubeconfig ) -> str: return "env KUBECONFIG={} {} rollback {} {} --wait".format( - kubeconfig, self._helm_command, kdu_instance, revision + kubeconfig, self._helm_command, quote(kdu_instance), revision ) def _get_uninstall_command( self, kdu_instance: str, namespace: str, kubeconfig: str ) -> str: return "env KUBECONFIG={} {} delete --purge {}".format( - kubeconfig, self._helm_command, kdu_instance + kubeconfig, self._helm_command, quote(kdu_instance) ) diff --git a/n2vc/n2vc_conn.py b/n2vc/n2vc_conn.py index c77d779..daee28f 100644 --- a/n2vc/n2vc_conn.py +++ b/n2vc/n2vc_conn.py @@ -24,6 +24,7 @@ import abc import asyncio from http import HTTPStatus +from shlex import quote import os import shlex import subprocess @@ -127,7 +128,7 @@ class N2VCConnector(abc.ABC, Loggable): # If we don't have a key generated, then we have to generate it using ssh-keygen if not os.path.exists(self.private_key_path): cmd = "ssh-keygen -t {} -b {} -N '' -f {}".format( - "rsa", "4096", self.private_key_path + "rsa", "4096", quote(self.private_key_path) ) # run command with arguments subprocess.check_output(shlex.split(cmd)) -- 2.17.1