From: aticig Date: Thu, 14 Jul 2022 21:43:09 +0000 (+0300) Subject: Fixing RO Security Vulnerabilities X-Git-Tag: release-v13.0-start~12 X-Git-Url: https://osm.etsi.org/gitweb/?a=commitdiff_plain;h=7b521f73dd996a279f23b2f512cd89a42c1c79f6;p=osm%2FRO.git Fixing RO Security Vulnerabilities Jinja2 sets autoescape to False, disabling SSL certificate checks, use of unsafe yaml load, Try-Except-Pass detected, use of assert detected. Removing openmanoconfig.py. Change-Id: I3353208e150ae6c2f91befa1a3bbed33ed0c528d Signed-off-by: aticig --- diff --git a/NG-RO/osm_ng_ro/__init__.py b/NG-RO/osm_ng_ro/__init__.py index 2995bad8..561c0f69 100644 --- a/NG-RO/osm_ng_ro/__init__.py +++ b/NG-RO/osm_ng_ro/__init__.py @@ -14,6 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. ## +import logging version = "8.0.1.post0" version_date = "2020-06-29" @@ -23,5 +24,5 @@ try: from pkg_resources import get_distribution version = get_distribution("osm_ng_ro").version -except Exception: - pass +except Exception as error: + logging.exception(f"{error} occured while getting the ro version") diff --git a/NG-RO/osm_ng_ro/ns.py b/NG-RO/osm_ng_ro/ns.py index a7c15628..102a0353 100644 --- a/NG-RO/osm_ng_ro/ns.py +++ b/NG-RO/osm_ng_ro/ns.py @@ -31,6 +31,7 @@ from cryptography.hazmat.primitives import serialization as crypto_serialization from cryptography.hazmat.primitives.asymmetric import rsa from jinja2 import ( Environment, + select_autoescape, StrictUndefined, TemplateError, TemplateNotFound, @@ -77,8 +78,8 @@ def get_process_id(): if text_id: return text_id - except Exception: - pass + except Exception as error: + logging.exception(f"{error} occured while getting process id") # Return a random id return "".join(random_choice("0123456789abcdef") for _ in range(12)) @@ -380,7 +381,10 @@ class Ns(object): str: [description] """ try: - env = Environment(undefined=StrictUndefined) + env = Environment( + undefined=StrictUndefined, + autoescape=select_autoescape(default_for_string=True, default=True), + ) template = env.from_string(cloud_init_content) return template.render(params or {}) diff --git a/NG-RO/osm_ng_ro/ns_thread.py b/NG-RO/osm_ng_ro/ns_thread.py index 7656a544..cb52d77c 100644 --- a/NG-RO/osm_ng_ro/ns_thread.py +++ b/NG-RO/osm_ng_ro/ns_thread.py @@ -504,8 +504,10 @@ class VimInteractionVdu(VimInteractionBase): vim_info_info = yaml.safe_load(vim_info["vim_info"]) if vim_info_info.get("name"): vim_info["name"] = vim_info_info["name"] - except Exception: - pass + except Exception as vim_info_error: + self.logger.exception( + f"{vim_info_error} occured while getting the vim_info from yaml" + ) except vimconn.VimConnException as e: # Mark all tasks at VIM_ERROR status self.logger.error( @@ -746,7 +748,7 @@ class VimInteractionFlavor(VimInteractionBase): flavor_data = task["find_params"]["flavor_data"] vim_flavor_id = target_vim.get_flavor_id_from_data(flavor_data) except vimconn.VimConnNotFoundException: - pass + self.logger.exception("VimConnNotFoundException occured.") if not vim_flavor_id and task.get("params"): # CREATE @@ -1600,7 +1602,9 @@ class NsWorker(threading.Thread): try: mkdir(file_name) except FileExistsError: - pass + self.logger.exception( + "FileExistsError occured while processing vim_config." + ) file_name = file_name + "/ca_cert" @@ -1653,7 +1657,8 @@ class NsWorker(threading.Thread): self.logger.info("Unloaded {}".format(target_id)) rmtree("{}:{}".format(target_id, self.worker_index)) except FileNotFoundError: - pass # this is raised by rmtree if folder does not exist + # This is raised by rmtree if folder does not exist. + self.logger.exception("FileNotFoundError occured while unloading VIM.") except Exception as e: self.logger.error("Cannot unload {}: {}".format(target_id, e)) diff --git a/NG-RO/osm_ng_ro/ro_main.py b/NG-RO/osm_ng_ro/ro_main.py index 376c087e..9500ea24 100644 --- a/NG-RO/osm_ng_ro/ro_main.py +++ b/NG-RO/osm_ng_ro/ro_main.py @@ -227,9 +227,7 @@ class Server(object): cherrypy.request.headers.pop("Content-File-MD5", None) elif "application/yaml" in cherrypy.request.headers["Content-Type"]: error_text = "Invalid yaml format " - indata = yaml.load( - cherrypy.request.body, Loader=yaml.SafeLoader - ) + indata = yaml.safe_load(cherrypy.request.body) cherrypy.request.headers.pop("Content-File-MD5", None) elif ( "application/binary" in cherrypy.request.headers["Content-Type"] @@ -262,13 +260,11 @@ class Server(object): # "Only 'Content-Type' of type 'application/json' or # 'application/yaml' for input format are available") error_text = "Invalid yaml format " - indata = yaml.load( - cherrypy.request.body, Loader=yaml.SafeLoader - ) + indata = yaml.safe_load(cherrypy.request.body) cherrypy.request.headers.pop("Content-File-MD5", None) else: error_text = "Invalid yaml format " - indata = yaml.load(cherrypy.request.body, Loader=yaml.SafeLoader) + indata = yaml.safe_load(cherrypy.request.body) cherrypy.request.headers.pop("Content-File-MD5", None) if not indata: @@ -284,9 +280,11 @@ class Server(object): kwargs[k] = None elif format_yaml: try: - kwargs[k] = yaml.load(v, Loader=yaml.SafeLoader) - except Exception: - pass + kwargs[k] = yaml.safe_load(v) + except Exception as yaml_error: + logging.exception( + f"{yaml_error} occured while parsing the yaml" + ) elif ( k.endswith(".gt") or k.endswith(".lt") @@ -298,8 +296,10 @@ class Server(object): except Exception: try: kwargs[k] = float(v) - except Exception: - pass + except Exception as keyword_error: + logging.exception( + f"{keyword_error} occured while getting the keyword arguments" + ) elif v.find(",") > 0: kwargs[k] = v.split(",") elif isinstance(v, (list, tuple)): @@ -308,9 +308,11 @@ class Server(object): v[index] = None elif format_yaml: try: - v[index] = yaml.load(v[index], Loader=yaml.SafeLoader) - except Exception: - pass + v[index] = yaml.safe_load(v[index]) + except Exception as error: + logging.exception( + f"{error} occured while parsing the yaml" + ) return indata except (ValueError, yaml.YAMLError) as exc: @@ -564,18 +566,14 @@ class Server(object): try: if cherrypy.request.method == "POST": - to_send = yaml.load(cherrypy.request.body, Loader=yaml.SafeLoader) + to_send = yaml.safe_load(cherrypy.request.body) for k, v in to_send.items(): self.ns.msg.write(main_topic, k, v) return_text += " {}: {}\n".format(k, v) elif cherrypy.request.method == "GET": for k, v in kwargs.items(): - self.ns.msg.write( - main_topic, k, yaml.load(v, Loader=yaml.SafeLoader) - ) - return_text += " {}: {}\n".format( - k, yaml.load(v, Loader=yaml.SafeLoader) - ) + self.ns.msg.write(main_topic, k, yaml.safe_load(v)) + return_text += " {}: {}\n".format(k, yaml.safe_load(v)) except Exception as e: return_text += "Error: " + str(e) @@ -990,7 +988,7 @@ if __name__ == "__main__": elif o in ("-c", "--config"): config_file = a else: - assert False, "Unhandled option" + raise ValueError("Unhandled option") if config_file: if not path.isfile(config_file): diff --git a/NG-RO/osm_ng_ro/tests/test_ns.py b/NG-RO/osm_ng_ro/tests/test_ns.py index 3dd699bc..e69a4c5c 100644 --- a/NG-RO/osm_ng_ro/tests/test_ns.py +++ b/NG-RO/osm_ng_ro/tests/test_ns.py @@ -18,7 +18,14 @@ import unittest from unittest.mock import MagicMock, Mock, patch -from jinja2 import TemplateError, TemplateNotFound, UndefinedError +from jinja2 import ( + Environment, + select_autoescape, + StrictUndefined, + TemplateError, + TemplateNotFound, + UndefinedError, +) from osm_ng_ro.ns import Ns, NsException @@ -2632,8 +2639,108 @@ class TestNs(unittest.TestCase): cloud_init_content=cloud_init_content, params=params, context=context ) - def test__parse_jinja2(self): - pass + def test_rendering_jinja2_temp_without_special_characters(self): + cloud_init_content = """ + disk_setup: + ephemeral0: + table_type: {{type}} + layout: True + overwrite: {{is_override}} + runcmd: + - [ ls, -l, / ] + - [ sh, -xc, "echo $(date) '{{command}}'" ] + """ + params = { + "type": "mbr", + "is_override": "False", + "command": "; mkdir abc", + } + context = "cloud-init for VM" + expected_result = """ + disk_setup: + ephemeral0: + table_type: mbr + layout: True + overwrite: False + runcmd: + - [ ls, -l, / ] + - [ sh, -xc, "echo $(date) '; mkdir abc'" ] + """ + result = Ns._parse_jinja2( + cloud_init_content=cloud_init_content, params=params, context=context + ) + self.assertEqual(result, expected_result) + + def test_rendering_jinja2_temp_with_special_characters(self): + cloud_init_content = """ + disk_setup: + ephemeral0: + table_type: {{type}} + layout: True + overwrite: {{is_override}} + runcmd: + - [ ls, -l, / ] + - [ sh, -xc, "echo $(date) '{{command}}'" ] + """ + params = { + "type": "mbr", + "is_override": "False", + "command": "& rm -rf", + } + context = "cloud-init for VM" + expected_result = """ + disk_setup: + ephemeral0: + table_type: mbr + layout: True + overwrite: False + runcmd: + - [ ls, -l, / ] + - [ sh, -xc, "echo $(date) '& rm -rf /'" ] + """ + result = Ns._parse_jinja2( + cloud_init_content=cloud_init_content, params=params, context=context + ) + self.assertNotEqual(result, expected_result) + + def test_rendering_jinja2_temp_with_special_characters_autoescape_is_false(self): + with patch("osm_ng_ro.ns.Environment") as mock_environment: + mock_environment.return_value = Environment( + undefined=StrictUndefined, + autoescape=select_autoescape(default_for_string=False, default=False), + ) + cloud_init_content = """ + disk_setup: + ephemeral0: + table_type: {{type}} + layout: True + overwrite: {{is_override}} + runcmd: + - [ ls, -l, / ] + - [ sh, -xc, "echo $(date) '{{command}}'" ] + """ + params = { + "type": "mbr", + "is_override": "False", + "command": "& rm -rf /", + } + context = "cloud-init for VM" + expected_result = """ + disk_setup: + ephemeral0: + table_type: mbr + layout: True + overwrite: False + runcmd: + - [ ls, -l, / ] + - [ sh, -xc, "echo $(date) '& rm -rf /'" ] + """ + result = Ns._parse_jinja2( + cloud_init_content=cloud_init_content, + params=params, + context=context, + ) + self.assertEqual(result, expected_result) def test__process_vdu_params_empty_kargs(self): pass diff --git a/NG-RO/osm_ng_ro/vim_admin.py b/NG-RO/osm_ng_ro/vim_admin.py index 2582ee2a..3350d434 100644 --- a/NG-RO/osm_ng_ro/vim_admin.py +++ b/NG-RO/osm_ng_ro/vim_admin.py @@ -334,7 +334,7 @@ class VimAdminThread(threading.Thread): self.logger.error("renew_locks task exception: {}".format(exc)) self.aiomain_task_renew_lock = None except asyncio.CancelledError: - pass + self.logger.exception("asyncio.CancelledError occured.") except Exception as e: if self.to_terminate: diff --git a/RO-SDN-arista_cloudvision/osm_rosdn_arista_cloudvision/wimconn_arista.py b/RO-SDN-arista_cloudvision/osm_rosdn_arista_cloudvision/wimconn_arista.py index 99ab4fa9..6be47e16 100644 --- a/RO-SDN-arista_cloudvision/osm_rosdn_arista_cloudvision/wimconn_arista.py +++ b/RO-SDN-arista_cloudvision/osm_rosdn_arista_cloudvision/wimconn_arista.py @@ -632,8 +632,8 @@ class AristaSdnConnector(SdnConnectorBase): try: self.__addMetadata(s_uid, service_type, s_connInf["vlan_id"]) - except Exception: - pass + except Exception as e: + self.logger.exception(f"{e} occured.") return (s_uid, s_connInf) except CvpLoginError as e: @@ -959,7 +959,6 @@ class AristaSdnConnector(SdnConnectorBase): self.logger.error( "Error removing configlets from device {}: {}".format(s, e) ) - pass for s in self.switches: if allLeafConfigured[s]: @@ -1136,7 +1135,7 @@ class AristaSdnConnector(SdnConnectorBase): found_in_cvp = True except CvpApiError as error: if "Entity does not exist" in error.msg: - pass + self.logger.exception(f"{error} occured.") else: raise error @@ -1356,7 +1355,7 @@ class AristaSdnConnector(SdnConnectorBase): found_in_cvp = True except CvpApiError as error: if "Entity does not exist" in error.msg: - pass + self.logger.exception(f"{error} occured.") else: raise error @@ -1378,7 +1377,6 @@ class AristaSdnConnector(SdnConnectorBase): service_uuid, str(e) ) ) - pass def __removeMetadata(self, service_uuid): """Removes the connectivity service from 'OSM_metadata' configLet""" @@ -1389,7 +1387,7 @@ class AristaSdnConnector(SdnConnectorBase): found_in_cvp = True except CvpApiError as error: if "Entity does not exist" in error.msg: - pass + self.logger.exception(f"{error} occured.") else: raise error @@ -1412,7 +1410,6 @@ class AristaSdnConnector(SdnConnectorBase): service_uuid, str(e) ) ) - pass def edit_connectivity_service( self, service_uuid, conn_info=None, connection_points=None, **kwargs @@ -1605,7 +1602,7 @@ class AristaSdnConnector(SdnConnectorBase): found_in_cvp = True except CvpApiError as error: if "Entity does not exist" in error.msg: - pass + self.logger.exception(f"{error} occured.") else: raise error @@ -1626,7 +1623,7 @@ class AristaSdnConnector(SdnConnectorBase): found_in_cvp = True except CvpApiError as error: if "Entity does not exist" in error.msg: - pass + self.logger.exception(f"{error} occured.") else: raise error @@ -1656,7 +1653,7 @@ class AristaSdnConnector(SdnConnectorBase): found_in_cvp = True except CvpApiError as error: if "Entity does not exist" in error.msg: - pass + self.logger.exception(f"{error} occured.") else: raise error diff --git a/RO-VIM-aws/osm_rovim_aws/vimconn_aws.py b/RO-VIM-aws/osm_rovim_aws/vimconn_aws.py index df8914be..5fc07041 100644 --- a/RO-VIM-aws/osm_rovim_aws/vimconn_aws.py +++ b/RO-VIM-aws/osm_rovim_aws/vimconn_aws.py @@ -141,9 +141,9 @@ class vimconnector(vimconn.VimConnector): try: if flavor_data[0] == "@": # read from a file with open(flavor_data[1:], "r") as stream: - self.flavor_info = yaml.load(stream, Loader=yaml.Loader) + self.flavor_info = yaml.safe_load(stream) else: - self.flavor_info = yaml.load(flavor_data, Loader=yaml.Loader) + self.flavor_info = yaml.safe_load(flavor_data) except yaml.YAMLError as e: self.flavor_info = None diff --git a/RO-VIM-azure/osm_rovim_azure/vimconn_azure.py b/RO-VIM-azure/osm_rovim_azure/vimconn_azure.py index 1034f842..0b31d029 100755 --- a/RO-VIM-azure/osm_rovim_azure/vimconn_azure.py +++ b/RO-VIM-azure/osm_rovim_azure/vimconn_azure.py @@ -415,7 +415,7 @@ class vimconnector(vimconn.VimConnector): return except CloudError as e: if e.error.error and "notfound" in e.error.error.lower(): - pass + self.logger.exception("CloudError Exception occured.") # continue and create it else: self._format_vimconn_exception(e) diff --git a/RO-plugin/osm_ro_plugin/openflow_conn.py b/RO-plugin/osm_ro_plugin/openflow_conn.py index f1869e85..afc0b821 100644 --- a/RO-plugin/osm_ro_plugin/openflow_conn.py +++ b/RO-plugin/osm_ro_plugin/openflow_conn.py @@ -369,7 +369,7 @@ class SdnConnectorOpenFlow(SdnConnectorBase): try: self.of_connector.del_flow(flow_id) except OpenflowConnNotFoundException: - pass + self.logger.exception("OpenflowConnNotFoundException occured.") except OpenflowConnException as e: error_text = "Cannot remove rule '{}': {}".format(flow_id, e) error_list.append(error_text) diff --git a/openmanoconfig.py b/openmanoconfig.py deleted file mode 100755 index 70268665..00000000 --- a/openmanoconfig.py +++ /dev/null @@ -1,124 +0,0 @@ -#!/usr/bin/env python -# -*- coding: utf-8 -*- - -## -# Copyright 2015 Telefonica Investigacion y Desarrollo, S.A.U. -# This file is part of openmano -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -# -# For those usages not covered by the Apache License, Version 2.0 please -# contact with: nfvlabs@tid.es -## - - -""" -Read openmanod.cfg file and creates envioronment variables for openmano client -Call it wusing execution quotes, or copy paste the output to set your shell envioronment -It read database to look for a ninc tenant / datacenter -""" - -from __future__ import print_function -from os import environ -from openmanod import load_configuration -#from socket import gethostname -from db_base import db_base_Exception -import nfvo_db -import getopt -import sys - - -__author__="Alfonso Tierno, Gerardo Garcia, Pablo Montes" -__date__ ="$26-aug-2014 11:09:29$" -__version__="0.0.1-r509" -version_date="Oct 2016" -database_version="0.16" #expected database schema version - - -def usage(): - print("Usage: ", sys.argv[0], "[options]") - print(" -v|--version: prints current version") - print(" -c|--config [configuration_file]: loads the configuration file (default: openmanod.cfg)") - print(" -h|--help: shows this help") - return - - -if __name__ == "__main__": - # Read parameters and configuration file - try: - # load parameters and configuration - opts, args = getopt.getopt(sys.argv[1:], "vhc:", - ["config=", "help", "version"]) - config_file = 'openmanod.cfg' - - for o, a in opts: - if o in ("-v", "--version"): - print("openmanoconfig.py version " + __version__ + ' ' + version_date) - print("(c) Copyright Telefonica") - exit() - elif o in ("-h", "--help"): - usage() - exit() - elif o in ("-c", "--config"): - config_file = a - else: - assert False, "Unhandled option" - global_config = load_configuration(config_file) - if global_config["http_host"] == "0.0.0.0": - global_config["http_host"] = "localhost" #gethostname() - environ["OPENMANO_HOST"]=global_config["http_host"] - print("export OPENMANO_HOST='{}'".format(global_config["http_host"])) - environ["OPENMANO_PORT"] = str(global_config["http_port"]) - print("export OPENMANO_PORT={}".format(global_config["http_port"])) - - mydb = nfvo_db.nfvo_db(); - mydb.connect(global_config['db_host'], global_config['db_user'], global_config['db_passwd'], global_config['db_name']) - try: - tenants = mydb.get_rows(FROM="nfvo_tenants") - if not tenants: - print("#No tenant found", file=sys.stderr) - elif len(tenants) > 1: - print("#Found several tenants export OPENMANO_TENANT=", file=sys.stderr, end="") - for tenant in tenants: - print(" '{}'".format(tenant["name"]), file=sys.stderr, end="") - print("") - else: - environ["OPENMANO_TENANT"] = tenants[0]["name"] - print("export OPENMANO_TENANT='{}'".format(tenants[0]["name"])) - - dcs = mydb.get_rows(FROM="datacenters") - if not dcs: - print("#No datacenter found", file=sys.stderr) - elif len(dcs) > 1: - print("#Found several datacenters export OPENMANO_DATACENTER=", file=sys.stderr, end="") - for dc in dcs: - print(" '{}'".format(dc["name"]), file=sys.stderr, end="") - print("") - else: - environ["OPENMANO_DATACENTER"] = dcs[0]["name"] - print("export OPENMANO_DATACENTER='{}'".format(dcs[0]["name"])) - - except db_base_Exception as e: - print("#DATABASE is not a MANO one or it is a '0.0' version. Try to upgrade to version '{}' with \ - './database_utils/migrate_mano_db.sh'".format(database_version), file=sys.stderr) - exit(-1) - - - - except db_base_Exception as e: - print("#"+str(e), file=sys.stderr) - exit(-1) - - except SystemExit: - pass diff --git a/releasenotes/notes/Fixing_security_vulnerabilities-bdca2f49083e5b6d.yaml b/releasenotes/notes/Fixing_security_vulnerabilities-bdca2f49083e5b6d.yaml new file mode 100644 index 00000000..56944c32 --- /dev/null +++ b/releasenotes/notes/Fixing_security_vulnerabilities-bdca2f49083e5b6d.yaml @@ -0,0 +1,22 @@ +####################################################################################### +# Copyright ETSI Contributors and Others. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. +####################################################################################### +--- +security: + - | + Fixing following RO security vulnerabilities. Improper Certificate Validation, jinja2 sets autoescape to False, + disabling SSL certificate checks, use of unsafe yaml load, try-except-pass detected, use of assert detected. + diff --git a/tox.ini b/tox.ini index 148a604a..43b1a92a 100644 --- a/tox.ini +++ b/tox.ini @@ -451,7 +451,8 @@ ignore = E125, E203, E226, - E241 + E241, + E501, exclude = .git, __pycache__,