Skip to content

Commit fa18def

Browse files
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) ## Checklist - [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)) ## Reviewer Checklist - [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)
1 parent 1247ac2 commit fa18def

File tree

29 files changed

+268
-105
lines changed

29 files changed

+268
-105
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/langchain/patch.py

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,29 +1090,6 @@ def unpatch():
10901090
delattr(langchain, "_datadog_integration")
10911091

10921092

1093-
def taint_outputs(instance, inputs, outputs):
1094-
from ddtrace.appsec._iast._metrics import _set_iast_error_metric
1095-
from ddtrace.appsec._iast._taint_tracking._taint_objects import get_tainted_ranges
1096-
from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject
1097-
1098-
try:
1099-
ranges = None
1100-
for key in filter(lambda x: x in inputs, instance.input_keys):
1101-
input_val = inputs.get(key)
1102-
if input_val:
1103-
ranges = get_tainted_ranges(input_val)
1104-
if ranges:
1105-
break
1106-
1107-
if ranges:
1108-
source = ranges[0].source
1109-
for key in filter(lambda x: x in outputs, instance.output_keys):
1110-
output_value = outputs[key]
1111-
outputs[key] = taint_pyobject(output_value, source.name, source.value, source.origin)
1112-
except Exception as e:
1113-
_set_iast_error_metric("IAST propagation error. langchain taint_outputs. {}".format(e))
1114-
1115-
11161093
def taint_parser_output(func, instance, args, kwargs):
11171094
from ddtrace.appsec._iast._metrics import _set_iast_error_metric
11181095
from ddtrace.appsec._iast._taint_tracking._taint_objects import get_tainted_ranges

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
@@ -60,6 +60,7 @@
6060
from ddtrace.internal.test_visibility.api import InternalTestSession
6161
from ddtrace.internal.test_visibility.api import InternalTestSuite
6262
from ddtrace.internal.test_visibility.coverage_lines import CoverageLines
63+
from ddtrace.settings.asm import config as asm_config
6364
from ddtrace.vendor.debtcollector import deprecate
6465

6566

@@ -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",

ddtrace/contrib/internal/requests/patch.py

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

66
from ddtrace import config
7-
from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request
8-
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
9-
from ddtrace.appsec._iast.constants import VULN_SSRF
107
from ddtrace.contrib.internal.trace_utils import unwrap as _u
118
from ddtrace.internal.schema import schematize_service_name
129
from ddtrace.internal.utils.formats import asbool
@@ -46,10 +43,16 @@ def patch():
4643

4744
_w("requests", "Session.send", _wrap_send)
4845
# IAST needs to wrap this function because `Session.send` is too late
49-
_w("requests", "Session.request", _wrap_request)
46+
if asm_config._load_modules:
47+
from ddtrace.appsec._common_module_patches import wrapped_request_D8CB81E472AF98A2 as _wrap_request
48+
49+
_w("requests", "Session.request", _wrap_request)
5050
Pin(_config=config.requests).onto(requests.Session)
5151

5252
if asm_config._iast_enabled:
53+
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
54+
from ddtrace.appsec._iast.constants import VULN_SSRF
55+
5356
_set_metric_iast_instrumented_sink(VULN_SSRF)
5457

5558

@@ -59,5 +62,13 @@ def unpatch():
5962
return
6063
requests.__datadog_patch = False
6164

62-
_u(requests.Session, "request")
63-
_u(requests.Session, "send")
65+
try:
66+
_u(requests.Session, "request")
67+
except AttributeError:
68+
# It was not patched
69+
pass
70+
try:
71+
_u(requests.Session, "send")
72+
except AttributeError:
73+
# It was not patched
74+
pass

ddtrace/contrib/internal/sqlalchemy/patch.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import sqlalchemy
22
from wrapt import wrap_function_wrapper as _w
33

4-
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
5-
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
64
from ddtrace.contrib.internal.trace_utils import unwrap
75
from ddtrace.settings.asm import config as asm_config
86

@@ -24,6 +22,9 @@ def patch():
2422
_w("sqlalchemy.engine", "create_engine", _wrap_create_engine)
2523

2624
if asm_config._iast_enabled:
25+
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
26+
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
27+
2728
_set_metric_iast_instrumented_sink(VULN_SQL_INJECTION)
2829

2930

ddtrace/contrib/internal/sqlite3/patch.py

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

77
from ddtrace import config
8-
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
9-
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
108
from ddtrace.contrib.dbapi import FetchTracedCursor
119
from ddtrace.contrib.dbapi import TracedConnection
1210
from ddtrace.contrib.dbapi import TracedCursor
@@ -47,6 +45,9 @@ def patch():
4745
sqlite3.dbapi2.connect = wrapped
4846

4947
if asm_config._iast_enabled:
48+
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
49+
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
50+
5051
_set_metric_iast_instrumented_sink(VULN_SQL_INJECTION)
5152

5253

ddtrace/contrib/internal/subprocess/patch.py

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ def del_lst_callback(name: str):
5858

5959

6060
def patch() -> List[str]:
61-
if not (asm_config._asm_enabled or asm_config._iast_enabled):
61+
if not asm_config._load_modules:
6262
return []
6363
patched: List[str] = []
6464

6565
import os # nosec
6666
import subprocess # nosec
6767

6868
should_patch_system = not trace_utils.iswrapped(os.system)
69-
should_patch_fork = not trace_utils.iswrapped(os.fork)
69+
should_patch_fork = (not trace_utils.iswrapped(os.fork)) if hasattr(os, "fork") else False
7070
spawnvef = getattr(os, "_spawnvef", None)
7171
should_patch_spawnvef = spawnvef is not None and not trace_utils.iswrapped(spawnvef)
7272

@@ -316,18 +316,19 @@ def unpatch() -> None:
316316
import os # nosec
317317
import subprocess # nosec
318318

319-
trace_utils.unwrap(os, "system")
320-
trace_utils.unwrap(os, "_spawnvef")
321-
trace_utils.unwrap(subprocess.Popen, "__init__")
322-
trace_utils.unwrap(subprocess.Popen, "wait")
319+
for obj, attr in [(os, "system"), (os, "_spawnvef"), (subprocess.Popen, "__init__"), (subprocess.Popen, "wait")]:
320+
try:
321+
trace_utils.unwrap(obj, attr)
322+
except AttributeError:
323+
pass
323324

324325
SubprocessCmdLine._clear_cache()
325326

326327

327328
@trace_utils.with_traced_module
328329
def _traced_ossystem(module, pin, wrapped, instance, args, kwargs):
329330
try:
330-
if asm_config._bypass_instrumentation_for_waf:
331+
if asm_config._bypass_instrumentation_for_waf or not (asm_config._asm_enabled or asm_config._iast_enabled):
331332
return wrapped(*args, **kwargs)
332333
if isinstance(args[0], str):
333334
for callback in _STR_CALLBACKS.values():
@@ -351,6 +352,8 @@ def _traced_ossystem(module, pin, wrapped, instance, args, kwargs):
351352

352353
@trace_utils.with_traced_module
353354
def _traced_fork(module, pin, wrapped, instance, args, kwargs):
355+
if not (asm_config._asm_enabled or asm_config._iast_enabled):
356+
return wrapped(*args, **kwargs)
354357
try:
355358
with pin.tracer.trace(COMMANDS.SPAN_NAME, resource="fork", span_type=SpanTypes.SYSTEM) as span:
356359
span.set_tag(COMMANDS.EXEC, ["os.fork"])
@@ -366,6 +369,8 @@ def _traced_fork(module, pin, wrapped, instance, args, kwargs):
366369

367370
@trace_utils.with_traced_module
368371
def _traced_osspawn(module, pin, wrapped, instance, args, kwargs):
372+
if not (asm_config._asm_enabled or asm_config._iast_enabled):
373+
return wrapped(*args, **kwargs)
369374
try:
370375
mode, file, func_args, _, _ = args
371376
if isinstance(func_args, (list, tuple, str)):
@@ -395,7 +400,7 @@ def _traced_osspawn(module, pin, wrapped, instance, args, kwargs):
395400
@trace_utils.with_traced_module
396401
def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs):
397402
try:
398-
if asm_config._bypass_instrumentation_for_waf:
403+
if asm_config._bypass_instrumentation_for_waf or not (asm_config._asm_enabled or asm_config._iast_enabled):
399404
return wrapped(*args, **kwargs)
400405
cmd_args = args[0] if len(args) else kwargs["args"]
401406
if isinstance(cmd_args, (list, tuple, str)):
@@ -429,7 +434,7 @@ def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs):
429434
@trace_utils.with_traced_module
430435
def _traced_subprocess_wait(module, pin, wrapped, instance, args, kwargs):
431436
try:
432-
if asm_config._bypass_instrumentation_for_waf:
437+
if asm_config._bypass_instrumentation_for_waf or not (asm_config._asm_enabled or asm_config._iast_enabled):
433438
return wrapped(*args, **kwargs)
434439
binary = core.get_item("subprocess_popen_binary")
435440

0 commit comments

Comments
 (0)