From de6984b39684c3a8587ea4111c757ab878942aba Mon Sep 17 00:00:00 2001 From: Daniel Arndt Date: Tue, 27 Jun 2023 16:42:41 -0300 Subject: [PATCH] Fix bug 2088 by quoting inputs for commands CVE-2022-35503 Change-Id: I392eda9138d399b956dd8072a082e15edab142b7 Signed-off-by: Daniel Arndt --- n2vc/k8s_helm3_conn.py | 46 ++++++++++++++++--------- n2vc/k8s_helm_base_conn.py | 59 ++++++++++++++++---------------- n2vc/k8s_helm_conn.py | 69 +++++++++++++++++++++----------------- n2vc/n2vc_conn.py | 9 +++-- 4 files changed, 103 insertions(+), 80 deletions(-) diff --git a/n2vc/k8s_helm3_conn.py b/n2vc/k8s_helm3_conn.py index 4baadae..675c851 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 @@ -258,7 +259,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 @@ -279,7 +280,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 @@ -297,9 +298,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 ) @@ -364,7 +367,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 @@ -373,7 +376,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 @@ -398,7 +405,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( @@ -455,7 +465,7 @@ class K8sHelm3Connector(K8sHelmBaseConnector): # namespace namespace_str = "" if namespace: - namespace_str = "--namespace {}".format(namespace) + namespace_str = "--namespace {}".format(quote(namespace)) # version version_str = "" @@ -467,12 +477,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, ) ) @@ -575,12 +585,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} {force}" @@ -588,13 +598,13 @@ class K8sHelm3Connector(K8sHelmBaseConnector): ).format( kubeconfig=kubeconfig, helm=self._helm_command, - name=kdu_instance, + name=quote(kdu_instance), namespace=namespace_str, atomic=atomic_str, force=force_str, params=params_str, timeout=timeout_str, - model=kdu_model, + model=quote(kdu_model), ver=version_str, ) return command @@ -603,14 +613,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 a897e0e..383ce7d 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 @@ -113,7 +114,7 @@ class K8sHelmBaseConnector(K8sConnector): namespace: str = "kube-system", reuse_cluster_uuid=None, **kwargs, - ) -> (str, bool): + ) -> tuple[str, bool]: """ It prepares a given K8s cluster environment to run Charts @@ -181,7 +182,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: @@ -191,13 +192,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( @@ -206,7 +207,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( @@ -232,7 +233,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 @@ -294,7 +295,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 @@ -1538,7 +1539,7 @@ class K8sHelmBaseConnector(K8sConnector): show_error_log: bool = True, encode_utf8: bool = False, env: dict = None, - ) -> (str, int): + ) -> tuple[str, int]: command = K8sHelmBaseConnector._remove_multiple_spaces(command) self.log.debug( "Executing async local command: {}, env: {}".format(command, env) @@ -1704,7 +1705,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( @@ -1755,20 +1759,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, ) @@ -1782,7 +1786,7 @@ class K8sHelmBaseConnector(K8sConnector): kdu_model: str, repo_url: str = None, resource_name: str = None, - ) -> (int, str): + ) -> tuple[int, str]: """Get the replica count value in the Helm Chart Values. Args: @@ -1957,7 +1961,7 @@ class K8sHelmBaseConnector(K8sConnector): # params for use in -f file # returns values file option and filename (in order to delete it at the end) - def _params_to_file_option(self, cluster_id: str, params: dict) -> (str, str): + def _params_to_file_option(self, cluster_id: str, params: dict) -> tuple[str, str]: if params and len(params) > 0: self._init_paths_env(cluster_name=cluster_id, create_if_not_exist=True) @@ -1986,19 +1990,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): @@ -2036,7 +2035,7 @@ class K8sHelmBaseConnector(K8sConnector): name = name + get_random_number() return name.lower() - def _split_version(self, kdu_model: str) -> (str, str): + def _split_version(self, kdu_model: str) -> tuple[str, str]: version = None if not self._is_helm_chart_a_file(kdu_model) and ":" in kdu_model: parts = kdu_model.split(sep=":") @@ -2045,7 +2044,7 @@ class K8sHelmBaseConnector(K8sConnector): kdu_model = parts[0] return kdu_model, version - def _split_repo(self, kdu_model: str) -> (str, str): + def _split_repo(self, kdu_model: str) -> tuple[str, str]: """Obtain the Helm Chart's repository and Chart's names from the KDU model Args: diff --git a/n2vc/k8s_helm_conn.py b/n2vc/k8s_helm_conn.py index bbe4c48..17e960f 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", ) @@ -237,9 +238,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 ) @@ -257,7 +260,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 @@ -282,7 +285,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 @@ -291,7 +294,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 ) @@ -302,10 +307,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", ) @@ -326,9 +331,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", ) @@ -362,7 +367,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 @@ -386,7 +391,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( @@ -397,16 +404,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( @@ -443,7 +450,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 @@ -451,7 +458,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 @@ -472,7 +479,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, @@ -610,7 +617,7 @@ class K8sHelmConnector(K8sHelmBaseConnector): # namespace namespace_str = "" if namespace: - namespace_str = "--namespace {}".format(namespace) + namespace_str = "--namespace {}".format(quote(namespace)) # version version_str = "" @@ -625,9 +632,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, ) ) @@ -730,12 +737,12 @@ class K8sHelmConnector(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 {namespace} {atomic} --output yaml {params} {timeout} {force}" @@ -748,8 +755,8 @@ class K8sHelmConnector(K8sHelmBaseConnector): force=force_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 @@ -758,12 +765,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 9e91a10..01d7df8 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 @@ -131,7 +132,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): command = "ssh-keygen -t {} -b {} -N '' -f {}".format( - "rsa", "4096", self.private_key_path + "rsa", "4096", quote(self.private_key_path) ) # run command with arguments args = shlex.split(command) @@ -151,7 +152,7 @@ class N2VCConnector(abc.ABC, Loggable): reuse_ee_id: str = None, progress_timeout: float = None, total_timeout: float = None, - ) -> (str, dict): + ) -> tuple[str, dict]: """Create an Execution Environment. Returns when it is created or raises an exception on failing @@ -398,7 +399,9 @@ class N2VCConnector(abc.ABC, Loggable): #################################################################################### """ - def _get_namespace_components(self, namespace: str) -> (str, str, str, str, str): + def _get_namespace_components( + self, namespace: str + ) -> tuple[str, str, str, str, str]: """ Split namespace components -- 2.17.1