Skip to content

Commit e8d87f7

Browse files
christophe-papazianemmettbutlerYun-Kimmajorgreysavara1986
authored
feat(asm): add RC support for API sec (#7518)
New API Security features: - Use last version of appsec event rules to provides static rules for api security - Remove temporary rule file previously used for api security - Add remote config support for api security (ability to remotely update scanners and processors from RC) (api security sample rate was already added in a previous PR) - Update api security unit tests accordingly API Security is still a private feature, no release note for now. ## 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] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [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)) ## Reviewer Checklist - [ ] Title is accurate. - [ ] No unnecessary changes are introduced. - [ ] Description motivates each change. - [ ] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Testing strategy adequately addresses listed risk(s). - [ ] Change is maintainable (easy to change, telemetry, documentation). - [ ] Release note makes sense to a user of the library. - [ ] Reviewer has explicitly 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) - [ ] If this PR 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`. - [ ] This PR doesn't touch any of that. --------- Signed-off-by: Juanjo Alvarez <[email protected]> Co-authored-by: Emmett Butler <[email protected]> Co-authored-by: Yun Kim <[email protected]> Co-authored-by: Tahir H. Butt <[email protected]> Co-authored-by: Alberto Vara <[email protected]> Co-authored-by: Kyle Verhoog <[email protected]> Co-authored-by: Zachary Groves <[email protected]> Co-authored-by: Federico Mon <[email protected]> Co-authored-by: Romain Komorn <[email protected]> Co-authored-by: Eric Navarro <[email protected]> Co-authored-by: Emmett Butler <[email protected]> Co-authored-by: Juanjo Alvarez Martinez <[email protected]> Co-authored-by: Brett Langdon <[email protected]> Co-authored-by: AJ Stuyvenberg <[email protected]> Co-authored-by: Munir Abdinur <[email protected]> Co-authored-by: Gabriele N. Tornetta <[email protected]> Co-authored-by: HovhannesV <[email protected]> Co-authored-by: Tahir H. Butt <[email protected]>
1 parent c328aea commit e8d87f7

File tree

6 files changed

+20
-254
lines changed

6 files changed

+20
-254
lines changed

ddtrace/appsec/_api_security/processors.json

Lines changed: 0 additions & 237 deletions
This file was deleted.

ddtrace/appsec/_constants.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ class LOGIN_EVENTS_MODE(metaclass=Constant_Class):
199199
class DEFAULT(metaclass=Constant_Class):
200200
ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
201201
RULES = os.path.join(ROOT_DIR, "rules.json")
202-
API_SECURITY_PARAMETERS = os.path.join(ROOT_DIR, "_api_security/processors.json")
203202
TRACE_RATE_LIMIT = 100
204203
WAF_TIMEOUT = 5.0 # float (milliseconds)
205204
APPSEC_OBFUSCATION_PARAMETER_KEY_REGEXP = (

ddtrace/appsec/_processor.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,6 @@ def __post_init__(self) -> None:
137137
try:
138138
with open(self.rules, "r") as f:
139139
rules = json.load(f)
140-
if asm_config._api_security_enabled:
141-
with open(DEFAULT.API_SECURITY_PARAMETERS, "r") as f_apisec:
142-
processors = json.load(f_apisec)
143-
rules["processors"] = processors["processors"]
144-
rules["scanners"] = processors["scanners"]
145140
self._update_actions(rules)
146141

147142
except EnvironmentError as err:

ddtrace/appsec/_remoteconfiguration.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ def _appsec_rules_data(features: Mapping[str, Any], test_tracer: Optional[Tracer
121121
_add_rules_to_list(features, "exclusions", "exclusion filters", ruleset)
122122
_add_rules_to_list(features, "rules_override", "rules override", ruleset)
123123
_add_rules_to_list(features, "scanners", "scanners", ruleset)
124+
_add_rules_to_list(features, "processors", "processors", ruleset)
124125
if ruleset:
125126
return tracer._appsec_processor._update_rules({k: v for k, v in ruleset.items() if v is not None})
126127

tests/contrib/django/test_django_appsec_api_security.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
import json
55

66
from ddtrace.appsec import _constants
7+
from ddtrace.appsec._constants import API_SECURITY
78
from ddtrace.settings.asm import config as asm_config
89
from tests.appsec.appsec.api_security.test_schema_fuzz import equal_with_meta
9-
from tests.appsec.appsec.test_processor import RULES_SRB
1010
from tests.utils import override_env
1111
from tests.utils import override_global_config
1212

@@ -45,7 +45,9 @@ def _aux_appsec_get_root_span(
4545
def test_api_security(client, test_spans, tracer):
4646
import django
4747

48-
with override_global_config(dict(_asm_enabled=True, _api_security_enabled=True, _api_security_sample_rate=1.0)):
48+
with override_global_config(
49+
dict(_asm_enabled=True, _api_security_enabled=True, _api_security_sample_rate=1.0)
50+
), override_env({API_SECURITY.SAMPLE_RATE: "1.0"}):
4951
payload = {"key": "secret", "ids": [0, 1, 2, 3]}
5052
root_span, response = _aux_appsec_get_root_span(
5153
client,
@@ -120,7 +122,7 @@ def test_api_security_with_srb(client, test_spans, tracer):
120122

121123
with override_global_config(
122124
dict(_asm_enabled=True, _api_security_enabled=True, _api_security_sample_rate=1.0)
123-
), override_env({"DD_APPSEC_RULES": RULES_SRB}):
125+
), override_env({API_SECURITY.SAMPLE_RATE: "1.0"}):
124126
payload = {"key": "secret", "ids": [0, 1, 2, 3]}
125127
root_span, response = _aux_appsec_get_root_span(
126128
client,
@@ -130,18 +132,19 @@ def test_api_security_with_srb(client, test_spans, tracer):
130132
payload=payload,
131133
cookies={"secret": "a1b2c3d4e5f6"},
132134
content_type="application/json",
135+
headers={"HTTP_USER_AGENT": "dd-test-scanner-log-block"},
133136
)
134137
assert response.status_code == 403
135138
loaded = json.loads(root_span.get_tag(_constants.APPSEC.JSON))
136-
assert [t["rule"]["id"] for t in loaded["triggers"]] == ["tst-037-001"]
139+
assert [t["rule"]["id"] for t in loaded["triggers"]] == ["ua0-600-56x"]
137140

138141
assert asm_config._api_security_enabled
139142

140143
for name, expected_value in [
141144
("_dd.appsec.s.req.body", [{"key": [8], "ids": [[[4]], {"len": 4}]}]),
142145
(
143146
"_dd.appsec.s.req.headers",
144-
[{"content-length": [8], "content-type": [8]}],
147+
[{"user-agent": [8], "content-length": [8], "content-type": [8]}],
145148
),
146149
("_dd.appsec.s.req.cookies", [{"secret": [8]}]),
147150
("_dd.appsec.s.req.query", [{"y": [8], "x": [8]}]),
@@ -152,14 +155,15 @@ def test_api_security_with_srb(client, test_spans, tracer):
152155
value = root_span.get_tag(name)
153156
assert value, name
154157
api = json.loads(gzip.decompress(base64.b64decode(value)).decode())
158+
print(api)
155159
assert equal_with_meta(api, expected_value), name
156160

157161

158162
def test_api_security_deactivated(client, test_spans, tracer):
159163
"""Test if blocking is still working as expected with api security deactivated"""
160164

161165
with override_global_config(dict(_asm_enabled=True, _api_security_enabled=False)), override_env(
162-
{_constants.API_SECURITY.SAMPLE_RATE: "1.0", "DD_APPSEC_RULES": RULES_SRB}
166+
{_constants.API_SECURITY.SAMPLE_RATE: "1.0"}
163167
):
164168
payload = {"key": "secret", "ids": [0, 1, 2, 3]}
165169
root_span, response = _aux_appsec_get_root_span(
@@ -170,10 +174,11 @@ def test_api_security_deactivated(client, test_spans, tracer):
170174
payload=payload,
171175
cookies={"secret": "a1b2c3d4e5f6"},
172176
content_type="application/json",
177+
headers={"HTTP_USER_AGENT": "dd-test-scanner-log-block"},
173178
)
174179
assert response.status_code == 403
175180
loaded = json.loads(root_span.get_tag(_constants.APPSEC.JSON))
176-
assert [t["rule"]["id"] for t in loaded["triggers"]] == ["tst-037-001"]
181+
assert [t["rule"]["id"] for t in loaded["triggers"]] == ["ua0-600-56x"]
177182

178183
assert not asm_config._api_security_enabled
179184

0 commit comments

Comments
 (0)