Skip to content

Commit 06ec846

Browse files
chore(asm): migrate appsec processor to drop python2 compatibility (#7121)
- drop python 2 and six and attr dependency in appsec/_processor.py - small fixes on names and types in appsec/_constants.py follows #7118 Motivation : we don't need attr anymore as features needed are covered by python 3 api in dataclasses. ## 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 - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] 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) - [x] 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`. - [x] This PR doesn't touch any of that.
1 parent 5480ca8 commit 06ec846

File tree

2 files changed

+49
-69
lines changed

2 files changed

+49
-69
lines changed

ddtrace/appsec/_constants.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ class WAF_ACTIONS(metaclass=Constant_Class):
149149
DEFAULT_PARAMETERS = STATUS_403_TYPE_AUTO
150150
BLOCK_ACTION = "block_request"
151151
REDIRECT_ACTION = "redirect_request"
152-
DEFAULT_ACTONS = {
152+
DEFAULT_ACTIONS = {
153153
BLOCK: {
154154
ID: BLOCK,
155155
TYPE: BLOCK_ACTION,
@@ -190,13 +190,13 @@ class DEFAULT(metaclass=Constant_Class):
190190
TRACE_RATE_LIMIT = 100
191191
WAF_TIMEOUT = 5.0 # float (milliseconds)
192192
APPSEC_OBFUSCATION_PARAMETER_KEY_REGEXP = (
193-
r"(?i)(?:p(?:ass)?w(?:or)?d|pass(?:_?phrase)?|secret|(?:api_?|private_?|public_?)key)|token|consumer_?"
194-
r"(?:id|key|secret)|sign(?:ed|ature)|bearer|authorization"
193+
rb"(?i)(?:p(?:ass)?w(?:or)?d|pass(?:_?phrase)?|secret|(?:api_?|private_?|public_?)key)|token|consumer_?"
194+
rb"(?:id|key|secret)|sign(?:ed|ature)|bearer|authorization"
195195
)
196196
APPSEC_OBFUSCATION_PARAMETER_VALUE_REGEXP = (
197-
r"(?i)(?:p(?:ass)?w(?:or)?d|pass(?:_?phrase)?|secret|(?:api_?|private_?|public_?|access_?|secret_?)"
198-
r"key(?:_?id)?|token|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)"
199-
r'(?:\s*=[^;]|"\s*:\s*"[^"]+")|bearer\s+[a-z0-9\._\-]+|token:[a-z0-9]{13}|gh[opsu]_[0-9a-zA-Z]{36}'
200-
r"|ey[I-L][\w=-]+\.ey[I-L][\w=-]+(?:\.[\w.+\/=-]+)?|[\-]{5}BEGIN[a-z\s]+PRIVATE\sKEY[\-]{5}[^\-]+[\-]"
201-
r"{5}END[a-z\s]+PRIVATE\sKEY|ssh-rsa\s*[a-z0-9\/\.+]{100,}"
197+
rb"(?i)(?:p(?:ass)?w(?:or)?d|pass(?:_?phrase)?|secret|(?:api_?|private_?|public_?|access_?|secret_?)"
198+
rb"key(?:_?id)?|token|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)"
199+
rb'(?:\s*=[^;]|"\s*:\s*"[^"]+")|bearer\s+[a-z0-9\._\-]+|token:[a-z0-9]{13}|gh[opsu]_[0-9a-zA-Z]{36}'
200+
rb"|ey[I-L][\w=-]+\.ey[I-L][\w=-]+(?:\.[\w.+\/=-]+)?|[\-]{5}BEGIN[a-z\s]+PRIVATE\sKEY[\-]{5}[^\-]+[\-]"
201+
rb"{5}END[a-z\s]+PRIVATE\sKEY|ssh-rsa\s*[a-z0-9\/\.+]{100,}"
202202
)

ddtrace/appsec/_processor.py

Lines changed: 41 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
1+
import dataclasses
12
import errno
23
import json
4+
from json.decoder import JSONDecodeError
35
import os
46
import os.path
57
import traceback
8+
from typing import Any
9+
from typing import Dict
10+
from typing import List
11+
from typing import Optional
612
from typing import Set
7-
from typing import TYPE_CHECKING
8-
9-
import attr
10-
from six import ensure_binary
13+
from typing import Tuple
14+
from typing import Union
1115

1216
from ddtrace.appsec import _asm_request_context
1317
from ddtrace.appsec._capabilities import _appsec_rc_file_is_not_static
@@ -17,6 +21,7 @@
1721
from ddtrace.appsec._constants import WAF_ACTIONS
1822
from ddtrace.appsec._constants import WAF_CONTEXT_NAMES
1923
from ddtrace.appsec._constants import WAF_DATA_NAMES
24+
from ddtrace.appsec._ddwaf.ddwaf_types import ddwaf_context_capsule
2025
from ddtrace.appsec._metrics import _set_waf_error_metric
2126
from ddtrace.appsec._metrics import _set_waf_init_metric
2227
from ddtrace.appsec._metrics import _set_waf_request_metrics
@@ -29,31 +34,14 @@
2934
from ddtrace.internal.logger import get_logger
3035
from ddtrace.internal.processor import SpanProcessor
3136
from ddtrace.internal.rate_limiter import RateLimiter
32-
33-
34-
try:
35-
from json.decoder import JSONDecodeError
36-
except ImportError:
37-
# handling python 2.X import error
38-
JSONDecodeError = ValueError # type: ignore
39-
40-
if TYPE_CHECKING: # pragma: no cover
41-
from typing import Any
42-
from typing import Dict
43-
from typing import List
44-
from typing import Tuple
45-
from typing import Union
46-
47-
from ddtrace.appsec._ddwaf.ddwaf_types import ddwaf_context_capsule
48-
from ddtrace.span import Span
37+
from ddtrace.span import Span
4938

5039

5140
log = get_logger(__name__)
5241

5342

54-
def _transform_headers(data):
55-
# type: (Union[Dict[str, str], List[Tuple[str, str]]]) -> Dict[str, Union[str, List[str]]]
56-
normalized = {} # type: Dict[str, Union[str, List[str]]]
43+
def _transform_headers(data: Union[Dict[str, str], List[Tuple[str, str]]]) -> Dict[str, Union[str, List[str]]]:
44+
normalized: Dict[str, Union[str, List[str]]] = {}
5745
headers = data if isinstance(data, list) else data.items()
5846
for header, value in headers:
5947
header = header.lower()
@@ -70,22 +58,17 @@ def _transform_headers(data):
7058
return normalized
7159

7260

73-
def get_rules():
74-
# type: () -> str
61+
def get_rules() -> str:
7562
return os.getenv("DD_APPSEC_RULES", default=DEFAULT.RULES)
7663

7764

78-
def get_appsec_obfuscation_parameter_key_regexp():
79-
# type: () -> bytes
80-
return ensure_binary(
81-
os.getenv("DD_APPSEC_OBFUSCATION_PARAMETER_KEY_REGEXP", DEFAULT.APPSEC_OBFUSCATION_PARAMETER_KEY_REGEXP)
82-
)
65+
def get_appsec_obfuscation_parameter_key_regexp() -> bytes:
66+
return os.getenvb(b"DD_APPSEC_OBFUSCATION_PARAMETER_KEY_REGEXP", DEFAULT.APPSEC_OBFUSCATION_PARAMETER_KEY_REGEXP)
8367

8468

85-
def get_appsec_obfuscation_parameter_value_regexp():
86-
# type: () -> bytes
87-
return ensure_binary(
88-
os.getenv("DD_APPSEC_OBFUSCATION_PARAMETER_VALUE_REGEXP", DEFAULT.APPSEC_OBFUSCATION_PARAMETER_VALUE_REGEXP)
69+
def get_appsec_obfuscation_parameter_value_regexp() -> bytes:
70+
return os.getenvb(
71+
b"DD_APPSEC_OBFUSCATION_PARAMETER_VALUE_REGEXP", DEFAULT.APPSEC_OBFUSCATION_PARAMETER_VALUE_REGEXP
8972
)
9073

9174

@@ -114,8 +97,7 @@ def get_appsec_obfuscation_parameter_value_regexp():
11497
}
11598

11699

117-
def _set_headers(span, headers, kind):
118-
# type: (Span, Any, str) -> None
100+
def _set_headers(span: Span, headers: Any, kind: str) -> None:
119101
from ddtrace.contrib.trace_utils import _normalize_tag_name
120102

121103
for k in headers:
@@ -128,25 +110,27 @@ def _set_headers(span, headers, kind):
128110
span.set_tag(_normalize_tag_name(kind, key), value)
129111

130112

131-
def _get_rate_limiter():
132-
# type: () -> RateLimiter
113+
def _get_rate_limiter() -> RateLimiter:
133114
return RateLimiter(int(os.getenv("DD_APPSEC_TRACE_RATE_LIMIT", DEFAULT.TRACE_RATE_LIMIT)))
134115

135116

136-
@attr.s(eq=False)
117+
@dataclasses.dataclass(eq=False)
137118
class AppSecSpanProcessor(SpanProcessor):
138-
rules = attr.ib(type=str, factory=get_rules)
139-
obfuscation_parameter_key_regexp = attr.ib(type=bytes, factory=get_appsec_obfuscation_parameter_key_regexp)
140-
obfuscation_parameter_value_regexp = attr.ib(type=bytes, factory=get_appsec_obfuscation_parameter_value_regexp)
141-
_addresses_to_keep = attr.ib(type=Set[str], factory=set)
142-
_rate_limiter = attr.ib(type=RateLimiter, factory=_get_rate_limiter)
119+
rules: str = dataclasses.field(default_factory=get_rules)
120+
obfuscation_parameter_key_regexp: bytes = dataclasses.field(
121+
default_factory=get_appsec_obfuscation_parameter_key_regexp
122+
)
123+
obfuscation_parameter_value_regexp: bytes = dataclasses.field(
124+
default_factory=get_appsec_obfuscation_parameter_value_regexp
125+
)
126+
_addresses_to_keep: Set[str] = dataclasses.field(default_factory=set)
127+
_rate_limiter: RateLimiter = dataclasses.field(default_factory=_get_rate_limiter)
143128

144129
@property
145130
def enabled(self):
146131
return self._ddwaf is not None
147132

148-
def __attrs_post_init__(self):
149-
# type: () -> None
133+
def __post_init__(self) -> None:
150134
from ddtrace import config
151135
from ddtrace.appsec._ddwaf import DDWaf
152136

@@ -196,16 +180,15 @@ def __attrs_post_init__(self):
196180
# we always need the response headers
197181
self._mark_needed(WAF_DATA_NAMES.RESPONSE_HEADERS_NO_COOKIES)
198182

199-
def _update_actions(self, rules):
183+
def _update_actions(self, rules: Dict[str, Any]) -> None:
200184
new_actions = rules.get("actions", [])
201-
self._actions = WAF_ACTIONS.DEFAULT_ACTONS
185+
self._actions: Dict[str, Dict[str, Any]] = WAF_ACTIONS.DEFAULT_ACTIONS
202186
for a in new_actions:
203187
self._actions[a.get(WAF_ACTIONS.ID, None)] = a
204188
if "actions" in rules:
205189
del rules["actions"]
206190

207-
def _update_rules(self, new_rules):
208-
# type: (Dict[str, Any]) -> bool
191+
def _update_rules(self, new_rules: Dict[str, Any]) -> bool:
209192
result = False
210193
if not _appsec_rc_file_is_not_static():
211194
return result
@@ -223,8 +206,7 @@ def _update_rules(self, new_rules):
223206
_set_waf_error_metric(error_msg, "", self._ddwaf.info)
224207
return result
225208

226-
def on_span_start(self, span):
227-
# type: (Span) -> None
209+
def on_span_start(self, span: Span) -> None:
228210
from ddtrace.contrib import trace_utils
229211

230212
if span.span_type != SpanTypes.WEB:
@@ -267,8 +249,9 @@ def waf_callable(custom_data=None):
267249
# _asm_request_context.call_callback()
268250
_asm_request_context.call_waf_callback({"REQUEST_HTTP_IP": None})
269251

270-
def _waf_action(self, span, ctx, custom_data=None):
271-
# type: (Span, ddwaf_context_capsule, dict[str, Any] | None) -> None | dict[str, Any]
252+
def _waf_action(
253+
self, span: Span, ctx: ddwaf_context_capsule, custom_data: Optional[Dict[str, Any]] = None
254+
) -> Optional[Dict[str, Any]]:
272255
"""
273256
Call the `WAF` with the given parameters. If `custom_data_names` is specified as
274257
a list of `(WAF_NAME, WAF_STR)` tuples specifying what values of the `WAF_DATA_NAMES`
@@ -411,16 +394,13 @@ def update_metric(name, value):
411394
span.set_tag_str(ORIGIN_KEY, APPSEC.ORIGIN_VALUE)
412395
return waf_results.derivatives
413396

414-
def _mark_needed(self, address):
415-
# type: (str) -> None
397+
def _mark_needed(self, address: str) -> None:
416398
self._addresses_to_keep.add(address)
417399

418-
def _is_needed(self, address):
419-
# type: (str) -> bool
400+
def _is_needed(self, address: str) -> bool:
420401
return address in self._addresses_to_keep
421402

422-
def on_span_finish(self, span):
423-
# type: (Span) -> None
403+
def on_span_finish(self, span: Span) -> None:
424404
try:
425405
if span.span_type == SpanTypes.WEB:
426406
# Force to set respond headers at the end

0 commit comments

Comments
 (0)