From d0fb0b7c5478720dd11d1fb418535bf39cc89a7e Mon Sep 17 00:00:00 2001 From: Seth Hollandsworth Date: Wed, 19 Apr 2023 10:16:05 -0400 Subject: [PATCH] making it so you can disable allow_elevated via the privileged field in the arm template --- src/confcom/azext_confcom/config.py | 3 + src/confcom/azext_confcom/container.py | 32 +++- src/confcom/azext_confcom/security_policy.py | 4 +- .../azext_confcom/tests/latest/README.md | 1 + .../tests/latest/test_confcom_arm.py | 167 +++++++++++++++++- .../tests/latest/test_confcom_tar.py | 24 ++- 6 files changed, 218 insertions(+), 13 deletions(-) diff --git a/src/confcom/azext_confcom/config.py b/src/confcom/azext_confcom/config.py index 3ceab644e42..7f7392efe55 100644 --- a/src/confcom/azext_confcom/config.py +++ b/src/confcom/azext_confcom/config.py @@ -25,6 +25,7 @@ ACI_FIELD_CONTAINERS_MOUNTS_READONLY = "readonly" ACI_FIELD_CONTAINERS_WAIT_MOUNT_POINTS = "wait_mount_points" ACI_FIELD_CONTAINERS_ALLOW_ELEVATED = "allow_elevated" +ACI_FIELD_CONTAINERS_SECURITY_CONTEXT = "securityContext" ACI_FIELD_CONTAINERS_REGO_FRAGMENTS = "fragments" ACI_FIELD_CONTAINERS_REGO_FRAGMENTS_FEED = "feed" ACI_FIELD_CONTAINERS_REGO_FRAGMENTS_ISS = "iss" @@ -51,6 +52,7 @@ ACI_FIELD_TEMPLATE_VARIABLES = "variables" ACI_FIELD_TEMPLATE_VOLUMES = "volumes" ACI_FIELD_TEMPLATE_IMAGE = "image" +ACI_FIELD_TEMPLATE_SECURITY_CONTEXT = "securityContext" ACI_FIELD_TEMPLATE_RESOURCE_LABEL = "Microsoft.ContainerInstance/containerGroups" ACI_FIELD_TEMPLATE_COMMAND = "command" ACI_FIELD_TEMPLATE_ENVS = "environmentVariables" @@ -60,6 +62,7 @@ ACI_FIELD_TEMPLATE_MOUNTS_READONLY = "readOnly" ACI_FIELD_TEMPLATE_CONFCOM_PROPERTIES = "confidentialComputeProperties" ACI_FIELD_TEMPLATE_CCE_POLICY = "ccePolicy" +ACI_FIELD_CONTAINERS_PRIVILEGED = "privileged" # output json values diff --git a/src/confcom/azext_confcom/container.py b/src/confcom/azext_confcom/container.py index b52be58638e..7e9de5d7235 100644 --- a/src/confcom/azext_confcom/container.py +++ b/src/confcom/azext_confcom/container.py @@ -214,6 +214,29 @@ def extract_exec_process(container_json: Any) -> List: def extract_allow_elevated(container_json: Any) -> bool: + # privileged is used for arm templates + security_context = case_insensitive_dict_get( + container_json, config.ACI_FIELD_CONTAINERS_SECURITY_CONTEXT + ) + + # get the field for privileged, default to false + privileged_value = case_insensitive_dict_get( + security_context, config.ACI_FIELD_CONTAINERS_PRIVILEGED + ) + if privileged_value and not isinstance(privileged_value, bool) and not isinstance(privileged_value, str): + eprint( + f'Field ["{config.ACI_FIELD_CONTAINERS}"]["{config.ACI_FIELD_CONTAINERS_SECURITY_CONTEXT}"]' + + f'["{config.ACI_FIELD_CONTAINERS_PRIVILEGED}"] can only be a boolean or string value.' + ) + + # force the field into a bool + if isinstance(privileged_value, str): + privileged_value = privileged_value.lower() == "true" + + if privileged_value is not None: + return privileged_value + + # allow_elevated is used for input.json _allow_elevated = case_insensitive_dict_get( container_json, config.ACI_FIELD_CONTAINERS_ALLOW_ELEVATED ) @@ -223,10 +246,11 @@ def extract_allow_elevated(container_json: Any) -> bool: f'Field ["{config.ACI_FIELD_CONTAINERS}"]' + f'["{config.ACI_FIELD_CONTAINERS_ALLOW_ELEVATED}"] can only be boolean value.' ) - else: - # default is allow_elevated should be true - _allow_elevated = True - return _allow_elevated + + if _allow_elevated is not None: + return _allow_elevated + # default value is true + return True def extract_allow_stdio_access(container_json: Any) -> bool: diff --git a/src/confcom/azext_confcom/security_policy.py b/src/confcom/azext_confcom/security_policy.py index 9cd583e71aa..434ef20ba98 100644 --- a/src/confcom/azext_confcom/security_policy.py +++ b/src/confcom/azext_confcom/security_policy.py @@ -571,13 +571,15 @@ def load_policy_from_arm_template_str( ) or [], config.ACI_FIELD_CONTAINERS_MOUNTS: process_mounts(image_properties, volumes), - config.ACI_FIELD_CONTAINERS_ALLOW_ELEVATED: False, config.ACI_FIELD_CONTAINERS_EXEC_PROCESSES: exec_processes + config.DEBUG_MODE_SETTINGS.get("execProcesses") if debug_mode else exec_processes, config.ACI_FIELD_CONTAINERS_SIGNAL_CONTAINER_PROCESSES: [], config.ACI_FIELD_CONTAINERS_ALLOW_STDIO_ACCESS: not disable_stdio, + config.ACI_FIELD_CONTAINERS_SECURITY_CONTEXT: case_insensitive_dict_get( + image_properties, config.ACI_FIELD_TEMPLATE_SECURITY_CONTEXT + ), } ) diff --git a/src/confcom/azext_confcom/tests/latest/README.md b/src/confcom/azext_confcom/tests/latest/README.md index 27a7bd518f1..5680c87a712 100644 --- a/src/confcom/azext_confcom/tests/latest/README.md +++ b/src/confcom/azext_confcom/tests/latest/README.md @@ -36,6 +36,7 @@ test_update_infrastructure_svn | python:3.6.14-slim-buster | Change the minimum test_multiple_policies | python:3.6.14-slim-buster & rust:1.52.1 | See if two unique policies are generated from a single ARM Template container multiple container groups. Also have an extra resource that is untouched. Also has a secureValue for an environment variable. test_arm_template_with_init_container | python:3.6.14-slim-buster & rust:1.52.1 | See if having an initContainer is picked up and added to the list of valid containers test_arm_template_without_stdio_access | rust:1.52.1 | See if disabling container stdio access gets passed down to individual containers +test_arm_template_allow_elevated_false | rust:1.52.1 | Disabling allow_elevated via securityContext test_arm_template_policy_regex | python:3.6.14-slim-buster | Make sure the regex generated from the ARM Template workflow matches that of the policy.json workflow test_wildcard_env_var | python:3.6.14-slim-buster | Check that an "allow all" regex is created when a value for env var is not provided via a parameter value test_wildcard_env_var_invalid | N/A | Make sure the process errors out if a value is not given for an env var or an undefined parameter is used for the name of an env var diff --git a/src/confcom/azext_confcom/tests/latest/test_confcom_arm.py b/src/confcom/azext_confcom/tests/latest/test_confcom_arm.py index d7e2f44c805..9e09535eb50 100644 --- a/src/confcom/azext_confcom/tests/latest/test_confcom_arm.py +++ b/src/confcom/azext_confcom/tests/latest/test_confcom_arm.py @@ -273,6 +273,18 @@ def test_default_infrastructure_svn(self): ], ) + def test_default_allow_elevated(self): + regular_image_json = json.loads( + self.aci_arm_policy.get_serialized_output( + output_type=OutputType.RAW, rego_boilerplate=False + ) + ) + + allow_elevated = regular_image_json[0][config.POLICY_FIELD_CONTAINERS_ELEMENTS_ALLOW_ELEVATED] + + # see if the remote image and the local one produce the same output + self.assertTrue(allow_elevated) + # @unittest.skip("not in use") @pytest.mark.run(order=2) @@ -2328,6 +2340,157 @@ def test_arm_template_without_stdio_access(self): # @unittest.skip("not in use") @pytest.mark.run(order=13) +class PolicyGeneratingAllowElevated(unittest.TestCase): + + custom_arm_json_default_value = """ + { + "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#", + "contentVersion": "1.0.0.0", + + + "parameters": { + "containergroupname": { + "type": "string", + "metadata": { + "description": "Name for the container group" + }, + "defaultValue":"simple-container-group" + }, + "image": { + "type": "string", + "metadata": { + "description": "Name for the container group" + }, + "defaultValue":"rust:1.52.1" + }, + "containername": { + "type": "string", + "metadata": { + "description": "Name for the container" + }, + "defaultValue":"simple-container" + }, + + "port": { + "type": "string", + "metadata": { + "description": "Port to open on the container and the public IP address." + }, + "defaultValue": "8080" + }, + "cpuCores": { + "type": "string", + "metadata": { + "description": "The number of CPU cores to allocate to the container." + }, + "defaultValue": "1.0" + }, + "memoryInGb": { + "type": "string", + "metadata": { + "description": "The amount of memory to allocate to the container in gigabytes." + }, + "defaultValue": "1.5" + }, + "location": { + "type": "string", + "defaultValue": "[resourceGroup().location]", + "metadata": { + "description": "Location for all resources." + } + } + }, + "resources": [ + { + "name": "[parameters('containergroupname')]", + "type": "Microsoft.ContainerInstance/containerGroups", + "apiVersion": "2022-04-01-preview", + "location": "[parameters('location')]", + + "properties": { + "containers": [ + { + "name": "[parameters('containername')]", + "properties": { + "securityContext":{ + "privileged":"false" + }, + "image": "[parameters('image')]", + "environmentVariables": [ + { + "name": "PORT", + "value": "80" + } + ], + + "ports": [ + { + "port": "[parameters('port')]" + } + ], + "command": [ + "/bin/bash", + "-c", + "while sleep 5; do cat /mnt/input/access.log; done" + ], + "resources": { + "requests": { + "cpu": "[parameters('cpuCores')]", + "memoryInGb": "[parameters('memoryInGb')]" + } + } + } + } + ], + + "osType": "Linux", + "restartPolicy": "OnFailure", + "confidentialComputeProperties": { + "IsolationType": "SevSnp" + }, + "ipAddress": { + "type": "Public", + "ports": [ + { + "protocol": "Tcp", + "port": "[parameters( 'port' )]" + } + ] + } + } + } + ], + "outputs": { + "containerIPv4Address": { + "type": "string", + "value": "[reference(resourceId('Microsoft.ContainerInstance/containerGroups/', parameters('containergroupname'))).ipAddress.ip]" + } + } + } + """ + + @classmethod + def setUpClass(cls): + cls.aci_arm_policy = load_policy_from_arm_template_str( + cls.custom_arm_json_default_value, "", disable_stdio=True + )[0] + cls.aci_arm_policy.populate_policy_content_for_all_images() + + def test_arm_template_allow_elevated_false(self): + regular_image_json = json.loads( + self.aci_arm_policy.get_serialized_output( + output_type=OutputType.RAW, rego_boilerplate=False + ) + ) + + allow_elevated = regular_image_json[0][config.POLICY_FIELD_CONTAINERS_ELEMENTS_ALLOW_ELEVATED] + + # see if the remote image and the local one produce the same output + self.assertFalse(allow_elevated) + + +# @unittest.skip("not in use") +@pytest.mark.run(order=14) class PrintExistingPolicy(unittest.TestCase): def test_printing_existing_policy(self): @@ -2628,7 +2791,7 @@ def test_printing_existing_policy(self): os.remove("test_template2.json") # @unittest.skip("not in use") -@pytest.mark.run(order=14) +@pytest.mark.run(order=15) class PolicyGeneratingArmWildcardEnvs(unittest.TestCase): custom_json = """ { @@ -3292,7 +3455,7 @@ def test_wildcard_env_var_invalid(self): # @unittest.skip("not in use") -@pytest.mark.run(order=15) +@pytest.mark.run(order=16) class PolicyGeneratingEdgeCases(unittest.TestCase): custom_arm_json_default_value = """ diff --git a/src/confcom/azext_confcom/tests/latest/test_confcom_tar.py b/src/confcom/azext_confcom/tests/latest/test_confcom_tar.py index 4a525697c6a..69bac2e8ad2 100644 --- a/src/confcom/azext_confcom/tests/latest/test_confcom_tar.py +++ b/src/confcom/azext_confcom/tests/latest/test_confcom_tar.py @@ -4,6 +4,7 @@ # -------------------------------------------------------------------------------------------- import os +import tempfile import unittest import pytest import deepdiff @@ -388,6 +389,12 @@ def test_arm_template_mixed_mode_tar(self): image = client.images.get("nginx:1.22") image_path = self.image_path + "2" # Note: Class setup and teardown shouldn't have side effects, and reading from the tar file fails when all the tests are running in parallel, so we want to save and delete this tar file as a part of the test. Not as a part of the testing class. + # make a temp directory for the tar file + temp_dir = tempfile.TemporaryDirectory() + + image_path = os.path.join( + temp_dir.name, "nginx.tar" + ) f = open(image_path, "wb") for chunk in image.save(named=True): f.write(chunk) @@ -396,9 +403,10 @@ def test_arm_template_mixed_mode_tar(self): tar_mapping_file = {"nginx:1.22": image_path} try: clean_room_image.populate_policy_content_for_all_images( - tar_mapping=tar_mapping_file + tar_mapping=image_path ) finally: + temp_dir.cleanup() # delete the tar file if os.path.isfile(image_path): os.remove(image_path) @@ -557,8 +565,11 @@ def test_arm_template_with_parameter_file_clean_room_tar_invalid(self): image = client.images.get("nginx:1.23") # Note: Class setup and teardown shouldn't have side effects, and reading from the tar file fails when all the tests are running in parallel, so we want to save and delete this tar file as a part of the test. Not as a part of the testing class. - image_path = self.image_path + "3" - tar_mapping_file = {"nginx:1.22": image_path} + temp_dir = tempfile.TemporaryDirectory() + + image_path = os.path.join( + temp_dir.name, "nginx.tar" + ) f = open(image_path, "wb") for chunk in image.save(named=True): f.write(chunk) @@ -567,15 +578,16 @@ def test_arm_template_with_parameter_file_clean_room_tar_invalid(self): try: clean_room_image.populate_policy_content_for_all_images( - tar_mapping=tar_mapping_file + tar_mapping=image_path ) raise AccContainerError("getting image should fail") except: pass finally: # delete the tar file - if os.path.isfile(image_path): - os.remove(image_path) + temp_dir.cleanup() + if os.path.isfile(self.image_path): + os.remove(self.image_path) def test_clean_room_fake_tar_invalid(self): custom_arm_json_default_value = """