Add more tests

Author: manuel-alvarez-alvarez

dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java

@@ -102,11 +102,12 @@ public class AppSecConfigServiceImpl implements AppSecConfigService {
   private final AtomicBoolean subscribedToRulesAndData = new AtomicBoolean();
   private final Set<String> usedDDWafConfigKeys =
       Collections.newSetFromMap(new ConcurrentHashMap<>());
-  private final Set<String> emptyConfigKeys = Collections.newSetFromMap(new ConcurrentHashMap<>());
+  private final Set<String> ignoredConfigKeys =
+      Collections.newSetFromMap(new ConcurrentHashMap<>());
   private final String DEFAULT_WAF_CONFIG_RULE = "DEFAULT_WAF_CONFIG";
   private String currentRuleVersion;
   private List<AppSecModule> modulesToUpdateVersionIn;
-  private long rulesAndDataCapabilities;
+  private long rulesAndDataCapabilities = -1L;
 
   public AppSecConfigServiceImpl(
       Config tracerConfig,
@@ -135,32 +136,34 @@ private void subscribeConfigurationPoller() {
   }
 
   private long buildRulesAndDataCapabilities() {
-    long capabilities =
-        CAPABILITY_ASM_DD_RULES
-            | CAPABILITY_ASM_IP_BLOCKING
-            | CAPABILITY_ASM_EXCLUSIONS
-            | CAPABILITY_ASM_EXCLUSION_DATA
-            | CAPABILITY_ASM_REQUEST_BLOCKING
-            | CAPABILITY_ASM_USER_BLOCKING
-            | CAPABILITY_ASM_CUSTOM_RULES
-            | CAPABILITY_ASM_CUSTOM_BLOCKING_RESPONSE
-            | CAPABILITY_ASM_TRUSTED_IPS
-            | CAPABILITY_ENDPOINT_FINGERPRINT
-            | CAPABILITY_ASM_SESSION_FINGERPRINT
-            | CAPABILITY_ASM_NETWORK_FINGERPRINT
-            | CAPABILITY_ASM_HEADER_FINGERPRINT;
-    if (tracerConfig.isAppSecRaspEnabled()) {
-      capabilities |= CAPABILITY_ASM_RASP_SQLI;
-      capabilities |= CAPABILITY_ASM_RASP_SSRF;
-      capabilities |= CAPABILITY_ASM_RASP_CMDI;
-      capabilities |= CAPABILITY_ASM_RASP_SHI;
-      // RASP LFI is only available in fully enabled mode as it's implemented using callsite
-      // instrumentation
-      if (tracerConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED) {
-        capabilities |= CAPABILITY_ASM_RASP_LFI;
+    if (rulesAndDataCapabilities == -1) {
+      rulesAndDataCapabilities =
+          CAPABILITY_ASM_DD_RULES
+              | CAPABILITY_ASM_IP_BLOCKING
+              | CAPABILITY_ASM_EXCLUSIONS
+              | CAPABILITY_ASM_EXCLUSION_DATA
+              | CAPABILITY_ASM_REQUEST_BLOCKING
+              | CAPABILITY_ASM_USER_BLOCKING
+              | CAPABILITY_ASM_CUSTOM_RULES
+              | CAPABILITY_ASM_CUSTOM_BLOCKING_RESPONSE
+              | CAPABILITY_ASM_TRUSTED_IPS
+              | CAPABILITY_ENDPOINT_FINGERPRINT
+              | CAPABILITY_ASM_SESSION_FINGERPRINT
+              | CAPABILITY_ASM_NETWORK_FINGERPRINT
+              | CAPABILITY_ASM_HEADER_FINGERPRINT;
+      if (tracerConfig.isAppSecRaspEnabled()) {
+        rulesAndDataCapabilities |= CAPABILITY_ASM_RASP_SQLI;
+        rulesAndDataCapabilities |= CAPABILITY_ASM_RASP_SSRF;
+        rulesAndDataCapabilities |= CAPABILITY_ASM_RASP_CMDI;
+        rulesAndDataCapabilities |= CAPABILITY_ASM_RASP_SHI;
+        // RASP LFI is only available in fully enabled mode as it's implemented using callsite
+        // instrumentation
+        if (tracerConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED) {
+          rulesAndDataCapabilities |= CAPABILITY_ASM_RASP_LFI;
+        }
       }
     }
-    return capabilities;
+    return rulesAndDataCapabilities;
   }
 
   private void updateRulesAndDataSubscription() {
@@ -179,7 +182,7 @@ private void subscribeRulesAndData() {
       this.configurationPoller.addListener(Product.ASM_DD, new AppSecConfigChangesDDListener());
       this.configurationPoller.addListener(Product.ASM_DATA, new AppSecConfigChangesListener());
       this.configurationPoller.addListener(Product.ASM, new AppSecConfigChangesListener());
-      this.configurationPoller.addCapabilities(rulesAndDataCapabilities);
+      this.configurationPoller.addCapabilities(buildRulesAndDataCapabilities());
     }
   }
 
@@ -188,7 +191,7 @@ private void unsubscribeRulesAndData() {
       this.configurationPoller.removeListeners(Product.ASM_DD);
       this.configurationPoller.removeListeners(Product.ASM_DATA);
       this.configurationPoller.removeListeners(Product.ASM);
-      this.configurationPoller.removeCapabilities(rulesAndDataCapabilities);
+      this.configurationPoller.removeCapabilities(buildRulesAndDataCapabilities());
     }
   }
 
@@ -204,35 +207,39 @@ private class AppSecConfigChangesListener implements ProductListener {
     @Override
     public void accept(ConfigKey configKey, byte[] content, PollingRateHinter pollingRateHinter)
         throws IOException {
-      maybeInitializeDefaultConfig();
       final String key = configKey.toString();
       if (content == null) {
-        if (!emptyConfigKeys.remove(key)) {
-          try {
-            wafBuilder.removeConfig(key);
-          } catch (UnclassifiedWafException e) {
-            throw new RuntimeException(e);
-          }
-        }
+        remove(configKey, pollingRateHinter);
+        return;
+      }
+      Map<String, Object> contentMap =
+          ADAPTER.fromJson(Okio.buffer(Okio.source(new ByteArrayInputStream(content))));
+      if (contentMap == null || contentMap.isEmpty()) {
+        ignoredConfigKeys.add(key);
       } else {
-        Map<String, Object> contentMap =
-            ADAPTER.fromJson(Okio.buffer(Okio.source(new ByteArrayInputStream(content))));
-        if (contentMap == null || contentMap.isEmpty()) {
-          emptyConfigKeys.add(key);
-        } else {
-          try {
-            handleWafUpdateResultReport(key, contentMap);
-          } catch (AppSecModule.AppSecModuleActivationException e) {
-            throw new RuntimeException(e);
-          }
+        ignoredConfigKeys.remove(key);
+        try {
+          maybeInitializeDefaultConfig();
+          handleWafUpdateResultReport(key, contentMap);
+        } catch (AppSecModule.AppSecModuleActivationException e) {
+          throw new RuntimeException(e);
         }
       }
     }
 
     @Override
     public void remove(ConfigKey configKey, PollingRateHinter pollingRateHinter)
         throws IOException {
-      accept(configKey, null, pollingRateHinter);
+      final String key = configKey.toString();
+      if (ignoredConfigKeys.remove(key)) {
+        return;
+      }
+      try {
+        maybeInitializeDefaultConfig();
+        wafBuilder.removeConfig(key);
+      } catch (UnclassifiedWafException e) {
+        throw new RuntimeException(e);
+      }
     }
 
     @Override
@@ -375,7 +382,6 @@ public void init() {
     }
     this.mergedAsmFeatures.clear();
     this.usedDDWafConfigKeys.clear();
-    this.emptyConfigKeys.clear();
     this.rulesAndDataCapabilities = buildRulesAndDataCapabilities();
 
     if (wafConfig.isEmpty()) {

dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/config/AppSecConfigServiceImplSpecification.groovy

@@ -715,6 +715,41 @@ class AppSecConfigServiceImplSpecification extends DDSpecification {
     service.usedDDWafConfigKeys.empty
   }
 
+  void 'test that empty configurations are acknowledged'() {
+    given:
+    final key = new ParsedConfigKey('Test', '1234', 1, 'ASM_DD', 'ID')
+
+    when:
+    AppSecSystem.active = true
+    config.getAppSecActivation() >> ProductActivation.FULLY_ENABLED
+    final service = new AppSecConfigServiceImpl(config, poller, reconf)
+    service.init()
+    service.maybeSubscribeConfigPolling()
+
+    then:
+    1 * poller.addListener(Product.ASM_DATA, _) >> {
+      listeners.savedWafDataChangesListener = it[1]
+    }
+    1 * poller.addConfigurationEndListener(_) >> {
+      listeners.savedConfEndListener = it[0]
+    }
+
+    when:
+    listeners.savedWafDataChangesListener.accept(key, '{}'.bytes, NOOP)
+    listeners.savedConfEndListener.onConfigurationEnd()
+
+    then:
+    noExceptionThrown()
+
+    when:
+    listeners.savedWafDataChangesListener.accept(key, null, NOOP)
+    listeners.savedConfEndListener.onConfigurationEnd()
+
+    then:
+    noExceptionThrown()
+  }
+
+
   private static AppSecFeatures autoUserInstrum(String mode) {
     return new AppSecFeatures().tap { features ->
       features.autoUserInstrum = new AppSecFeatures.AutoUserInstrum().tap { instrum ->

dd-smoke-tests/dynamic-config/src/main/java/datadog/smoketest/dynamicconfig/AppSecApplication.java

@@ -4,7 +4,7 @@
 
 public class AppSecApplication {
 
-  public static final long TIMEOUT_IN_SECONDS = 10;
+  public static final long TIMEOUT_IN_SECONDS = 15;
 
   public static void main(String[] args) throws InterruptedException {
     // just wait as we want to test RC payloads

dd-smoke-tests/dynamic-config/src/test/groovy/datadog/smoketest/AppSecActivationSmokeTest.groovy

@@ -1,7 +1,8 @@
 package datadog.smoketest
 
+import datadog.remoteconfig.Capabilities
+import datadog.remoteconfig.Product
 import datadog.smoketest.dynamicconfig.AppSecApplication
-import datadog.trace.test.util.Flaky
 
 class AppSecActivationSmokeTest extends AbstractSmokeTest {
 
@@ -23,22 +24,55 @@ class AppSecActivationSmokeTest extends AbstractSmokeTest {
     processBuilder.directory(new File(buildDirectory))
   }
 
-  @Flaky
-  void 'test activation config change is sent via RC'() {
-    when:
+  void 'test activation via RC workflow'() {
+    given:
+    final asmRuleProducts = [Product.ASM, Product.ASM_DD, Product.ASM_DATA]
+
+    when: 'appsec is enabled but inactive'
+    final request = waitForRcClientRequest {req ->
+      decodeProducts(req).find { asmRuleProducts.contains(it) } == null
+    }
+    final capabilities = decodeCapabilities(request)
+
+    then: 'only ASM_ACTIVATION capability should be reported'
+    assert hasCapability(capabilities, Capabilities.CAPABILITY_ASM_ACTIVATION)
+    assert !hasCapability(capabilities, Capabilities.CAPABILITY_ASM_CUSTOM_RULES)
+
+    when: 'appsec is enabled via RC'
     setRemoteConfig('datadog/2/ASM_FEATURES/asm_features_activation/config', '{"asm":{"enabled":true}}')
 
-    then:
+    then: 'we should receive a product change for appsec'
     waitForTelemetryFlat {
-      if (it['request_type'] != 'app-client-configuration-change') {
-        return false
-      }
       final configurations = (List<Map<String, Object>>) it?.payload?.configuration ?: []
       final enabledConfig = configurations.find { it.name == 'appsec_enabled' }
       if (!enabledConfig) {
         return false
       }
       return enabledConfig.value == 'true' && enabledConfig .origin == 'remote_config'
     }
+
+    and: 'we should have set the capabilities for ASM rules and data'
+    final newRequest = waitForRcClientRequest {req ->
+      decodeProducts(req).containsAll(asmRuleProducts)
+    }
+    final newCapabilities = decodeCapabilities(newRequest)
+    assert hasCapability(newCapabilities, Capabilities.CAPABILITY_ASM_CUSTOM_RULES)
+  }
+
+  private static Set<Product> decodeProducts(final Map<String, Object> request) {
+    return request.client.products.collect { Product.valueOf(it)}
+  }
+
+  private static long decodeCapabilities(final Map<String, Object> request) {
+    final clientCapabilities = request.client.capabilities as byte[]
+    long capabilities = 0l
+    for (int i = 0; i < clientCapabilities.length; i++) {
+      capabilities |= (clientCapabilities[i] & 0xFFL) << ((clientCapabilities.length - i - 1) * 8)
+    }
+    return capabilities
+  }
+
+  private static boolean hasCapability(final long capabilities, final long test) {
+    return (capabilities & test) > 0
   }
 }

dd-smoke-tests/src/main/groovy/datadog/smoketest/AbstractSmokeTest.groovy

@@ -50,6 +50,12 @@ abstract class AbstractSmokeTest extends ProcessManager {
   @Shared
   protected TestHttpServer.Headers lastTraceRequestHeaders = null
 
+  @Shared
+  protected CopyOnWriteArrayList<Map<String, Object>> rcClientMessages = new CopyOnWriteArrayList()
+
+  @Shared
+  private Throwable rcClientDecodingFailure = null
+
   @Shared
   protected final PollingConditions defaultPoll = new PollingConditions(timeout: 30, initialDelay: 0, delay: 1, factor: 1)
 
@@ -129,6 +135,10 @@ abstract class AbstractSmokeTest extends ProcessManager {
         response.status(200).send()
       }
       prefix("/v0.7/config") {
+        if (request.getBody() != null) {
+          final msg = new JsonSlurper().parseText(new String(request.getBody(), StandardCharsets.UTF_8)) as Map<String, Object>
+          rcClientMessages.add(msg)
+        }
         response.status(200).send(remoteConfigResponse)
       }
       prefix("/telemetry/proxy/api/v2/apmtelemetry") {
@@ -349,6 +359,21 @@ abstract class AbstractSmokeTest extends ProcessManager {
     }
   }
 
+  Map<String, Object> waitForRcClientRequest(final Function<Map<String, Object>, Boolean> predicate) {
+    waitForRcClientRequest(defaultPoll, predicate)
+  }
+
+  Map<String, Object> waitForRcClientRequest(final PollingConditions poll, final Function<Map<String, Object>, Boolean> predicate) {
+    def message = null
+    poll.eventually {
+      if (rcClientDecodingFailure != null) {
+        throw rcClientDecodingFailure
+      }
+      assert (message = rcClientMessages.find { predicate.apply(it) }) != null
+    }
+    return message
+  }
+
   List<DecodedTrace> getTraces() {
     decodeTraces
   }