From 4c0e6805c44f9ed1d0bb35161bf69645f5b84151 Mon Sep 17 00:00:00 2001 From: Gabriel Cuba Date: Mon, 9 Oct 2023 13:22:38 -0500 Subject: [PATCH] Fix multiple minor security vulnerabilities Change-Id: I7c2a42c8bd025767de12ebca2573ab01d200c100 Signed-off-by: Gabriel Cuba --- osm_lcm/ROclient.py | 11 +++++---- osm_lcm/lcm.py | 44 ++++++++++++++++----------------- osm_lcm/lcm_utils.py | 2 +- osm_lcm/ns.py | 10 +++++--- osm_lcm/tests/test_lcm_utils.py | 28 ++++++++++----------- 5 files changed, 48 insertions(+), 47 deletions(-) diff --git a/osm_lcm/ROclient.py b/osm_lcm/ROclient.py index cdbde9c..e731ec1 100644 --- a/osm_lcm/ROclient.py +++ b/osm_lcm/ROclient.py @@ -99,7 +99,7 @@ def remove_envelop(item, indata=None): if len(indata) == 1 and "instance" in indata: clean_indata = indata["instance"] else: - assert False, "remove_envelop with unknown item {}".format(item) + raise ROClientException("remove_envelop with unknown item {}".format(item)) return clean_indata @@ -272,7 +272,7 @@ class ROClient: elif item == "sdn": return {"sdn_controller": indata} else: - assert False, "_create_envelop with unknown item {}".format(item) + raise ROClientException("remove_envelop with unknown item {}".format(item)) @staticmethod def update_descriptor(desc, kwargs): @@ -359,9 +359,10 @@ class ROClient: if item_id: return item_id desc = content[item] - assert isinstance( - desc, list - ), "_get_item_uuid get a non dict with a list inside {}".format(type(desc)) + if not isinstance(desc, list): + raise ROClientException( + "_get_item_uuid get a non dict with a list inside {}".format(type(desc)) + ) uuid = None for i in desc: if item_id_name and i["name"] != item_id_name: diff --git a/osm_lcm/lcm.py b/osm_lcm/lcm.py index 2fc479f..5ed39ab 100644 --- a/osm_lcm/lcm.py +++ b/osm_lcm/lcm.py @@ -19,7 +19,6 @@ # DEBUG WITH PDB -import os import pdb import asyncio @@ -28,6 +27,7 @@ import logging import logging.handlers import getopt import sys +from random import SystemRandom from osm_lcm import ns, vim_sdn, netslice from osm_lcm.ng_ro import NgRoException, NgRoClient @@ -46,12 +46,11 @@ from osm_lcm.data_utils.database.database import Database from osm_lcm.data_utils.filesystem.filesystem import Filesystem from osm_lcm.data_utils.lcm_config import LcmCfg from osm_lcm.lcm_hc import get_health_check_file -from os import path -from random import choice as random_choice +from os import path, getenv from n2vc import version as n2vc_version import traceback -if os.getenv("OSMLCM_PDB_DEBUG", None) is not None: +if getenv("OSMLCM_PDB_DEBUG", None) is not None: pdb.set_trace() @@ -761,18 +760,22 @@ class Lcm: will provide a random one :return: Obtained ID """ - # Try getting docker id. If fails, get pid - try: - with open("/proc/self/cgroup", "r") as f: - text_id_ = f.readline() - _, _, text_id = text_id_.rpartition("/") - text_id = text_id.replace("\n", "")[:12] - if text_id: - return text_id - except Exception: - pass - # Return a random id - return "".join(random_choice("0123456789abcdef") for _ in range(12)) + + def get_docker_id(): + try: + with open("/proc/self/cgroup", "r") as f: + text_id_ = f.readline() + _, _, text_id = text_id_.rpartition("/") + return text_id.replace("\n", "")[:12] + except Exception: + return None + + def generate_random_id(): + return "".join(SystemRandom().choice("0123456789abcdef") for _ in range(12)) + + # Try getting docker id. If it fails, generate a random id + docker_id = get_docker_id() + return docker_id if docker_id else generate_random_id() def usage(): @@ -813,14 +816,9 @@ if __name__ == "__main__": from osm_lcm.lcm_hc import health_check health_check(config_file, Lcm.ping_interval_pace) - # elif o == "--log-socket-port": - # log_socket_port = a - # elif o == "--log-socket-host": - # log_socket_host = a - # elif o == "--log-file": - # log_file = a else: - assert False, "Unhandled option" + print(f"Unhandled option: {o}") + exit(1) if config_file: if not path.isfile(config_file): diff --git a/osm_lcm/lcm_utils.py b/osm_lcm/lcm_utils.py index 639c5ec..5817b16 100644 --- a/osm_lcm/lcm_utils.py +++ b/osm_lcm/lcm_utils.py @@ -236,7 +236,7 @@ class LcmBase: Returns: hex digest (str): The hash of the charm file """ - filehash = hashlib.md5() + filehash = hashlib.sha256() with open(zipped_file, mode="rb") as file: contents = file.read() filehash.update(contents) diff --git a/osm_lcm/ns.py b/osm_lcm/ns.py index 63ae2a4..fad3972 100644 --- a/osm_lcm/ns.py +++ b/osm_lcm/ns.py @@ -3628,9 +3628,11 @@ class NsLcm(LcmBase): vnfr_data.get("_id"), {"kdur.{}.status".format(kdu_index): "ERROR"}, ) - except Exception: + except Exception as error: # ignore to keep original exception - pass + self.logger.warning( + f"An exception occurred while updating DB: {str(error)}" + ) # reraise original error raise @@ -3790,8 +3792,8 @@ class NsLcm(LcmBase): kdumodel = self.fs.path + filename except (asyncio.TimeoutError, asyncio.CancelledError): raise - except Exception: # it is not a file - pass + except Exception as e: # it is not a file + self.logger.warning(f"An exception occurred: {str(e)}") k8s_cluster_id = kdur["k8s-cluster"]["id"] step = "Synchronize repos for k8s cluster '{}'".format( diff --git a/osm_lcm/tests/test_lcm_utils.py b/osm_lcm/tests/test_lcm_utils.py index bcd242f..14aa5a5 100644 --- a/osm_lcm/tests/test_lcm_utils.py +++ b/osm_lcm/tests/test_lcm_utils.py @@ -253,14 +253,14 @@ class TestLcmBase(TestCase): charm = tmpfile hexdigest = self.hexdigest mock_file_hash = MagicMock() - mock_hashlib.md5.return_value = mock_file_hash + mock_hashlib.sha256.return_value = mock_file_hash mock_file_hash.hexdigest.return_value = hexdigest result = LcmBase.calculate_charm_hash(charm) self.assertEqual(result, hexdigest) self.assertEqual(mocking_open.call_count, 1) self.assertEqual(mock_file_hash.update.call_count, 1) self.assertEqual(mock_file_hash.hexdigest.call_count, 1) - self.assertEqual(mock_hashlib.md5.call_count, 1) + self.assertEqual(mock_hashlib.sha256.call_count, 1) @patch("builtins.open", new_callable=mock_open(read_data="charm content")) @patch("osm_lcm.lcm_utils.hashlib") @@ -269,7 +269,7 @@ class TestLcmBase(TestCase): charm = tmpfile hexdigest = self.hexdigest mock_file_hash = MagicMock() - mock_hashlib.md5.return_value = mock_file_hash + mock_hashlib.sha256.return_value = mock_file_hash mock_file_hash.hexdigest.return_value = hexdigest mocking_open.side_effect = IOError with self.assertRaises(IOError): @@ -277,7 +277,7 @@ class TestLcmBase(TestCase): self.assertEqual(mocking_open.call_count, 1) mock_file_hash.update.assert_not_called() mock_file_hash.hexdigest.assert_not_called() - self.assertEqual(mock_hashlib.md5.call_count, 1) + self.assertEqual(mock_hashlib.sha256.call_count, 1) @patch("builtins.open", new_callable=mock_open(read_data="charm content")) @patch("osm_lcm.lcm_utils.hashlib") @@ -287,14 +287,14 @@ class TestLcmBase(TestCase): hexdigest = self.hexdigest mock_file_hash = MagicMock() mock_file_hash.update.side_effect = Exception - mock_hashlib.md5.return_value = mock_file_hash + mock_hashlib.sha256.return_value = mock_file_hash mock_file_hash.hexdigest.return_value = hexdigest with self.assertRaises(Exception): LcmBase.calculate_charm_hash(charm) self.assertEqual(mocking_open.call_count, 1) self.assertEqual(mock_file_hash.update.call_count, 1) mock_file_hash.hexdigest.assert_not_called() - self.assertEqual(mock_hashlib.md5.call_count, 1) + self.assertEqual(mock_hashlib.sha256.call_count, 1) @patch("builtins.open", new_callable=mock_open(read_data="charm content")) @patch("osm_lcm.lcm_utils.hashlib") @@ -304,27 +304,27 @@ class TestLcmBase(TestCase): """Filehash hexdigest raises exception.""" charm = tmpfile mock_file_hash = MagicMock() - mock_hashlib.md5.return_value = mock_file_hash + mock_hashlib.sha256.return_value = mock_file_hash mock_file_hash.hexdigest.side_effect = Exception with self.assertRaises(Exception): LcmBase.calculate_charm_hash(charm) self.assertEqual(mocking_open.call_count, 1) self.assertEqual(mock_file_hash.update.call_count, 1) mock_file_hash.hexdigest.assert_called_once() - self.assertEqual(mock_hashlib.md5.call_count, 1) + self.assertEqual(mock_hashlib.sha256.call_count, 1) mock_file_hash.update.assert_called_once() @patch("builtins.open", new_callable=mock_open(read_data="charm content")) @patch("osm_lcm.lcm_utils.hashlib") - def test_calculate_charm_filehash_hashlib_md5_raises( + def test_calculate_charm_filehash_hashlib_sha256_raises( self, mock_hashlib, mocking_open ): - """Filehash hashlib md5 raises exception.""" + """Filehash hashlib sha256 raises exception.""" charm = tmpfile - mock_hashlib.md5.side_effect = Exception + mock_hashlib.sha256.side_effect = Exception with self.assertRaises(Exception): LcmBase.calculate_charm_hash(charm) - self.assertEqual(mock_hashlib.md5.call_count, 1) + self.assertEqual(mock_hashlib.sha256.call_count, 1) mocking_open.assert_not_called() @patch("builtins.open", new_callable=mock_open(read_data="charm content")) @@ -333,14 +333,14 @@ class TestLcmBase(TestCase): """Calculate charm hash, charm file does not exist.""" file = None mock_file_hash = MagicMock() - mock_hashlib.md5.return_value = mock_file_hash + mock_hashlib.sha256.return_value = mock_file_hash mocking_open.side_effect = FileNotFoundError with self.assertRaises(FileNotFoundError): LcmBase.calculate_charm_hash(file) self.assertEqual(mocking_open.call_count, 1) mock_file_hash.update.assert_not_called() mock_file_hash.hexdigest.assert_not_called() - self.assertEqual(mock_hashlib.md5.call_count, 1) + self.assertEqual(mock_hashlib.sha256.call_count, 1) @patch("osm_lcm.lcm_utils.LcmBase.calculate_charm_hash") def test_compare_charm_hash_charm_changed(self, mock_calculate_charm_hash): -- 2.17.1