From befa9acd54a747a6b944123f6b7eb50d56b520ce Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Tue, 4 Feb 2025 10:21:31 +0100 Subject: [PATCH 01/23] decouple monkey --- ddtrace/_monkey.py | 6 ++++-- ddtrace/settings/asm.py | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ddtrace/_monkey.py b/ddtrace/_monkey.py index fad8c8f4d2b..bbf5f3e55db 100644 --- a/ddtrace/_monkey.py +++ b/ddtrace/_monkey.py @@ -5,7 +5,6 @@ from wrapt.importer import when_imported -from ddtrace.appsec import load_common_appsec_modules from ddtrace.internal.telemetry.constants import TELEMETRY_NAMESPACE from ddtrace.settings.asm import config as asm_config @@ -247,7 +246,10 @@ def patch_all(**patch_modules): patch_iast() enable_iast_propagation() - load_common_appsec_modules() + if asm_config._load_modules: + from ddtrace.appsec import load_common_appsec_modules + + load_common_appsec_modules() def patch(raise_errors=True, **patch_modules): diff --git a/ddtrace/settings/asm.py b/ddtrace/settings/asm.py index 4024c13f982..a6b11dd4662 100644 --- a/ddtrace/settings/asm.py +++ b/ddtrace/settings/asm.py @@ -233,6 +233,9 @@ def __init__(self): self._api_security_enabled = False if not self._iast_supported: self._iast_enabled = False + self._load_modules: bool = bool( + self._iast_supported or (self._ep_enabled and (self._asm_enabled or self._asm_can_be_enabled)) + ) def reset(self): """For testing purposes, reset the configuration to its default values given current environment variables.""" From abf96da64f5bc35661aad59f05f3d7c3d6c9625a Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Tue, 4 Feb 2025 11:52:56 +0100 Subject: [PATCH 02/23] fix patch logic --- ddtrace/_monkey.py | 6 ++---- ddtrace/appsec/__init__.py | 7 +++++-- ddtrace/appsec/_common_module_patches.py | 8 ++++---- ddtrace/settings/asm.py | 10 +++++----- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/ddtrace/_monkey.py b/ddtrace/_monkey.py index bbf5f3e55db..fad8c8f4d2b 100644 --- a/ddtrace/_monkey.py +++ b/ddtrace/_monkey.py @@ -5,6 +5,7 @@ from wrapt.importer import when_imported +from ddtrace.appsec import load_common_appsec_modules from ddtrace.internal.telemetry.constants import TELEMETRY_NAMESPACE from ddtrace.settings.asm import config as asm_config @@ -246,10 +247,7 @@ def patch_all(**patch_modules): patch_iast() enable_iast_propagation() - if asm_config._load_modules: - from ddtrace.appsec import load_common_appsec_modules - - load_common_appsec_modules() + load_common_appsec_modules() def patch(raise_errors=True, **patch_modules): diff --git a/ddtrace/appsec/__init__.py b/ddtrace/appsec/__init__.py index 05d1a852710..a32442ccc0a 100644 --- a/ddtrace/appsec/__init__.py +++ b/ddtrace/appsec/__init__.py @@ -1,5 +1,6 @@ +# this module must not load any other appsec module + from ddtrace.internal import core -from ddtrace.settings.asm import config as asm_config _APPSEC_TO_BE_LOADED = True @@ -28,7 +29,9 @@ def load_iast(): def load_common_appsec_modules(): """Lazily load the common module patches.""" - if (asm_config._ep_enabled and asm_config._asm_enabled) or asm_config._iast_enabled: + from ddtrace.settings.asm import config as asm_config + + if asm_config._load_modules: from ddtrace.appsec._common_module_patches import patch_common_modules patch_common_modules() diff --git a/ddtrace/appsec/_common_module_patches.py b/ddtrace/appsec/_common_module_patches.py index 8c834b80e6f..ac3c2c4e775 100644 --- a/ddtrace/appsec/_common_module_patches.py +++ b/ddtrace/appsec/_common_module_patches.py @@ -44,6 +44,10 @@ def is_iast_request_enabled() -> bool: def patch_common_modules(): global _is_patched + # ensure that the subprocess patch is applied even after one click activation + subprocess_patch.patch() + subprocess_patch.add_str_callback(_RASP_SYSTEM, wrapped_system_5542593D237084A7) + subprocess_patch.add_lst_callback(_RASP_POPEN, popen_FD233052260D8B4D) if _is_patched: return # for testing purposes, we need to update is_iast_request_enabled @@ -60,10 +64,6 @@ def is_iast_request_enabled() -> bool: try_wrap_function_wrapper("urllib.request", "OpenerDirector.open", wrapped_open_ED4CF71136E15EBF) try_wrap_function_wrapper("_io", "BytesIO.read", wrapped_read_F3E51D71B4EC16EF) try_wrap_function_wrapper("_io", "StringIO.read", wrapped_read_F3E51D71B4EC16EF) - # ensure that the subprocess patch is applied even after one click activation - subprocess_patch.patch() - subprocess_patch.add_str_callback(_RASP_SYSTEM, wrapped_system_5542593D237084A7) - subprocess_patch.add_lst_callback(_RASP_POPEN, popen_FD233052260D8B4D) core.on("asm.block.dbapi.execute", execute_4C9BAC8E228EB347) if asm_config._iast_enabled: from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink diff --git a/ddtrace/settings/asm.py b/ddtrace/settings/asm.py index a6b11dd4662..717763304d0 100644 --- a/ddtrace/settings/asm.py +++ b/ddtrace/settings/asm.py @@ -224,8 +224,6 @@ class ASMConfig(Env): def __init__(self): super().__init__() - # Is one click available? - self._eval_asm_can_be_enabled() if not self._asm_libddwaf_available: self._asm_enabled = False self._asm_can_be_enabled = False @@ -233,9 +231,8 @@ def __init__(self): self._api_security_enabled = False if not self._iast_supported: self._iast_enabled = False - self._load_modules: bool = bool( - self._iast_supported or (self._ep_enabled and (self._asm_enabled or self._asm_can_be_enabled)) - ) + # Is one click available? + self._eval_asm_can_be_enabled() def reset(self): """For testing purposes, reset the configuration to its default values given current environment variables.""" @@ -243,6 +240,9 @@ def reset(self): def _eval_asm_can_be_enabled(self): self._asm_can_be_enabled = APPSEC_ENV not in os.environ and tracer_config._remote_config_enabled + self._load_modules: bool = bool( + self._iast_supported or (self._ep_enabled and (self._asm_enabled or self._asm_can_be_enabled)) + ) @property def _api_security_feature_active(self) -> bool: From 27bee9549d827ba38707f463cc4ac8732317eafd Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Tue, 4 Feb 2025 12:58:12 +0100 Subject: [PATCH 03/23] import iast only if enabled in asm_request_context --- ddtrace/appsec/_asm_request_context.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ddtrace/appsec/_asm_request_context.py b/ddtrace/appsec/_asm_request_context.py index d8f258d43a7..bd6c8b21a2a 100644 --- a/ddtrace/appsec/_asm_request_context.py +++ b/ddtrace/appsec/_asm_request_context.py @@ -15,9 +15,6 @@ from ddtrace.appsec._constants import APPSEC from ddtrace.appsec._constants import EXPLOIT_PREVENTION from ddtrace.appsec._constants import SPAN_DATA_NAMES -from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled -from ddtrace.appsec._iast._taint_tracking import OriginType -from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject from ddtrace.appsec._utils import add_context_log from ddtrace.appsec._utils import get_triggers from ddtrace.internal import core @@ -28,6 +25,16 @@ from ddtrace.trace import Span +if asm_config._iast_enabled: + from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled + from ddtrace.appsec._iast._taint_tracking import OriginType + from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject +else: + + def is_iast_request_enabled() -> bool: + return False + + if TYPE_CHECKING: from ddtrace.appsec._ddwaf import DDWaf_info from ddtrace.appsec._ddwaf import DDWaf_result From ee6950de2586034ad009dd8d650b55fcc0cf6c24 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Tue, 4 Feb 2025 13:53:07 +0100 Subject: [PATCH 04/23] factor logic --- ddtrace/internal/appsec/product.py | 5 ++--- ddtrace/internal/writer/writer.py | 4 +--- ddtrace/settings/asm.py | 18 +++++++++++++----- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/ddtrace/internal/appsec/product.py b/ddtrace/internal/appsec/product.py index 126d6d2a04f..e0854ff2a2a 100644 --- a/ddtrace/internal/appsec/product.py +++ b/ddtrace/internal/appsec/product.py @@ -1,4 +1,3 @@ -from ddtrace import config from ddtrace.settings.asm import config as asm_config @@ -10,14 +9,14 @@ def post_preload(): def start(): - if asm_config._asm_enabled or config._remote_config_enabled: + if asm_config._asm_rc_enabled: from ddtrace.appsec._remoteconfiguration import enable_appsec_rc enable_appsec_rc() def restart(join=False): - if asm_config._asm_enabled or config._remote_config_enabled: + if asm_config._asm_rc_enabled: from ddtrace.appsec._remoteconfiguration import _forksafe_appsec_rc _forksafe_appsec_rc() diff --git a/ddtrace/internal/writer/writer.py b/ddtrace/internal/writer/writer.py index da3f09a99b1..301d0400c7c 100644 --- a/ddtrace/internal/writer/writer.py +++ b/ddtrace/internal/writer/writer.py @@ -578,9 +578,7 @@ def start(self): try: # appsec remote config should be enabled/started after the global tracer and configs # are initialized - if os.getenv("AWS_LAMBDA_FUNCTION_NAME") is None and ( - asm_config._asm_enabled or config._remote_config_enabled - ): + if asm_config._asm_rc_enabled: from ddtrace.appsec._remoteconfiguration import enable_appsec_rc enable_appsec_rc() diff --git a/ddtrace/settings/asm.py b/ddtrace/settings/asm.py index 717763304d0..ff1c0b3d767 100644 --- a/ddtrace/settings/asm.py +++ b/ddtrace/settings/asm.py @@ -17,6 +17,7 @@ from ddtrace.appsec._constants import LOGIN_EVENTS_MODE from ddtrace.appsec._constants import TELEMETRY_INFORMATION_NAME from ddtrace.constants import APPSEC_ENV +from ddtrace.internal.serverless import in_aws_lambda from ddtrace.settings._core import report_telemetry as _report_telemetry @@ -224,15 +225,21 @@ class ASMConfig(Env): def __init__(self): super().__init__() - if not self._asm_libddwaf_available: + if not self._iast_supported: + self._iast_enabled = False + if not self._asm_libddwaf_available or in_aws_lambda(): self._asm_enabled = False self._asm_can_be_enabled = False self._iast_enabled = False self._api_security_enabled = False - if not self._iast_supported: - self._iast_enabled = False - # Is one click available? - self._eval_asm_can_be_enabled() + self._ep_enabled = False + self._auto_user_instrumentation_enabled = False + self._auto_user_instrumentation_local_mode = LOGIN_EVENTS_MODE.DISABLED + self._load_modules = False + self._asm_rc_enabled = False + else: + # Is one click available? + self._eval_asm_can_be_enabled() def reset(self): """For testing purposes, reset the configuration to its default values given current environment variables.""" @@ -243,6 +250,7 @@ def _eval_asm_can_be_enabled(self): self._load_modules: bool = bool( self._iast_supported or (self._ep_enabled and (self._asm_enabled or self._asm_can_be_enabled)) ) + self._asm_rc_enabled = (self._asm_enabled and tracer_config._remote_config_enabled) or self._asm_can_be_enabled @property def _api_security_feature_active(self) -> bool: From bdca8014581dbd6c8861454ba54462c45e16599b Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Tue, 4 Feb 2025 16:27:29 +0100 Subject: [PATCH 05/23] fix subprocess tests --- ddtrace/contrib/internal/subprocess/patch.py | 2 +- ddtrace/settings/asm.py | 2 +- tests/contrib/subprocess/test_subprocess.py | 4 ++-- tests/utils.py | 10 +++++----- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index 2d66edd4737..4fff343578a 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -58,7 +58,7 @@ def del_lst_callback(name: str): def patch() -> List[str]: - if not (asm_config._asm_enabled or asm_config._iast_enabled): + if not asm_config._load_modules: return [] patched: List[str] = [] diff --git a/ddtrace/settings/asm.py b/ddtrace/settings/asm.py index ff1c0b3d767..8cb35132d47 100644 --- a/ddtrace/settings/asm.py +++ b/ddtrace/settings/asm.py @@ -248,7 +248,7 @@ def reset(self): def _eval_asm_can_be_enabled(self): self._asm_can_be_enabled = APPSEC_ENV not in os.environ and tracer_config._remote_config_enabled self._load_modules: bool = bool( - self._iast_supported or (self._ep_enabled and (self._asm_enabled or self._asm_can_be_enabled)) + self._iast_enabled or (self._ep_enabled and (self._asm_enabled or self._asm_can_be_enabled)) ) self._asm_rc_enabled = (self._asm_enabled and tracer_config._remote_config_enabled) or self._asm_can_be_enabled diff --git a/tests/contrib/subprocess/test_subprocess.py b/tests/contrib/subprocess/test_subprocess.py index 40e7ab67431..6cf28c2fef9 100644 --- a/tests/contrib/subprocess/test_subprocess.py +++ b/tests/contrib/subprocess/test_subprocess.py @@ -253,7 +253,7 @@ def test_unpatch(tracer): assert span.get_tag(COMMANDS.SHELL) == "dir -l /" unpatch() - with override_global_config(dict(_asm_enabled=True)): + with override_global_config(dict(_ep_enabled=False)): Pin.get_from(os).clone(tracer=tracer).onto(os) with tracer.trace("os.system_unpatch"): ret = os.system("dir -l /") @@ -273,7 +273,7 @@ def test_unpatch(tracer): def test_ossystem_noappsec(tracer): - with override_global_config(dict(_asm_enabled=False)): + with override_global_config(dict(_ep_enabled=False)): patch() assert not hasattr(os.system, "__wrapped__") assert not hasattr(os._spawnvef, "__wrapped__") diff --git a/tests/utils.py b/tests/utils.py index bc7acd68b84..c06471262fc 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -170,7 +170,7 @@ def override_global_config(values): ddtrace.config._subscriptions = [] # Grab the current values of all keys originals = dict((key, getattr(ddtrace.config, key)) for key in global_config_keys) - asm_originals = dict((key, getattr(ddtrace.settings.asm.config, key)) for key in asm_config_keys) + asm_originals = dict((key, getattr(asm_config, key)) for key in asm_config_keys) # Override from the passed in keys for key, value in values.items(): @@ -179,9 +179,9 @@ def override_global_config(values): # rebuild asm config from env vars and global config for key, value in values.items(): if key in asm_config_keys: - setattr(ddtrace.settings.asm.config, key, value) + setattr(asm_config, key, value) # If ddtrace.settings.asm.config has changed, check _asm_can_be_enabled again - ddtrace.settings.asm.config._eval_asm_can_be_enabled() + asm_config._eval_asm_can_be_enabled() try: core.dispatch("test.config.override") yield @@ -190,9 +190,9 @@ def override_global_config(values): for key, value in originals.items(): setattr(ddtrace.config, key, value) - ddtrace.settings.asm.config.reset() + asm_config.reset() for key, value in asm_originals.items(): - setattr(ddtrace.settings.asm.config, key, value) + setattr(asm_config, key, value) ddtrace.config._reset() ddtrace.config._subscriptions = subscriptions From 517659ddce52e22edf425928b1d74032ce67d070 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Tue, 4 Feb 2025 16:59:08 +0100 Subject: [PATCH 06/23] ensure patch is working on windows --- ddtrace/contrib/internal/subprocess/patch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index 4fff343578a..90d2a333ba6 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -66,7 +66,7 @@ def patch() -> List[str]: import subprocess # nosec should_patch_system = not trace_utils.iswrapped(os.system) - should_patch_fork = not trace_utils.iswrapped(os.fork) + should_patch_fork = (not trace_utils.iswrapped(os.fork)) if hasattr(os, "fork") else False spawnvef = getattr(os, "_spawnvef", None) should_patch_spawnvef = spawnvef is not None and not trace_utils.iswrapped(spawnvef) From f85e7d62eb1dfa6a3a7bccdbf0c46a6a54cfa975 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Tue, 4 Feb 2025 17:02:54 +0100 Subject: [PATCH 07/23] ensure unpatch is guarded --- ddtrace/contrib/internal/subprocess/patch.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index 90d2a333ba6..189cc703496 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -316,10 +316,11 @@ def unpatch() -> None: import os # nosec import subprocess # nosec - trace_utils.unwrap(os, "system") - trace_utils.unwrap(os, "_spawnvef") - trace_utils.unwrap(subprocess.Popen, "__init__") - trace_utils.unwrap(subprocess.Popen, "wait") + for obj, attr in [(os, "system"), (os, "_spawnvef"), (subprocess.Popen, "__init__"), (subprocess.Popen, "wait")]: + try: + trace_utils.unwrap(obj, attr) + except AttributeError: + pass SubprocessCmdLine._clear_cache() From c0a5e17578e505f9eba23b66c64700468bec20b6 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Tue, 4 Feb 2025 17:33:00 +0100 Subject: [PATCH 08/23] fix test --- tests/contrib/subprocess/test_subprocess_patch.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/contrib/subprocess/test_subprocess_patch.py b/tests/contrib/subprocess/test_subprocess_patch.py index 57778f798c1..471f096fbae 100644 --- a/tests/contrib/subprocess/test_subprocess_patch.py +++ b/tests/contrib/subprocess/test_subprocess_patch.py @@ -19,6 +19,8 @@ class TestSubprocessPatch(PatchTestCase.Base): def __init__(self, *args, **kwargs): asm_config._asm_enabled = True + asm_config._ep_enabled = True + asm_config._load_modules = True super(TestSubprocessPatch, self).__init__(*args, **kwargs) def assert_module_patched(self, subprocess): From c460b9373ee42b2d9b6569fba07b9e8a5c526e69 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 5 Feb 2025 10:03:50 +0100 Subject: [PATCH 09/23] fix selenium test --- hatch.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/hatch.toml b/hatch.toml index 3e80f24a5e7..fbfba2794c3 100644 --- a/hatch.toml +++ b/hatch.toml @@ -655,6 +655,7 @@ dependencies = [ [evs.selenium.env-vars] DD_AGENT_TRACER_URL = "9126" CMAKE_BUILD_PARALLEL_LEVEL = "12" +DD_APPSEC_RASP_ENABLED = "false" [envs.selenium.scripts] test = [ From 1582b1369cff5f30678c5e634df02a5e89042daf Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 5 Feb 2025 10:30:29 +0100 Subject: [PATCH 10/23] disable subprocess patches when asm disabled --- ddtrace/contrib/internal/subprocess/patch.py | 10 +++++++--- hatch.toml | 1 - 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index 189cc703496..929fd58d05d 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -328,7 +328,7 @@ def unpatch() -> None: @trace_utils.with_traced_module def _traced_ossystem(module, pin, wrapped, instance, args, kwargs): try: - if asm_config._bypass_instrumentation_for_waf: + if asm_config._bypass_instrumentation_for_waf or not asm_config._asm_enabled: return wrapped(*args, **kwargs) if isinstance(args[0], str): for callback in _STR_CALLBACKS.values(): @@ -352,6 +352,8 @@ def _traced_ossystem(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_fork(module, pin, wrapped, instance, args, kwargs): + if not asm_config._asm_enabled: + return wrapped(*args, **kwargs) try: with pin.tracer.trace(COMMANDS.SPAN_NAME, resource="fork", span_type=SpanTypes.SYSTEM) as span: span.set_tag(COMMANDS.EXEC, ["os.fork"]) @@ -367,6 +369,8 @@ def _traced_fork(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_osspawn(module, pin, wrapped, instance, args, kwargs): + if not asm_config._asm_enabled: + return wrapped(*args, **kwargs) try: mode, file, func_args, _, _ = args if isinstance(func_args, (list, tuple, str)): @@ -396,7 +400,7 @@ def _traced_osspawn(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): try: - if asm_config._bypass_instrumentation_for_waf: + if asm_config._bypass_instrumentation_for_waf or not asm_config._asm_enabled: return wrapped(*args, **kwargs) cmd_args = args[0] if len(args) else kwargs["args"] if isinstance(cmd_args, (list, tuple, str)): @@ -430,7 +434,7 @@ def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_wait(module, pin, wrapped, instance, args, kwargs): try: - if asm_config._bypass_instrumentation_for_waf: + if asm_config._bypass_instrumentation_for_waf or not asm_config._asm_enabled: return wrapped(*args, **kwargs) binary = core.get_item("subprocess_popen_binary") diff --git a/hatch.toml b/hatch.toml index fbfba2794c3..3e80f24a5e7 100644 --- a/hatch.toml +++ b/hatch.toml @@ -655,7 +655,6 @@ dependencies = [ [evs.selenium.env-vars] DD_AGENT_TRACER_URL = "9126" CMAKE_BUILD_PARALLEL_LEVEL = "12" -DD_APPSEC_RASP_ENABLED = "false" [envs.selenium.scripts] test = [ From 4532d044066a5b55228066c8aaa03a426f50cda1 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 5 Feb 2025 10:40:52 +0100 Subject: [PATCH 11/23] enable subprocess patches when iast enabled --- ddtrace/contrib/internal/subprocess/patch.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index 929fd58d05d..1ffc1f2d733 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -328,7 +328,7 @@ def unpatch() -> None: @trace_utils.with_traced_module def _traced_ossystem(module, pin, wrapped, instance, args, kwargs): try: - if asm_config._bypass_instrumentation_for_waf or not asm_config._asm_enabled: + if asm_config._bypass_instrumentation_for_waf or not (asm_config._asm_enabled or asm_config._iast_enabled): return wrapped(*args, **kwargs) if isinstance(args[0], str): for callback in _STR_CALLBACKS.values(): @@ -352,7 +352,7 @@ def _traced_ossystem(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_fork(module, pin, wrapped, instance, args, kwargs): - if not asm_config._asm_enabled: + if not (asm_config._asm_enabled or asm_config._iast_enabled): return wrapped(*args, **kwargs) try: with pin.tracer.trace(COMMANDS.SPAN_NAME, resource="fork", span_type=SpanTypes.SYSTEM) as span: @@ -369,7 +369,7 @@ def _traced_fork(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_osspawn(module, pin, wrapped, instance, args, kwargs): - if not asm_config._asm_enabled: + if not (asm_config._asm_enabled or asm_config._iast_enabled): return wrapped(*args, **kwargs) try: mode, file, func_args, _, _ = args @@ -400,7 +400,7 @@ def _traced_osspawn(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): try: - if asm_config._bypass_instrumentation_for_waf or not asm_config._asm_enabled: + if asm_config._bypass_instrumentation_for_waf or not (asm_config._asm_enabled or asm_config._iast_enabled): return wrapped(*args, **kwargs) cmd_args = args[0] if len(args) else kwargs["args"] if isinstance(cmd_args, (list, tuple, str)): @@ -434,7 +434,7 @@ def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_wait(module, pin, wrapped, instance, args, kwargs): try: - if asm_config._bypass_instrumentation_for_waf or not asm_config._asm_enabled: + if asm_config._bypass_instrumentation_for_waf or not (asm_config._asm_enabled or asm_config._iast_enabled): return wrapped(*args, **kwargs) binary = core.get_item("subprocess_popen_binary") From 5ec6f3b278dc9b4d26a88fab2f70cb542ca81684 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 5 Feb 2025 11:29:10 +0100 Subject: [PATCH 12/23] ensure IAST is enabled at start for appsec_integrations_flask tests --- hatch.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hatch.toml b/hatch.toml index 3e80f24a5e7..02a22818501 100644 --- a/hatch.toml +++ b/hatch.toml @@ -374,7 +374,7 @@ dependencies = [ test = [ "uname -a", "pip freeze", - "DD_TRACE_AGENT_URL=\"http://testagent:9126\" DD_CIVISIBILITY_ITR_ENABLED=0 DD_IAST_REQUEST_SAMPLING=100 DD_IAST_DEDUPLICATION_ENABLED=false python -m pytest -vvv {args:tests/appsec/integrations/flask_tests/}", + "DD_TRACE_AGENT_URL=\"http://testagent:9126\" DD_CIVISIBILITY_ITR_ENABLED=0 DD_IAST_ENABLED=true DD_IAST_REQUEST_SAMPLING=100 DD_IAST_DEDUPLICATION_ENABLED=false python -m pytest -vvv {args:tests/appsec/integrations/flask_tests/}", ] [[envs.appsec_integrations_flask.matrix]] From 05de3c1874cef503b094f411ace0f3ffdf6e533f Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 5 Feb 2025 11:52:20 +0100 Subject: [PATCH 13/23] disable iast explicitely when required --- .../flask_tests/test_iast_flask.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/appsec/integrations/flask_tests/test_iast_flask.py b/tests/appsec/integrations/flask_tests/test_iast_flask.py index be45e6bb82f..5ef471199ee 100644 --- a/tests/appsec/integrations/flask_tests/test_iast_flask.py +++ b/tests/appsec/integrations/flask_tests/test_iast_flask.py @@ -1621,18 +1621,19 @@ def test_sqli(): return "OK", 200 - if tuple(map(int, werkzeug_version.split("."))) >= (2, 3): - self.client.set_cookie(domain="localhost", key="sqlite_master", value="sqlite_master3") - else: - self.client.set_cookie(server_name="localhost", key="sqlite_master", value="sqlite_master3") + with override_global_config(dict(_iast_enabled=False)): + if tuple(map(int, werkzeug_version.split("."))) >= (2, 3): + self.client.set_cookie(domain="localhost", key="sqlite_master", value="sqlite_master3") + else: + self.client.set_cookie(server_name="localhost", key="sqlite_master", value="sqlite_master3") - resp = self.client.post("/sqli/cookies/") - assert resp.status_code == 200 + resp = self.client.post("/sqli/cookies/") + assert resp.status_code == 200 - root_span = self.pop_spans()[0] - assert root_span.get_metric(IAST.ENABLED) is None + root_span = self.pop_spans()[0] + assert root_span.get_metric(IAST.ENABLED) is None - assert root_span.get_tag(IAST.JSON) is None + assert root_span.get_tag(IAST.JSON) is None @pytest.mark.skipif(not asm_config._iast_supported, reason="Python version not supported by IAST") def test_flask_full_sqli_iast_disabled_http_request_header_getitem(self): From 15a2561bb96debc9340a724f0ebafd5e32f6326b Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 5 Feb 2025 13:47:26 +0100 Subject: [PATCH 14/23] more iast guards on pytest and langchain. remove dead code --- ddtrace/contrib/internal/langchain/patch.py | 23 ------------------- ddtrace/contrib/internal/pytest/_plugin_v2.py | 6 +++-- ddtrace/contrib/internal/pytest/plugin.py | 5 +++- 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/ddtrace/contrib/internal/langchain/patch.py b/ddtrace/contrib/internal/langchain/patch.py index f9681dd1302..b67a7826994 100644 --- a/ddtrace/contrib/internal/langchain/patch.py +++ b/ddtrace/contrib/internal/langchain/patch.py @@ -1113,29 +1113,6 @@ def unpatch(): delattr(langchain, "_datadog_integration") -def taint_outputs(instance, inputs, outputs): - from ddtrace.appsec._iast._metrics import _set_iast_error_metric - from ddtrace.appsec._iast._taint_tracking._taint_objects import get_tainted_ranges - from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject - - try: - ranges = None - for key in filter(lambda x: x in inputs, instance.input_keys): - input_val = inputs.get(key) - if input_val: - ranges = get_tainted_ranges(input_val) - if ranges: - break - - if ranges: - source = ranges[0].source - for key in filter(lambda x: x in outputs, instance.output_keys): - output_value = outputs[key] - outputs[key] = taint_pyobject(output_value, source.name, source.value, source.origin) - except Exception as e: - _set_iast_error_metric("IAST propagation error. langchain taint_outputs. {}".format(e)) - - def taint_parser_output(func, instance, args, kwargs): from ddtrace.appsec._iast._metrics import _set_iast_error_metric from ddtrace.appsec._iast._taint_tracking._taint_objects import get_tainted_ranges diff --git a/ddtrace/contrib/internal/pytest/_plugin_v2.py b/ddtrace/contrib/internal/pytest/_plugin_v2.py index 79435f94576..a9736374114 100644 --- a/ddtrace/contrib/internal/pytest/_plugin_v2.py +++ b/ddtrace/contrib/internal/pytest/_plugin_v2.py @@ -60,6 +60,7 @@ from ddtrace.internal.test_visibility.api import InternalTestSession from ddtrace.internal.test_visibility.api import InternalTestSuite from ddtrace.internal.test_visibility.coverage_lines import CoverageLines +from ddtrace.settings.asm import config as asm_config from ddtrace.vendor.debtcollector import deprecate @@ -574,9 +575,10 @@ def _pytest_terminal_summary_post_yield(terminalreporter, failed_reports_initial def pytest_terminal_summary(terminalreporter, exitstatus, config): """Report flaky or failed tests""" try: - from ddtrace.appsec._iast._pytest_plugin import print_iast_report + if asm_config._iast_enabled: + from ddtrace.appsec._iast._pytest_plugin import print_iast_report - print_iast_report(terminalreporter) + print_iast_report(terminalreporter) except Exception: # noqa: E722 log.debug("Encountered error during code security summary", exc_info=True) diff --git a/ddtrace/contrib/internal/pytest/plugin.py b/ddtrace/contrib/internal/pytest/plugin.py index 23bf58d7dcb..cee6d13ce6b 100644 --- a/ddtrace/contrib/internal/pytest/plugin.py +++ b/ddtrace/contrib/internal/pytest/plugin.py @@ -17,13 +17,16 @@ import pytest from ddtrace import config -from ddtrace.appsec._iast._pytest_plugin import ddtrace_iast # noqa:F401 from ddtrace.contrib.internal.pytest._utils import _USE_PLUGIN_V2 from ddtrace.contrib.internal.pytest._utils import _extract_span from ddtrace.contrib.internal.pytest._utils import _pytest_version_supports_itr from ddtrace.settings.asm import config as asm_config +if asm_config._iast_enabled: + from ddtrace.appsec._iast._pytest_plugin import ddtrace_iast # noqa:F401 + + # pytest default settings config._add( "pytest", From a265dd03dcf5e8112265e0e441ffc80348172f85 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 5 Feb 2025 15:22:32 +0100 Subject: [PATCH 15/23] complete decoupling --- ddtrace/appsec/__init__.py | 2 +- ddtrace/appsec/_constants.py | 2 + ddtrace/appsec/_utils.py | 4 +- ddtrace/contrib/internal/httplib/patch.py | 5 +- ddtrace/contrib/internal/mysql/patch.py | 5 +- ddtrace/contrib/internal/mysqldb/patch.py | 5 +- ddtrace/contrib/internal/requests/patch.py | 11 ++-- ddtrace/contrib/internal/sqlalchemy/patch.py | 5 +- ddtrace/contrib/internal/sqlite3/patch.py | 5 +- ddtrace/contrib/internal/urllib/patch.py | 12 ++-- ddtrace/contrib/internal/urllib3/patch.py | 19 +++--- ddtrace/contrib/internal/webbrowser/patch.py | 11 ++-- tests/internal/test_serverless.py | 63 ++++++++++++++------ 13 files changed, 98 insertions(+), 51 deletions(-) diff --git a/ddtrace/appsec/__init__.py b/ddtrace/appsec/__init__.py index a32442ccc0a..6b5758a95c2 100644 --- a/ddtrace/appsec/__init__.py +++ b/ddtrace/appsec/__init__.py @@ -1,4 +1,4 @@ -# this module must not load any other appsec module +# this module must not load any other unsafe appsec module directly from ddtrace.internal import core diff --git a/ddtrace/appsec/_constants.py b/ddtrace/appsec/_constants.py index 2172548205b..454483fcf17 100644 --- a/ddtrace/appsec/_constants.py +++ b/ddtrace/appsec/_constants.py @@ -1,3 +1,5 @@ +# this module must not load any other unsafe appsec module directly + import os from re import Match import sys diff --git a/ddtrace/appsec/_utils.py b/ddtrace/appsec/_utils.py index 79f8f8b5311..e4dbae7a27f 100644 --- a/ddtrace/appsec/_utils.py +++ b/ddtrace/appsec/_utils.py @@ -1,3 +1,5 @@ +# this module must not load any other unsafe appsec module directly + import logging import sys from typing import Any @@ -5,6 +7,7 @@ from ddtrace.appsec._constants import API_SECURITY from ddtrace.appsec._constants import APPSEC +from ddtrace.appsec._constants import SPAN_DATA_NAMES from ddtrace.internal._unpatched import unpatched_json_loads from ddtrace.internal.compat import to_unicode from ddtrace.internal.logger import get_logger @@ -21,7 +24,6 @@ def parse_response_body(raw_body): import xmltodict from ddtrace.appsec import _asm_request_context - from ddtrace.appsec._constants import SPAN_DATA_NAMES from ddtrace.contrib.internal.trace_utils import _get_header_value_case_insensitive if not raw_body: diff --git a/ddtrace/contrib/internal/httplib/patch.py b/ddtrace/contrib/internal/httplib/patch.py index 79a8ea2816f..7db5d59d31c 100644 --- a/ddtrace/contrib/internal/httplib/patch.py +++ b/ddtrace/contrib/internal/httplib/patch.py @@ -5,7 +5,6 @@ import wrapt from ddtrace import config -from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request_asm from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils @@ -77,12 +76,14 @@ def _wrap_getresponse(func, instance, args, kwargs): def _call_asm_wrap(func, instance, *args, **kwargs): + from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request_asm + _wrap_request_asm(func, instance, args, kwargs) def _wrap_request(func, instance, args, kwargs): # Use any attached tracer if available, otherwise use the global tracer - if asm_config._iast_enabled or asm_config._asm_enabled: + if asm_config._iast_enabled or (asm_config._asm_enabled and asm_config._ep_enabled): func_to_call = functools.partial(_call_asm_wrap, func, instance) else: func_to_call = func diff --git a/ddtrace/contrib/internal/mysql/patch.py b/ddtrace/contrib/internal/mysql/patch.py index 6425bd33766..0aad999e546 100644 --- a/ddtrace/contrib/internal/mysql/patch.py +++ b/ddtrace/contrib/internal/mysql/patch.py @@ -4,8 +4,6 @@ import wrapt from ddtrace import config -from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink -from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION from ddtrace.contrib.dbapi import TracedConnection from ddtrace.contrib.internal.trace_utils import _convert_to_string from ddtrace.ext import db @@ -51,6 +49,9 @@ def patch(): mysql.connector.Connect = mysql.connector.connect if asm_config._iast_enabled: + from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink + from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION + _set_metric_iast_instrumented_sink(VULN_SQL_INJECTION) mysql.connector._datadog_patch = True diff --git a/ddtrace/contrib/internal/mysqldb/patch.py b/ddtrace/contrib/internal/mysqldb/patch.py index cde0f58629f..fe9c62bbd5e 100644 --- a/ddtrace/contrib/internal/mysqldb/patch.py +++ b/ddtrace/contrib/internal/mysqldb/patch.py @@ -4,8 +4,6 @@ from wrapt import wrap_function_wrapper as _w from ddtrace import config -from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink -from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION from ddtrace.constants import _SPAN_MEASURED_KEY from ddtrace.constants import SPAN_KIND from ddtrace.contrib.dbapi import TracedConnection @@ -67,6 +65,9 @@ def patch(): _w("MySQLdb", "connect", _connect) if asm_config._iast_enabled: + from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink + from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION + _set_metric_iast_instrumented_sink(VULN_SQL_INJECTION) diff --git a/ddtrace/contrib/internal/requests/patch.py b/ddtrace/contrib/internal/requests/patch.py index eab51c2c0a4..47518c6341f 100644 --- a/ddtrace/contrib/internal/requests/patch.py +++ b/ddtrace/contrib/internal/requests/patch.py @@ -4,9 +4,6 @@ from wrapt import wrap_function_wrapper as _w from ddtrace import config -from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request -from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink -from ddtrace.appsec._iast.constants import VULN_SSRF from ddtrace.contrib.internal.trace_utils import unwrap as _u from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.utils.formats import asbool @@ -46,10 +43,16 @@ def patch(): _w("requests", "Session.send", _wrap_send) # IAST needs to wrap this function because `Session.send` is too late - _w("requests", "Session.request", _wrap_request) + if asm_config._load_modules: + from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request + + _w("requests", "Session.request", _wrap_request) Pin(_config=config.requests).onto(requests.Session) if asm_config._iast_enabled: + from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink + from ddtrace.appsec._iast.constants import VULN_SSRF + _set_metric_iast_instrumented_sink(VULN_SSRF) diff --git a/ddtrace/contrib/internal/sqlalchemy/patch.py b/ddtrace/contrib/internal/sqlalchemy/patch.py index 916cc53daa4..c6d6df476c1 100644 --- a/ddtrace/contrib/internal/sqlalchemy/patch.py +++ b/ddtrace/contrib/internal/sqlalchemy/patch.py @@ -1,8 +1,6 @@ import sqlalchemy from wrapt import wrap_function_wrapper as _w -from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink -from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION from ddtrace.contrib.internal.trace_utils import unwrap from ddtrace.settings.asm import config as asm_config @@ -24,6 +22,9 @@ def patch(): _w("sqlalchemy.engine", "create_engine", _wrap_create_engine) if asm_config._iast_enabled: + from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink + from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION + _set_metric_iast_instrumented_sink(VULN_SQL_INJECTION) diff --git a/ddtrace/contrib/internal/sqlite3/patch.py b/ddtrace/contrib/internal/sqlite3/patch.py index 03c79789661..68ee5779983 100644 --- a/ddtrace/contrib/internal/sqlite3/patch.py +++ b/ddtrace/contrib/internal/sqlite3/patch.py @@ -5,8 +5,6 @@ import wrapt from ddtrace import config -from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink -from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION from ddtrace.contrib.dbapi import FetchTracedCursor from ddtrace.contrib.dbapi import TracedConnection from ddtrace.contrib.dbapi import TracedCursor @@ -47,6 +45,9 @@ def patch(): sqlite3.dbapi2.connect = wrapped if asm_config._iast_enabled: + from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink + from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION + _set_metric_iast_instrumented_sink(VULN_SQL_INJECTION) diff --git a/ddtrace/contrib/internal/urllib/patch.py b/ddtrace/contrib/internal/urllib/patch.py index ed5e2891f06..1ba279fb20a 100644 --- a/ddtrace/contrib/internal/urllib/patch.py +++ b/ddtrace/contrib/internal/urllib/patch.py @@ -2,9 +2,6 @@ from wrapt import wrap_function_wrapper as _w -from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_open -from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink -from ddtrace.appsec._iast.constants import VULN_SSRF from ddtrace.contrib.internal.trace_utils import unwrap as _u from ddtrace.settings.asm import config as asm_config @@ -20,8 +17,15 @@ def patch(): return urllib.request.__datadog_patch = True - _w("urllib.request", "urlopen", _wrap_open) + if asm_config._load_modules: + from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_open + + _w("urllib.request", "urlopen", _wrap_open) + if asm_config._iast_enabled: + from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink + from ddtrace.appsec._iast.constants import VULN_SSRF + _set_metric_iast_instrumented_sink(VULN_SSRF) diff --git a/ddtrace/contrib/internal/urllib3/patch.py b/ddtrace/contrib/internal/urllib3/patch.py index 6c10526c125..7c5d6adc28d 100644 --- a/ddtrace/contrib/internal/urllib3/patch.py +++ b/ddtrace/contrib/internal/urllib3/patch.py @@ -4,9 +4,6 @@ from wrapt import wrap_function_wrapper as _w from ddtrace import config -from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request -from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink -from ddtrace.appsec._iast.constants import VULN_SSRF from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils @@ -54,14 +51,20 @@ def patch(): urllib3.__datadog_patch = True _w("urllib3", "connectionpool.HTTPConnectionPool.urlopen", _wrap_urlopen) - if hasattr(urllib3, "_request_methods"): - _w("urllib3._request_methods", "RequestMethods.request", _wrap_request) - else: - # Old version before https://github.com/urllib3/urllib3/pull/2398 - _w("urllib3.request", "RequestMethods.request", _wrap_request) + if asm_config._load_modules: + from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request + + if hasattr(urllib3, "_request_methods"): + _w("urllib3._request_methods", "RequestMethods.request", _wrap_request) + else: + # Old version before https://github.com/urllib3/urllib3/pull/2398 + _w("urllib3.request", "RequestMethods.request", _wrap_request) Pin().onto(urllib3.connectionpool.HTTPConnectionPool) if asm_config._iast_enabled: + from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink + from ddtrace.appsec._iast.constants import VULN_SSRF + _set_metric_iast_instrumented_sink(VULN_SSRF) diff --git a/ddtrace/contrib/internal/webbrowser/patch.py b/ddtrace/contrib/internal/webbrowser/patch.py index 1387df37ac9..1f90e9cf9aa 100644 --- a/ddtrace/contrib/internal/webbrowser/patch.py +++ b/ddtrace/contrib/internal/webbrowser/patch.py @@ -2,9 +2,6 @@ from wrapt import wrap_function_wrapper as _w -from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_open -from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink -from ddtrace.appsec._iast.constants import VULN_SSRF from ddtrace.contrib.internal.trace_utils import unwrap as _u from ddtrace.settings.asm import config as asm_config @@ -20,9 +17,15 @@ def patch(): return webbrowser.__datadog_patch = True - _w("webbrowser", "open", _wrap_open) + if asm_config._load_modules: + from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_open + + _w("webbrowser", "open", _wrap_open) if asm_config._iast_enabled: + from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink + from ddtrace.appsec._iast.constants import VULN_SSRF + _set_metric_iast_instrumented_sink(VULN_SSRF) diff --git a/tests/internal/test_serverless.py b/tests/internal/test_serverless.py index 48d7586d25e..2f0dad15087 100644 --- a/tests/internal/test_serverless.py +++ b/tests/internal/test_serverless.py @@ -1,4 +1,5 @@ import mock +import pytest from ddtrace.internal.serverless import in_azure_function from ddtrace.internal.serverless import in_gcp_function @@ -78,7 +79,41 @@ def test_not_azure_function_consumption_plan(): assert in_azure_function() is False -def test_slow_imports(): +standard_blocklist = [ + "ddtrace.appsec._api_security.api_manager", + "ddtrace.appsec._iast._ast.ast_patching", + "ddtrace.internal.telemetry.telemetry_writer", + "email.mime.application", + "email.mime.multipart", + "logging.handlers", + "multiprocessing", + "importlib_metadata", + + # These modules must not be imported because their source files are + # specifically removed from the serverless python layer. + # See https://github.com/DataDog/datadog-lambda-python/blob/main/Dockerfile + "ddtrace.appsec._iast._taint_tracking._native", + "ddtrace.appsec._iast._stacktrace", + "ddtrace.internal.datadog.profiling.libdd_wrapper", + "ddtrace.internal.datadog.profiling.ddup._ddup", + "ddtrace.internal.datadog.profiling.stack_v2._stack_v2", +] +expanded_blocklist = standard_blocklist + [ + "importlib.metadata", +] + +@pytest.mark.parametrize('package,blocklist', [ + ('ddtrace', expanded_blocklist), + ('ddtrace.contrib.internal.aws_lambda', expanded_blocklist), + ('ddtrace.contrib.internal.psycopg', expanded_blocklist), + # requests imports urlib3 which imports importlib.metadata + pytest.param('ddtrace.contrib.requests', standard_blocklist, + # Currently this package will import `ddtrace.appsec._iast._taint_tracking._native` + # and so is expected to fail for now. Once that is fixed and this test + # begins to XPASS, the xfail should be removed. + marks=pytest.mark.xfail(strict=True)), +]) +def test_slow_imports(package, blocklist): # We should lazy load certain modules to avoid slowing down the startup # time when running in a serverless environment. This test will fail if # any of those modules are imported during the import of ddtrace. @@ -93,18 +128,6 @@ def test_slow_imports(): } ) - blocklist = [ - "ddtrace.appsec._api_security.api_manager", - "ddtrace.appsec._iast._ast.ast_patching", - "ddtrace.internal.telemetry.telemetry_writer", - "email.mime.application", - "email.mime.multipart", - "logging.handlers", - "multiprocessing", - "importlib.metadata", - "importlib_metadata", - ] - class BlockListFinder: def find_spec(self, fullname, *args): for lib in blocklist: @@ -113,12 +136,14 @@ def find_spec(self, fullname, *args): return None try: - sys.meta_path.insert(0, BlockListFinder()) + orig_meta_path = sys.meta_path + sys.meta_path = [BlockListFinder()] + orig_meta_path + + for mod in sys.modules.copy(): + if mod in blocklist or mod.startswith("ddtrace"): + del sys.modules[mod] - import ddtrace - import ddtrace.contrib.internal.aws_lambda # noqa:F401 - import ddtrace.contrib.internal.psycopg # noqa:F401 + __import__(package) finally: - if isinstance(sys.meta_path[0], BlockListFinder): - sys.meta_path = sys.meta_path[1:] + sys.meta_path = orig_meta_path From 20dbe8d59658dce01ab3dc48c031adc192cd9aad Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 5 Feb 2025 15:41:03 +0100 Subject: [PATCH 16/23] fix lint --- tests/internal/test_serverless.py | 58 +++++++++++++++++-------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/tests/internal/test_serverless.py b/tests/internal/test_serverless.py index 2f0dad15087..03c41d6b742 100644 --- a/tests/internal/test_serverless.py +++ b/tests/internal/test_serverless.py @@ -80,39 +80,45 @@ def test_not_azure_function_consumption_plan(): standard_blocklist = [ - "ddtrace.appsec._api_security.api_manager", - "ddtrace.appsec._iast._ast.ast_patching", - "ddtrace.internal.telemetry.telemetry_writer", - "email.mime.application", - "email.mime.multipart", - "logging.handlers", - "multiprocessing", - "importlib_metadata", - - # These modules must not be imported because their source files are - # specifically removed from the serverless python layer. - # See https://github.com/DataDog/datadog-lambda-python/blob/main/Dockerfile - "ddtrace.appsec._iast._taint_tracking._native", - "ddtrace.appsec._iast._stacktrace", - "ddtrace.internal.datadog.profiling.libdd_wrapper", - "ddtrace.internal.datadog.profiling.ddup._ddup", - "ddtrace.internal.datadog.profiling.stack_v2._stack_v2", + "ddtrace.appsec._api_security.api_manager", + "ddtrace.appsec._iast._ast.ast_patching", + "ddtrace.internal.telemetry.telemetry_writer", + "email.mime.application", + "email.mime.multipart", + "logging.handlers", + "multiprocessing", + "importlib_metadata", + # These modules must not be imported because their source files are + # specifically removed from the serverless python layer. + # See https://github.com/DataDog/datadog-lambda-python/blob/main/Dockerfile + "ddtrace.appsec._iast._taint_tracking._native", + "ddtrace.appsec._iast._stacktrace", + "ddtrace.internal.datadog.profiling.libdd_wrapper", + "ddtrace.internal.datadog.profiling.ddup._ddup", + "ddtrace.internal.datadog.profiling.stack_v2._stack_v2", ] expanded_blocklist = standard_blocklist + [ - "importlib.metadata", + "importlib.metadata", ] -@pytest.mark.parametrize('package,blocklist', [ - ('ddtrace', expanded_blocklist), - ('ddtrace.contrib.internal.aws_lambda', expanded_blocklist), - ('ddtrace.contrib.internal.psycopg', expanded_blocklist), - # requests imports urlib3 which imports importlib.metadata - pytest.param('ddtrace.contrib.requests', standard_blocklist, + +@pytest.mark.parametrize( + "package,blocklist", + [ + ("ddtrace", expanded_blocklist), + ("ddtrace.contrib.internal.aws_lambda", expanded_blocklist), + ("ddtrace.contrib.internal.psycopg", expanded_blocklist), + # requests imports urlib3 which imports importlib.metadata + pytest.param( + "ddtrace.contrib.requests", + standard_blocklist, # Currently this package will import `ddtrace.appsec._iast._taint_tracking._native` # and so is expected to fail for now. Once that is fixed and this test # begins to XPASS, the xfail should be removed. - marks=pytest.mark.xfail(strict=True)), -]) + marks=pytest.mark.xfail(strict=True), + ), + ], +) def test_slow_imports(package, blocklist): # We should lazy load certain modules to avoid slowing down the startup # time when running in a serverless environment. This test will fail if From fed3053015d11f5e33f5b29a99a1229d3eabe02f Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 5 Feb 2025 17:15:15 +0100 Subject: [PATCH 17/23] unpatch less strict --- ddtrace/contrib/internal/requests/patch.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ddtrace/contrib/internal/requests/patch.py b/ddtrace/contrib/internal/requests/patch.py index 47518c6341f..a885e5575de 100644 --- a/ddtrace/contrib/internal/requests/patch.py +++ b/ddtrace/contrib/internal/requests/patch.py @@ -62,5 +62,13 @@ def unpatch(): return requests.__datadog_patch = False - _u(requests.Session, "request") - _u(requests.Session, "send") + try: + _u(requests.Session, "request") + except AttributeError: + # It was not patched + pass + try: + _u(requests.Session, "send") + except AttributeError: + # It was not patched + pass From 0da9beacff7563ba363ab7063148c6716262931f Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 6 Feb 2025 10:29:52 +0100 Subject: [PATCH 18/23] revert test_serverless --- tests/internal/test_serverless.py | 69 +++++++++---------------------- 1 file changed, 19 insertions(+), 50 deletions(-) diff --git a/tests/internal/test_serverless.py b/tests/internal/test_serverless.py index 03c41d6b742..48d7586d25e 100644 --- a/tests/internal/test_serverless.py +++ b/tests/internal/test_serverless.py @@ -1,5 +1,4 @@ import mock -import pytest from ddtrace.internal.serverless import in_azure_function from ddtrace.internal.serverless import in_gcp_function @@ -79,47 +78,7 @@ def test_not_azure_function_consumption_plan(): assert in_azure_function() is False -standard_blocklist = [ - "ddtrace.appsec._api_security.api_manager", - "ddtrace.appsec._iast._ast.ast_patching", - "ddtrace.internal.telemetry.telemetry_writer", - "email.mime.application", - "email.mime.multipart", - "logging.handlers", - "multiprocessing", - "importlib_metadata", - # These modules must not be imported because their source files are - # specifically removed from the serverless python layer. - # See https://github.com/DataDog/datadog-lambda-python/blob/main/Dockerfile - "ddtrace.appsec._iast._taint_tracking._native", - "ddtrace.appsec._iast._stacktrace", - "ddtrace.internal.datadog.profiling.libdd_wrapper", - "ddtrace.internal.datadog.profiling.ddup._ddup", - "ddtrace.internal.datadog.profiling.stack_v2._stack_v2", -] -expanded_blocklist = standard_blocklist + [ - "importlib.metadata", -] - - -@pytest.mark.parametrize( - "package,blocklist", - [ - ("ddtrace", expanded_blocklist), - ("ddtrace.contrib.internal.aws_lambda", expanded_blocklist), - ("ddtrace.contrib.internal.psycopg", expanded_blocklist), - # requests imports urlib3 which imports importlib.metadata - pytest.param( - "ddtrace.contrib.requests", - standard_blocklist, - # Currently this package will import `ddtrace.appsec._iast._taint_tracking._native` - # and so is expected to fail for now. Once that is fixed and this test - # begins to XPASS, the xfail should be removed. - marks=pytest.mark.xfail(strict=True), - ), - ], -) -def test_slow_imports(package, blocklist): +def test_slow_imports(): # We should lazy load certain modules to avoid slowing down the startup # time when running in a serverless environment. This test will fail if # any of those modules are imported during the import of ddtrace. @@ -134,6 +93,18 @@ def test_slow_imports(package, blocklist): } ) + blocklist = [ + "ddtrace.appsec._api_security.api_manager", + "ddtrace.appsec._iast._ast.ast_patching", + "ddtrace.internal.telemetry.telemetry_writer", + "email.mime.application", + "email.mime.multipart", + "logging.handlers", + "multiprocessing", + "importlib.metadata", + "importlib_metadata", + ] + class BlockListFinder: def find_spec(self, fullname, *args): for lib in blocklist: @@ -142,14 +113,12 @@ def find_spec(self, fullname, *args): return None try: - orig_meta_path = sys.meta_path - sys.meta_path = [BlockListFinder()] + orig_meta_path - - for mod in sys.modules.copy(): - if mod in blocklist or mod.startswith("ddtrace"): - del sys.modules[mod] + sys.meta_path.insert(0, BlockListFinder()) - __import__(package) + import ddtrace + import ddtrace.contrib.internal.aws_lambda # noqa:F401 + import ddtrace.contrib.internal.psycopg # noqa:F401 finally: - sys.meta_path = orig_meta_path + if isinstance(sys.meta_path[0], BlockListFinder): + sys.meta_path = sys.meta_path[1:] From e2d8767b6b41d1d031116cf2a8eb9e87ccb1ec20 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 6 Feb 2025 10:49:29 +0100 Subject: [PATCH 19/23] fix subprocess test --- tests/contrib/subprocess/test_subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/contrib/subprocess/test_subprocess.py b/tests/contrib/subprocess/test_subprocess.py index 6cf28c2fef9..d4661d9ccfb 100644 --- a/tests/contrib/subprocess/test_subprocess.py +++ b/tests/contrib/subprocess/test_subprocess.py @@ -194,7 +194,7 @@ def test_truncation(cmdline_obj, expected_str, expected_list, truncated): def test_ossystem(tracer): with override_global_config(dict(_asm_enabled=True)): patch() - Pin.get_from(os).clone(tracer=tracer).onto(os) + Pin.get_from(os)._clone(tracer=tracer).onto(os) with tracer.trace("ossystem_test"): ret = os.system("dir -l /") assert ret == 0 From cfa82c94a737d6a6fc9a356f26b8193db8d53530 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 6 Feb 2025 11:02:15 +0100 Subject: [PATCH 20/23] fix subprocess test --- tests/contrib/subprocess/test_subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/contrib/subprocess/test_subprocess.py b/tests/contrib/subprocess/test_subprocess.py index ea54842e6ab..ff6b30bb0dd 100644 --- a/tests/contrib/subprocess/test_subprocess.py +++ b/tests/contrib/subprocess/test_subprocess.py @@ -254,7 +254,7 @@ def test_unpatch(tracer): unpatch() with override_global_config(dict(_ep_enabled=False)): - Pin.get_from(os).clone(tracer=tracer).onto(os) + Pin.get_from(os)._clone(tracer=tracer).onto(os) with tracer.trace("os.system_unpatch"): ret = os.system("dir -l /") assert ret == 0 From 2d9d36c3ad2370d35b082aff63e977eeaf72f04c Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 6 Feb 2025 13:26:43 +0100 Subject: [PATCH 21/23] add minimal test for loading module --- tests/appsec/integrations/flask_tests/mini.py | 36 +++++++++ .../test_appsec_loading_modules.py | 80 +++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 tests/appsec/integrations/flask_tests/mini.py create mode 100644 tests/appsec/integrations/flask_tests/test_appsec_loading_modules.py diff --git a/tests/appsec/integrations/flask_tests/mini.py b/tests/appsec/integrations/flask_tests/mini.py new file mode 100644 index 00000000000..5254d2ad5bd --- /dev/null +++ b/tests/appsec/integrations/flask_tests/mini.py @@ -0,0 +1,36 @@ +import ddtrace.auto # noqa: F401 + + +"""do not move this import""" + +import os # noqa: E402 +import sys # noqa: E402 + +from flask import Flask # noqa: E402 +import requests # noqa: E402 F401 + +from ddtrace.settings.asm import config as asm_config # noqa: E402 +from ddtrace.version import get_version # noqa: E402 + + +app = Flask(__name__) + + +@app.route("/") +def hello_world(): + res = [] + for m in sys.modules: + if m.startswith("ddtrace.appsec"): + res.append(m) + return { + "appsec": list(sorted(res)), + "asm_config": { + k: getattr(asm_config, k) for k in dir(asm_config) if isinstance(getattr(asm_config, k), (int, bool, float)) + }, + "aws": "AWS_LAMBDA_FUNCTION_NAME" in os.environ, + "version": get_version(), + } + + +if __name__ == "__main__": + app.run(debug=True, port=8475) diff --git a/tests/appsec/integrations/flask_tests/test_appsec_loading_modules.py b/tests/appsec/integrations/flask_tests/test_appsec_loading_modules.py new file mode 100644 index 00000000000..a83241109cb --- /dev/null +++ b/tests/appsec/integrations/flask_tests/test_appsec_loading_modules.py @@ -0,0 +1,80 @@ +import json +import os +import pathlib +import subprocess +import time +from urllib.error import HTTPError +from urllib.error import URLError +from urllib.request import urlopen + +import pytest + + +MODULES_ALWAYS_LOADED = ["ddtrace.appsec", "ddtrace.appsec._capabilities", "ddtrace.appsec._constants"] +MODULE_ASM_ONLY = ["ddtrace.appsec._processor", "ddtrace.appsec._ddwaf"] +MODULE_IAST_ONLY = [ + "ddtrace.appsec._iast", + "ddtrace.appsec._iast._taint_tracking._native", + "ddtrace.appsec._iast._stacktrace", +] + + +@pytest.mark.parametrize("appsec_enabled", ["true", "false"]) +@pytest.mark.parametrize("iast_enabled", ["true", None]) +@pytest.mark.parametrize("aws_lambda", ["any", None]) +def test_loading(appsec_enabled, iast_enabled, aws_lambda): + flask_app = pathlib.Path(__file__).parent / "mini.py" + env = {} + if appsec_enabled: + env["DD_APPSEC_ENABLED"] = appsec_enabled + if iast_enabled: + env["DD_IAST_ENABLED"] = iast_enabled + if aws_lambda: + env["AWS_LAMBDA_FUNCTION_NAME"] = aws_lambda + + all_env = os.environ | env + + process = subprocess.Popen( + ["python", str(flask_app)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=all_env, + ) + for i in range(16): + time.sleep(1) + try: + with urlopen("http://localhost:8475") as response: + assert response.status == 200 + payload = response.read().decode() + data = json.loads(payload) + assert "appsec" in data + # appsec is always enabled + for m in MODULES_ALWAYS_LOADED: + assert m in data["appsec"], f"{m} not in {data['appsec']}" + for m in MODULE_ASM_ONLY: + if appsec_enabled == "true" and not aws_lambda: + assert m in data["appsec"], f"{m} not in {data['appsec']}" + else: + assert m not in data["appsec"], f"{m} in {data['appsec']}" + for m in MODULE_IAST_ONLY: + if iast_enabled and not aws_lambda: + assert m in data["appsec"], f"{m} not in {data['appsec']}" + else: + assert m not in data["appsec"], f"{m} in {data['appsec']}" + process.terminate() + process.wait() + break + except HTTPError as e: + process.terminate() + process.wait() + raise AssertionError(e.read().decode()) + except URLError: + continue + except AssertionError: + process.terminate() + process.wait() + raise + else: + process.terminate() + process.wait() + raise AssertionError("Server did not start") From 6cad0b78024daa9f43468d941ad6e495919a4867 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 6 Feb 2025 13:30:53 +0100 Subject: [PATCH 22/23] add release note --- .../notes/ensure_no_appsec_loading-8ce46c58d6ecf81f.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 releasenotes/notes/ensure_no_appsec_loading-8ce46c58d6ecf81f.yaml diff --git a/releasenotes/notes/ensure_no_appsec_loading-8ce46c58d6ecf81f.yaml b/releasenotes/notes/ensure_no_appsec_loading-8ce46c58d6ecf81f.yaml new file mode 100644 index 00000000000..b7c34f83779 --- /dev/null +++ b/releasenotes/notes/ensure_no_appsec_loading-8ce46c58d6ecf81f.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + ASM: This ensures that no module from ASM are loaded when ASM is disabled or unavailable. + SCA: This ensures that no module from IAST are loaded when IAST is disabled or unavailable. \ No newline at end of file From 72b3bbc973b5f1454944ec5d700a5d098ded08a7 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 6 Feb 2025 13:49:13 +0100 Subject: [PATCH 23/23] make sure env vars are updated as expected for load_module test --- .../flask_tests/test_appsec_loading_modules.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/appsec/integrations/flask_tests/test_appsec_loading_modules.py b/tests/appsec/integrations/flask_tests/test_appsec_loading_modules.py index a83241109cb..e989ee5d612 100644 --- a/tests/appsec/integrations/flask_tests/test_appsec_loading_modules.py +++ b/tests/appsec/integrations/flask_tests/test_appsec_loading_modules.py @@ -24,21 +24,25 @@ @pytest.mark.parametrize("aws_lambda", ["any", None]) def test_loading(appsec_enabled, iast_enabled, aws_lambda): flask_app = pathlib.Path(__file__).parent / "mini.py" - env = {} + env = os.environ.copy() if appsec_enabled: env["DD_APPSEC_ENABLED"] = appsec_enabled + else: + env.pop("DD_APPSEC_ENABLED", None) if iast_enabled: env["DD_IAST_ENABLED"] = iast_enabled + else: + env.pop("DD_IAST_ENABLED", None) if aws_lambda: env["AWS_LAMBDA_FUNCTION_NAME"] = aws_lambda - - all_env = os.environ | env + else: + env.pop("AWS_LAMBDA_FUNCTION_NAME", None) process = subprocess.Popen( ["python", str(flask_app)], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - env=all_env, + env=env, ) for i in range(16): time.sleep(1)