Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand SSRF support in IAST to apache-httpclient-5 and apache-httpasyncclient-4 #7920

Merged
merged 17 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ protected URI url(final HttpUriRequest request) throws URISyntaxException {
return request.getURI();
}

@Override
protected URI sourceUrl(final HttpUriRequest request) {
return request.getURI();
}

@Override
protected int status(final HttpContext context) {
final Object responseObject = context.getAttribute(HttpCoreContext.HTTP_RESPONSE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ protected URI url(final HttpRequest request) throws URISyntaxException {
return request.getUri();
}

@Override
protected HttpRequest sourceUrl(final HttpRequest request) {
return request;
}

@Override
protected int status(final HttpResponse httpResponse) {
return httpResponse.getCode();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.instrumentation.apachehttpclient5;

import datadog.trace.api.iast.util.PropagationUtils;
import java.net.URI;
import java.net.URISyntaxException;
import org.apache.hc.core5.http.Header;
Expand All @@ -15,6 +16,9 @@ public class HostAndRequestAsHttpUriRequest extends BasicClassicHttpRequest {
public HostAndRequestAsHttpUriRequest(final HttpHost httpHost, final HttpRequest httpRequest) {
super(httpRequest.getMethod(), httpHost, httpRequest.getPath());
actualRequest = httpRequest;
// Propagate in case the host or request is tainted
PropagationUtils.taintObjectIfTainted(this, httpHost);
PropagationUtils.taintObjectIfTainted(this, httpRequest);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package datadog.trace.instrumentation.apachehttpclient5;

import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.agent.tooling.bytebuddy.iast.TaintableVisitor;
import datadog.trace.api.iast.InstrumentationBridge;
import datadog.trace.api.iast.Propagation;
import datadog.trace.api.iast.propagation.PropagationModule;
import java.net.URI;
import net.bytebuddy.asm.Advice;
import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;

@AutoService(InstrumenterModule.class)
public class IastHttpUriRequestBaseInstrumentation extends InstrumenterModule.Iast
implements Instrumenter.ForSingleType, Instrumenter.HasTypeAdvice {

public IastHttpUriRequestBaseInstrumentation() {
super("apache-httpclient", "httpclient5");
}

@Override
public String instrumentedType() {
return "org.apache.hc.client5.http.classic.methods.HttpUriRequestBase";
}

@Override
public void typeAdvice(TypeTransformer transformer) {
transformer.applyAdvice(new TaintableVisitor(instrumentedType()));
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
isConstructor().and(takesArguments(String.class, URI.class)),
IastHttpUriRequestBaseInstrumentation.class.getName() + "$CtorAdvice");
}

public static class CtorAdvice {
@Advice.OnMethodExit()
@Propagation
public static void afterCtor(
@Advice.This final HttpUriRequestBase self, @Advice.Argument(1) final URI uri) {
final PropagationModule module = InstrumentationBridge.PROPAGATION;
if (module != null) {
module.taintObjectIfTainted(self, uri);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.iast.InstrumentationBridge
import datadog.trace.api.iast.propagation.PropagationModule
import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase

class IastHttpUriRequestBaseInstrumentationTest extends AgentTestRunner {

@Override
protected void configurePreAgent() {
injectSysConfig('dd.iast.enabled', 'true')
}

void 'test constructor'() {
given:
final module = Mock(PropagationModule)
InstrumentationBridge.registerIastModule(module)

when:
HttpUriRequestBase.newInstance(method, new URI(uri))

then:
1 * module.taintObjectIfTainted(_ as HttpUriRequestBase, _ as URI)
0 * _

where:
method | uri
"GET" | 'http://localhost.com'
}
}
20 changes: 20 additions & 0 deletions dd-java-agent/instrumentation/apache-httpcore-5/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
muzzle {
pass {
group = "org.apache.httpcomponents.core5"
module = "httpcore5"
versions = "[5.0,)"
assertInverse = true
}
}

apply from: "$rootDir/gradle/java.gradle"

addTestSuiteForDir('latestDepTest', 'test')

dependencies {
compileOnly group: 'org.apache.httpcomponents.core5', name: 'httpcore5', version: '5.0'

testImplementation group: 'org.apache.httpcomponents.core5', name: 'httpcore5', version: '5.0'

latestDepTestImplementation group: 'org.apache.httpcomponents.core5', name: 'httpcore5', version: '+'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package datadog.trace.instrumentation.apachehttpcore5;

import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.api.iast.InstrumentationBridge;
import datadog.trace.api.iast.Propagation;
import datadog.trace.api.iast.propagation.PropagationModule;
import java.net.InetAddress;
import net.bytebuddy.asm.Advice;

@AutoService(InstrumenterModule.class)
public class IastHttpHostInstrumentation extends InstrumenterModule.Iast
implements Instrumenter.ForSingleType {

public IastHttpHostInstrumentation() {
super("httpcore-5", "apache-httpcore-5", "apache-http-core-5");
}

@Override
public String instrumentedType() {
return "org.apache.hc.core5.http.HttpHost";
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
isConstructor()
.and(takesArguments(String.class, InetAddress.class, String.class, int.class)),
IastHttpHostInstrumentation.class.getName() + "$CtorAdvice");
}

public static class CtorAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
@Propagation
public static void afterCtor(
@Advice.This final Object self, @Advice.Argument(2) final Object host) {
Mariovido marked this conversation as resolved.
Show resolved Hide resolved
final PropagationModule module = InstrumentationBridge.PROPAGATION;
if (module != null) {
module.taintObjectIfTainted(self, host);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package datadog.trace.instrumentation.apachehttpcore5

import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.iast.InstrumentationBridge
import datadog.trace.api.iast.propagation.PropagationModule
import org.apache.hc.core5.http.HttpHost

class IastHttpHostInstrumentationTest extends AgentTestRunner {

@Override
protected void configurePreAgent() {
injectSysConfig('dd.iast.enabled', 'true')
}

void 'test constructor'(){
given:
final module = Mock(PropagationModule)
InstrumentationBridge.registerIastModule(module)

when:
HttpHost.newInstance(*args)

then:
1 * module.taintObjectIfTainted( _ as HttpHost, 'localhost')

where:
args | _
['localhost'] | _
['localhost', 8080] | _
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@
1 org.apache.*
#apache httpClient needs URI propagation
0 org.apache.http.client.methods.*
0 org.apache.hc.client5.http.classic.methods.*
# apache compiled jsps
0 org.apache.jsp.*
1 org.apiguardian.*
Expand Down
2 changes: 2 additions & 0 deletions dd-smoke-tests/iast-util/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@ dependencies {
compileOnly group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.0'
compileOnly group: 'com.squareup.okhttp', name: 'okhttp', version: '2.2.0'
compileOnly group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0'
compileOnly group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.0'
compileOnly group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.0'
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,16 @@
import org.apache.commons.httpclient.HttpClient;
import org.apache.commons.httpclient.HttpMethod;
import org.apache.commons.httpclient.methods.GetMethod;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.impl.classic.HttpClients;
import org.apache.http.HttpHost;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.DefaultHttpClient;
import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
import org.apache.http.impl.nio.client.HttpAsyncClients;
import org.apache.http.message.BasicHttpRequest;
import org.apache.http.nio.client.methods.HttpAsyncMethods;
import org.apache.http.nio.protocol.HttpAsyncRequestProducer;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
Expand Down Expand Up @@ -89,4 +95,60 @@ public String okHttp3(@RequestParam(value = "url") final String url) {
client.connectionPool().evictAll();
return "ok";
}

@PostMapping("/apache-httpclient5")
public String apacheHttpClient5(
@RequestParam(value = "url", required = false) final String url,
@RequestParam(value = "urlHandler", required = false) final String urlHandler,
@RequestParam(value = "host", required = false) final String host) {
CloseableHttpClient client = HttpClients.createDefault();
try {
if (host != null) {
final org.apache.hc.core5.http.HttpHost httpHost =
new org.apache.hc.core5.http.HttpHost(host);
final org.apache.hc.client5.http.classic.methods.HttpGet request =
new org.apache.hc.client5.http.classic.methods.HttpGet("/");
client.execute(httpHost, request);
} else if (url != null) {
final org.apache.hc.client5.http.classic.methods.HttpGet request =
new org.apache.hc.client5.http.classic.methods.HttpGet(url);
client.execute(request);
} else if (urlHandler != null) {
final org.apache.hc.client5.http.classic.methods.HttpGet request =
new org.apache.hc.client5.http.classic.methods.HttpGet(urlHandler);
client.execute(request, response -> null);
}
client.close();
} catch (Exception e) {
}
return "ok";
}

@PostMapping("/apache-httpasyncclient")
public String apacheHttpAsyncClient(
@RequestParam(value = "url", required = false) final String url,
@RequestParam(value = "host", required = false) final String host,
@RequestParam(value = "urlProducer", required = false) final String urlProducer) {
final CloseableHttpAsyncClient client = HttpAsyncClients.createDefault();
client.start();
try {
if (host != null) {
final HttpHost httpHost = new HttpHost(host);
client.execute(httpHost, new HttpGet("/"), null);
} else if (url != null) {
final HttpGet request = new HttpGet(url);
client.execute(request, null);
} else if (urlProducer != null) {
final HttpAsyncRequestProducer producer = HttpAsyncMethods.create(new HttpGet(urlProducer));
client.execute(producer, null, null);
}
} catch (Exception e) {
} finally {
try {
client.close();
} catch (Exception e) {
}
}
return "ok";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
'host' | 'dd.datad0g.com'
}

void 'ssrf is present (#path)'() {
void 'ssrf is present (#path) (#parameter)'() {
setup:
final url = "http://localhost:${httpPort}/ssrf/${path}"
final body = new FormBody.Builder().add(parameter, value).build()
Expand All @@ -744,21 +744,29 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
&& parts[0].value == value && parts[0].source.origin == 'http.request.parameter' && parts[0].source.name == parameter
} else if (parameter == 'host') {
String protocol = protocolSecure ? 'https://' : 'http://'
return parts.size() == 2
&& parts[0].value == protocol + value && parts[0].source.origin == 'http.request.parameter' && parts[0].source.name == parameter
&& parts[1].value == '/' && parts[1].source == null
String finalValue = protocol + value + (endSlash ? '/' : '')
return parts[0].value.endsWith(finalValue) && parts[0].source.origin == 'http.request.parameter' && parts[0].source.name == parameter
} else if (parameter == 'urlProducer' || parameter == 'urlHandler') {
return parts.size() == 1
&& parts[0].value.endsWith(value) && parts[0].source.origin == 'http.request.parameter' && parts[0].source.name == parameter
} else {
throw new IllegalArgumentException("Parameter $parameter not supported")
}
}

where:
path | parameter | value | protocolSecure
"apache-httpclient4" | "url" | "https://dd.datad0g.com/" | true
"apache-httpclient4" | "host" | "dd.datad0g.com" | false
"commons-httpclient2" | "url" | "https://dd.datad0g.com/" | true
"okHttp2" | "url" | "https://dd.datad0g.com/" | true
"okHttp3" | "url" | "https://dd.datad0g.com/" | true
path | parameter | value | protocolSecure | endSlash
"apache-httpclient4" | "url" | "https://dd.datad0g.com/" | true | true
"apache-httpclient4" | "host" | "dd.datad0g.com" | false | false
"apache-httpasyncclient" | "url" | "https://dd.datad0g.com/" | true | true
"apache-httpasyncclient" | "urlProducer" | "https://dd.datad0g.com/" | true | true
"apache-httpasyncclient" | "host" | "dd.datad0g.com" | false | false
"apache-httpclient5" | "url" | "https://dd.datad0g.com/" | true | true
"apache-httpclient5" | "urlHandler" | "https://dd.datad0g.com/" | true | true
"apache-httpclient5" | "host" | "dd.datad0g.com" | false | true
"commons-httpclient2" | "url" | "https://dd.datad0g.com/" | true | true
"okHttp2" | "url" | "https://dd.datad0g.com/" | true | true
"okHttp3" | "url" | "https://dd.datad0g.com/" | true | true
}

void 'test iast metrics stored in spans'() {
Expand Down
2 changes: 2 additions & 0 deletions dd-smoke-tests/spring-boot-2.6-webmvc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ dependencies {
implementation group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.0'
implementation group: 'com.squareup.okhttp', name: 'okhttp', version: '2.2.0'
implementation group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0'
implementation group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.0'
implementation group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.0'

testImplementation project(':dd-smoke-tests')
implementation project(':dd-smoke-tests:iast-util')
Expand Down
3 changes: 2 additions & 1 deletion dd-smoke-tests/springboot/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ dependencies {
implementation group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.0'
implementation group: 'com.squareup.okhttp', name: 'okhttp', version: '2.2.0'
implementation group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0'

implementation group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.0'
implementation group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.0'

testImplementation project(':dd-smoke-tests')
testImplementation(testFixtures(project(":dd-smoke-tests:iast-util")))
Expand Down
Loading
Loading