Appsec: create top span for process executions

dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie

@@ -42,6 +42,8 @@
 1 java.*
 # allow exception profiling instrumentation
 0 java.lang.Throwable
+# allow ProcessImpl instrumentation
+0 java.lang.ProcessImpl
 0 java.net.HttpURLConnection
 0 java.net.URL
 0 java.nio.DirectByteBuffer

dd-java-agent/instrumentation/java-lang/java-lang-9/build.gradle

@@ -37,5 +37,8 @@ project.tasks.withType(AbstractCompile).configureEach {
     sourceCompatibility = JavaVersion.VERSION_1_9
     targetCompatibility = JavaVersion.VERSION_1_9
     setJavaVersion(it, 11)
+    if (it instanceof JavaCompile) {
+      it.options.release.set(9)
+    }
   }
 }

dd-java-agent/instrumentation/java-lang/java-lang.gradle

@@ -0,0 +1,11 @@
+ext {
+  minJavaVersionForTests = JavaVersion.VERSION_1_8
+}
+
+muzzle {
+  pass {
+    coreJdk()
+  }
+}
+
+apply from: "$rootDir/gradle/java.gradle"

dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentation.java

@@ -0,0 +1,42 @@
+package datadog.trace.instrumentation.java.lang;
+
+import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.api.Platform;
+import java.util.Map;
+
+@AutoService(Instrumenter.class)
+public class ProcessImplInstrumentation extends Instrumenter.Tracing
+    implements Instrumenter.ForSingleType, Instrumenter.ForBootstrap {
+
+  public ProcessImplInstrumentation() {
+    super("java-lang-appsec");
+  }
+
+  @Override
+  public String instrumentedType() {
+    return "java.lang.ProcessImpl";
+  }
+
+  @Override
+  public boolean isEnabled() {
+    return Platform.isJavaVersionAtLeast(8) && super.isEnabled();
+  }
+
+  @Override
+  public void adviceTransformations(AdviceTransformation transformation) {
+    transformation.applyAdvice(
+        named("start")
+            .and(
+                takesArguments(
+                    String[].class,
+                    Map.class,
+                    String.class,
+                    ProcessBuilder.Redirect[].class,
+                    boolean.class)),
+        packageName + ".ProcessImplStartAdvice");
+  }
+}

dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/ProcessImplStartAdvice.java

@@ -0,0 +1,47 @@
+package datadog.trace.instrumentation.java.lang;
+
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
+import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
+import datadog.trace.bootstrap.instrumentation.api.TagContext;
+import datadog.trace.bootstrap.instrumentation.api.java.lang.ProcessImplInstrumentationHelpers;
+import java.io.IOException;
+import java.util.Map;
+import net.bytebuddy.asm.Advice;
+
+class ProcessImplStartAdvice {
+  @Advice.OnMethodEnter(suppress = Throwable.class)
+  public static AgentSpan startSpan(@Advice.Argument(0) final String[] command) throws IOException {
+    if (!ProcessImplInstrumentationHelpers.ONLINE) {
+      return null;
+    }
+
+    if (command.length == 0) {
+      return null;
+    }
+
+    AgentTracer.TracerAPI tracer = AgentTracer.get();
+
+    Map<String, String> tags = ProcessImplInstrumentationHelpers.createTags(command);
+    TagContext tagContext = new TagContext("appsec", tags);
+    AgentSpan span = tracer.startSpan("command_execution", tagContext);
+    span.setSpanType("system");
+    span.setResourceName(ProcessImplInstrumentationHelpers.determineResource(command));
+    return span;
+  }
+
+  @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
+  public static void endSpan(
+      @Advice.Return Process p, @Advice.Enter AgentSpan span, @Advice.Thrown Throwable t) {
+    if (span == null) {
+      return;
+    }
+    if (t != null) {
+      span.setError(true);
+      span.setErrorMessage(t.getMessage());
+      span.finish();
+      return;
+    }
+
+    ProcessImplInstrumentationHelpers.addProcessCompletionHook(p, span);
+  }
+}

dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessBuilderCallSiteTest.groovy

@@ -25,6 +25,7 @@ class ProcessBuilderCallSiteTest extends AgentTestRunner {
     then:
     thrown(IOException)
     1 * iastModule.onProcessBuilderStart(command)
+    _ * TEST_CHECKPOINTER._
     0 * _
   }
 }

dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationSpecification.groovy

@@ -0,0 +1,165 @@
+package datadog.trace.instrumentation.java.lang
+
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.gateway.RequestContextSlot
+import datadog.trace.bootstrap.ActiveSubsystems
+import datadog.trace.core.DDSpan
+import spock.lang.Requires
+
+import java.util.concurrent.TimeUnit
+
+import static datadog.trace.api.Platform.isJavaVersionAtLeast
+
+@Requires({
+  isJavaVersionAtLeast(8)
+})
+class ProcessImplInstrumentationSpecification extends AgentTestRunner {
+  def ss = TEST_TRACER.getSubscriptionService(RequestContextSlot.APPSEC)
+
+  void cleanup() {
+    ss.reset()
+  }
+
+  void 'creates a span in a normal case'() {
+    when:
+    def builder = new ProcessBuilder('/bin/sh', '-c', 'echo 42')
+    Process p = builder.start()
+    String output = p.inputStream.text
+    TEST_WRITER.waitForTraces(1)
+    DDSpan span = TEST_WRITER[0][0]
+
+    then:
+    output == "42\n"
+    span.tags['cmd.exec'] == '["/bin/sh","-c","echo 42"]'
+    span.tags['cmd.exit_code'] == 0
+    span.spanType == 'system'
+    span.resourceName == 'sh'
+    span.spanName == 'command_execution'
+  }
+
+  void 'span only has executable if appsec is disabled'() {
+    setup:
+    ActiveSubsystems.APPSEC_ACTIVE = false
+
+    when:
+    def builder = new ProcessBuilder('/bin/sh', '-c', 'echo 42')
+    Process p = builder.start()
+    Thread.start { p.inputStream.text }
+    p.waitFor(5, TimeUnit.SECONDS)
+    TEST_WRITER.waitForTraces(1)
+    DDSpan span = TEST_WRITER[0][0]
+
+    then:
+    span.tags['cmd.exec'] == '["/bin/sh"]'
+  }
+
+  void 'variant with Runtime exec'() {
+    when:
+    Process p = Runtime.runtime.exec('/bin/sh -c true')
+    p.waitFor(5, TimeUnit.SECONDS)
+    TEST_WRITER.waitForTraces(1)
+    DDSpan span = TEST_WRITER[0][0]
+
+    then:
+    span.tags['cmd.exec'] == '["/bin/sh","-c","true"]'
+  }
+
+  void 'the exit code is correctly reported'() {
+    when:
+    def builder = new ProcessBuilder('/bin/sh', '-c', 'exit 33')
+    Process p = builder.start()
+    p.waitFor(5, TimeUnit.SECONDS)
+    TEST_WRITER.waitForTraces(1)
+    DDSpan span = TEST_WRITER[0][0]
+
+    then:
+    span.tags['cmd.exit_code'] == 33
+  }
+
+  void 'can handle waiting on another thread'() {
+    when:
+    // sleep a bit so that it doesn't all happen on the same thread
+    def builder = new ProcessBuilder('/bin/sh', '-c', 'sleep 0.5; echo 42')
+    Process p = builder.start()
+    def out
+    Thread.start {
+      out = p.inputStream.text
+      p.waitFor()
+    }.join(5000)
+    TEST_WRITER.waitForTraces(1)
+    DDSpan span = TEST_WRITER[0][0]
+
+    then:
+    out == '42\n'
+    span.getDurationNano() >= 500_000_000 // 500 ms (we sleep for 0.5 s)
+    span.tags['cmd.exit_code'] == 0
+  }
+
+  void 'command cannot be executed'() {
+    when:
+    def builder = new ProcessBuilder('/bin/does-not-exist')
+    builder.start()
+
+    then:
+    thrown IOException
+
+    when:
+    TEST_WRITER.waitForTraces(1)
+    DDSpan span = TEST_WRITER[0][0]
+
+    then:
+    span.tags['cmd.exec'] == '["/bin/does-not-exist"]'
+    span.tags['error.msg'] != null
+    span.isError() == true
+  }
+
+  void 'process is destroyed'() {
+    when:
+    def builder = new ProcessBuilder('/bin/sh', '-c', 'sleep 3600')
+    Process p = builder.start()
+    Thread.start {
+      p.destroy()
+    }
+    p.waitFor(5, TimeUnit.SECONDS)
+    TEST_WRITER.waitForTraces(1)
+    DDSpan span = TEST_WRITER[0][0]
+
+    then:
+    span.tags['cmd.exit_code'] != 0
+  }
+
+  void 'command is truncated'() {
+    when:
+    def builder = new ProcessBuilder('/bin/sh', '-c', 'echo ' + ('a' * (4096 - 14 + 1)))
+    Process p = builder.start()
+    Thread.start { p.inputStream.text }
+    p.waitFor(5, TimeUnit.SECONDS)
+    TEST_WRITER.waitForTraces(1)
+    DDSpan span = TEST_WRITER[0][0]
+
+    then:
+    span.tags['cmd.truncated'] == 'true'
+    span.tags['cmd.exec'] == '["/bin/sh","-c"]'
+  }
+
+  void redactions() {
+    when:
+    def builder = new ProcessBuilder(command)
+    builder.start()
+
+    then:
+    thrown IOException
+
+    when:
+    TEST_WRITER.waitForTraces(1)
+    DDSpan span = TEST_WRITER[0][0]
+
+    then:
+    span.tags['cmd.exec'] == expected
+
+    where:
+    command | expected
+    ['cmd', '--pass', 'abc', '--token=def'] | '["cmd","--pass","?","--token=?"]'
+    ['/does/not/exist/md5', '-s', 'pony'] | '["/does/not/exist/md5","?","?"]'
+  }
+}

dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy

@@ -57,6 +57,7 @@ class StringBuilderCallSiteTest extends AgentTestRunner {
     } else {
       1 * iastModule.onStringBuilderAppend(target, param)
     }
+    _ * TEST_CHECKPOINTER._
     0 * _
 
     where:
@@ -93,6 +94,7 @@ class StringBuilderCallSiteTest extends AgentTestRunner {
     1 * iastModule.onStringBuilderAppend(_ as StringBuilder, 'Hello ')
     1 * iastModule.onStringBuilderAppend(_ as StringBuilder, 'World!')
     1 * iastModule.onStringBuilderToString(_ as StringBuilder, 'Hello World!')
+    _ * TEST_CHECKPOINTER._
     0 * _
   }
 
@@ -134,6 +136,7 @@ class StringBuilderCallSiteTest extends AgentTestRunner {
 
     then:
     1 * iastModule.onStringBuilderAppend(_, 'Hello')
+    _ * TEST_CHECKPOINTER._
     0 * _
     final ex = thrown(NuclearException)
     ex.stackTrace.find {it.className == StringBuilderCallSite.name } == null

dd-smoke-tests/springboot-openliberty/src/test/groovy/datadog/smoketest/SpringBootOpenLibertySnapshotTest.groovy

@@ -21,6 +21,7 @@ class SpringBootOpenLibertySnapshotTest extends AbstractTestAgentSmokeTest {
     command.addAll(defaultJavaProperties)
     command.addAll((String[]) [
       "-Ddd.jmxfetch.enabled=false",
+      '-Ddd.trace.integration.java-lang-appsec.enabled=false',
       "-jar",
       openLibertyShadowJar,
       "--server.port=${httpPort}"

gradle/forbiddenApiFilters/main.txt

@@ -2,7 +2,6 @@
 java.lang.Class#forName(java.lang.String)
 
 # String methods which uses regexes for matching
-java.lang.String#replace(java.lang.CharSequence,java.lang.CharSequence)
 java.lang.String#split(java.lang.String)
 java.lang.String#split(java.lang.String,int)
 java.lang.String#replaceAll(java.lang.String,java.lang.String)

internal-api/internal-api-8/build.gradle

@@ -23,7 +23,10 @@ targetCompatibility = JavaVersion.VERSION_1_8
 minimumBranchCoverage = 0.8
 minimumInstructionCoverage = 0.8
 
-excludedClassesCoverage += ["datadog.trace.api.sampling.ConstantSampler"]
+excludedClassesCoverage += [
+  'datadog.trace.api.sampling.ConstantSampler',
+  'datadog.trace.bootstrap.instrumentation.api.java.lang.ProcessImplInstrumentationHelpers',
+]
 
 excludedClassesBranchCoverage = [
   'datadog.trace.util.stacktrace.HotSpotStackWalker',