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

@@ -44,6 +44,8 @@
 0 java.security.MessageDigest
 # 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.gradle

@@ -0,0 +1,18 @@
+ext {
+  minJavaVersionForTests = JavaVersion.VERSION_1_8
+}
+
+muzzle {
+  pass {
+    coreJdk()
+  }
+}
+
+apply from: "$rootDir/gradle/java.gradle"
+
+tasks.withType(Test).configureEach {
+  jvmArgs += ['-Dnet.bytebuddy.dump=/tmp/aa']
+}
+
+dependencies {
+}

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

@@ -0,0 +1,47 @@
+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.AppSec
+    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 String[] helperClassNames() {
+    return new String[] {packageName + ".ProcessImplInstrumentationHelpers"};
+  }
+
+  @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/java8/datadog/trace/instrumentation/java/lang/ProcessImplInstrumentationHelpers.java

@@ -0,0 +1,111 @@
+package datadog.trace.instrumentation.java.lang;
+
+import static java.lang.invoke.MethodType.methodType;
+
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Field;
+import java.lang.reflect.UndeclaredThrowableException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Executor;
+
+public class ProcessImplInstrumentationHelpers {
+  private static final int LIMIT = 4096;
+  public static final boolean ONLINE;
+  private static final MethodHandle PROCESS_ON_EXIT;
+  private static final Executor EXECUTOR;
+
+  static {
+    MethodHandle processOnExit = null;
+    Executor executor = null;
+    try {
+      // java 9
+      processOnExit =
+          MethodHandles.publicLookup()
+              .findVirtual(Process.class, "onExit", methodType(CompletableFuture.class));
+    } catch (Throwable e) {
+      try {
+        // java 8
+        Class<?> unixProcessCls =
+            ClassLoader.getSystemClassLoader().loadClass("java.lang.UNIXProcess");
+        Field f = unixProcessCls.getDeclaredField("processReaperExecutor");
+        f.setAccessible(true);
+        executor = (Executor) f.get(null);
+      } catch (ClassNotFoundException | NoSuchFieldException | IllegalAccessException ex) {
+      }
+    }
+    PROCESS_ON_EXIT = processOnExit;
+    EXECUTOR = executor;
+    ONLINE = PROCESS_ON_EXIT != null || EXECUTOR != null;
+  }
+
+  private ProcessImplInstrumentationHelpers() {}
+
+  public static Map<String, String> createTags(String[] command) {
+    Map<String, String> ret = new HashMap<>(4);
+    StringBuilder sb = new StringBuilder("[");
+    long remaining = LIMIT;
+    for (int i = 0; i < command.length; i++) {
+      String cur = command[i];
+      remaining -= cur.length();
+      if (remaining < 0) {
+        ret.put("cmd.truncated", "true");
+        break;
+      }
+      if (i != 0) {
+        sb.append(',');
+      }
+      sb.append('"');
+      sb.append(cur.replace("\\", "\\\\").replace("\"", "\\\""));
+      sb.append('"');
+    }
+    sb.append("]");
+    ret.put("cmd.exec", sb.toString());
+    return ret;
+  }
+
+  public static void addProcessCompletionHook(Process p, AgentSpan span) {
+    if (PROCESS_ON_EXIT != null) {
+      CompletableFuture<Process> future;
+      try {
+        future = (CompletableFuture<Process>) PROCESS_ON_EXIT.invokeExact(p);
+      } catch (Throwable e) {
+        if (e instanceof Error) {
+          throw (Error) e;
+        } else if (e instanceof RuntimeException) {
+          throw (RuntimeException) e;
+        } else {
+          throw new UndeclaredThrowableException(e);
+        }
+      }
+
+      future.whenComplete(
+          (process, thr) -> {
+            span.finishThreadMigration();
+            if (thr != null) {
+              span.setError(true);
+              span.setErrorMessage(thr.getMessage());
+            } else {
+              span.setTag("cmd.exit_code", process.exitValue());
+            }
+            span.finish();
+          });
+    } else if (EXECUTOR != null) {
+      EXECUTOR.execute(
+          () -> {
+            try {
+              int exitCode = p.waitFor();
+              span.finishThreadMigration();
+              span.setTag("cmd.exit_code", exitCode);
+            } catch (InterruptedException e) {
+              span.setError(true);
+              span.setErrorMessage(e.getMessage());
+            }
+            span.finish();
+          });
+    }
+  }
+}

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

@@ -0,0 +1,64 @@
+package datadog.trace.instrumentation.java.lang;
+
+import datadog.trace.api.function.Supplier;
+import datadog.trace.api.gateway.CallbackProvider;
+import datadog.trace.api.gateway.Events;
+import datadog.trace.api.gateway.Flow;
+import datadog.trace.api.gateway.RequestContextSlot;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
+import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
+import datadog.trace.bootstrap.instrumentation.api.TagContext;
+import java.io.Closeable;
+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;
+    }
+
+    AgentTracer.TracerAPI tracer = AgentTracer.get();
+
+    // hacky/expensive way to determine if appsec is active (AppSecSystem.ACTIVE is not accessible)
+    CallbackProvider cbp = tracer.getCallbackProvider(RequestContextSlot.APPSEC);
+    if (cbp == null) {
+      return null;
+    }
+    Supplier<Flow<Object>> callbackSupplier = cbp.getCallback(Events.EVENTS.requestStarted());
+    if (callbackSupplier == null) {
+      return null;
+    }
+    Object result = callbackSupplier.get().getResult();
+    if (!(result instanceof Closeable)) {
+      return null;
+    }
+    ((Closeable) result).close(); // noop actually, but just in case
+
+    Map<String, String> tags = ProcessImplInstrumentationHelpers.createTags(command);
+    TagContext tagContext = new TagContext("appsec", tags);
+    AgentSpan span = tracer.startSpan("command_execution", tagContext, true);
+    span.setSpanType("system");
+    span.startThreadMigration();
+    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.finishThreadMigration();
+      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/ProcessImplInstrumentationSpecification.groovy

@@ -0,0 +1,124 @@
+package datadog.trace.instrumentation.java.lang
+
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.function.Supplier
+import datadog.trace.api.gateway.Events
+import datadog.trace.api.gateway.Flow
+import datadog.trace.api.gateway.RequestContextSlot
+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 {
+  Closeable c = Mock()
+  def ss = TEST_TRACER.getSubscriptionService(RequestContextSlot.APPSEC)
+
+  def setup() {
+    ss.registerCallback(Events.EVENTS.requestStarted(), {
+      new Flow.ResultFlow<Object>(c)
+    } as Supplier<Flow<Object>>)
+  }
+
+  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.spanName == 'command_execution'
+  }
+
+  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"]'
+  }
+}

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.groovy

@@ -18,6 +18,7 @@ import datadog.trace.api.Checkpointer
 import datadog.trace.api.Config
 import datadog.trace.api.DDId
 import datadog.trace.api.Platform
+import datadog.trace.api.ProductActivationConfig
 import datadog.trace.api.StatsDClient
 import datadog.trace.api.WellKnownTags
 import datadog.trace.api.config.TracerConfig
@@ -253,7 +254,8 @@ abstract class AgentTestRunner extends DDSpecification implements AgentBuilder.L
           ClassLoader classLoader,
           JavaModule module,
           ProtectionDomain pd) {
-          builder.method(named("isAppSecEnabled")).intercept(FixedValue.value(true))
+          builder.method(named("getAppSecEnabledConfig"))
+            .intercept(FixedValue.value(ProductActivationConfig.FULLY_ENABLED))
         }
       }).installOn(INSTRUMENTATION)
   }