Skip to content

Commit 6553226

Browse files
manuel-alvarez-alvarezmanuel-alvarez-alvarez
authored
Store the http.route inside the appsec request context in Play (#8991)
1 parent 1f2b612 commit 6553226

28 files changed

Lines changed: 768 additions & 0 deletions

File tree

‎dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ public void init() {
158158
subscriptionService.registerCallback(EVENTS.shellCmd(), this::onShellCmd);
159159
subscriptionService.registerCallback(EVENTS.user(), this::onUser);
160160
subscriptionService.registerCallback(EVENTS.loginEvent(), this::onLoginEvent);
161+
subscriptionService.registerCallback(EVENTS.httpRoute(), this::onHttpRoute);
161162

162163
if (additionalIGEvents.contains(EVENTS.requestPathParams())) {
163164
subscriptionService.registerCallback(EVENTS.requestPathParams(), this::onRequestPathParams);
@@ -224,6 +225,14 @@ private Flow<Void> onUser(final RequestContext ctx_, final String user) {
224225
}
225226
}
226227

228+
private void onHttpRoute(final RequestContext ctx_, final String route) {
229+
final AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
230+
if (ctx == null) {
231+
return;
232+
}
233+
ctx.setRoute(route);
234+
}
235+
227236
private Flow<Void> onLoginEvent(
228237
final RequestContext ctx_, final LoginEvent event, final String login) {
229238
final AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);

‎dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ class GatewayBridgeSpecification extends DDSpecification {
114114
BiFunction<RequestContext, String, Flow<Void>> shellCmdCB
115115
BiFunction<RequestContext, String, Flow<Void>> userCB
116116
TriFunction<RequestContext, LoginEvent, String, Flow<Void>> loginEventCB
117+
BiConsumer<RequestContext, String> httpRouteCB
117118

118119
WafMetricCollector wafMetricCollector = Mock(WafMetricCollector)
119120

@@ -477,6 +478,7 @@ class GatewayBridgeSpecification extends DDSpecification {
477478
1 * ig.registerCallback(EVENTS.shellCmd(), _) >> { shellCmdCB = it[1]; null }
478479
1 * ig.registerCallback(EVENTS.user(), _) >> { userCB = it[1]; null }
479480
1 * ig.registerCallback(EVENTS.loginEvent(), _) >> { loginEventCB = it[1]; null }
481+
1 * ig.registerCallback(EVENTS.httpRoute(), _) >> { httpRouteCB = it[1]; null }
480482
0 * ig.registerCallback(_, _)
481483

482484
bridge.init()
@@ -1327,4 +1329,15 @@ class GatewayBridgeSpecification extends DDSpecification {
13271329
0 * traceSegment.setTagTop(_, _)
13281330
}
13291331
1332+
void 'test on httpRoute'() {
1333+
given:
1334+
final route = 'dummy-route'
1335+
1336+
when:
1337+
httpRouteCB.accept(ctx, route)
1338+
1339+
then:
1340+
arCtx.getRoute() == route
1341+
}
1342+
13301343
}

‎dd-java-agent/instrumentation/play-2.3/src/main/java/datadog/trace/instrumentation/play23/PlayHttpServerDecorator.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package datadog.trace.instrumentation.play23;
22

3+
import static datadog.trace.api.gateway.Events.EVENTS;
34
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
45

56
import datadog.trace.api.Config;
7+
import datadog.trace.api.gateway.CallbackProvider;
8+
import datadog.trace.api.gateway.RequestContext;
9+
import datadog.trace.api.gateway.RequestContextSlot;
610
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
711
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
812
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
@@ -12,13 +16,17 @@
1216
import java.lang.reflect.InvocationTargetException;
1317
import java.lang.reflect.UndeclaredThrowableException;
1418
import java.util.concurrent.CompletionException;
19+
import java.util.function.BiConsumer;
20+
import org.slf4j.Logger;
21+
import org.slf4j.LoggerFactory;
1522
import play.api.Routes;
1623
import play.api.mvc.Headers;
1724
import play.api.mvc.Request;
1825
import scala.Option;
1926

2027
public class PlayHttpServerDecorator
2128
extends HttpServerDecorator<Request, Request, play.api.mvc.Result, Headers> {
29+
private static final Logger LOG = LoggerFactory.getLogger(PlayHttpServerDecorator.class);
2230
public static final boolean REPORT_HTTP_STATUS = Config.get().getPlayReportHttpStatus();
2331
public static final CharSequence PLAY_REQUEST = UTF8BytesString.create("play.request");
2432
public static final CharSequence PLAY_ACTION = UTF8BytesString.create("play-action");
@@ -88,11 +96,35 @@ public AgentSpan onRequest(
8896
if (!pathOption.isEmpty()) {
8997
final String path = (String) pathOption.get();
9098
HTTP_RESOURCE_DECORATOR.withRoute(span, request.method(), path);
99+
dispatchRoute(span, path);
91100
}
92101
}
93102
return span;
94103
}
95104

105+
/**
106+
* Play does not set the http.route in the local root span so we need to store it in the context
107+
* for API security
108+
*/
109+
private void dispatchRoute(final AgentSpan span, final String route) {
110+
try {
111+
final RequestContext ctx = span.getRequestContext();
112+
if (ctx == null) {
113+
return;
114+
}
115+
final CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC);
116+
if (cbp == null) {
117+
return;
118+
}
119+
final BiConsumer<RequestContext, String> cb = cbp.getCallback(EVENTS.httpRoute());
120+
if (cb != null) {
121+
cb.accept(ctx, route);
122+
}
123+
} catch (final Throwable t) {
124+
LOG.debug("Failed to dispatch route", t);
125+
}
126+
}
127+
96128
@Override
97129
public AgentSpan onError(final AgentSpan span, Throwable throwable) {
98130
if (REPORT_HTTP_STATUS) {

‎dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package datadog.trace.instrumentation.play24;
22

3+
import static datadog.trace.api.gateway.Events.EVENTS;
34
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
45

56
import datadog.trace.api.Config;
7+
import datadog.trace.api.gateway.CallbackProvider;
8+
import datadog.trace.api.gateway.RequestContext;
9+
import datadog.trace.api.gateway.RequestContextSlot;
610
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
711
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
812
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
@@ -12,13 +16,17 @@
1216
import java.lang.reflect.InvocationTargetException;
1317
import java.lang.reflect.UndeclaredThrowableException;
1418
import java.util.concurrent.CompletionException;
19+
import java.util.function.BiConsumer;
20+
import org.slf4j.Logger;
21+
import org.slf4j.LoggerFactory;
1522
import play.api.mvc.Headers;
1623
import play.api.mvc.Request;
1724
import play.api.mvc.Result;
1825
import scala.Option;
1926

2027
public class PlayHttpServerDecorator
2128
extends HttpServerDecorator<Request, Request, Result, Headers> {
29+
private static final Logger LOG = LoggerFactory.getLogger(PlayHttpServerDecorator.class);
2230
public static final boolean REPORT_HTTP_STATUS = Config.get().getPlayReportHttpStatus();
2331
public static final CharSequence PLAY_REQUEST = UTF8BytesString.create("play.request");
2432
public static final CharSequence PLAY_ACTION = UTF8BytesString.create("play-action");
@@ -88,11 +96,35 @@ public AgentSpan onRequest(
8896
if (!pathOption.isEmpty()) {
8997
final String path = (String) pathOption.get();
9098
HTTP_RESOURCE_DECORATOR.withRoute(span, request.method(), path);
99+
dispatchRoute(span, path);
91100
}
92101
}
93102
return span;
94103
}
95104

105+
/**
106+
* Play does not set the http.route in the local root span so we need to store it in the context
107+
* for API security
108+
*/
109+
private void dispatchRoute(final AgentSpan span, final String route) {
110+
try {
111+
final RequestContext ctx = span.getRequestContext();
112+
if (ctx == null) {
113+
return;
114+
}
115+
final CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC);
116+
if (cbp == null) {
117+
return;
118+
}
119+
final BiConsumer<RequestContext, String> cb = cbp.getCallback(EVENTS.httpRoute());
120+
if (cb != null) {
121+
cb.accept(ctx, route);
122+
}
123+
} catch (final Throwable t) {
124+
LOG.debug("Failed to dispatch route", t);
125+
}
126+
}
127+
96128
@Override
97129
public AgentSpan onError(final AgentSpan span, Throwable throwable) {
98130
if (REPORT_HTTP_STATUS) {

‎dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
package datadog.trace.instrumentation.play26;
22

3+
import static datadog.trace.api.gateway.Events.EVENTS;
34
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
45

56
import datadog.trace.api.Config;
67
import datadog.trace.api.cache.DDCache;
78
import datadog.trace.api.cache.DDCaches;
9+
import datadog.trace.api.gateway.CallbackProvider;
10+
import datadog.trace.api.gateway.RequestContext;
11+
import datadog.trace.api.gateway.RequestContextSlot;
812
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
913
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1014
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
1115
import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities;
1216
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
17+
import datadog.trace.bootstrap.instrumentation.api.URIUtils;
1318
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
1419
import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator;
1520
import java.lang.invoke.MethodHandle;
@@ -18,6 +23,9 @@
1823
import java.lang.reflect.InvocationTargetException;
1924
import java.lang.reflect.UndeclaredThrowableException;
2025
import java.util.concurrent.CompletionException;
26+
import java.util.function.BiConsumer;
27+
import org.slf4j.Logger;
28+
import org.slf4j.LoggerFactory;
2129
import play.api.mvc.Headers;
2230
import play.api.mvc.Request;
2331
import play.api.mvc.Result;
@@ -29,6 +37,7 @@
2937

3038
public class PlayHttpServerDecorator
3139
extends HttpServerDecorator<Request, Request, Result, Headers> {
40+
private static final Logger LOG = LoggerFactory.getLogger(PlayHttpServerDecorator.class);
3241
public static final boolean REPORT_HTTP_STATUS = Config.get().getPlayReportHttpStatus();
3342
public static final CharSequence PLAY_REQUEST = UTF8BytesString.create("play.request");
3443
public static final CharSequence PLAY_ACTION = UTF8BytesString.create("play-action");
@@ -143,11 +152,35 @@ public AgentSpan onRequest(
143152
PATH_CACHE.computeIfAbsent(
144153
defOption.get().path(), p -> addMissingSlash(p, request.path()));
145154
HTTP_RESOURCE_DECORATOR.withRoute(span, request.method(), path, true);
155+
dispatchRoute(span, path);
146156
}
147157
}
148158
return span;
149159
}
150160

161+
/**
162+
* Play does not set the http.route in the local root span so we need to store it in the context
163+
* for API security
164+
*/
165+
private void dispatchRoute(final AgentSpan span, final CharSequence route) {
166+
try {
167+
final RequestContext ctx = span.getRequestContext();
168+
if (ctx == null) {
169+
return;
170+
}
171+
final CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC);
172+
if (cbp == null) {
173+
return;
174+
}
175+
final BiConsumer<RequestContext, String> cb = cbp.getCallback(EVENTS.httpRoute());
176+
if (cb != null) {
177+
cb.accept(ctx, URIUtils.decode(route.toString()));
178+
}
179+
} catch (final Throwable t) {
180+
LOG.debug("Failed to dispatch route", t);
181+
}
182+
}
183+
151184
/*
152185
This is a workaround to add a `/` if it is missing when using split routes.
153186

‎dd-smoke-tests/play-2.4/app/controllers/AppSecController.scala‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package controllers
2+
3+
import play.api.mvc.{Action, AnyContent, Controller}
4+
5+
class AppSecController extends Controller {
6+
7+
def apiSecuritySampling(statusCode: Int, test: String): Action[AnyContent] = Action {
8+
Status(statusCode)("EXECUTED")
9+
}
10+
11+
}

‎dd-smoke-tests/play-2.4/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ dependencies {
6363
implementation group: 'io.opentracing', name: 'opentracing-util', version: '0.32.0'
6464

6565
testImplementation project(':dd-smoke-tests')
66+
testImplementation project(':dd-smoke-tests:appsec')
6667
}
6768

6869
configurations.testImplementation {

‎dd-smoke-tests/play-2.4/conf/routes‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,6 @@
66
# An example controller showing a sample home page
77
GET /welcomej controllers.JController.doGet(id: Int ?= 0)
88
GET /welcomes controllers.SController.doGet(id: Option[Int])
9+
10+
# AppSec endpoints for testing
11+
GET /api_security/sampling/:statusCode controllers.AppSecController.apiSecuritySampling(statusCode: Int, test: String)

‎dd-smoke-tests/play-2.4/src/test/groovy/datadog/smoketest/AppSecPlayNettySmokeTest.groovy‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package datadog.smoketest
2+
3+
import datadog.smoketest.appsec.AbstractAppSecServerSmokeTest
4+
import datadog.trace.agent.test.utils.OkHttpUtils
5+
import okhttp3.Request
6+
import okhttp3.Response
7+
import spock.lang.Shared
8+
9+
import java.nio.file.Files
10+
11+
import static java.util.concurrent.TimeUnit.SECONDS
12+
13+
class AppSecPlayNettySmokeTest extends AbstractAppSecServerSmokeTest {
14+
15+
@Shared
16+
File playDirectory = new File("${buildDirectory}/stage/main")
17+
18+
@Override
19+
ProcessBuilder createProcessBuilder() {
20+
// If the server is not shut down correctly, this file can be left there and will block
21+
// the start of a new test
22+
def runningPid = new File(playDirectory.getPath(), "RUNNING_PID")
23+
if (runningPid.exists()) {
24+
runningPid.delete()
25+
}
26+
def command = isWindows() ? 'main.bat' : 'main'
27+
ProcessBuilder processBuilder = new ProcessBuilder("${playDirectory}/bin/${command}")
28+
processBuilder.directory(playDirectory)
29+
processBuilder.environment().put("JAVA_OPTS",
30+
(defaultAppSecProperties + defaultJavaProperties).collect({ it.replace(' ', '\\ ')}).join(" ")
31+
+ " -Dconfig.file=${playDirectory}/conf/application.conf"
32+
+ " -Dhttp.port=${httpPort}"
33+
+ " -Dhttp.address=127.0.0.1"
34+
+ " -Dplay.server.provider=play.core.server.NettyServerProvider"
35+
+ " -Ddd.writer.type=MultiWriter:TraceStructureWriter:${output.getAbsolutePath()},DDAgentWriter")
36+
return processBuilder
37+
}
38+
39+
@Override
40+
File createTemporaryFile() {
41+
return new File("${buildDirectory}/tmp/trace-structure-play-2.4-appsec-netty.out")
42+
}
43+
44+
void 'API Security samples only one request per endpoint'() {
45+
given:
46+
def url = "http://localhost:${httpPort}/api_security/sampling/200?test=value"
47+
def client = OkHttpUtils.clientBuilder().build()
48+
def request = new Request.Builder()
49+
.url(url)
50+
.addHeader('X-My-Header', "value")
51+
.get()
52+
.build()
53+
54+
when:
55+
List<Response> responses = (1..3).collect {
56+
client.newCall(request).execute()
57+
}
58+
59+
then:
60+
responses.each {
61+
assert it.code() == 200
62+
}
63+
waitForTraceCount(3)
64+
def spans = rootSpans.toList().toSorted { it.span.duration }
65+
spans.size() == 3
66+
def sampledSpans = spans.findAll { it.meta.keySet().any { it.startsWith('_dd.appsec.s.req.') } }
67+
sampledSpans.size() == 1
68+
def span = sampledSpans[0]
69+
span.meta.containsKey('_dd.appsec.s.req.query')
70+
span.meta.containsKey('_dd.appsec.s.req.headers')
71+
}
72+
73+
// Ensure to clean up server and not only the shell script that starts it
74+
def cleanupSpec() {
75+
def pid = runningServerPid()
76+
if (pid) {
77+
def commands = isWindows() ? ['taskkill', '/PID', pid, '/T', '/F'] : ['kill', '-9', pid]
78+
new ProcessBuilder(commands).start().waitFor(10, SECONDS)
79+
}
80+
}
81+
82+
def runningServerPid() {
83+
def runningPid = new File(playDirectory.getPath(), 'RUNNING_PID')
84+
if (runningPid.exists()) {
85+
return Files.lines(runningPid.toPath()).findAny().orElse(null)
86+
}
87+
}
88+
89+
static isWindows() {
90+
return System.getProperty('os.name').toLowerCase().contains('win')
91+
}
92+
}

‎dd-smoke-tests/play-2.5/app/controllers/AppSecController.scala‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package controllers
2+
3+
import play.api.mvc.{Action, AnyContent, Controller}
4+
5+
class AppSecController extends Controller {
6+
7+
def apiSecuritySampling(statusCode: Int, test: String): Action[AnyContent] = Action {
8+
Status(statusCode)("EXECUTED")
9+
}
10+
11+
}

0 commit comments

Comments
 (0)