Fixing RO Security Vulnerabilities 48/12348/9
authoraticig <gulsum.atici@canonical.com>
Thu, 14 Jul 2022 21:43:09 +0000 (00:43 +0300)
committeraticig <gulsum.atici@canonical.com>
Sun, 31 Jul 2022 21:13:55 +0000 (00:13 +0300)
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 <gulsum.atici@canonical.com>
13 files changed:
NG-RO/osm_ng_ro/__init__.py
NG-RO/osm_ng_ro/ns.py
NG-RO/osm_ng_ro/ns_thread.py
NG-RO/osm_ng_ro/ro_main.py
NG-RO/osm_ng_ro/tests/test_ns.py
NG-RO/osm_ng_ro/vim_admin.py
RO-SDN-arista_cloudvision/osm_rosdn_arista_cloudvision/wimconn_arista.py
RO-VIM-aws/osm_rovim_aws/vimconn_aws.py
RO-VIM-azure/osm_rovim_azure/vimconn_azure.py
RO-plugin/osm_ro_plugin/openflow_conn.py
openmanoconfig.py [deleted file]
releasenotes/notes/Fixing_security_vulnerabilities-bdca2f49083e5b6d.yaml [new file with mode: 0644]
tox.ini

index 2995bad..561c0f6 100644 (file)
@@ -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")
index a7c1562..102a035 100644 (file)
@@ -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 {})
index 7656a54..cb52d77 100644 (file)
@@ -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))
 
index 376c087..9500ea2 100644 (file)
@@ -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):
index 3dd699b..e69a4c5 100644 (file)
 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
index 2582ee2..3350d43 100644 (file)
@@ -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:
index 99ab4fa..6be47e1 100644 (file)
@@ -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
 
index df8914b..5fc0704 100644 (file)
@@ -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
 
index 1034f84..0b31d02 100755 (executable)
@@ -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)
index f1869e8..afc0b82 100644 (file)
@@ -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 (executable)
index 7026866..0000000
+++ /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 (file)
index 0000000..56944c3
--- /dev/null
@@ -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 148a604..43b1a92 100644 (file)
--- a/tox.ini
+++ b/tox.ini
@@ -451,7 +451,8 @@ ignore =
         E125,
         E203,
         E226,
-        E241
+        E241,
+        E501,
 exclude =
         .git,
         __pycache__,