From 566bafc71e82d874fdf81069437709934e05b823 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 29 Feb 2024 11:17:41 +0100 Subject: [PATCH] add test_api_security_sampling for new sampling algorithm --- ddtrace/appsec/_api_security/api_manager.py | 2 +- ddtrace/appsec/_remoteconfiguration.py | 2 +- ddtrace/appsec/_utils.py | 2 +- ddtrace/settings/asm.py | 2 +- .../appsec/contrib_appsec/django_app/urls.py | 6 +++ .../appsec/contrib_appsec/fastapi_app/app.py | 21 ++++++++- tests/appsec/contrib_appsec/flask_app/app.py | 6 +++ tests/appsec/contrib_appsec/utils.py | 45 ++++++++++++++++--- 8 files changed, 74 insertions(+), 12 deletions(-) diff --git a/ddtrace/appsec/_api_security/api_manager.py b/ddtrace/appsec/_api_security/api_manager.py index 89b03c37f51..e97f8bbf5ff 100644 --- a/ddtrace/appsec/_api_security/api_manager.py +++ b/ddtrace/appsec/_api_security/api_manager.py @@ -126,7 +126,7 @@ def _schema_callback(self, env): return try: - if not self._should_collect_schema(env, root.context.sampling_priority): + if not self._should_collect_schema(env, env.span.context.sampling_priority): return except Exception: log.warning("Failed to sample request for schema generation", exc_info=True) diff --git a/ddtrace/appsec/_remoteconfiguration.py b/ddtrace/appsec/_remoteconfiguration.py index a8a716247c1..2634e573fd6 100644 --- a/ddtrace/appsec/_remoteconfiguration.py +++ b/ddtrace/appsec/_remoteconfiguration.py @@ -101,7 +101,6 @@ def _add_rules_to_list(features: Mapping[str, Any], feature: str, message: str, def _appsec_callback(features: Mapping[str, Any], test_tracer: Optional[Tracer] = None) -> None: config = features.get("config", {}) _appsec_1click_activation(config, test_tracer) - _appsec_api_security_settings(config, test_tracer) _appsec_rules_data(config, test_tracer) @@ -235,6 +234,7 @@ def _appsec_1click_activation(features: Mapping[str, Any], test_tracer: Optional def _appsec_api_security_settings(features: Mapping[str, Any], test_tracer: Optional[Tracer] = None) -> None: """ + Deprecated Update API Security settings from remote config Actually: Update sample rate """ diff --git a/ddtrace/appsec/_utils.py b/ddtrace/appsec/_utils.py index 23befd40b7d..a84f5e8e823 100644 --- a/ddtrace/appsec/_utils.py +++ b/ddtrace/appsec/_utils.py @@ -73,7 +73,7 @@ def _appsec_rc_features_is_enabled() -> bool: def _appsec_apisec_features_is_active() -> bool: - return asm_config._asm_enabled and asm_config._api_security_enabled and asm_config._api_security_sample_rate > 0.0 + return asm_config._asm_enabled and asm_config._api_security_enabled def _safe_userid(user_id): diff --git a/ddtrace/settings/asm.py b/ddtrace/settings/asm.py index 1ab4f2bad9d..6b996dfbd37 100644 --- a/ddtrace/settings/asm.py +++ b/ddtrace/settings/asm.py @@ -48,7 +48,7 @@ class ASMConfig(Env): _user_model_email_field = Env.var(str, APPSEC.USER_MODEL_EMAIL_FIELD, default="") _user_model_name_field = Env.var(str, APPSEC.USER_MODEL_NAME_FIELD, default="") _api_security_enabled = Env.var(bool, API_SECURITY.ENV_VAR_ENABLED, default=True) - _api_security_sample_rate = Env.var(float, API_SECURITY.SAMPLE_RATE, validator=_validate_sample_rate, default=0.1) + _api_security_sample_rate = 0.0 _api_security_parse_response_body = Env.var(bool, API_SECURITY.PARSE_RESPONSE_BODY, default=True) _asm_libddwaf = build_libddwaf_filename() _asm_libddwaf_available = os.path.exists(_asm_libddwaf) diff --git a/tests/appsec/contrib_appsec/django_app/urls.py b/tests/appsec/contrib_appsec/django_app/urls.py index 13d70e4f893..187743a2c77 100644 --- a/tests/appsec/contrib_appsec/django_app/urls.py +++ b/tests/appsec/contrib_appsec/django_app/urls.py @@ -9,6 +9,7 @@ from django.views.decorators.csrf import csrf_exempt from ddtrace import tracer +import ddtrace.constants # django.conf.urls.url was deprecated in django 3 and removed in django 4 @@ -40,6 +41,11 @@ def multi_view(request, param_int=0, param_str=""): } status = int(query_params.get("status", "200")) headers_query = query_params.get("headers", "").split(",") + priority = query_params.get("priority", None) + if priority in ("keep", "drop"): + tracer.current_span().set_tag( + ddtrace.constants.MANUAL_KEEP_KEY if priority == "keep" else ddtrace.constants.MANUAL_DROP_KEY + ) response_headers = {} for header in headers_query: vk = header.split("=") diff --git a/tests/appsec/contrib_appsec/fastapi_app/app.py b/tests/appsec/contrib_appsec/fastapi_app/app.py index 99fdbee5b81..c0f33ee778c 100644 --- a/tests/appsec/contrib_appsec/fastapi_app/app.py +++ b/tests/appsec/contrib_appsec/fastapi_app/app.py @@ -6,6 +6,9 @@ from fastapi.responses import JSONResponse from pydantic import BaseModel +from ddtrace import tracer +import ddtrace.constants + fake_secret_token = "DataDog" @@ -51,6 +54,11 @@ async def multi_view(param_int: int, param_str: str, request: Request): # noqa: } status = int(query_params.get("status", "200")) headers_query = query_params.get("headers", "").split(",") + priority = query_params.get("priority", None) + if priority in ("keep", "drop"): + tracer.current_span().set_tag( + ddtrace.constants.MANUAL_KEEP_KEY if priority == "keep" else ddtrace.constants.MANUAL_DROP_KEY + ) response_headers = {} for header in headers_query: vk = header.split("=") @@ -71,7 +79,18 @@ async def multi_view_no_param(request: Request): # noqa: B008 "method": request.method, } status = int(query_params.get("status", "200")) - return JSONResponse(body, status_code=status) + headers_query = query_params.get("headers", "").split(",") + priority = query_params.get("priority", None) + if priority in ("keep", "drop"): + tracer.current_span().set_tag( + ddtrace.constants.MANUAL_KEEP_KEY if priority == "keep" else ddtrace.constants.MANUAL_DROP_KEY + ) + response_headers = {} + for header in headers_query: + vk = header.split("=") + if len(vk) == 2: + response_headers[vk[0]] = vk[1] + return JSONResponse(body, status_code=status, headers=response_headers) @app.get("/new_service/{service_name:str}/") @app.post("/new_service/{service_name:str}/") diff --git a/tests/appsec/contrib_appsec/flask_app/app.py b/tests/appsec/contrib_appsec/flask_app/app.py index d4daf829e50..99eff1cc88f 100644 --- a/tests/appsec/contrib_appsec/flask_app/app.py +++ b/tests/appsec/contrib_appsec/flask_app/app.py @@ -4,6 +4,7 @@ from flask import request from ddtrace import tracer +import ddtrace.constants from tests.webclient import PingFilter @@ -36,6 +37,11 @@ def multi_view(param_int=0, param_str=""): } status = int(query_params.get("status", "200")) headers_query = query_params.get("headers", "").split(",") + priority = query_params.get("priority", None) + if priority in ("keep", "drop"): + tracer.current_span().set_tag( + ddtrace.constants.MANUAL_KEEP_KEY if priority == "keep" else ddtrace.constants.MANUAL_DROP_KEY + ) response_headers = {} for header in headers_query: vk = header.split("=") diff --git a/tests/appsec/contrib_appsec/utils.py b/tests/appsec/contrib_appsec/utils.py index cc1e9951d71..811bee9efe5 100644 --- a/tests/appsec/contrib_appsec/utils.py +++ b/tests/appsec/contrib_appsec/utils.py @@ -885,7 +885,6 @@ def test_nested_appsec_events( ({"User-Agent": "AllOK"}, False, False), ], ) - @pytest.mark.parametrize("sample_rate", [0.0, 1.0]) def test_api_security_schemas( self, interface: Interface, @@ -897,16 +896,13 @@ def test_api_security_schemas( headers, event, blocked, - sample_rate, ): import base64 import gzip from ddtrace.ext import http - with override_global_config( - dict(_asm_enabled=True, _api_security_enabled=apisec_enabled, _api_security_sample_rate=sample_rate) - ): + with override_global_config(dict(_asm_enabled=True, _api_security_enabled=apisec_enabled)): self.update_tracer(interface) response = interface.client.post( "/asm/324/huj/?x=1&y=2", @@ -916,7 +912,6 @@ def test_api_security_schemas( content_type="application/json", ) assert asm_config._api_security_enabled == apisec_enabled - assert asm_config._api_security_sample_rate == sample_rate assert self.status(response) == 403 if blocked else 200 assert get_tag(http.STATUS_CODE) == "403" if blocked else "200" @@ -925,7 +920,7 @@ def test_api_security_schemas( else: assert get_triggers(root_span()) is None value = get_tag(name) - if apisec_enabled and sample_rate: + if apisec_enabled: assert value, name api = json.loads(gzip.decompress(base64.b64decode(value)).decode()) assert api, name @@ -976,6 +971,42 @@ def test_api_security_scanners(self, interface: Interface, get_tag, apisec_enabl else: assert value is None + @pytest.mark.parametrize("apisec_enabled", [True, False]) + @pytest.mark.parametrize("priority", ["keep", "drop"]) + def test_api_security_sampling(self, interface: Interface, get_tag, apisec_enabled, priority): + from ddtrace.ext import http + + payload = {"mastercard": "5123456789123456"} + with override_global_config(dict(_asm_enabled=True, _api_security_enabled=apisec_enabled)): + self.update_tracer(interface) + response = interface.client.post( + f"/asm/?priority={priority}", + data=json.dumps(payload), + content_type="application/json", + ) + assert self.status(response) == 200 + assert get_tag(http.STATUS_CODE) == "200" + assert asm_config._api_security_enabled == apisec_enabled + + value = get_tag("_dd.appsec.s.req.body") + if apisec_enabled and priority == "keep": + assert value + else: + assert value is None + # second request must be ignored + self.update_tracer(interface) + response = interface.client.post( + f"/asm/?priority={priority}", + data=json.dumps(payload), + content_type="application/json", + ) + assert self.status(response) == 200 + assert get_tag(http.STATUS_CODE) == "200" + assert asm_config._api_security_enabled == apisec_enabled + + value = get_tag("_dd.appsec.s.req.body") + assert value is None + def test_request_invalid_rule_file(self, interface): """ When the rule file is invalid, the tracer should not crash or prevent normal behavior