Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(asm): switch back to default if remote config stop sending the security rule file [backport 2.9] #10053

Merged
merged 1 commit into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions ddtrace/appsec/_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def _get_rate_limiter() -> RateLimiter:

@dataclasses.dataclass(eq=False)
class AppSecSpanProcessor(SpanProcessor):
rules: str = dataclasses.field(default_factory=get_rules)
rule_filename: str = dataclasses.field(default_factory=get_rules)
obfuscation_parameter_key_regexp: bytes = dataclasses.field(
default_factory=get_appsec_obfuscation_parameter_key_regexp
)
Expand All @@ -159,28 +159,35 @@ def __post_init__(self) -> None:
from ddtrace.appsec._ddwaf import DDWaf

try:
with open(self.rules, "r") as f:
rules = json.load(f)
with open(self.rule_filename, "r") as f:
self._rules = json.load(f)

except EnvironmentError as err:
if err.errno == errno.ENOENT:
log.error("[DDAS-0001-03] ASM could not read the rule file %s. Reason: file does not exist", self.rules)
log.error(
"[DDAS-0001-03] ASM could not read the rule file %s. Reason: file does not exist",
self.rule_filename,
)
else:
# TODO: try to log reasons
log.error("[DDAS-0001-03] ASM could not read the rule file %s.", self.rules)
log.error("[DDAS-0001-03] ASM could not read the rule file %s.", self.rule_filename)
raise
except JSONDecodeError:
log.error("[DDAS-0001-03] ASM could not read the rule file %s. Reason: invalid JSON file", self.rules)
log.error(
"[DDAS-0001-03] ASM could not read the rule file %s. Reason: invalid JSON file", self.rule_filename
)
raise
except Exception:
# TODO: try to log reasons
log.error("[DDAS-0001-03] ASM could not read the rule file %s.", self.rules)
log.error("[DDAS-0001-03] ASM could not read the rule file %s.", self.rule_filename)
raise
try:
self._ddwaf = DDWaf(rules, self.obfuscation_parameter_key_regexp, self.obfuscation_parameter_value_regexp)
self._ddwaf = DDWaf(
self._rules, self.obfuscation_parameter_key_regexp, self.obfuscation_parameter_value_regexp
)
if not self._ddwaf._handle or self._ddwaf.info.failed:
stack_trace = "DDWAF.__init__: invalid rules\n ruleset: %s\nloaded:%s\nerrors:%s\n" % (
rules,
self._rules,
self._ddwaf.info.loaded,
self._ddwaf.info.errors,
)
Expand Down
17 changes: 11 additions & 6 deletions ddtrace/appsec/_remoteconfiguration.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ def _add_rules_to_list(features: Mapping[str, Any], feature: str, message: str,
if rules is not None:
try:
if ruleset.get(feature) is None:
ruleset[feature] = []
ruleset[feature] += rules
ruleset[feature] = rules
else:
ruleset[feature] = ruleset[feature] + rules
log.debug("Reloading Appsec %s: %s", message, str(rules)[:20])
except json.JSONDecodeError:
log.error("ERROR Appsec %s: invalid JSON content from remote configuration", message)
Expand All @@ -114,14 +115,18 @@ def _appsec_rules_data(features: Mapping[str, Any], test_tracer: Optional[Tracer

if features and tracer._appsec_processor:
ruleset = {} # type: dict[str, Optional[list[Any]]]
_add_rules_to_list(features, "rules_data", "rules data", ruleset)
if features.get("rules", None) == []:
# if rules is empty, we need to switch back to the default rules
ruleset = tracer._appsec_processor._rules.copy() or {}
_add_rules_to_list(features, "actions", "actions", ruleset)
_add_rules_to_list(features, "custom_rules", "custom rules", ruleset)
_add_rules_to_list(features, "rules", "Datadog rules", ruleset)
_add_rules_to_list(features, "exclusions", "exclusion filters", ruleset)
_add_rules_to_list(features, "exclusion_data", "exclusion data", ruleset)
_add_rules_to_list(features, "processors", "processors", ruleset)
_add_rules_to_list(features, "rules", "Datadog rules", ruleset)
_add_rules_to_list(features, "rules_data", "rules data", ruleset)
_add_rules_to_list(features, "rules_override", "rules override", ruleset)
_add_rules_to_list(features, "scanners", "scanners", ruleset)
_add_rules_to_list(features, "processors", "processors", ruleset)
_add_rules_to_list(features, "actions", "actions", ruleset)
if ruleset:
return tracer._appsec_processor._update_rules({k: v for k, v in ruleset.items() if v is not None})

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
fixes:
- |
ASM: This fix resolves an issue where the WAF could be disabled if the ASM_DD rule file was not found in Remote Config.
2 changes: 1 addition & 1 deletion tests/appsec/appsec/test_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def test_enable_custom_rules():
processor = AppSecSpanProcessor()

assert processor.enabled
assert processor.rules == rules.RULES_GOOD_PATH
assert processor.rule_filename == rules.RULES_GOOD_PATH


def test_ddwaf_ctx(tracer_appsec):
Expand Down
12 changes: 12 additions & 0 deletions tests/appsec/appsec/test_remoteconfiguration.py
Original file line number Diff line number Diff line change
Expand Up @@ -1033,3 +1033,15 @@ def test_rc_rules_data_error_ddwaf(tracer):
"rules": [{"invalid": mock.MagicMock()}],
}
assert not _appsec_rules_data(config, tracer)


def test_rules_never_empty(tracer):
with override_global_config(dict(_asm_enabled=True)):
tracer.configure(appsec_enabled=True, api_version="v0.4")
with mock.patch("ddtrace.appsec._processor.AppSecSpanProcessor._update_rules", autospec=True) as mock_update:
mock_update.reset_mock()
_appsec_rules_data({"rules": []}, tracer)
call = mock_update.mock_calls
args = call[-1][1][1]
assert "rules" in args
assert args["rules"], "empty rules should not be possible, it must switch to default."
Loading