diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 3cf73374fb2..f754a57e797 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -387,6 +387,11 @@ abstract class HttpServerTest extends WithHttpServer { false } + /** Override to false when the multipart implementation uses a set with non-deterministic ordering (e.g. HashSet). */ + boolean testBodyFilesContentOrdering() { + true + } + boolean testBodyJson() { false } @@ -1767,7 +1772,7 @@ abstract class HttpServerTest extends WithHttpServer { def 'test instrumentation gateway file upload content max files limit'() { setup: - assumeTrue(testBodyFilesContent()) + assumeTrue(testBodyFilesContent() && testBodyFilesContentOrdering()) def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount() def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM) (1..maxFilesToInspect + 1).each { diff --git a/dd-java-agent/instrumentation/vertx/vertx-rx-3.5/src/test/groovy/server/VertxRxCircuitBreakerHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-rx-3.5/src/test/groovy/server/VertxRxCircuitBreakerHttpServerForkedTest.groovy index 0c79e4e10f3..96b8b9b3a54 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-rx-3.5/src/test/groovy/server/VertxRxCircuitBreakerHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-rx-3.5/src/test/groovy/server/VertxRxCircuitBreakerHttpServerForkedTest.groovy @@ -44,6 +44,11 @@ class VertxRxCircuitBreakerHttpServerForkedTest extends VertxHttpServerForkedTes false } + @Override + boolean testBodyFilesContent() { + false + } + @Override boolean testBodyJson() { false diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelper.java new file mode 100644 index 00000000000..913003e8c28 --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/FileUploadHelper.java @@ -0,0 +1,51 @@ +package datadog.trace.instrumentation.vertx_3_4.server; + +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.http.MultipartContentDecoder; +import io.vertx.ext.web.FileUpload; +import java.io.FileInputStream; +import java.util.List; +import java.util.function.BiFunction; + +public class FileUploadHelper { + + public static BlockingException commitBlockingResponse( + BiFunction, Flow> cb, + RequestContext reqCtx, + List data, + String reason) { + Flow flow = cb.apply(reqCtx, data); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse( + reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action); + return new BlockingException(reason); + } + } + return null; + } + + public static String readUploadContent(FileUpload upload, int maxBytes) { + try { + String path = upload.uploadedFileName(); + if (path == null || path.isEmpty()) { + return ""; + } + String charSet = upload.charSet(); + String contentType = + charSet != null && !charSet.isEmpty() + ? upload.contentType() + "; charset=" + charSet + : upload.contentType(); + try (FileInputStream fis = new FileInputStream(path)) { + return MultipartContentDecoder.readInputStream(fis, maxBytes, contentType); + } + } catch (Exception ignored) { + return ""; + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextFilenamesAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextFilenamesAdvice.java index 4ab0bbec651..c1abb2e0a97 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextFilenamesAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextFilenamesAdvice.java @@ -2,10 +2,9 @@ import static datadog.trace.api.gateway.Events.EVENTS; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.advice.ActiveRequestContext; import datadog.trace.advice.RequiresRequestContext; -import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.Config; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -38,35 +37,52 @@ static void after( return; } - List filenames = new ArrayList<>(uploads.size()); + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> filenamesCb = + cbp.getCallback(EVENTS.requestFilesFilenames()); + BiFunction, Flow> contentCb = + cbp.getCallback(EVENTS.requestFilesContent()); + if (filenamesCb == null && contentCb == null) { + return; + } + + int maxFiles = Config.get().getAppSecMaxFileContentCount(); + int maxBytes = Config.get().getAppSecMaxFileContentBytes(); + List filenames = null; + List filesContent = null; + for (FileUpload upload : uploads) { String name = upload.fileName(); - if (name != null && !name.isEmpty()) { + if (filenamesCb != null && name != null && !name.isEmpty()) { + if (filenames == null) { + filenames = new ArrayList<>(); + } filenames.add(name); } + if (contentCb != null + && maxFiles > 0 + && (filesContent == null || filesContent.size() < maxFiles)) { + if (filesContent == null) { + filesContent = new ArrayList<>(); + } + filesContent.add(FileUploadHelper.readUploadContent(upload, maxBytes)); + } } - if (filenames.isEmpty()) { - return; + + if (filenamesCb != null && filenames != null) { + throwable = + FileUploadHelper.commitBlockingResponse( + filenamesCb, reqCtx, filenames, "Blocked request (multipart file upload)"); } - CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - BiFunction, Flow> cb = - cbp.getCallback(EVENTS.requestFilesFilenames()); - if (cb == null) { + if (throwable != null) { return; } - Flow flow = cb.apply(reqCtx, filenames); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse( - reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action); - if (throwable == null) { - throwable = new BlockingException("Blocked request (multipart file upload)"); - } - } + if (contentCb != null && filesContent != null) { + throwable = + FileUploadHelper.commitBlockingResponse( + contentCb, reqCtx, filesContent, "Blocked request (file content)"); } } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextImplInstrumentation.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextImplInstrumentation.java index e13eef7f18e..af8ee74bf18 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextImplInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextImplInstrumentation.java @@ -18,12 +18,20 @@ public class RoutingContextImplInstrumentation extends InstrumenterModule.AppSec private static final Reference FILE_UPLOAD_REF = new Reference.Builder("io.vertx.ext.web.FileUpload") .withMethod(new String[0], 0, "fileName", "Ljava/lang/String;") + .withMethod(new String[0], 0, "uploadedFileName", "Ljava/lang/String;") + .withMethod(new String[0], 0, "contentType", "Ljava/lang/String;") + .withMethod(new String[0], 0, "charSet", "Ljava/lang/String;") .build(); public RoutingContextImplInstrumentation() { super("vertx", "vertx-3.4"); } + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".FileUploadHelper"}; + } + @Override public String instrumentedType() { return "io.vertx.ext.web.impl.RoutingContextImpl"; diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy index c64753affe7..7fadc69bac5 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -1,6 +1,8 @@ package server +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static org.junit.jupiter.api.Assumptions.assumeTrue import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.LOGIN import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND @@ -10,6 +12,10 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCES import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.Config +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody import datadog.trace.api.DDSpanTypes import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator @@ -87,6 +93,42 @@ class VertxHttpServerForkedTest extends HttpServerTest { true } + @Override + boolean testBodyFilesContent() { + true + } + + @Override + boolean testBodyFilesContentOrdering() { + false + } + + // fileUploads() returns a HashSet in Vert.x: check count instead of which specific file is excluded + def 'test instrumentation gateway file upload content max files limit count'() { + setup: + assumeTrue(testBodyFilesContent()) + def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount() + def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM) + (1..maxFilesToInspect + 1).each { i -> + bodyBuilder.addFormDataPart("file${i}", "file${i}.bin", + RequestBody.create(MediaType.parse('application/octet-stream'), "content_of_file_${i}")) + } + def httpRequest = request(BODY_MULTIPART, 'POST', bodyBuilder.build()).build() + def response = client.newCall(httpRequest).execute() + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { span -> + def tag = span.getTag('request.body.files_content') as String + tag != null && tag.count('content_of_file_') == maxFilesToInspect + } + + cleanup: + response.close() + } + @Override boolean testBodyJson() { true diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/latestDepTest/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/latestDepTest/groovy/server/VertxHttpServerForkedTest.groovy index 8f276d08f23..f08e9dd7fd2 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/latestDepTest/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/latestDepTest/groovy/server/VertxHttpServerForkedTest.groovy @@ -1,6 +1,8 @@ package server +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static org.junit.jupiter.api.Assumptions.assumeTrue import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.LOGIN import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND @@ -10,6 +12,10 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCES import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.Config +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody import datadog.trace.api.DDSpanTypes import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator @@ -82,6 +88,42 @@ class VertxHttpServerForkedTest extends HttpServerTest { true } + @Override + boolean testBodyFilesContent() { + true + } + + @Override + boolean testBodyFilesContentOrdering() { + false + } + + // fileUploads() returns a HashSet in Vert.x: check count instead of which specific file is excluded + def 'test instrumentation gateway file upload content max files limit count'() { + setup: + assumeTrue(testBodyFilesContent()) + def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount() + def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM) + (1..maxFilesToInspect + 1).each { i -> + bodyBuilder.addFormDataPart("file${i}", "file${i}.bin", + RequestBody.create(MediaType.parse('application/octet-stream'), "content_of_file_${i}")) + } + def httpRequest = request(BODY_MULTIPART, 'POST', bodyBuilder.build()).build() + def response = client.newCall(httpRequest).execute() + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { span -> + def tag = span.getTag('request.body.files_content') as String + tag != null && tag.count('content_of_file_') == maxFilesToInspect + } + + cleanup: + response.close() + } + @Override boolean testBodyJson() { true diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelper.java new file mode 100644 index 00000000000..c0eef8cab2e --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/FileUploadHelper.java @@ -0,0 +1,51 @@ +package datadog.trace.instrumentation.vertx_4_0.server; + +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.http.MultipartContentDecoder; +import io.vertx.ext.web.FileUpload; +import java.io.FileInputStream; +import java.util.List; +import java.util.function.BiFunction; + +public class FileUploadHelper { + + public static BlockingException commitBlockingResponse( + BiFunction, Flow> cb, + RequestContext reqCtx, + List data, + String reason) { + Flow flow = cb.apply(reqCtx, data); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse( + reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action); + return new BlockingException(reason); + } + } + return null; + } + + public static String readUploadContent(FileUpload upload, int maxBytes) { + try { + String path = upload.uploadedFileName(); + if (path == null || path.isEmpty()) { + return ""; + } + String charSet = upload.charSet(); + String contentType = + charSet != null && !charSet.isEmpty() + ? upload.contentType() + "; charset=" + charSet + : upload.contentType(); + try (FileInputStream fis = new FileInputStream(path)) { + return MultipartContentDecoder.readInputStream(fis, maxBytes, contentType); + } + } catch (Exception ignored) { + return ""; + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextFilenamesAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextFilenamesAdvice.java index e9ac149e7bb..ce4cd89ed71 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextFilenamesAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextFilenamesAdvice.java @@ -2,10 +2,9 @@ import static datadog.trace.api.gateway.Events.EVENTS; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.advice.ActiveRequestContext; import datadog.trace.advice.RequiresRequestContext; -import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.Config; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -38,35 +37,52 @@ static void after( return; } - List filenames = new ArrayList<>(uploads.size()); + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> filenamesCb = + cbp.getCallback(EVENTS.requestFilesFilenames()); + BiFunction, Flow> contentCb = + cbp.getCallback(EVENTS.requestFilesContent()); + if (filenamesCb == null && contentCb == null) { + return; + } + + int maxFiles = Config.get().getAppSecMaxFileContentCount(); + int maxBytes = Config.get().getAppSecMaxFileContentBytes(); + List filenames = null; + List filesContent = null; + for (FileUpload upload : uploads) { String name = upload.fileName(); - if (name != null && !name.isEmpty()) { + if (filenamesCb != null && name != null && !name.isEmpty()) { + if (filenames == null) { + filenames = new ArrayList<>(); + } filenames.add(name); } + if (contentCb != null + && maxFiles > 0 + && (filesContent == null || filesContent.size() < maxFiles)) { + if (filesContent == null) { + filesContent = new ArrayList<>(); + } + filesContent.add(FileUploadHelper.readUploadContent(upload, maxBytes)); + } } - if (filenames.isEmpty()) { - return; + + if (filenamesCb != null && filenames != null) { + throwable = + FileUploadHelper.commitBlockingResponse( + filenamesCb, reqCtx, filenames, "Blocked request (multipart file upload)"); } - CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - BiFunction, Flow> cb = - cbp.getCallback(EVENTS.requestFilesFilenames()); - if (cb == null) { + if (throwable != null) { return; } - Flow flow = cb.apply(reqCtx, filenames); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse( - reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action); - if (throwable == null) { - throwable = new BlockingException("Blocked request (multipart file upload)"); - } - } + if (contentCb != null && filesContent != null) { + throwable = + FileUploadHelper.commitBlockingResponse( + contentCb, reqCtx, filesContent, "Blocked request (file content)"); } } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextImplInstrumentation.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextImplInstrumentation.java index ae863b6ec09..97623985225 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextImplInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RoutingContextImplInstrumentation.java @@ -21,12 +21,20 @@ public class RoutingContextImplInstrumentation extends InstrumenterModule.AppSec private static final Reference FILE_UPLOAD_REF = new Reference.Builder("io.vertx.ext.web.FileUpload") .withMethod(new String[0], 0, "fileName", "Ljava/lang/String;") + .withMethod(new String[0], 0, "uploadedFileName", "Ljava/lang/String;") + .withMethod(new String[0], 0, "contentType", "Ljava/lang/String;") + .withMethod(new String[0], 0, "charSet", "Ljava/lang/String;") .build(); public RoutingContextImplInstrumentation() { super("vertx", "vertx-4.0"); } + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".FileUploadHelper"}; + } + @Override public Reference[] additionalMuzzleReferences() { return new Reference[] {VertxVersionMatcher.HTTP_1X_SERVER_RESPONSE, FILE_UPLOAD_REF}; diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 1d2f4ec9971..62455354611 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -1,6 +1,8 @@ package server +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static org.junit.jupiter.api.Assumptions.assumeTrue import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.LOGIN import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND @@ -10,6 +12,10 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCES import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.Config +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody import datadog.trace.api.DDSpanTypes import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator @@ -83,6 +89,42 @@ class VertxHttpServerForkedTest extends HttpServerTest { true } + @Override + boolean testBodyFilesContent() { + true + } + + @Override + boolean testBodyFilesContentOrdering() { + false + } + + // fileUploads() returns a HashSet in Vert.x: check count instead of which specific file is excluded + def 'test instrumentation gateway file upload content max files limit count'() { + setup: + assumeTrue(testBodyFilesContent()) + def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount() + def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM) + (1..maxFilesToInspect + 1).each { i -> + bodyBuilder.addFormDataPart("file${i}", "file${i}.bin", + RequestBody.create(MediaType.parse('application/octet-stream'), "content_of_file_${i}")) + } + def httpRequest = request(BODY_MULTIPART, 'POST', bodyBuilder.build()).build() + def response = client.newCall(httpRequest).execute() + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { span -> + def tag = span.getTag('request.body.files_content') as String + tag != null && tag.count('content_of_file_') == maxFilesToInspect + } + + cleanup: + response.close() + } + @Override boolean testBodyJson() { true diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelper.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelper.java new file mode 100644 index 00000000000..045d825444a --- /dev/null +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/FileUploadHelper.java @@ -0,0 +1,51 @@ +package datadog.trace.instrumentation.vertx_5_0.server; + +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.http.MultipartContentDecoder; +import io.vertx.ext.web.FileUpload; +import java.io.FileInputStream; +import java.util.List; +import java.util.function.BiFunction; + +public class FileUploadHelper { + + public static BlockingException commitBlockingResponse( + BiFunction, Flow> cb, + RequestContext reqCtx, + List data, + String reason) { + Flow flow = cb.apply(reqCtx, data); + Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); + if (brf != null) { + brf.tryCommitBlockingResponse( + reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action); + return new BlockingException(reason); + } + } + return null; + } + + public static String readUploadContent(FileUpload upload, int maxBytes) { + try { + String path = upload.uploadedFileName(); + if (path == null || path.isEmpty()) { + return ""; + } + String charSet = upload.charSet(); + String contentType = + charSet != null && !charSet.isEmpty() + ? upload.contentType() + "; charset=" + charSet + : upload.contentType(); + try (FileInputStream fis = new FileInputStream(path)) { + return MultipartContentDecoder.readInputStream(fis, maxBytes, contentType); + } + } catch (Exception ignored) { + return ""; + } + } +} diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/RoutingContextFilenamesAdvice.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/RoutingContextFilenamesAdvice.java index cf4aa6482f6..2d8a77e342c 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/RoutingContextFilenamesAdvice.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/RoutingContextFilenamesAdvice.java @@ -2,10 +2,9 @@ import static datadog.trace.api.gateway.Events.EVENTS; -import datadog.appsec.api.blocking.BlockingException; import datadog.trace.advice.ActiveRequestContext; import datadog.trace.advice.RequiresRequestContext; -import datadog.trace.api.gateway.BlockResponseFunction; +import datadog.trace.api.Config; import datadog.trace.api.gateway.CallbackProvider; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.RequestContext; @@ -37,35 +36,52 @@ static void after( return; } - List filenames = new ArrayList<>(uploads.size()); + CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); + BiFunction, Flow> filenamesCb = + cbp.getCallback(EVENTS.requestFilesFilenames()); + BiFunction, Flow> contentCb = + cbp.getCallback(EVENTS.requestFilesContent()); + if (filenamesCb == null && contentCb == null) { + return; + } + + int maxFiles = Config.get().getAppSecMaxFileContentCount(); + int maxBytes = Config.get().getAppSecMaxFileContentBytes(); + List filenames = null; + List filesContent = null; + for (FileUpload upload : uploads) { String name = upload.fileName(); - if (name != null && !name.isEmpty()) { + if (filenamesCb != null && name != null && !name.isEmpty()) { + if (filenames == null) { + filenames = new ArrayList<>(); + } filenames.add(name); } + if (contentCb != null + && maxFiles > 0 + && (filesContent == null || filesContent.size() < maxFiles)) { + if (filesContent == null) { + filesContent = new ArrayList<>(); + } + filesContent.add(FileUploadHelper.readUploadContent(upload, maxBytes)); + } } - if (filenames.isEmpty()) { - return; + + if (filenamesCb != null && filenames != null) { + throwable = + FileUploadHelper.commitBlockingResponse( + filenamesCb, reqCtx, filenames, "Blocked request (multipart file upload)"); } - CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC); - BiFunction, Flow> cb = - cbp.getCallback(EVENTS.requestFilesFilenames()); - if (cb == null) { + if (throwable != null) { return; } - Flow flow = cb.apply(reqCtx, filenames); - Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - BlockResponseFunction brf = reqCtx.getBlockResponseFunction(); - if (brf != null) { - brf.tryCommitBlockingResponse( - reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action); - if (throwable == null) { - throwable = new BlockingException("Blocked request (multipart file upload)"); - } - } + if (contentCb != null && filesContent != null) { + throwable = + FileUploadHelper.commitBlockingResponse( + contentCb, reqCtx, filesContent, "Blocked request (file content)"); } } } diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/RoutingContextImplInstrumentation.java b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/RoutingContextImplInstrumentation.java index 93ff71d4d82..627d7798f9e 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/RoutingContextImplInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/main/java/datadog/trace/instrumentation/vertx_5_0/server/RoutingContextImplInstrumentation.java @@ -15,12 +15,20 @@ public class RoutingContextImplInstrumentation extends InstrumenterModule.AppSec private static final Reference FILE_UPLOAD_REF = new Reference.Builder("io.vertx.ext.web.FileUpload") .withMethod(new String[0], 0, "fileName", "Ljava/lang/String;") + .withMethod(new String[0], 0, "uploadedFileName", "Ljava/lang/String;") + .withMethod(new String[0], 0, "contentType", "Ljava/lang/String;") + .withMethod(new String[0], 0, "charSet", "Ljava/lang/String;") .build(); public RoutingContextImplInstrumentation() { super("vertx", "vertx-5.0"); } + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".FileUploadHelper"}; + } + @Override public String instrumentedType() { return "io.vertx.ext.web.impl.RoutingContextImpl"; diff --git a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy index 2be39db6f4a..cdd4bca1c19 100644 --- a/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy +++ b/dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-5.0/src/test/groovy/server/VertxHttpServerForkedTest.groovy @@ -1,7 +1,9 @@ package server import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static org.junit.jupiter.api.Assumptions.assumeTrue import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.LOGIN import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND @@ -11,14 +13,16 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCES import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.Config +import okhttp3.MediaType +import okhttp3.MultipartBody +import okhttp3.RequestBody import datadog.trace.api.DDSpanTypes import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator import datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator import io.vertx.core.AbstractVerticle import io.vertx.core.Vertx -import okhttp3.MediaType -import okhttp3.RequestBody class VertxHttpServerForkedTest extends HttpServerTest { @Override @@ -90,6 +94,42 @@ class VertxHttpServerForkedTest extends HttpServerTest { true } + @Override + boolean testBodyFilesContent() { + true + } + + @Override + boolean testBodyFilesContentOrdering() { + false + } + + // fileUploads() returns a HashSet in Vert.x: check count instead of which specific file is excluded + def 'test instrumentation gateway file upload content max files limit count'() { + setup: + assumeTrue(testBodyFilesContent()) + def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount() + def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM) + (1..maxFilesToInspect + 1).each { i -> + bodyBuilder.addFormDataPart("file${i}", "file${i}.bin", + RequestBody.create(MediaType.parse('application/octet-stream'), "content_of_file_${i}")) + } + def httpRequest = request(BODY_MULTIPART, 'POST', bodyBuilder.build()).build() + def response = client.newCall(httpRequest).execute() + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { span -> + def tag = span.getTag('request.body.files_content') as String + tag != null && tag.count('content_of_file_') == maxFilesToInspect + } + + cleanup: + response.close() + } + @Override boolean testBodyJson() { true