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 <gulsum.atici@canonical.com>
diff --git a/NG-RO/osm_ng_ro/__init__.py b/NG-RO/osm_ng_ro/__init__.py
index 2995bad..561c0f6 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 @@
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 a7c1562..102a035 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.asymmetric import rsa
from jinja2 import (
Environment,
+ select_autoescape,
StrictUndefined,
TemplateError,
TemplateNotFound,
@@ -77,8 +78,8 @@
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 @@
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 7656a54..cb52d77 100644
--- a/NG-RO/osm_ng_ro/ns_thread.py
+++ b/NG-RO/osm_ng_ro/ns_thread.py
@@ -504,8 +504,10 @@
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 @@
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 @@
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 @@
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 376c087..9500ea2 100644
--- a/NG-RO/osm_ng_ro/ro_main.py
+++ b/NG-RO/osm_ng_ro/ro_main.py
@@ -227,9 +227,7 @@
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 @@
# "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 @@
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 @@
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 @@
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 @@
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 @@
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 3dd699b..e69a4c5 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 @@
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 2582ee2..3350d43 100644
--- a/NG-RO/osm_ng_ro/vim_admin.py
+++ b/NG-RO/osm_ng_ro/vim_admin.py
@@ -334,7 +334,7 @@
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: