chore(asm): move asm waf triggers from meta to meta_struct (DataDog#8474

)

After DataDog#8441
This PR migrates WAF result data from meta to meta_struct, ensuring more
efficient serialization and removing the need for python
serialisation/deserialisation.

This new feature is tested in unit tests for suspicious request
blocking.

**This feature is not activated by default for now, it's hidden behind a
private asm config flag**

DataDog/system-tests#2171 allow system tests to
handle this new transport mechanism.

## Checklist

- [x] Change(s) are motivated and described in the PR description
- [x] Testing strategy is described if automated tests are not included
in the PR
- [x] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/))
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.
- [x] If change touches code that signs or publishes builds or packages,
or handles credentials of any kind, I've requested a review from
`@DataDog/security-design-and-guidance`.

## Reviewer Checklist

- [ ] Title is accurate
- [ ] All changes are related to the pull request's stated goal
- [ ] Description motivates each change
- [ ] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [ ] Testing strategy adequately addresses listed risks
- [ ] Change is maintainable (easy to change, telemetry, documentation)
- [ ] Release note makes sense to a user of the library
- [ ] 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)

ddtrace/appsec/_asm_request_context.py

@@ -17,6 +17,7 @@
 from ddtrace.appsec._constants import SPAN_DATA_NAMES
 from ddtrace.appsec._constants import WAF_CONTEXT_NAMES
 from ddtrace.appsec._iast._utils import _is_iast_enabled
+from ddtrace.appsec._utils import get_triggers
 from ddtrace.internal import core
 from ddtrace.internal.constants import REQUEST_PATH_PARAMS
 from ddtrace.internal.logger import get_logger
@@ -109,19 +110,15 @@ def unregister(span: Span) -> None:
 def flush_waf_triggers(env: ASM_Environment) -> None:
     if env.waf_triggers and env.span:
         root_span = env.span._local_root or env.span
-        old_tags = root_span.get_tag(APPSEC.JSON)
-        if old_tags is not None:
-            try:
-                new_json = json.loads(old_tags)
-                if "triggers" not in new_json:
-                    new_json["triggers"] = []
-                new_json["triggers"].extend(env.waf_triggers)
-            except BaseException:
-                new_json = {"triggers": env.waf_triggers}
+        report_list = get_triggers(root_span)
+        if report_list is not None:
+            report_list.extend(env.waf_triggers)
         else:
-            new_json = {"triggers": env.waf_triggers}
-        root_span.set_tag_str(APPSEC.JSON, json.dumps(new_json, separators=(",", ":")))
-
+            report_list = env.waf_triggers
+        if asm_config._use_metastruct_for_triggers:
+            root_span.set_struct_tag(APPSEC.STRUCT, {"triggers": report_list})
+        else:
+            root_span.set_tag(APPSEC.JSON, json.dumps({"triggers": report_list}, separators=(",", ":")))
         env.waf_triggers = []
 
 

ddtrace/appsec/_constants.py

@@ -44,6 +44,7 @@ class APPSEC(metaclass=Constant_Class):
     ENV = "DD_APPSEC_ENABLED"
     ENABLED = "_dd.appsec.enabled"
     JSON = "_dd.appsec.json"
+    STRUCT = "appsec"
     EVENT_RULE_VERSION = "_dd.appsec.event_rules.version"
     EVENT_RULE_ERRORS = "_dd.appsec.event_rules.errors"
     EVENT_RULE_LOADED = "_dd.appsec.event_rules.loaded"

ddtrace/appsec/_processor.py

@@ -29,6 +29,7 @@
 from ddtrace.appsec._metrics import _set_waf_request_metrics
 from ddtrace.appsec._metrics import _set_waf_updates_metric
 from ddtrace.appsec._trace_utils import _asm_manual_keep
+from ddtrace.appsec._utils import has_triggers
 from ddtrace.constants import ORIGIN_KEY
 from ddtrace.constants import RUNTIME_FAMILY
 from ddtrace.ext import SpanTypes
@@ -399,7 +400,7 @@ def on_span_finish(self, span: Span) -> None:
                     _set_headers(span, headers_req, kind="response")
 
                 # this call is only necessary for tests or frameworks that are not using blocking
-                if span.get_tag(APPSEC.JSON) is None and _asm_request_context.in_context():
+                if not has_triggers(span) and _asm_request_context.in_context():
                     log.debug("metrics waf call")
                     _asm_request_context.call_waf_callback()
 

ddtrace/appsec/_utils.py

@@ -1,7 +1,9 @@
 import os
+from typing import Any
 import uuid
 
 from ddtrace.appsec._constants import API_SECURITY
+from ddtrace.appsec._constants import APPSEC
 from ddtrace.constants import APPSEC_ENV
 from ddtrace.internal.compat import to_unicode
 from ddtrace.internal.logger import get_logger
@@ -174,3 +176,23 @@ def get_user_info(self):
             return None, {}
 
         return user_id, user_extra_info
+
+
+def has_triggers(span) -> bool:
+    if asm_config._use_metastruct_for_triggers:
+        return (span.get_struct_tag(APPSEC.STRUCT) or {}).get("triggers", None) is not None
+    return span.get_tag(APPSEC.JSON) is not None
+
+
+def get_triggers(span) -> Any:
+    import json
+
+    if asm_config._use_metastruct_for_triggers:
+        return (span.get_struct_tag(APPSEC.STRUCT) or {}).get("triggers", None)
+    json_payload = span.get_tag(APPSEC.JSON)
+    if json_payload:
+        try:
+            return json.loads(json_payload).get("triggers", None)
+        except Exception:
+            log.debug("Failed to parse triggers", exc_info=True)
+    return None

ddtrace/settings/asm.py

@@ -41,6 +41,7 @@ class ASMConfig(Env):
     _asm_enabled = Env.var(bool, APPSEC_ENV, default=False)
     _asm_can_be_enabled = (APPSEC_ENV not in os.environ and tracer_config._remote_config_enabled) or _asm_enabled
     _iast_enabled = Env.var(bool, IAST_ENV, default=False)
+    _use_metastruct_for_triggers = False
 
     _automatic_login_events_mode = Env.var(str, APPSEC.AUTOMATIC_USER_EVENTS_TRACKING, default="safe")
     _user_model_login_field = Env.var(str, APPSEC.USER_MODEL_LOGIN_FIELD, default="")
@@ -82,6 +83,7 @@ class ASMConfig(Env):
     _asm_config_keys = [
         "_asm_enabled",
         "_iast_enabled",
+        "_use_metastruct_for_triggers",
         "_automatic_login_events_mode",
         "_user_model_login_field",
         "_user_model_email_field",

tests/appsec/appsec/test_processor.py

@@ -12,6 +12,7 @@
 from ddtrace.appsec._ddwaf import DDWaf
 from ddtrace.appsec._processor import AppSecSpanProcessor
 from ddtrace.appsec._processor import _transform_headers
+from ddtrace.appsec._utils import get_triggers
 from ddtrace.constants import USER_KEEP
 from ddtrace.contrib.trace_utils import set_http_meta
 from ddtrace.ext import SpanTypes
@@ -30,6 +31,9 @@
     JSONDecodeError = ValueError  # type: ignore
 
 
+APPSEC_JSON_TAG = f"meta.{APPSEC.JSON}"
+
+
 @pytest.fixture
 def tracer_appsec(tracer):
     with override_global_config(dict(_asm_enabled=True)):
@@ -104,7 +108,7 @@ def test_valid_json(tracer_appsec):
     with _asm_request_context.asm_request_context_manager(), tracer.trace("test", span_type=SpanTypes.WEB) as span:
         set_http_meta(span, {}, raw_uri="http://example.com/.git", status_code="404")
 
-    assert "triggers" in json.loads(span.get_tag(APPSEC.JSON))
+    assert get_triggers(span)
 
 
 def test_header_attack(tracer_appsec):
@@ -122,7 +126,7 @@ def test_header_attack(tracer_appsec):
                 },
             )
 
-        assert "triggers" in json.loads(span.get_tag(APPSEC.JSON))
+        assert get_triggers(span)
         assert span.get_tag("actor.ip") == "8.8.8.8"
 
 
@@ -156,9 +160,10 @@ def test_headers_collection(tracer_appsec):
 @snapshot(
     include_tracer=True,
     ignores=[
+        "meta_struct",
         "metrics._dd.appsec.waf.duration",
         "metrics._dd.appsec.waf.duration_ext",
-        "meta._dd.appsec.json",
+        APPSEC_JSON_TAG,
     ],
 )
 def test_appsec_cookies_no_collection_snapshot(tracer):
@@ -175,15 +180,16 @@ def test_appsec_cookies_no_collection_snapshot(tracer):
                 request_cookies={"cookie1": "im the cookie1"},
             )
 
-        assert "triggers" in json.loads(span.get_tag(APPSEC.JSON))
+        assert get_triggers(span)
 
 
 @snapshot(
     include_tracer=True,
     ignores=[
+        "meta_struct",
         "metrics._dd.appsec.waf.duration",
         "metrics._dd.appsec.waf.duration_ext",
-        "meta._dd.appsec.json",
+        APPSEC_JSON_TAG,
     ],
 )
 def test_appsec_body_no_collection_snapshot(tracer):
@@ -198,7 +204,7 @@ def test_appsec_body_no_collection_snapshot(tracer):
                 request_body={"somekey": "somekey value"},
             )
 
-        assert "triggers" in json.loads(span.get_tag(APPSEC.JSON))
+        assert get_triggers(span)
 
 
 def test_ip_block(tracer):
@@ -211,7 +217,7 @@ def test_ip_block(tracer):
                     rules.Config(),
                 )
 
-            assert "triggers" in json.loads(span.get_tag(APPSEC.JSON))
+            assert get_triggers(span)
             assert core.get_item("http.request.remote_ip", span) == rules._IP.BLOCKED
             assert core.get_item("http.request.blocked", span)
 
@@ -288,9 +294,10 @@ def test_ip_update_rules_expired_no_block(tracer):
 @snapshot(
     include_tracer=True,
     ignores=[
+        "meta_struct",
         "metrics._dd.appsec.waf.duration",
         "metrics._dd.appsec.waf.duration_ext",
-        "meta._dd.appsec.json",
+        APPSEC_JSON_TAG,
     ],
 )
 def test_appsec_span_tags_snapshot(tracer):
@@ -302,16 +309,17 @@ def test_appsec_span_tags_snapshot(tracer):
             span.set_tag("http.url", "http://example.com/.git")
             set_http_meta(span, {}, raw_uri="http://example.com/.git", status_code="404")
 
-        assert "triggers" in json.loads(span.get_tag(APPSEC.JSON))
+        assert get_triggers(span)
 
 
 @flaky(1735812000)
 @snapshot(
     include_tracer=True,
     ignores=[
+        "meta_struct",
         "metrics._dd.appsec.waf.duration",
         "metrics._dd.appsec.waf.duration_ext",
-        "meta._dd.appsec.json",
+        APPSEC_JSON_TAG,
         "meta._dd.appsec.event_rules.errors",
     ],
 )
@@ -325,7 +333,7 @@ def test_appsec_span_tags_snapshot_with_errors(tracer):
                 span.set_tag("http.url", "http://example.com/.git")
                 set_http_meta(span, {}, raw_uri="http://example.com/.git", status_code="404")
 
-        assert span.get_tag(APPSEC.JSON) is None
+        assert get_triggers(span) is None
 
 
 def test_appsec_span_rate_limit(tracer):
@@ -342,9 +350,9 @@ def test_appsec_span_rate_limit(tracer):
             set_http_meta(span3, {}, raw_uri="http://example.com/.git", status_code="404")
             span2.start_ns = span1.start_ns + 2
 
-        assert span1.get_tag(APPSEC.JSON) is not None
-        assert span2.get_tag(APPSEC.JSON) is None
-        assert span3.get_tag(APPSEC.JSON) is None
+        assert get_triggers(span1)
+        assert get_triggers(span2) is None
+        assert get_triggers(span3) is None
 
 
 def test_ddwaf_not_raises_exception():
@@ -409,11 +417,17 @@ def test_obfuscation_parameter_value_unconfigured_not_matching(tracer_appsec):
     with _asm_request_context.asm_request_context_manager(), tracer.trace("test", span_type=SpanTypes.WEB) as span:
         set_http_meta(span, rules.Config(), raw_uri="http://example.com/.git?hello=goodbye", status_code="404")
 
-    assert "triggers" in json.loads(span.get_tag("_dd.appsec.json"))
-
-    assert "hello" in span.get_tag("_dd.appsec.json")
-    assert "goodbye" in span.get_tag("_dd.appsec.json")
-    assert "<Redacted>" not in span.get_tag("_dd.appsec.json")
+    triggers = get_triggers(span)
+    assert triggers
+    values = [
+        value.get("value")
+        for rule in triggers
+        for match in rule.get("rule_matches", [])
+        for value in match.get("parameters", [])
+    ]
+    assert any("hello" in value for value in values)
+    assert any("goodbye" in value for value in values)
+    assert all("<Redacted>" not in value for value in values)
 
 
 def test_obfuscation_parameter_value_unconfigured_matching(tracer_appsec):
@@ -422,11 +436,17 @@ def test_obfuscation_parameter_value_unconfigured_matching(tracer_appsec):
     with _asm_request_context.asm_request_context_manager(), tracer.trace("test", span_type=SpanTypes.WEB) as span:
         set_http_meta(span, rules.Config(), raw_uri="http://example.com/.git?password=goodbye", status_code="404")
 
-    assert "triggers" in json.loads(span.get_tag("_dd.appsec.json"))
-
-    assert "password" not in span.get_tag("_dd.appsec.json")
-    assert "goodbye" not in span.get_tag("_dd.appsec.json")
-    assert "<Redacted>" in span.get_tag("_dd.appsec.json")
+    triggers = get_triggers(span)
+    assert triggers
+    values = [
+        value.get("value")
+        for rule in triggers
+        for match in rule.get("rule_matches", [])
+        for value in match.get("parameters", [])
+    ]
+    assert all("password" not in value for value in values)
+    assert all("goodbye" not in value for value in values)
+    assert any("<Redacted>" in value for value in values)
 
 
 def test_obfuscation_parameter_value_configured_not_matching(tracer):
@@ -438,11 +458,17 @@ def test_obfuscation_parameter_value_configured_not_matching(tracer):
         with _asm_request_context.asm_request_context_manager(), tracer.trace("test", span_type=SpanTypes.WEB) as span:
             set_http_meta(span, rules.Config(), raw_uri="http://example.com/.git?password=goodbye", status_code="404")
 
-        assert "triggers" in json.loads(span.get_tag("_dd.appsec.json"))
-
-        assert "password" in span.get_tag("_dd.appsec.json")
-        assert "goodbye" in span.get_tag("_dd.appsec.json")
-        assert "<Redacted>" not in span.get_tag("_dd.appsec.json")
+    triggers = get_triggers(span)
+    assert triggers
+    values = [
+        value.get("value")
+        for rule in triggers
+        for match in rule.get("rule_matches", [])
+        for value in match.get("parameters", [])
+    ]
+    assert any("password" in value for value in values)
+    assert any("goodbye" in value for value in values)
+    assert all("<Redacted>" not in value for value in values)
 
 
 def test_obfuscation_parameter_value_configured_matching(tracer):
@@ -454,11 +480,17 @@ def test_obfuscation_parameter_value_configured_matching(tracer):
         with _asm_request_context.asm_request_context_manager(), tracer.trace("test", span_type=SpanTypes.WEB) as span:
             set_http_meta(span, rules.Config(), raw_uri="http://example.com/.git?token=goodbye", status_code="404")
 
-        assert "triggers" in json.loads(span.get_tag("_dd.appsec.json"))
-
-        assert "token" not in span.get_tag("_dd.appsec.json")
-        assert "goodbye" not in span.get_tag("_dd.appsec.json")
-        assert "<Redacted>" in span.get_tag("_dd.appsec.json")
+    triggers = get_triggers(span)
+    assert triggers
+    values = [
+        value.get("value")
+        for rule in triggers
+        for match in rule.get("rule_matches", [])
+        for value in match.get("parameters", [])
+    ]
+    assert all("token" not in value for value in values)
+    assert all("goodbye" not in value for value in values)
+    assert any("<Redacted>" in value for value in values)
 
 
 def test_ddwaf_run():
@@ -606,7 +638,7 @@ def test_ddwaf_run_contained_typeerror(tracer_appsec, caplog):
                 request_body={"_authentication_token": "2b0297348221f294de3a047e2ecf1235abb866b6"},
             )
 
-    assert span.get_tag(APPSEC.JSON) is None
+    assert get_triggers(span) is None
     assert "TypeError: expected c_long instead of int" in caplog.text
 
 
@@ -644,7 +676,7 @@ def test_ddwaf_run_contained_oserror(tracer_appsec, caplog):
                 request_body={"_authentication_token": "2b0297348221f294de3a047e2ecf1235abb866b6"},
             )
 
-    assert span.get_tag(APPSEC.JSON) is None
+    assert get_triggers(span) is None
     assert "OSError: ddwaf run failed" in caplog.text