Skip to content

Commit fe131d6

Browse files
jandro996devflow.devflow-routing-intake
authored and
TophrC-dd
committed
fix: report correct block outcome tag in rasp.rule.match metric (#11723)
fix: report correct block outcome tag in rasp.rule.match metric rasp.rule.match was always reporting block:N/A because raspRuleMatch() was called before the action loop; flow.isBlocking() is only valid after setAction() runs inside the loop. - Move raspRuleMatch() call to after the action loop (setRaspMatched stays in place) - Add boolean blocked param; double counter array for per-outcome tracking - Update prepareMetrics() to drain both blocked/non-blocked slots per RuleType - Add block: tag to both variant and non-variant RaspRuleMatch branches - Guard flow.setAction(rba) against null return from createBlockRequestAction/createRedirectRequestAction Merge branch 'master' into rasp-rule-match-fix Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 57112a7 commit fe131d6

3 files changed

Lines changed: 103 additions & 17 deletions

File tree

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/ddwaf/WAFModule.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,6 @@ public void onDataAvailable(
360360

361361
if (gwCtx.isRasp) {
362362
reqCtx.setRaspMatched(true);
363-
WafMetricCollector.get().raspRuleMatch(gwCtx.raspRuleType);
364363
}
365364

366365
String securityResponseId = null;
@@ -375,13 +374,17 @@ public void onDataAvailable(
375374
securityResponseId = (String) actionInfo.parameters.get("security_response_id");
376375
Flow.Action.RequestBlockingAction rba =
377376
createBlockRequestAction(actionInfo, reqCtx, gwCtx.isRasp, securityResponseId);
378-
flow.setAction(rba);
377+
if (rba != null) {
378+
flow.setAction(rba);
379+
}
379380
} else if ("redirect_request".equals(actionInfo.type)) {
380381
// Extract security_response_id from action parameters for use in triggers
381382
securityResponseId = (String) actionInfo.parameters.get("security_response_id");
382383
Flow.Action.RequestBlockingAction rba =
383384
createRedirectRequestAction(actionInfo, reqCtx, gwCtx.isRasp, securityResponseId);
384-
flow.setAction(rba);
385+
if (rba != null) {
386+
flow.setAction(rba);
387+
}
385388
} else if ("generate_stack".equals(actionInfo.type)) {
386389
if (Config.get().isAppSecStackTraceEnabled()) {
387390
String stackId = (String) actionInfo.parameters.get("stack_id");
@@ -417,6 +420,9 @@ public void onDataAvailable(
417420
}
418421
}
419422
}
423+
if (gwCtx.isRasp) {
424+
WafMetricCollector.get().raspRuleMatch(gwCtx.raspRuleType, flow.isBlocking());
425+
}
420426
Collection<AppSecEvent> events = buildEvents(resultWithData, securityResponseId);
421427
boolean isThrottled = reqCtx.isThrottled(rateLimiter);
422428

‎internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ private WafMetricCollector() {
4444
private static final AtomicLongArray raspRuleSkippedCounter =
4545
new AtomicLongArray(RuleType.getNumValues());
4646
private static final AtomicLongArray raspRuleMatchCounter =
47-
new AtomicLongArray(RuleType.getNumValues());
47+
new AtomicLongArray(RuleType.getNumValues() * 2);
4848
private static final AtomicLongArray raspTimeoutCounter =
4949
new AtomicLongArray(RuleType.getNumValues());
5050
private static final AtomicLongArray raspErrorCodeCounter =
@@ -154,8 +154,8 @@ public void raspRuleSkipped(final RuleType ruleType) {
154154
raspRuleSkippedCounter.incrementAndGet(ruleType.ordinal());
155155
}
156156

157-
public void raspRuleMatch(final RuleType ruleType) {
158-
raspRuleMatchCounter.incrementAndGet(ruleType.ordinal());
157+
public void raspRuleMatch(final RuleType ruleType, final boolean blocked) {
158+
raspRuleMatchCounter.incrementAndGet(ruleType.ordinal() * 2 + (blocked ? 1 : 0));
159159
}
160160

161161
public void raspTimeout(final RuleType ruleType) {
@@ -272,12 +272,20 @@ public void prepareMetrics() {
272272
}
273273
}
274274

275-
// RASP rule match per rule type
275+
// RASP rule match per rule type: two slots per RuleType: ordinal*2 (non-blocked),
276+
// ordinal*2+1 (blocked)
276277
for (RuleType ruleType : RuleType.values()) {
277-
long counter = raspRuleMatchCounter.getAndSet(ruleType.ordinal(), 0);
278-
if (counter > 0) {
278+
long blockedCount = raspRuleMatchCounter.getAndSet(ruleType.ordinal() * 2 + 1, 0);
279+
if (blockedCount > 0) {
279280
if (!rawMetricsQueue.offer(
280-
new RaspRuleMatch(counter, ruleType, WafMetricCollector.wafVersion))) {
281+
new RaspRuleMatch(blockedCount, ruleType, WafMetricCollector.wafVersion, true))) {
282+
return;
283+
}
284+
}
285+
long nonBlockedCount = raspRuleMatchCounter.getAndSet(ruleType.ordinal() * 2, 0);
286+
if (nonBlockedCount > 0) {
287+
if (!rawMetricsQueue.offer(
288+
new RaspRuleMatch(nonBlockedCount, ruleType, WafMetricCollector.wafVersion, false))) {
281289
return;
282290
}
283291
}
@@ -552,7 +560,11 @@ public AfterRequestRaspRuleSkipped(final long counter, final RuleType ruleType)
552560
}
553561

554562
public static class RaspRuleMatch extends WafMetric {
555-
public RaspRuleMatch(final long counter, final RuleType ruleType, final String wafVersion) {
563+
public RaspRuleMatch(
564+
final long counter,
565+
final RuleType ruleType,
566+
final String wafVersion,
567+
final boolean blocked) {
556568
super(
557569
"rasp.rule.match",
558570
counter,
@@ -561,9 +573,12 @@ public RaspRuleMatch(final long counter, final RuleType ruleType, final String w
561573
"rule_type:" + ruleType.type,
562574
"rule_variant:" + ruleType.variant,
563575
"waf_version:" + wafVersion,
564-
"event_rules_version:" + rulesVersion
576+
"event_rules_version:" + rulesVersion,
577+
"block:" + blocked
565578
}
566-
: new String[] {"rule_type:" + ruleType.type, "waf_version:" + wafVersion});
579+
: new String[] {
580+
"rule_type:" + ruleType.type, "waf_version:" + wafVersion, "block:" + blocked
581+
});
567582
}
568583
}
569584

‎internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy‎

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class WafMetricCollectorTest extends DDSpecification {
3737
WafMetricCollector.get().wafUpdates('rules.3', false)
3838
WafMetricCollector.get().raspRuleEval(RuleType.SQL_INJECTION)
3939
WafMetricCollector.get().raspRuleEval(RuleType.SQL_INJECTION)
40-
WafMetricCollector.get().raspRuleMatch(RuleType.SQL_INJECTION)
40+
WafMetricCollector.get().raspRuleMatch(RuleType.SQL_INJECTION, false)
4141
WafMetricCollector.get().raspRuleEval(RuleType.SQL_INJECTION)
4242
WafMetricCollector.get().raspTimeout(RuleType.SQL_INJECTION)
4343
WafMetricCollector.get().raspErrorCode(RuleType.SHELL_INJECTION, DD_WAF_RUN_INTERNAL_ERROR)
@@ -85,7 +85,7 @@ class WafMetricCollectorTest extends DDSpecification {
8585
raspRuleMatch.value == 1
8686
raspRuleMatch.namespace == 'appsec'
8787
raspRuleMatch.metricName == 'rasp.rule.match'
88-
raspRuleMatch.tags.toSet() == ['rule_type:sql_injection', 'waf_version:waf_ver1'].toSet()
88+
raspRuleMatch.tags.toSet() == ['rule_type:sql_injection', 'waf_version:waf_ver1', 'block:false'].toSet()
8989

9090
def raspTimeout = (WafMetricCollector.RaspTimeout) metrics[5]
9191
raspTimeout.type == 'count'
@@ -317,7 +317,7 @@ class WafMetricCollectorTest extends DDSpecification {
317317
WafMetricCollector.get().wafInit('waf_ver1', 'rules.1', true)
318318
WafMetricCollector.get().raspRuleEval(ruleType)
319319
WafMetricCollector.get().raspRuleEval(ruleType)
320-
WafMetricCollector.get().raspRuleMatch(ruleType)
320+
WafMetricCollector.get().raspRuleMatch(ruleType, false)
321321
WafMetricCollector.get().raspRuleEval(ruleType)
322322
WafMetricCollector.get().raspTimeout(ruleType)
323323
WafMetricCollector.get().raspErrorCode(ruleType, DD_WAF_RUN_INTERNAL_ERROR)
@@ -349,7 +349,8 @@ class WafMetricCollectorTest extends DDSpecification {
349349
'rule_type:command_injection',
350350
'rule_variant:' + ruleType.variant,
351351
'waf_version:waf_ver1',
352-
'event_rules_version:rules.1'
352+
'event_rules_version:rules.1',
353+
'block:false'
353354
].toSet()
354355

355356
def raspTimeout = (WafMetricCollector.RaspTimeout) metrics[3]
@@ -585,6 +586,70 @@ class WafMetricCollectorTest extends DDSpecification {
585586
type << [MESSAGES, CONTENT]
586587
}
587588

589+
void 'test rasp rule match block tag'() {
590+
given:
591+
final collector = WafMetricCollector.get()
592+
collector.wafInit('waf_ver1', 'rules.1', true)
593+
594+
when:
595+
collector.raspRuleMatch(ruleType, blocked)
596+
597+
then:
598+
collector.prepareMetrics()
599+
final metrics = collector.drain()
600+
final matchMetrics = metrics.findAll { it.metricName == 'rasp.rule.match' }
601+
602+
matchMetrics.size() == 1
603+
final metric = matchMetrics[0]
604+
metric.type == 'count'
605+
metric.value == 1
606+
metric.namespace == 'appsec'
607+
final expectedTags = ruleType.variant != null
608+
? [
609+
'rule_type:' + ruleType.type,
610+
'rule_variant:' + ruleType.variant,
611+
'waf_version:waf_ver1',
612+
'event_rules_version:rules.1',
613+
'block:' + blocked
614+
].toSet()
615+
: ['rule_type:' + ruleType.type, 'waf_version:waf_ver1', 'block:' + blocked].toSet()
616+
metric.tags.toSet() == expectedTags
617+
618+
where:
619+
ruleType | blocked
620+
RuleType.SQL_INJECTION | true
621+
RuleType.SQL_INJECTION | false
622+
RuleType.LFI | true
623+
RuleType.LFI | false
624+
RuleType.SSRF_REQUEST | true
625+
RuleType.SSRF_REQUEST | false
626+
RuleType.SSRF_RESPONSE | true
627+
RuleType.SSRF_RESPONSE | false
628+
RuleType.SHELL_INJECTION | true
629+
RuleType.SHELL_INJECTION | false
630+
RuleType.COMMAND_INJECTION | true
631+
RuleType.COMMAND_INJECTION | false
632+
}
633+
634+
void 'test rasp rule match drains blocked and non-blocked as separate metrics'() {
635+
given:
636+
final collector = WafMetricCollector.get()
637+
collector.wafInit('waf_ver1', 'rules.1', true)
638+
639+
when:
640+
collector.raspRuleMatch(RuleType.SQL_INJECTION, true)
641+
collector.raspRuleMatch(RuleType.SQL_INJECTION, false)
642+
643+
then:
644+
collector.prepareMetrics()
645+
final metrics = collector.drain()
646+
final matchMetrics = metrics.findAll { it.metricName == 'rasp.rule.match' }
647+
648+
matchMetrics.size() == 2
649+
matchMetrics.find { it.tags.contains('block:true') }?.value == 1
650+
matchMetrics.find { it.tags.contains('block:false') }?.value == 1
651+
}
652+
588653
/**
589654
* Helper method to generate all combinations of n boolean values.
590655
*/

0 commit comments

Comments
 (0)