diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java index aaa177335a7..dc450baeff1 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java @@ -320,15 +320,25 @@ private void handleWafUpdateResultReport(String configKey, Map r } } } catch (InvalidRuleSetException e) { - log.debug( + log.warn( "Invalid rule during waf config update for config key {}: {}", configKey, e.wafDiagnostics); if (e.wafDiagnostics.getNumConfigError() > 0) { WafMetricCollector.get().addWafConfigError(e.wafDiagnostics.getNumConfigError()); } - // TODO: Propagate diagostics back to remote config apply_error - + // TODO: Propagate diagnostics back to remote config apply_error + if (e.wafDiagnostics.rulesetVersion != null + && !e.wafDiagnostics.rulesetVersion.isEmpty() + && (!defaultConfigActivated || currentRuleVersion == null)) { + currentRuleVersion = e.wafDiagnostics.rulesetVersion; + if (!e.wafDiagnostics.rules.getLoaded().isEmpty()) { + statsReporter.setRulesVersion(currentRuleVersion); + } + if (modulesToUpdateVersionIn != null) { + modulesToUpdateVersionIn.forEach(module -> module.setRuleVersion(currentRuleVersion)); + } + } initReporter.setReportForPublication(e.wafDiagnostics); throw new RuntimeException(e); } catch (UnclassifiedWafException e) { diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy index e42b82a4b7d..cc650c3c7e6 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy @@ -1,5 +1,6 @@ package com.datadog.appsec.config +import com.datadog.appsec.AppSecModule import com.datadog.appsec.AppSecSystem import com.datadog.appsec.util.AbortStartupException import static datadog.remoteconfig.Capabilities.CAPABILITY_ASM_EXTENDED_DATA_COLLECTION @@ -703,6 +704,49 @@ class AppSecConfigServiceImplSpecification extends DDSpecification { noExceptionThrown() } + void 'currentRuleVersion is updated from diagnostics when InvalidRuleSetException is thrown on RC update'() { + setup: + final key = new ParsedConfigKey('Test', '1234', 1, 'ASM_DD', 'ID') + final service = new AppSecConfigServiceImpl(config, poller, reconf) + AppSecSystem.active = true + config.getAppSecActivation() >> ProductActivation.FULLY_ENABLED + AppSecModule wafModule = Mock() + wafModule.isWafBuilderSet() >> false + service.modulesToUpdateVersionIn([wafModule]) + + when: + service.maybeSubscribeConfigPolling() + + then: + 1 * poller.addListener(Product.ASM_DD, _) >> { + listeners.savedWafDataChangesListener = it[1] + } + + when: + listeners.savedWafDataChangesListener.accept( + key, + '''{ + "version": "2.2", + "metadata": {"rules_version": "1.99.0"}, + "rules": [{ + "id": "invalid-rule", + "name": "Invalid Rule", + "tags": {"type": "attack_attempt", "category": "attack_attempt"}, + "conditions": [{ + "operator": "invalid_operator", + "parameters": {"inputs": [{"address": "server.request.query"}]} + }] + }] + }'''.getBytes(), + NOOP) + + then: + thrown RuntimeException + service.getCurrentRuleVersion() == '1.99.0' + 1 * wafModule.setWafBuilder(_) + 1 * wafModule.setRuleVersion('1.99.0') + } + void 'config keys are added and removed to the set when receiving ASM_DD payloads'() { setup: final key = new ParsedConfigKey('Test', '1234', 1, 'ASM_DD', 'ID') diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java index 032212a7f1a..5d6914a4b1e 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java @@ -33,9 +33,6 @@ private WafMetricCollector() { private static final BlockingQueue rawMetricsQueue = new ArrayBlockingQueue<>(RAW_QUEUE_SIZE); - private static final AtomicInteger wafInitCounter = new AtomicInteger(); - private static final AtomicInteger wafUpdatesCounter = new AtomicInteger(); - private static final int WAF_REQUEST_COMBINATIONS = 128; // 2^7 private final AtomicLongArray wafRequestCounter = new AtomicLongArray(WAF_REQUEST_COMBINATIONS); @@ -80,14 +77,11 @@ private WafMetricCollector() { public void wafInit(final String wafVersion, final String rulesVersion, final boolean success) { WafMetricCollector.wafVersion = wafVersion; WafMetricCollector.rulesVersion = rulesVersion; - rawMetricsQueue.offer( - new WafInitRawMetric(wafInitCounter.incrementAndGet(), wafVersion, rulesVersion, success)); + rawMetricsQueue.offer(new WafInitRawMetric(1L, wafVersion, rulesVersion, success)); } public void wafUpdates(final String rulesVersion, final boolean success) { - rawMetricsQueue.offer( - new WafUpdatesRawMetric( - wafUpdatesCounter.incrementAndGet(), wafVersion, rulesVersion, success)); + rawMetricsQueue.offer(new WafUpdatesRawMetric(1L, wafVersion, rulesVersion, success)); // Flush request metrics to get the new version. if (rulesVersion != null diff --git a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy index 251bdb4baef..0ee8ed04e6e 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy @@ -67,7 +67,7 @@ class WafMetricCollectorTest extends DDSpecification { def updateMetric2 = (WafMetricCollector.WafUpdatesRawMetric) metrics[2] updateMetric2.type == 'count' - updateMetric2.value == 2 + updateMetric2.value == 1 updateMetric2.namespace == 'appsec' updateMetric2.metricName == 'waf.updates' updateMetric2.tags.toSet() == ['waf_version:waf_ver1', 'event_rules_version:rules.3', 'success:false'].toSet() diff --git a/telemetry/src/test/groovy/datadog/telemetry/metric/WafMetricPeriodicActionSpecification.groovy b/telemetry/src/test/groovy/datadog/telemetry/metric/WafMetricPeriodicActionSpecification.groovy index 4d5459edcae..b14a10e431f 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/metric/WafMetricPeriodicActionSpecification.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/metric/WafMetricPeriodicActionSpecification.groovy @@ -34,7 +34,7 @@ class WafMetricPeriodicActionSpecification extends DDSpecification { 1 * telemetryService.addMetric( { Metric metric -> metric.namespace == 'appsec' && metric.metric == 'waf.updates' && - metric.points[0][1] == 2 && + metric.points[0][1] == 1 && metric.tags == ['waf_version:0.0.0', 'event_rules_version:rules_ver_3', 'success:true'] } ) 0 * _._