From d8763e90f424211d2e3cd3c3143f16ca71f22688 Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Mon, 6 Mar 2023 12:15:18 +0100 Subject: [PATCH 1/9] feat(rcm): store payloads target files content --- ddtrace/appsec/_remoteconfiguration.py | 37 ++-- ddtrace/internal/remoteconfig/client.py | 64 ++++-- tests/appsec/test_remoteconfiguration.py | 207 ++++++++++++++++++ .../remoteconfig/test_remoteconfig_client.py | 137 +++++++++++- .../test_remoteconfig_client_e2e.py | 8 +- 5 files changed, 419 insertions(+), 34 deletions(-) diff --git a/ddtrace/appsec/_remoteconfiguration.py b/ddtrace/appsec/_remoteconfiguration.py index 50b2f0b3f32..a1855778584 100644 --- a/ddtrace/appsec/_remoteconfiguration.py +++ b/ddtrace/appsec/_remoteconfiguration.py @@ -31,7 +31,6 @@ from typing import Union from ddtrace import Tracer - from ddtrace.internal.remoteconfig.client import ConfigMetadata log = get_logger(__name__) @@ -45,20 +44,21 @@ def enable_appsec_rc(test_tracer=None): else: tracer = test_tracer - appsec_features_callback = RCAppSecFeaturesCallBack(tracer) - appsec_callback = RCAppSecCallBack(tracer) + asm_features_callback = RCAppSecFeaturesCallBack(tracer) + asm_dd_callback = RCASMDDCallBack(tracer) + asm_callback = RCAppSecCallBack(tracer) if _appsec_rc_features_is_enabled(): from ddtrace.internal.remoteconfig import RemoteConfig - RemoteConfig.register(PRODUCTS.ASM_FEATURES, appsec_features_callback) + RemoteConfig.register(PRODUCTS.ASM_FEATURES, asm_features_callback) if tracer._appsec_enabled: from ddtrace.internal.remoteconfig import RemoteConfig - RemoteConfig.register(PRODUCTS.ASM_DATA, appsec_callback) # IP Blocking - RemoteConfig.register(PRODUCTS.ASM, appsec_callback) # Exclusion Filters & Custom Rules - RemoteConfig.register(PRODUCTS.ASM_DD, appsec_callback) # DD Rules + RemoteConfig.register(PRODUCTS.ASM_DATA, asm_callback) # IP Blocking + RemoteConfig.register(PRODUCTS.ASM, asm_callback) # Exclusion Filters & Custom Rules + RemoteConfig.register(PRODUCTS.ASM_DD, asm_dd_callback) # DD Rules def _add_rules_to_list(features, feature, message, rule_list): @@ -87,6 +87,16 @@ def _appsec_rules_data(tracer, features): return False +class RCASMDDCallBack(RemoteConfigCallBack): + def __init__(self, tracer): + # type: (Tracer) -> None + self.tracer = tracer + + def __call__(self, metadata, features): + if features is not None: + _appsec_rules_data(self.tracer, features) + + class RCAppSecFeaturesCallBack(RemoteConfigCallBack): def __init__(self, tracer): # type: (Tracer) -> None @@ -126,10 +136,11 @@ def _appsec_1click_activation(self, features): log.debug("Updating ASM Remote Configuration ASM_FEATURES: %s", rc_appsec_enabled) if rc_appsec_enabled: - appsec_callback = RCAppSecCallBack(self.tracer) - RemoteConfig.register(PRODUCTS.ASM_DATA, appsec_callback) # IP Blocking - RemoteConfig.register(PRODUCTS.ASM, appsec_callback) # Exclusion Filters & Custom Rules - RemoteConfig.register(PRODUCTS.ASM_DD, appsec_callback) # DD Rules + asm_dd_callback = RCASMDDCallBack(self.tracer) + asm_callback = RCAppSecCallBack(self.tracer) + RemoteConfig.register(PRODUCTS.ASM_DATA, asm_callback) # IP Blocking + RemoteConfig.register(PRODUCTS.ASM, asm_callback) # Exclusion Filters & Custom Rules + RemoteConfig.register(PRODUCTS.ASM_DD, asm_dd_callback) # DD Rules if not self.tracer._appsec_enabled: self.tracer.configure(appsec_enabled=True) else: @@ -152,7 +163,7 @@ def __init__(self, tracer): # type: (Tracer) -> None self.tracer = tracer - def __call__(self, metadata, features): - # type: (Optional[ConfigMetadata], Any) -> None + def __call__(self, target, features): + # type: (str, Any) -> None if features is not None: _appsec_rules_data(self.tracer, features) diff --git a/ddtrace/internal/remoteconfig/client.py b/ddtrace/internal/remoteconfig/client.py index 0d41f43244c..084890c4d27 100644 --- a/ddtrace/internal/remoteconfig/client.py +++ b/ddtrace/internal/remoteconfig/client.py @@ -154,15 +154,35 @@ def __call__(self, metadata, config): pass -class RemoteConfigCallBackAfterMerge(RemoteConfigCallBack): +class RemoteConfigCallBackAfterMerge(six.with_metaclass(abc.ABCMeta)): configs = {} # type: Dict[str, Any] - def append(self, config): - self.configs.update(config) + @abc.abstractmethod + def __call__(self, target, config): + # type: (str, Any) -> None + pass + + def append(self, target, config): + if not self.configs.get(target): + self.configs[target] = {} + if config is False: + del self.configs[target] + elif config is not None: + self.configs[target].update(config) def dispatch(self): try: - self.__call__(None, self.configs) + config_result = {} + for target, config in self.configs.items(): + for key, value in config.items(): + if config_result.get(key): + if isinstance(value, list): + config_result[key] = config_result[key] + value + else: + raise ValueError("target %s key %s has type of %s" % (target, key, type(value))) + else: + config_result[key] = value + self.__call__("", config_result) finally: self.configs = {} @@ -351,10 +371,22 @@ def _process_targets(self, payload): backend_state = signed.custom.get("opaque_backend_state") return signed.version, backend_state, targets + @staticmethod + def _apply_callback(list_callbacks, callback, config_content, target, config): + # type: (List[RemoteConfigCallBackAfterMerge], Any, Any, str, ConfigMetadata) -> None + if isinstance(callback, RemoteConfigCallBackAfterMerge): + callback.append(target, config_content) + if callback not in list_callbacks and not any( + filter(lambda x: isinstance(x, type(callback)), list_callbacks) + ): + list_callbacks.append(callback) + else: + callback(config, config_content) + def _remove_previously_applied_configurations(self, applied_configs, client_configs, targets): # type: (AppliedConfigType, TargetsType, TargetsType) -> None + list_callbacks = [] # type: List[RemoteConfigCallBackAfterMerge] for target, config in self._applied_configs.items(): - callback_action = None if target in client_configs and targets.get(target) == config: # The configuration has not changed. applied_configs[target] = config @@ -362,18 +394,24 @@ def _remove_previously_applied_configurations(self, applied_configs, client_conf elif target not in client_configs: log.debug("Disable configuration: %s", target) callback_action = False + else: + continue callback = self._products.get(config.product_name) if callback: try: - callback(config, callback_action) + log.debug("Disable configuration 2: %s. ", target) + self._apply_callback(list_callbacks, callback, callback_action, target, config) except Exception: log.debug("error while removing product %s config %r", config.product_name, config) continue + for callback_to_dispach in list_callbacks: + callback_to_dispach.dispatch() + def _load_new_configurations(self, applied_configs, client_configs, payload): # type: (AppliedConfigType, TargetsType, AgentPayload) -> None - list_callbacks = [] + list_callbacks = [] # type: List[RemoteConfigCallBackAfterMerge] for target, config in client_configs.items(): callback = self._products.get(config.product_name) if callback: @@ -387,12 +425,7 @@ def _load_new_configurations(self, applied_configs, client_configs, payload): try: log.debug("Load new configuration: %s. content ", target) - if isinstance(callback, RemoteConfigCallBackAfterMerge): - callback.append(config_content) - if callback not in list_callbacks: - list_callbacks.append(callback) - else: - callback(config, config_content) + self._apply_callback(list_callbacks, callback, config_content, target, config) except Exception: log.debug( "Failed to load configuration %s for product %r", config, config.product_name, exc_info=True @@ -400,8 +433,9 @@ def _load_new_configurations(self, applied_configs, client_configs, payload): continue else: applied_configs[target] = config - for callback in list_callbacks: - callback.dispatch() + + for callback_to_dispach in list_callbacks: + callback_to_dispach.dispatch() def _add_apply_config_to_cache(self): if self._applied_configs: diff --git a/tests/appsec/test_remoteconfiguration.py b/tests/appsec/test_remoteconfiguration.py index bf731e6f9db..47030feb08f 100644 --- a/tests/appsec/test_remoteconfiguration.py +++ b/tests/appsec/test_remoteconfiguration.py @@ -283,6 +283,213 @@ def test_load_new_configurations_remove_config_and_dispatch_applied_configs_erro RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) +@mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") +@mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") +def test_load_multiple_targets_file_same_product( + mock_appsec_rules_data, mock_appsec_1click_activation, remote_config_worker, tracer +): + with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True, api_version="v0.4") + enable_appsec_rc(tracer) + enable_appsec_rc() + asm_features_data = b'{"asm":{"enabled":true}}' + asm_data_data1 = b'{"data": [{"a":1}]}' + asm_data_data2 = b'{"data": [{"b":2}]}' + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), + TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data1)), + TargetFile(path="mock/ASM_DATA/2", raw=base64.b64encode(asm_data_data2)), + ] + ) + client_configs = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ), + } + + RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) + mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}, {"b": 2}]}) + mock_appsec_1click_activation.assert_called_with({"asm": {"enabled": True}}) + + +@mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") +@mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") +def test_remove_targets_file_same_product( + mock_appsec_rules_data, mock_appsec_1click_activation, remote_config_worker, tracer +): + with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True, api_version="v0.4") + enable_appsec_rc(tracer) + enable_appsec_rc() + asm_features_data = b'{"asm":{"enabled":true}}' + asm_data_data1 = b'{"data": [{"a":1}]}' + asm_data_data2 = b'{"data": [{"b":2}]}' + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), + TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data1)), + TargetFile(path="mock/ASM_DATA/2", raw=base64.b64encode(asm_data_data2)), + ] + ) + applied_configs = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ), + } + + client_configs = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + } + + target_file = { + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ) + } + + RemoteConfig._worker._client._applied_configs = applied_configs + RemoteConfig._worker._client._remove_previously_applied_configurations({}, client_configs, target_file) + + RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) + mock_appsec_rules_data.assert_called_with(ANY, {}) + + +@mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") +@mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") +def test_load_new_config_and_remove_targets_file_same_product( + mock_appsec_rules_data, mock_appsec_1click_activation, remote_config_worker, tracer +): + with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True, api_version="v0.4") + enable_appsec_rc(tracer) + enable_appsec_rc() + asm_features_data = b'{"asm":{"enabled":true}}' + asm_data_data1 = b'{"data": [{"a":1}]}' + asm_data_data2 = b'{"data": [{"b":2}]}' + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), + TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data1)), + TargetFile(path="mock/ASM_DATA/2", raw=base64.b64encode(asm_data_data2)), + ] + ) + first_config = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ), + } + + second_config = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + } + + target_file = { + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ) + } + + RemoteConfig._worker._client._remove_previously_applied_configurations({}, first_config, {}) + + RemoteConfig._worker._client._load_new_configurations({}, first_config, payload=payload) + mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}, {"b": 2}]}) + mock_appsec_rules_data.reset_mock() + + RemoteConfig._worker._client._remove_previously_applied_configurations({}, second_config, target_file) + + RemoteConfig._worker._client._load_new_configurations({}, second_config, payload=payload) + mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}]}) + + def test_rc_activation_ip_blocking_data(tracer, remote_config_worker): with override_env({APPSEC_ENV: "true"}): tracer.configure(appsec_enabled=True, api_version="v0.4") diff --git a/tests/internal/remoteconfig/test_remoteconfig_client.py b/tests/internal/remoteconfig/test_remoteconfig_client.py index 1bf9af580cb..0247e4d6516 100644 --- a/tests/internal/remoteconfig/test_remoteconfig_client.py +++ b/tests/internal/remoteconfig/test_remoteconfig_client.py @@ -5,6 +5,7 @@ import pytest from ddtrace.internal.remoteconfig.client import ConfigMetadata +from ddtrace.internal.remoteconfig.client import RemoteConfigCallBack from ddtrace.internal.remoteconfig.client import RemoteConfigCallBackAfterMerge from ddtrace.internal.remoteconfig.client import RemoteConfigClient from ddtrace.internal.remoteconfig.client import RemoteConfigError @@ -72,7 +73,7 @@ def __call__(self, payload, target, config): rc_client._load_new_configurations(applied_configs, client_configs, payload=payload) - mock_callback.assert_called_once_with(None, expected_results) + mock_callback.assert_called_once_with("", expected_results) assert applied_configs == client_configs rc_client._products = {} @@ -265,3 +266,137 @@ def test_remote_config_client_tags_override(): assert tags["env"] == "foo" assert tags["version"] == "bar" + + +def test_apply_default_callback(): + class callbackClass(RemoteConfigCallBack): + result = None + + def __call__(self, *args, **kwargs): + self.result = args + + callback = callbackClass() + callback_content = {"a": 1} + target = "1/ASM/2" + config = {"Config": "data"} + test_list_callbacks = [] + RemoteConfigClient._apply_callback(test_list_callbacks, callback, callback_content, target, config) + + assert callback.result == ({"Config": "data"}, {"a": 1}) + assert test_list_callbacks == [] + + +def test_apply_merge_callback(): + class callbackClass(RemoteConfigCallBackAfterMerge): + result = None + + def __call__(self, *args, **kwargs): + self.result = args + + callback = callbackClass() + callback_content = {"a": [1]} + target = "1/ASM/2" + config = {"Config": "data"} + test_list_callbacks = [] + RemoteConfigClient._apply_callback(test_list_callbacks, callback, callback_content, target, config) + + assert len(test_list_callbacks) == 1 + test_list_callbacks[0].dispatch() + + assert callback.result == ("", {"a": [1]}) + + +def test_apply_merge_multiple_callback(): + class callbackClass(RemoteConfigCallBackAfterMerge): + result = None + + def __call__(self, *args, **kwargs): + self.result = args + + callback1 = callbackClass() + callback2 = callbackClass() + callback_content1 = {"a": [1]} + callback_content2 = {"b": [2]} + target = "1/ASM/2" + config = {"Config": "data"} + test_list_callbacks = [] + RemoteConfigClient._apply_callback(test_list_callbacks, callback1, callback_content1, target, config) + RemoteConfigClient._apply_callback(test_list_callbacks, callback2, callback_content2, target, config) + + assert len(test_list_callbacks) == 1 + test_list_callbacks[0].dispatch() + + assert callback1.result == ("", {"a": [1], "b": [2]}) + assert callback2.result is None + + +def test_apply_merge_different_callback(): + class callback1And2Class(RemoteConfigCallBackAfterMerge): + configs = {} + result = None + + def __call__(self, *args, **kwargs): + self.result = args + + class callback3Class(RemoteConfigCallBackAfterMerge): + configs = {} + result = None + + def __call__(self, *args, **kwargs): + self.result = args + + callback1 = callback1And2Class() + callback2 = callback1And2Class() + callback3 = callback3Class() + callback_content1 = {"a": [1]} + callback_content2 = {"b": [2]} + target = "1/ASM/2" + config = {"Config": "data"} + test_list_callbacks = [] + RemoteConfigClient._apply_callback(test_list_callbacks, callback1, callback_content1, target, config) + RemoteConfigClient._apply_callback(test_list_callbacks, callback2, callback_content2, target, config) + RemoteConfigClient._apply_callback(test_list_callbacks, callback3, callback_content2, target, config) + + assert len(test_list_callbacks) == 2 + test_list_callbacks[0].dispatch() + test_list_callbacks[1].dispatch() + + assert callback1.result == ("", {"a": [1], "b": [2]}) + assert callback2.result is None + assert callback3.result == ("", {"b": [2]}) + + +def test_apply_merge_different_target_callback(): + class callback1And2Class(RemoteConfigCallBackAfterMerge): + configs = {} + result = None + + def __call__(self, *args, **kwargs): + self.result = args + + class callback3Class(RemoteConfigCallBackAfterMerge): + configs = {} + result = None + + def __call__(self, *args, **kwargs): + self.result = args + + callback1 = callback1And2Class() + callback2 = callback1And2Class() + callback3 = callback3Class() + callback_content1 = {"a": [1]} + callback_content2 = {"b": [2]} + callback_content3 = {"b": [3]} + config = {"Config": "data"} + test_list_callbacks = [] + RemoteConfigClient._apply_callback(test_list_callbacks, callback1, callback_content1, "1/ASM/1", config) + RemoteConfigClient._apply_callback(test_list_callbacks, callback2, callback_content2, "1/ASM/2", config) + RemoteConfigClient._apply_callback(test_list_callbacks, callback3, callback_content3, "1/ASM/3", config) + + assert len(test_list_callbacks) == 2 + test_list_callbacks[0].dispatch() + test_list_callbacks[1].dispatch() + + assert callback1.result == ("", {"a": [1], "b": [2]}) + assert callback2.result is None + assert callback3.result == ("", {"b": [3]}) diff --git a/tests/internal/remoteconfig/test_remoteconfig_client_e2e.py b/tests/internal/remoteconfig/test_remoteconfig_client_e2e.py index 75dfaf4d54f..cda08b37e18 100644 --- a/tests/internal/remoteconfig/test_remoteconfig_client_e2e.py +++ b/tests/internal/remoteconfig/test_remoteconfig_client_e2e.py @@ -4,8 +4,8 @@ import mock +from ddtrace.appsec._remoteconfiguration import RCASMDDCallBack from ddtrace.internal import runtime -from ddtrace.internal.remoteconfig.client import RemoteConfigCallBackAfterMerge from ddtrace.internal.remoteconfig.client import RemoteConfigClient from ddtrace.internal.utils.version import _pep440_to_semver from tests.utils import override_env @@ -73,9 +73,7 @@ def test_remote_config_client_steps(mock_appsec_rc_capabilities, mock_send_reque with open(MOCK_AGENT_RESPONSES_FILE, "r") as f: MOCK_AGENT_RESPONSES = json.load(f) - class RCAppSecCallBack(RemoteConfigCallBackAfterMerge): - configs = {} - + class RCAppSecMockCallBack(RCASMDDCallBack): def __call__(self, metadata, features): mock_callback(metadata, features) @@ -83,7 +81,7 @@ def __call__(self, metadata, features): rc_client = RemoteConfigClient() mock_callback = mock.mock.MagicMock() - callback = RCAppSecCallBack() + callback = RCAppSecMockCallBack(mock.mock.MagicMock()) rc_client.register_product("ASM_FEATURES", callback) with override_env(dict(DD_REMOTE_CONFIGURATION_ENABLED="false")): From d67cae47d07ba744a72b455062b2a2f2ababbed9 Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Mon, 6 Mar 2023 12:49:33 +0100 Subject: [PATCH 2/9] feat(rcm): fix test --- ddtrace/appsec/_remoteconfiguration.py | 3 +-- ddtrace/internal/remoteconfig/client.py | 13 ++++++++++++- tests/appsec/test_remoteconfiguration.py | 2 -- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/ddtrace/appsec/_remoteconfiguration.py b/ddtrace/appsec/_remoteconfiguration.py index a1855778584..ef625ace268 100644 --- a/ddtrace/appsec/_remoteconfiguration.py +++ b/ddtrace/appsec/_remoteconfiguration.py @@ -157,10 +157,9 @@ def _appsec_1click_activation(self, features): class RCAppSecCallBack(RemoteConfigCallBackAfterMerge): - configs = {} - def __init__(self, tracer): # type: (Tracer) -> None + super(RCAppSecCallBack, self).__init__() self.tracer = tracer def __call__(self, target, features): diff --git a/ddtrace/internal/remoteconfig/client.py b/ddtrace/internal/remoteconfig/client.py index 084890c4d27..c588b6fd09a 100644 --- a/ddtrace/internal/remoteconfig/client.py +++ b/ddtrace/internal/remoteconfig/client.py @@ -155,7 +155,8 @@ def __call__(self, metadata, config): class RemoteConfigCallBackAfterMerge(six.with_metaclass(abc.ABCMeta)): - configs = {} # type: Dict[str, Any] + def __init__(self): + self.configs = {} @abc.abstractmethod def __call__(self, target, config): @@ -412,14 +413,24 @@ def _remove_previously_applied_configurations(self, applied_configs, client_conf def _load_new_configurations(self, applied_configs, client_configs, payload): # type: (AppliedConfigType, TargetsType, AgentPayload) -> None list_callbacks = [] # type: List[RemoteConfigCallBackAfterMerge] + print("_applied_configs") + print(self._applied_configs) for target, config in client_configs.items(): callback = self._products.get(config.product_name) if callback: applied_config = self._applied_configs.get(target) + print("applied_config!!!!!") + print(applied_config) + print("config!!!!!") + print(config) + print("applied_config == config!!!!!!!!!!!!!1") + print(applied_config == config) if applied_config == config: continue config_content = self._extract_target_file(payload, target, config) + print("config_content!!!!!!!!!!!!!1") + print(config_content) if config_content is None: continue diff --git a/tests/appsec/test_remoteconfiguration.py b/tests/appsec/test_remoteconfiguration.py index 47030feb08f..d39bfb6d7a4 100644 --- a/tests/appsec/test_remoteconfiguration.py +++ b/tests/appsec/test_remoteconfiguration.py @@ -291,7 +291,6 @@ def test_load_multiple_targets_file_same_product( with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): tracer.configure(appsec_enabled=True, api_version="v0.4") enable_appsec_rc(tracer) - enable_appsec_rc() asm_features_data = b'{"asm":{"enabled":true}}' asm_data_data1 = b'{"data": [{"a":1}]}' asm_data_data2 = b'{"data": [{"b":2}]}' @@ -339,7 +338,6 @@ def test_remove_targets_file_same_product( with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): tracer.configure(appsec_enabled=True, api_version="v0.4") enable_appsec_rc(tracer) - enable_appsec_rc() asm_features_data = b'{"asm":{"enabled":true}}' asm_data_data1 = b'{"data": [{"a":1}]}' asm_data_data2 = b'{"data": [{"b":2}]}' From 0fe83a5cac41d48fc8a92ae3d5f8ce76eecff290 Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Mon, 6 Mar 2023 13:38:26 +0100 Subject: [PATCH 3/9] feat(rcm): fix test --- ddtrace/internal/remoteconfig/client.py | 36 ++++++++---------------- tests/appsec/test_remoteconfiguration.py | 8 ++++-- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/ddtrace/internal/remoteconfig/client.py b/ddtrace/internal/remoteconfig/client.py index c588b6fd09a..d8c1e847ccc 100644 --- a/ddtrace/internal/remoteconfig/client.py +++ b/ddtrace/internal/remoteconfig/client.py @@ -155,8 +155,7 @@ def __call__(self, metadata, config): class RemoteConfigCallBackAfterMerge(six.with_metaclass(abc.ABCMeta)): - def __init__(self): - self.configs = {} + configs = {} # type: Dict[str, Any] @abc.abstractmethod def __call__(self, target, config): @@ -172,20 +171,17 @@ def append(self, target, config): self.configs[target].update(config) def dispatch(self): - try: - config_result = {} - for target, config in self.configs.items(): - for key, value in config.items(): - if config_result.get(key): - if isinstance(value, list): - config_result[key] = config_result[key] + value - else: - raise ValueError("target %s key %s has type of %s" % (target, key, type(value))) + config_result = {} + for target, config in self.configs.items(): + for key, value in config.items(): + if config_result.get(key): + if isinstance(value, list): + config_result[key] = config_result[key] + value else: - config_result[key] = value - self.__call__("", config_result) - finally: - self.configs = {} + raise ValueError("target %s key %s has type of %s" % (target, key, type(value))) + else: + config_result[key] = value + self.__call__("", config_result) class RemoteConfigClient(object): @@ -413,24 +409,14 @@ def _remove_previously_applied_configurations(self, applied_configs, client_conf def _load_new_configurations(self, applied_configs, client_configs, payload): # type: (AppliedConfigType, TargetsType, AgentPayload) -> None list_callbacks = [] # type: List[RemoteConfigCallBackAfterMerge] - print("_applied_configs") - print(self._applied_configs) for target, config in client_configs.items(): callback = self._products.get(config.product_name) if callback: applied_config = self._applied_configs.get(target) - print("applied_config!!!!!") - print(applied_config) - print("config!!!!!") - print(config) - print("applied_config == config!!!!!!!!!!!!!1") - print(applied_config == config) if applied_config == config: continue config_content = self._extract_target_file(payload, target, config) - print("config_content!!!!!!!!!!!!!1") - print(config_content) if config_content is None: continue diff --git a/tests/appsec/test_remoteconfiguration.py b/tests/appsec/test_remoteconfiguration.py index d39bfb6d7a4..cade1fb934b 100644 --- a/tests/appsec/test_remoteconfiguration.py +++ b/tests/appsec/test_remoteconfiguration.py @@ -336,6 +336,7 @@ def test_remove_targets_file_same_product( mock_appsec_rules_data, mock_appsec_1click_activation, remote_config_worker, tracer ): with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + RCAppSecCallBack.configs = {} tracer.configure(appsec_enabled=True, api_version="v0.4") enable_appsec_rc(tracer) asm_features_data = b'{"asm":{"enabled":true}}' @@ -413,8 +414,8 @@ def test_load_new_config_and_remove_targets_file_same_product( ): with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): tracer.configure(appsec_enabled=True, api_version="v0.4") + applied_configs = {} enable_appsec_rc(tracer) - enable_appsec_rc() asm_features_data = b'{"asm":{"enabled":true}}' asm_data_data1 = b'{"data": [{"a":1}]}' asm_data_data2 = b'{"data": [{"b":2}]}' @@ -475,10 +476,11 @@ def test_load_new_config_and_remove_targets_file_same_product( tuf_version=5, ) } - RemoteConfig._worker._client._remove_previously_applied_configurations({}, first_config, {}) - RemoteConfig._worker._client._load_new_configurations({}, first_config, payload=payload) + RemoteConfig._worker._client._load_new_configurations(applied_configs, first_config, payload=payload) + RemoteConfig._worker._client._applied_configs = applied_configs + mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}, {"b": 2}]}) mock_appsec_rules_data.reset_mock() From 9091fada17db06dcd40f1b7bf00252cffd283cba Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Mon, 6 Mar 2023 14:52:56 +0100 Subject: [PATCH 4/9] feat(rcm): fix test --- tests/appsec/test_remoteconfiguration.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/appsec/test_remoteconfiguration.py b/tests/appsec/test_remoteconfiguration.py index cade1fb934b..15ce11a9afc 100644 --- a/tests/appsec/test_remoteconfiguration.py +++ b/tests/appsec/test_remoteconfiguration.py @@ -8,6 +8,7 @@ import mock from mock.mock import ANY import pytest +import six from ddtrace.appsec import _asm_request_context from ddtrace.appsec._constants import PRODUCTS @@ -283,6 +284,7 @@ def test_load_new_configurations_remove_config_and_dispatch_applied_configs_erro RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) +@pytest.mark.skipif(six.PY2, reason="Mock return order is different in python 2.7") @mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") @mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") def test_load_multiple_targets_file_same_product( @@ -407,6 +409,7 @@ def test_remove_targets_file_same_product( mock_appsec_rules_data.assert_called_with(ANY, {}) +@pytest.mark.skipif(six.PY2, reason="Mock return order is different in python 2.7") @mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") @mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") def test_load_new_config_and_remove_targets_file_same_product( From df1de70d13f6d2fa915e4bb0b6440ca30c8c56ec Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Mon, 6 Mar 2023 15:08:03 +0100 Subject: [PATCH 5/9] feat(rcm): fix test --- tests/appsec/test_remoteconfiguration.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/appsec/test_remoteconfiguration.py b/tests/appsec/test_remoteconfiguration.py index 15ce11a9afc..891922308e5 100644 --- a/tests/appsec/test_remoteconfiguration.py +++ b/tests/appsec/test_remoteconfiguration.py @@ -8,7 +8,6 @@ import mock from mock.mock import ANY import pytest -import six from ddtrace.appsec import _asm_request_context from ddtrace.appsec._constants import PRODUCTS @@ -284,7 +283,7 @@ def test_load_new_configurations_remove_config_and_dispatch_applied_configs_erro RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) -@pytest.mark.skipif(six.PY2, reason="Mock return order is different in python 2.7") +@pytest.mark.skipif(sys.version_info[:2] < (3, 6), reason="Mock return order is different in python <= 3.5") @mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") @mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") def test_load_multiple_targets_file_same_product( @@ -409,7 +408,7 @@ def test_remove_targets_file_same_product( mock_appsec_rules_data.assert_called_with(ANY, {}) -@pytest.mark.skipif(six.PY2, reason="Mock return order is different in python 2.7") +@pytest.mark.skipif(sys.version_info[:2] < (3, 6), reason="Mock return order is different in python <= 3.5") @mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") @mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") def test_load_new_config_and_remove_targets_file_same_product( From cae9f575c032f593763b9b97ca4d51b8340f531d Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Mon, 6 Mar 2023 16:54:27 +0100 Subject: [PATCH 6/9] feat(rcm): add new corner case --- ddtrace/internal/remoteconfig/client.py | 14 +++- tests/appsec/test_remoteconfiguration.py | 82 ++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/ddtrace/internal/remoteconfig/client.py b/ddtrace/internal/remoteconfig/client.py index d8c1e847ccc..0ec55a18501 100644 --- a/ddtrace/internal/remoteconfig/client.py +++ b/ddtrace/internal/remoteconfig/client.py @@ -166,9 +166,20 @@ def append(self, target, config): if not self.configs.get(target): self.configs[target] = {} if config is False: + # Remove old config from the configs dict. _remove_previously_applied_configurations function should + # call to this method del self.configs[target] elif config is not None: - self.configs[target].update(config) + # Append the new config to the configs dict. _load_new_configurations function should + # call to this method + if isinstance(config, dict): + if not any(v for k, v in config.items()): + # Ups, no, if new config is empty, it's the same behavior of remove config + del self.configs[target] + else: + self.configs[target].update(config) + else: + raise ValueError("target %s config %s has type of %s" % (target, config, type(config))) def dispatch(self): config_result = {} @@ -181,6 +192,7 @@ def dispatch(self): raise ValueError("target %s key %s has type of %s" % (target, key, type(value))) else: config_result[key] = value + print("DISPATCH CONTENT {}".format(config_result)) self.__call__("", config_result) diff --git a/tests/appsec/test_remoteconfiguration.py b/tests/appsec/test_remoteconfiguration.py index 891922308e5..7690a798992 100644 --- a/tests/appsec/test_remoteconfiguration.py +++ b/tests/appsec/test_remoteconfiguration.py @@ -492,6 +492,88 @@ def test_load_new_config_and_remove_targets_file_same_product( mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}]}) +@pytest.mark.skipif(sys.version_info[:2] < (3, 6), reason="Mock return order is different in python <= 3.5") +@mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") +@mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") +def test_load_new_empty_config_and_remove_targets_file_same_product( + mock_appsec_rules_data, mock_appsec_1click_activation, remote_config_worker, tracer +): + with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True, api_version="v0.4") + applied_configs = {} + enable_appsec_rc(tracer) + asm_features_data = b'{"asm":{"enabled":true}}' + asm_data_data1 = b'{"data": [{"a":1}]}' + asm_data_data2 = b'{"data2": [{"b":2}]}' + asm_data_data_empty = b'{"data2": []}' + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), + TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data1)), + TargetFile(path="mock/ASM_DATA/2", raw=base64.b64encode(asm_data_data2)), + ] + ) + first_config = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ), + } + + second_config = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data_empty).hexdigest(), + length=5, + tuf_version=5, + ), + } + + RemoteConfig._worker._client._remove_previously_applied_configurations({}, first_config, {}) + + RemoteConfig._worker._client._load_new_configurations(applied_configs, first_config, payload=payload) + RemoteConfig._worker._client._applied_configs = {} + + mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}], "data2": [{"b": 2}]}) + mock_appsec_rules_data.reset_mock() + + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), + TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data1)), + TargetFile(path="mock/ASM_DATA/2", raw=base64.b64encode(asm_data_data_empty)), + ] + ) + + RemoteConfig._worker._client._load_new_configurations({}, second_config, payload=payload) + mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}]}) + + def test_rc_activation_ip_blocking_data(tracer, remote_config_worker): with override_env({APPSEC_ENV: "true"}): tracer.configure(appsec_enabled=True, api_version="v0.4") From 8aa167163d69622d27d13e6ea6f28f81de1a1c5c Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Mon, 6 Mar 2023 17:22:18 +0100 Subject: [PATCH 7/9] feat(rcm): add new corner case --- ddtrace/internal/remoteconfig/client.py | 10 +-- tests/appsec/test_remoteconfiguration.py | 97 ++++++++++++++++++------ 2 files changed, 78 insertions(+), 29 deletions(-) diff --git a/ddtrace/internal/remoteconfig/client.py b/ddtrace/internal/remoteconfig/client.py index 0ec55a18501..5daef3f5e9d 100644 --- a/ddtrace/internal/remoteconfig/client.py +++ b/ddtrace/internal/remoteconfig/client.py @@ -185,15 +185,13 @@ def dispatch(self): config_result = {} for target, config in self.configs.items(): for key, value in config.items(): - if config_result.get(key): + if value: if isinstance(value, list): - config_result[key] = config_result[key] + value + config_result[key] = config_result.get(key, []) + value else: raise ValueError("target %s key %s has type of %s" % (target, key, type(value))) - else: - config_result[key] = value - print("DISPATCH CONTENT {}".format(config_result)) - self.__call__("", config_result) + if config_result: + self.__call__("", config_result) class RemoteConfigClient(object): diff --git a/tests/appsec/test_remoteconfiguration.py b/tests/appsec/test_remoteconfiguration.py index 7690a798992..b2ca8ae3caa 100644 --- a/tests/appsec/test_remoteconfiguration.py +++ b/tests/appsec/test_remoteconfiguration.py @@ -160,6 +160,38 @@ def test_rc_activation_check_asm_features_product_disables_rest_of_products(trac assert RemoteConfig._worker._client._products.get(PRODUCTS.ASM_FEATURES) +def test_load_new_configurations_invalid_content(remote_config_worker, tracer): + with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True, api_version="v0.4") + enable_appsec_rc(tracer) + asm_features_data = b'{"asm":{"enabled":true}}' + asm_data_data = b'{"data": "data"}' + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), + TargetFile(path="mock/ASM_DATA", raw=base64.b64encode(asm_data_data)), + ] + ) + client_configs = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data).hexdigest(), + length=5, + tuf_version=5, + ), + } + with pytest.raises(ValueError): + RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) + + @mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") @mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") def test_load_new_configurations_dispatch_applied_configs( @@ -169,7 +201,45 @@ def test_load_new_configurations_dispatch_applied_configs( tracer.configure(appsec_enabled=True, api_version="v0.4") enable_appsec_rc(tracer) asm_features_data = b'{"asm":{"enabled":true}}' - asm_data_data = b'{"data":{}}' + asm_data_data = b'{"data": [{"test": "data"}]}' + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), + TargetFile(path="mock/ASM_DATA", raw=base64.b64encode(asm_data_data)), + ] + ) + client_configs = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data).hexdigest(), + length=5, + tuf_version=5, + ), + } + + RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) + mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"test": "data"}]}) + mock_appsec_1click_activation.assert_called_with({"asm": {"enabled": True}}) + + +@mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") +@mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") +def test_load_new_configurations_empty_config( + mock_appsec_rules_data, mock_appsec_1click_activation, remote_config_worker, tracer +): + with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True, api_version="v0.4") + enable_appsec_rc(tracer) + asm_features_data = b'{"asm":{"enabled":true}}' + asm_data_data = b'{"data": []}' payload = AgentPayload( target_files=[ TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), @@ -194,7 +264,7 @@ def test_load_new_configurations_dispatch_applied_configs( } RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) - mock_appsec_rules_data.assert_called_with(ANY, {"data": {}}) + mock_appsec_rules_data.assert_not_called() mock_appsec_1click_activation.assert_called_with({"asm": {"enabled": True}}) @@ -331,33 +401,21 @@ def test_load_multiple_targets_file_same_product( mock_appsec_1click_activation.assert_called_with({"asm": {"enabled": True}}) -@mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") @mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") -def test_remove_targets_file_same_product( - mock_appsec_rules_data, mock_appsec_1click_activation, remote_config_worker, tracer -): +def test_remove_targets_file_with_previous_configuration(mock_appsec_rules_data, remote_config_worker, tracer): with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): RCAppSecCallBack.configs = {} tracer.configure(appsec_enabled=True, api_version="v0.4") enable_appsec_rc(tracer) - asm_features_data = b'{"asm":{"enabled":true}}' asm_data_data1 = b'{"data": [{"a":1}]}' asm_data_data2 = b'{"data": [{"b":2}]}' payload = AgentPayload( target_files=[ - TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data1)), TargetFile(path="mock/ASM_DATA/2", raw=base64.b64encode(asm_data_data2)), ] ) applied_configs = { - "mock/ASM_FEATURES": ConfigMetadata( - id="", - product_name="ASM_FEATURES", - sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), - length=5, - tuf_version=5, - ), "mock/ASM_DATA/1": ConfigMetadata( id="", product_name="ASM_DATA", @@ -375,13 +433,6 @@ def test_remove_targets_file_same_product( } client_configs = { - "mock/ASM_FEATURES": ConfigMetadata( - id="", - product_name="ASM_FEATURES", - sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), - length=5, - tuf_version=5, - ), "mock/ASM_DATA/1": ConfigMetadata( id="", product_name="ASM_DATA", @@ -405,7 +456,7 @@ def test_remove_targets_file_same_product( RemoteConfig._worker._client._remove_previously_applied_configurations({}, client_configs, target_file) RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) - mock_appsec_rules_data.assert_called_with(ANY, {}) + mock_appsec_rules_data.assert_not_called() @pytest.mark.skipif(sys.version_info[:2] < (3, 6), reason="Mock return order is different in python <= 3.5") From 6cac78a812556cdedc67205e034395bcb6cb7b2a Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Mon, 6 Mar 2023 17:25:11 +0100 Subject: [PATCH 8/9] feat(rcm): fix test --- tests/internal/remoteconfig/test_remoteconfig_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/internal/remoteconfig/test_remoteconfig_client.py b/tests/internal/remoteconfig/test_remoteconfig_client.py index 0247e4d6516..aac648ac9b0 100644 --- a/tests/internal/remoteconfig/test_remoteconfig_client.py +++ b/tests/internal/remoteconfig/test_remoteconfig_client.py @@ -48,7 +48,7 @@ class MockExtractFile: def __call__(self, payload, target, config): self.counter += 1 - result = {"test{}".format(self.counter): target} + result = {"test{}".format(self.counter): [target]} expected_results.update(result) return result From ba17480ccb70828c90caf288c2183aaef9fd989d Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Tue, 7 Mar 2023 13:36:12 +0100 Subject: [PATCH 9/9] feat: merge #5263 --- ddtrace/appsec/_remoteconfiguration.py | 33 ++-- ddtrace/internal/remoteconfig/client.py | 15 +- tests/appsec/test_remoteconfiguration.py | 216 ++++++++++++++++++++++- 3 files changed, 238 insertions(+), 26 deletions(-) diff --git a/ddtrace/appsec/_remoteconfiguration.py b/ddtrace/appsec/_remoteconfiguration.py index ef625ace268..88a72a8b054 100644 --- a/ddtrace/appsec/_remoteconfiguration.py +++ b/ddtrace/appsec/_remoteconfiguration.py @@ -20,6 +20,7 @@ if TYPE_CHECKING: # pragma: no cover from typing import Any + from typing import Dict try: from typing import Literal @@ -61,12 +62,14 @@ def enable_appsec_rc(test_tracer=None): RemoteConfig.register(PRODUCTS.ASM_DD, asm_dd_callback) # DD Rules -def _add_rules_to_list(features, feature, message, rule_list): - # type: (Mapping[str, Any], str, str, list[Any]) -> None - rules = features.get(feature, []) - if rules: +def _add_rules_to_list(features, feature, message, ruleset): + # type: (Mapping[str, Any], str, str, Dict[str, Any]) -> None + rules = features.get(feature, None) + if rules is not None: try: - rule_list += rules + if ruleset.get(feature) is None: + ruleset[feature] = [] + ruleset[feature] += rules log.debug("Reloading Appsec %s: %s", message, rules) except JSONDecodeError: log.error("ERROR Appsec %s: invalid JSON content from remote configuration", message) @@ -75,14 +78,18 @@ def _add_rules_to_list(features, feature, message, rule_list): def _appsec_rules_data(tracer, features): # type: (Tracer, Mapping[str, Any]) -> bool if features and tracer._appsec_processor: - ruleset = {"rules": [], "rules_data": [], "exclusions": [], "rules_override": []} # type: dict[str, list[Any]] - _add_rules_to_list(features, "rules_data", "rules data", ruleset["rules_data"]) - _add_rules_to_list(features, "custom_rules", "custom rules", ruleset["rules"]) - _add_rules_to_list(features, "rules", "Datadog rules", ruleset["rules"]) - _add_rules_to_list(features, "exclusions", "exclusion filters", ruleset["exclusions"]) - _add_rules_to_list(features, "rules_override", "rules override", ruleset["rules_override"]) - if any(ruleset.values()): - return tracer._appsec_processor._update_rules({k: v for k, v in ruleset.items() if v}) + ruleset = { + "rules": None, + "rules_data": None, + "exclusions": None, + "rules_override": None, + } # type: dict[str, Optional[list[Any]]] + _add_rules_to_list(features, "rules_data", "rules data", ruleset) + _add_rules_to_list(features, "custom_rules", "custom rules", ruleset) + _add_rules_to_list(features, "rules", "Datadog rules", ruleset) + _add_rules_to_list(features, "exclusions", "exclusion filters", ruleset) + _add_rules_to_list(features, "rules_override", "rules override", ruleset) + return tracer._appsec_processor._update_rules({k: v for k, v in ruleset.items() if v is not None}) return False diff --git a/ddtrace/internal/remoteconfig/client.py b/ddtrace/internal/remoteconfig/client.py index 5daef3f5e9d..55d8f86f0a2 100644 --- a/ddtrace/internal/remoteconfig/client.py +++ b/ddtrace/internal/remoteconfig/client.py @@ -173,11 +173,7 @@ def append(self, target, config): # Append the new config to the configs dict. _load_new_configurations function should # call to this method if isinstance(config, dict): - if not any(v for k, v in config.items()): - # Ups, no, if new config is empty, it's the same behavior of remove config - del self.configs[target] - else: - self.configs[target].update(config) + self.configs[target].update(config) else: raise ValueError("target %s config %s has type of %s" % (target, config, type(config))) @@ -185,11 +181,10 @@ def dispatch(self): config_result = {} for target, config in self.configs.items(): for key, value in config.items(): - if value: - if isinstance(value, list): - config_result[key] = config_result.get(key, []) + value - else: - raise ValueError("target %s key %s has type of %s" % (target, key, type(value))) + if isinstance(value, list): + config_result[key] = config_result.get(key, []) + value + else: + raise ValueError("target %s key %s has type of %s" % (target, key, type(value))) if config_result: self.__call__("", config_result) diff --git a/tests/appsec/test_remoteconfiguration.py b/tests/appsec/test_remoteconfiguration.py index b2ca8ae3caa..12b6fadef6d 100644 --- a/tests/appsec/test_remoteconfiguration.py +++ b/tests/appsec/test_remoteconfiguration.py @@ -15,6 +15,7 @@ from ddtrace.appsec._remoteconfiguration import RCAppSecFeaturesCallBack from ddtrace.appsec._remoteconfiguration import _appsec_rules_data from ddtrace.appsec._remoteconfiguration import enable_appsec_rc +from ddtrace.appsec.processor import AppSecSpanProcessor from ddtrace.appsec.utils import _appsec_rc_capabilities from ddtrace.appsec.utils import _appsec_rc_features_is_enabled from ddtrace.constants import APPSEC_ENV @@ -264,7 +265,7 @@ def test_load_new_configurations_empty_config( } RemoteConfig._worker._client._load_new_configurations({}, client_configs, payload=payload) - mock_appsec_rules_data.assert_not_called() + mock_appsec_rules_data.assert_called_with(ANY, {"data": []}) mock_appsec_1click_activation.assert_called_with({"asm": {"enabled": True}}) @@ -543,14 +544,223 @@ def test_load_new_config_and_remove_targets_file_same_product( mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}]}) +@pytest.mark.skipif(sys.version_info[:2] < (3, 6), reason="Mock return order is different in python <= 3.5") +@mock.patch.object(AppSecSpanProcessor, "_update_rules") +def test_fullpath_appsec_rules_data(mock_update_rules, remote_config_worker, tracer): + with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True, api_version="v0.4") + applied_configs = {} + enable_appsec_rc(tracer) + asm_features_data = b'{"asm":{"enabled":true}}' + asm_data_data1 = b'{"exclusions": [{"a":1}]}' + asm_data_data2 = b'{"exclusions": [{"b":2}]}' + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_FEATURES", raw=base64.b64encode(asm_features_data)), + TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data1)), + TargetFile(path="mock/ASM_DATA/2", raw=base64.b64encode(asm_data_data2)), + ] + ) + first_config = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ), + } + + second_config = { + "mock/ASM_FEATURES": ConfigMetadata( + id="", + product_name="ASM_FEATURES", + sha256_hash=hashlib.sha256(asm_features_data).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + } + + target_file = { + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ) + } + RemoteConfig._worker._client._remove_previously_applied_configurations({}, first_config, {}) + + RemoteConfig._worker._client._load_new_configurations(applied_configs, first_config, payload=payload) + RemoteConfig._worker._client._applied_configs = applied_configs + + mock_update_rules.assert_called_with({"exclusions": [{"a": 1}, {"b": 2}]}) + mock_update_rules.reset_mock() + + RemoteConfig._worker._client._remove_previously_applied_configurations({}, second_config, target_file) + + RemoteConfig._worker._client._load_new_configurations({}, second_config, payload=payload) + mock_update_rules.assert_called_with({"exclusions": [{"a": 1}]}) + + +@pytest.mark.skipif(sys.version_info[:2] < (3, 6), reason="Mock return order is different in python <= 3.5") +@mock.patch.object(AppSecSpanProcessor, "_update_rules") +def test_fullpath_appsec_rules_data_empty_data(mock_update_rules, remote_config_worker, tracer): + with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True, api_version="v0.4") + applied_configs = {} + enable_appsec_rc(tracer) + asm_data_data1 = b'{"exclusions": [{"a":1}]}' + asm_data_data2 = b'{"exclusions": []}' + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data1)), + TargetFile(path="mock/ASM_DATA/2", raw=base64.b64encode(asm_data_data2)), + ] + ) + first_config = { + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ), + } + + second_config = { + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + } + + target_file = { + "mock/ASM_DATA/2": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ) + } + RemoteConfig._worker._client._remove_previously_applied_configurations({}, first_config, {}) + + RemoteConfig._worker._client._load_new_configurations(applied_configs, first_config, payload=payload) + RemoteConfig._worker._client._applied_configs = applied_configs + + mock_update_rules.assert_called_with({"exclusions": [{"a": 1}]}) + mock_update_rules.reset_mock() + + RemoteConfig._worker._client._remove_previously_applied_configurations({}, second_config, target_file) + + RemoteConfig._worker._client._load_new_configurations({}, second_config, payload=payload) + mock_update_rules.assert_called_with({"exclusions": [{"a": 1}]}) + + +@pytest.mark.skipif(sys.version_info[:2] < (3, 6), reason="Mock return order is different in python <= 3.5") +@mock.patch.object(AppSecSpanProcessor, "_update_rules") +def test_fullpath_appsec_rules_data_add_delete_file(mock_update_rules, remote_config_worker, tracer): + with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): + tracer.configure(appsec_enabled=True, api_version="v0.4") + applied_configs = {} + enable_appsec_rc(tracer) + asm_data_data1 = b'{"exclusions": [{"a":1}]}' + asm_data_data2 = b'{"exclusions": []}' + payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data1)), + ] + ) + first_config = { + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ), + } + + second_payload = AgentPayload( + target_files=[ + TargetFile(path="mock/ASM_DATA/1", raw=base64.b64encode(asm_data_data2)), + ] + ) + second_config = { + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data2).hexdigest(), + length=5, + tuf_version=5, + ), + } + + target_file = { + "mock/ASM_DATA/1": ConfigMetadata( + id="", + product_name="ASM_DATA", + sha256_hash=hashlib.sha256(asm_data_data1).hexdigest(), + length=5, + tuf_version=5, + ) + } + RemoteConfig._worker._client._remove_previously_applied_configurations({}, first_config, {}) + + RemoteConfig._worker._client._load_new_configurations(applied_configs, first_config, payload=payload) + RemoteConfig._worker._client._applied_configs = applied_configs + + mock_update_rules.assert_called_with({"exclusions": [{"a": 1}]}) + mock_update_rules.reset_mock() + + RemoteConfig._worker._client._remove_previously_applied_configurations({}, second_config, target_file) + + RemoteConfig._worker._client._load_new_configurations({}, second_config, payload=second_payload) + mock_update_rules.assert_called_with({"exclusions": []}) + + @pytest.mark.skipif(sys.version_info[:2] < (3, 6), reason="Mock return order is different in python <= 3.5") @mock.patch.object(RCAppSecFeaturesCallBack, "_appsec_1click_activation") @mock.patch("ddtrace.appsec._remoteconfiguration._appsec_rules_data") def test_load_new_empty_config_and_remove_targets_file_same_product( - mock_appsec_rules_data, mock_appsec_1click_activation, remote_config_worker, tracer + mock_appsec_rules_data, remote_config_worker, tracer ): with override_global_config(dict(_appsec_enabled=True, api_version="v0.4")): tracer.configure(appsec_enabled=True, api_version="v0.4") + RCAppSecCallBack.configs = {} applied_configs = {} enable_appsec_rc(tracer) asm_features_data = b'{"asm":{"enabled":true}}' @@ -622,7 +832,7 @@ def test_load_new_empty_config_and_remove_targets_file_same_product( ) RemoteConfig._worker._client._load_new_configurations({}, second_config, payload=payload) - mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}]}) + mock_appsec_rules_data.assert_called_with(ANY, {"data": [{"a": 1}], "data2": []}) def test_rc_activation_ip_blocking_data(tracer, remote_config_worker):