Skip to content

Commit 1e7af66

Browse files
christophe-papazianavara1986
authored andcommitted
fix(asm): decouple appsec (#12212)
Second part of #12198 - Ensure we don't load appsec modules if appsec is disabled or unavailable (except for a few safe modules). - Ensure we don't load iast modules if iast is disabled or unavailable. - Ensure all initialisation logic for enable flags are handled in ddtrace.settings.asm (small factorisation) - Add test on module loading with a mini flask application in `tests/appsec/integrations/flask_tests/test_appsec_loading_modules.py`, checking with all possible combinations of appsec/iast/aws_lambda enabled or disabled. - remove dead code in `ddtrace/contrib/internal/langchain/patch.py` APPSEC-56626 (should be backported to 3.0 when possible) - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) (cherry picked from commit fa18def)
1 parent 6731d2e commit 1e7af66

File tree

28 files changed

+275
-83
lines changed

28 files changed

+275
-83
lines changed

ddtrace/appsec/__init__.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
# this module must not load any other unsafe appsec module directly
2+
13
from ddtrace.internal import core
2-
from ddtrace.settings.asm import config as asm_config
34

45

56
_APPSEC_TO_BE_LOADED = True
@@ -28,7 +29,9 @@ def load_iast():
2829

2930
def load_common_appsec_modules():
3031
"""Lazily load the common module patches."""
31-
if (asm_config._ep_enabled and asm_config._asm_enabled) or asm_config._iast_enabled:
32+
from ddtrace.settings.asm import config as asm_config
33+
34+
if asm_config._load_modules:
3235
from ddtrace.appsec._common_module_patches import patch_common_modules
3336

3437
patch_common_modules()

ddtrace/appsec/_asm_request_context.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
from ddtrace.appsec._constants import APPSEC
1616
from ddtrace.appsec._constants import EXPLOIT_PREVENTION
1717
from ddtrace.appsec._constants import SPAN_DATA_NAMES
18-
from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled
19-
from ddtrace.appsec._iast._taint_tracking import OriginType
20-
from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject
2118
from ddtrace.appsec._utils import add_context_log
2219
from ddtrace.appsec._utils import get_triggers
2320
from ddtrace.internal import core
@@ -28,6 +25,16 @@
2825
from ddtrace.trace import Span
2926

3027

28+
if asm_config._iast_enabled:
29+
from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled
30+
from ddtrace.appsec._iast._taint_tracking import OriginType
31+
from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject
32+
else:
33+
34+
def is_iast_request_enabled() -> bool:
35+
return False
36+
37+
3138
if TYPE_CHECKING:
3239
from ddtrace.appsec._ddwaf import DDWaf_info
3340
from ddtrace.appsec._ddwaf import DDWaf_result

ddtrace/appsec/_common_module_patches.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ def is_iast_request_enabled() -> bool:
4444

4545
def patch_common_modules():
4646
global _is_patched
47+
# ensure that the subprocess patch is applied even after one click activation
48+
subprocess_patch.patch()
49+
subprocess_patch.add_str_callback(_RASP_SYSTEM, wrapped_system_5542593D237084A7)
50+
subprocess_patch.add_lst_callback(_RASP_POPEN, popen_FD233052260D8B4D)
4751
if _is_patched:
4852
return
4953
# for testing purposes, we need to update is_iast_request_enabled
@@ -60,10 +64,6 @@ def is_iast_request_enabled() -> bool:
6064
try_wrap_function_wrapper("urllib.request", "OpenerDirector.open", wrapped_open_ED4CF71136E15EBF)
6165
try_wrap_function_wrapper("_io", "BytesIO.read", wrapped_read_F3E51D71B4EC16EF)
6266
try_wrap_function_wrapper("_io", "StringIO.read", wrapped_read_F3E51D71B4EC16EF)
63-
# ensure that the subprocess patch is applied even after one click activation
64-
subprocess_patch.patch()
65-
subprocess_patch.add_str_callback(_RASP_SYSTEM, wrapped_system_5542593D237084A7)
66-
subprocess_patch.add_lst_callback(_RASP_POPEN, popen_FD233052260D8B4D)
6767
core.on("asm.block.dbapi.execute", execute_4C9BAC8E228EB347)
6868
if asm_config._iast_enabled:
6969
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink

ddtrace/appsec/_constants.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# this module must not load any other unsafe appsec module directly
2+
13
import os
24
from re import Match
35
import sys

ddtrace/appsec/_utils.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1+
# this module must not load any other unsafe appsec module directly
2+
13
import logging
24
import sys
35
from typing import Any
46
import uuid
57

68
from ddtrace.appsec._constants import API_SECURITY
79
from ddtrace.appsec._constants import APPSEC
10+
from ddtrace.appsec._constants import SPAN_DATA_NAMES
811
from ddtrace.internal._unpatched import unpatched_json_loads
912
from ddtrace.internal.compat import to_unicode
1013
from ddtrace.internal.logger import get_logger
@@ -21,7 +24,6 @@ def parse_response_body(raw_body):
2124
import xmltodict
2225

2326
from ddtrace.appsec import _asm_request_context
24-
from ddtrace.appsec._constants import SPAN_DATA_NAMES
2527
from ddtrace.contrib.internal.trace_utils import _get_header_value_case_insensitive
2628

2729
if not raw_body:

ddtrace/contrib/internal/httplib/patch.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import wrapt
66

77
from ddtrace import config
8-
from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request_asm
98
from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY
109
from ddtrace.constants import SPAN_KIND
1110
from ddtrace.contrib import trace_utils
@@ -77,12 +76,14 @@ def _wrap_getresponse(func, instance, args, kwargs):
7776

7877

7978
def _call_asm_wrap(func, instance, *args, **kwargs):
79+
from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request_asm
80+
8081
_wrap_request_asm(func, instance, args, kwargs)
8182

8283

8384
def _wrap_request(func, instance, args, kwargs):
8485
# Use any attached tracer if available, otherwise use the global tracer
85-
if asm_config._iast_enabled or asm_config._asm_enabled:
86+
if asm_config._iast_enabled or (asm_config._asm_enabled and asm_config._ep_enabled):
8687
func_to_call = functools.partial(_call_asm_wrap, func, instance)
8788
else:
8889
func_to_call = func

ddtrace/contrib/internal/mysql/patch.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
import wrapt
55

66
from ddtrace import config
7-
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
8-
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
97
from ddtrace.contrib.dbapi import TracedConnection
108
from ddtrace.contrib.internal.trace_utils import _convert_to_string
119
from ddtrace.ext import db
@@ -51,6 +49,9 @@ def patch():
5149
mysql.connector.Connect = mysql.connector.connect
5250

5351
if asm_config._iast_enabled:
52+
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
53+
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
54+
5455
_set_metric_iast_instrumented_sink(VULN_SQL_INJECTION)
5556
mysql.connector._datadog_patch = True
5657

ddtrace/contrib/internal/mysqldb/patch.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
from wrapt import wrap_function_wrapper as _w
55

66
from ddtrace import config
7-
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
8-
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
97
from ddtrace.constants import _SPAN_MEASURED_KEY
108
from ddtrace.constants import SPAN_KIND
119
from ddtrace.contrib.dbapi import TracedConnection
@@ -67,6 +65,9 @@ def patch():
6765
_w("MySQLdb", "connect", _connect)
6866

6967
if asm_config._iast_enabled:
68+
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
69+
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
70+
7071
_set_metric_iast_instrumented_sink(VULN_SQL_INJECTION)
7172

7273

ddtrace/contrib/internal/pytest/_plugin_v2.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
from ddtrace.internal.test_visibility.api import InternalTestSession
6060
from ddtrace.internal.test_visibility.api import InternalTestSuite
6161
from ddtrace.internal.test_visibility.coverage_lines import CoverageLines
62+
from ddtrace.settings.asm import config as asm_config
6263
from ddtrace.vendor.debtcollector import deprecate
6364

6465

@@ -574,9 +575,10 @@ def _pytest_terminal_summary_post_yield(terminalreporter, failed_reports_initial
574575
def pytest_terminal_summary(terminalreporter, exitstatus, config):
575576
"""Report flaky or failed tests"""
576577
try:
577-
from ddtrace.appsec._iast._pytest_plugin import print_iast_report
578+
if asm_config._iast_enabled:
579+
from ddtrace.appsec._iast._pytest_plugin import print_iast_report
578580

579-
print_iast_report(terminalreporter)
581+
print_iast_report(terminalreporter)
580582
except Exception: # noqa: E722
581583
log.debug("Encountered error during code security summary", exc_info=True)
582584

ddtrace/contrib/internal/pytest/plugin.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@
1717
import pytest
1818

1919
from ddtrace import config
20-
from ddtrace.appsec._iast._pytest_plugin import ddtrace_iast # noqa:F401
2120
from ddtrace.contrib.internal.pytest._utils import _USE_PLUGIN_V2
2221
from ddtrace.contrib.internal.pytest._utils import _extract_span
2322
from ddtrace.contrib.internal.pytest._utils import _pytest_version_supports_itr
2423
from ddtrace.settings.asm import config as asm_config
2524

2625

26+
if asm_config._iast_enabled:
27+
from ddtrace.appsec._iast._pytest_plugin import ddtrace_iast # noqa:F401
28+
29+
2730
# pytest default settings
2831
config._add(
2932
"pytest",

0 commit comments

Comments
 (0)