Skip to content

Commit 1eab8be

Browse files
author
dmitrii
committed
Track and protect WebClient outbound requests, fix private-IP SSRF regression
Follow-up to the reverted #308/#310. Customer flood was InetAddress.getAllByName() picking up Reactor Netty's own DNS-resolver bootstrap noise (0.0.0.0, ::, /etc/resolv.conf nameservers) as "new outbound connections" on port 0, and blocking them in lockdown mode. #310 fixed the flood with an early return in DNSRecordCollector.report() that also skipped the SSRF check below it - verified with a regression test that this let an attacker-supplied private-IP literal (e.g. a webhook field pointing straight at 169.254.169.254) through undetected. Investigating further found the actual root cause is bigger: Spring's WebClient was never instrumented at all, and Reactor Netty's default HTTP client bypasses InetAddress.getAllByName() entirely (it uses its own async DNS resolver). So even after wrapping WebClient to register pending ports, DNSRecordCollector was never invoked for real WebClient targets - confirmed empirically via trace logs against a live running app, with distinct markers proving InetAddressWrapper never fires for WebClient/Reactor Netty traffic in this configuration. WebClient had zero outbound-domain visibility and zero SSRF protection, independent of the original bug. - DNSRecordCollector: narrow the private-IP-literal gate to only skip recording + outbound blocking when there's no pending port (genuine infra noise). SSRF checks are unconditional again, fixing the bypass above. - SpringWebClientWrapper: register pending host+port for every WebClient request by hooking ExchangeFunction.exchange(), the interface every WebClient call goes through, same pattern as the existing OkHttp/Apache/JDK HttpClient wrappers. Uses string-based ByteBuddy matchers (hasSuperType(named(...))) instead of .class literals, since spring-webflux is compileOnly and only present on the target app's classloader - a .class reference in the matcher crashes the agent at premain with NoClassDefFoundError. - SocketChannelWrapper: hook java.nio.channels.SocketChannel.connect(), the JDK-level call every NIO-based client (including Reactor Netty) makes once it has a resolved address, regardless of which DNS resolver produced it. This is what actually closes the gap for WebClient, and it also catches literal IP targets that never go through any resolver at all. Not Netty-specific instrumentation - it's a generic JDK hook with no references to io.netty.* types. - DNSRecordCollector.reportConnect(): entry point for the new hook. Peeks the pending port instead of consuming it (report()'s getAndRemove), because a single request can trigger multiple connect() calls to the same hostname (e.g. the IPv4 then IPv6 address of a dual-stack host like localhost). Consuming on the first attempt let a blocked SSRF target succeed on the second attempt via the other address family - found live, fixed, covered by a regression test. - PendingHostnamesStore: peeking instead of consuming means entries rely on WebRequestCollector's per-incoming-request clear() for cleanup, which never fires for WebClient calls made outside any incoming-request context (e.g. a @scheduled background task). Capped the store at 1000 entries per thread, evicting the least-recently-used one once exceeded - the same bounded-LRU pattern (LinkedHashMap with accessOrder=true + removeEldestEntry()) already used by Hostnames.java for the same class of problem. Deliberately not a time-based TTL, to avoid a timing-dependent race reopening the dual-stack gap under load. - RequestController (SpringWebfluxSampleApp): new /api/request endpoint used to validate all of the above against a real running app end to end. Known limitation, not fixed here: Spring WebFlux has no request-body taint tracking at all (SpringWebfluxContextObject never populates ContextObject.body), so SSRF via JSON body can't be detected for WebFlux apps regardless of this change - flagged separately, doesn't regress anything.
1 parent 8a92b6b commit 1eab8be

9 files changed

Lines changed: 477 additions & 24 deletions

File tree

agent/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ dependencies {
1212
compileOnly 'io.projectreactor.netty:reactor-netty-http:1.2.1' // For Spring Webflux
1313
compileOnly 'io.javalin:javalin:6.4.0'
1414
compileOnly 'org.springframework:spring-web:5.3.20'
15+
compileOnly 'org.springframework:spring-webflux:5.3.20' // For Spring WebClient
1516
}
1617

1718
shadowJar {

agent/src/main/java/dev/aikido/agent/Wrappers.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import dev.aikido.agent.wrappers.spring.SpringWebfluxWrapper;
1010
import dev.aikido.agent.wrappers.spring.SpringControllerWrapper;
1111
import dev.aikido.agent.wrappers.spring.SpringMVCJakartaWrapper;
12+
import dev.aikido.agent.wrappers.spring.SpringWebClientWrapper;
1213

1314
import java.util.Arrays;
1415
import java.util.List;
@@ -30,11 +31,13 @@ private Wrappers() {}
3031
// SSRF/HTTP wrappers
3132
new HttpURLConnectionWrapper(),
3233
new InetAddressWrapper(),
34+
new SocketChannelWrapper(),
3335
new HttpClientWrapper(),
3436
new HttpConnectionRedirectWrapper(),
3537
new HttpClientSendWrapper(),
3638
new OkHttpWrapper(),
3739
new ApacheHttpClientWrapper(),
40+
new SpringWebClientWrapper(),
3841

3942
new PathWrapper(),
4043
new PathsWrapper(),
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package dev.aikido.agent.wrappers;
2+
import net.bytebuddy.asm.Advice;
3+
import net.bytebuddy.description.method.MethodDescription;
4+
import net.bytebuddy.description.type.TypeDescription;
5+
import net.bytebuddy.matcher.ElementMatcher;
6+
import net.bytebuddy.matcher.ElementMatchers;
7+
8+
import java.lang.reflect.InvocationTargetException;
9+
import java.lang.reflect.Method;
10+
import java.net.InetAddress;
11+
import java.net.InetSocketAddress;
12+
import java.net.MalformedURLException;
13+
import java.net.SocketAddress;
14+
import java.net.URL;
15+
import java.net.URLClassLoader;
16+
import java.nio.channels.SocketChannel;
17+
18+
public class SocketChannelWrapper implements Wrapper {
19+
public String getName() {
20+
// Wrap connect(SocketAddress) on SocketChannel. Clients that resolve hostnames with
21+
// their own DNS resolver instead of InetAddress.getAllByName() (e.g. Reactor Netty's
22+
// async resolver, used by default by Spring's WebClient) never trigger
23+
// InetAddressWrapper, so this is the only point where we see the resolved address
24+
// before the connection is made. Also catches literal IP targets, which never go
25+
// through any resolver at all.
26+
// https://docs.oracle.com/javase/8/docs/api/java/nio/channels/SocketChannel.html#connect-java.net.SocketAddress-
27+
return SocketChannelAdvice.class.getName();
28+
}
29+
public ElementMatcher<? super MethodDescription> getMatcher() {
30+
return ElementMatchers.named("connect");
31+
}
32+
@Override
33+
public ElementMatcher<? super TypeDescription> getTypeMatcher() {
34+
return ElementMatchers.isSubTypeOf(SocketChannel.class);
35+
}
36+
public static class SocketChannelAdvice {
37+
// Since we have to wrap a native Java Class stuff gets more complicated
38+
// The classpath is not the same anymore, and we can't import our modules directly.
39+
// To bypass this issue we load collectors from a .jar file
40+
@Advice.OnMethodEnter
41+
public static void before(
42+
@Advice.Argument(0) SocketAddress remoteAddress
43+
) throws Throwable {
44+
if (!(remoteAddress instanceof InetSocketAddress)) {
45+
return;
46+
}
47+
InetSocketAddress inetSocketAddress = (InetSocketAddress) remoteAddress;
48+
InetAddress resolvedAddress = inetSocketAddress.getAddress();
49+
if (resolvedAddress == null) {
50+
// Unresolved: nothing to report yet, connect() will throw on its own.
51+
return;
52+
}
53+
String hostname = inetSocketAddress.getHostString();
54+
55+
String jarFilePath = System.getProperty("AIK_agent_api_jar");
56+
URLClassLoader classLoader = null;
57+
try {
58+
URL[] urls = { new URL(jarFilePath) };
59+
classLoader = new URLClassLoader(urls);
60+
} catch (MalformedURLException ignored) {}
61+
if (classLoader == null) {
62+
return;
63+
}
64+
65+
try {
66+
// Load the class from the JAR
67+
Class<?> clazz = classLoader.loadClass("dev.aikido.agent_api.collectors.DNSRecordCollector");
68+
69+
// Run reportConnect with "argument"
70+
for (Method method2: clazz.getMethods()) {
71+
if(method2.getName().equals("reportConnect")) {
72+
method2.invoke(null, hostname, resolvedAddress);
73+
break;
74+
}
75+
}
76+
classLoader.close(); // Close the class loader
77+
} catch (InvocationTargetException invocationTargetException) {
78+
if(invocationTargetException.getCause().toString().startsWith("dev.aikido.agent_api.vulnerabilities")) {
79+
throw invocationTargetException.getCause();
80+
}
81+
// Ignore non-aikido throwables.
82+
} catch(Throwable e) {
83+
System.out.println("AIKIDO: " + e.getMessage());
84+
}
85+
}
86+
}
87+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package dev.aikido.agent.wrappers.spring;
2+
3+
import dev.aikido.agent.wrappers.Wrapper;
4+
import dev.aikido.agent_api.collectors.URLCollector;
5+
import net.bytebuddy.asm.Advice;
6+
import net.bytebuddy.description.method.MethodDescription;
7+
import net.bytebuddy.description.type.TypeDescription;
8+
import net.bytebuddy.matcher.ElementMatcher;
9+
import net.bytebuddy.matcher.ElementMatchers;
10+
import org.springframework.web.reactive.function.client.ClientRequest;
11+
12+
import java.net.MalformedURLException;
13+
14+
public class SpringWebClientWrapper implements Wrapper {
15+
// Referenced by name (not by .class) in the matchers below: ExchangeFunction is only on
16+
// the target application's classloader (spring-webflux is compileOnly here), not on the
17+
// agent's own classloader, so a .class literal would throw NoClassDefFoundError at premain.
18+
private static final String EXCHANGE_FUNCTION_CLASS_NAME =
19+
"org.springframework.web.reactive.function.client.ExchangeFunction";
20+
21+
public String getName() {
22+
// Wrap exchange(ClientRequest) on ExchangeFunction, the interface every WebClient
23+
// request goes through before Reactor Netty resolves/connects.
24+
// https://docs.spring.io/spring-framework/docs/5.3.20/javadoc-api/org/springframework/web/reactive/function/client/ExchangeFunction.html
25+
return SpringWebClientAdvice.class.getName();
26+
}
27+
public ElementMatcher<? super MethodDescription> getMatcher() {
28+
return ElementMatchers.isDeclaredBy(getTypeMatcher())
29+
.and(ElementMatchers.named("exchange"));
30+
}
31+
@Override
32+
public ElementMatcher<? super TypeDescription> getTypeMatcher() {
33+
return ElementMatchers.hasSuperType(ElementMatchers.named(EXCHANGE_FUNCTION_CLASS_NAME));
34+
}
35+
public static class SpringWebClientAdvice {
36+
@Advice.OnMethodEnter(suppress = Throwable.class)
37+
public static void before(
38+
@Advice.Argument(0) ClientRequest request
39+
) throws MalformedURLException {
40+
if (request == null || request.url() == null) {
41+
return;
42+
}
43+
// Report the URL before the request is sent, so DNSRecordCollector can match the
44+
// DNS lookup that follows to this outgoing request.
45+
URLCollector.report(request.url().toURL());
46+
}
47+
}
48+
}

agent_api/src/main/java/dev/aikido/agent_api/collectors/DNSRecordCollector.java

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import dev.aikido.agent_api.vulnerabilities.ssrf.SSRFException;
1313
import dev.aikido.agent_api.helpers.logging.LogManager;
1414
import dev.aikido.agent_api.helpers.logging.Logger;
15+
import dev.aikido.agent_api.vulnerabilities.ssrf.IsPrivateIP;
1516
import dev.aikido.agent_api.vulnerabilities.ssrf.StoredSSRFDetector;
1617
import dev.aikido.agent_api.vulnerabilities.ssrf.StoredSSRFException;
1718

@@ -26,30 +27,44 @@
2627
public final class DNSRecordCollector {
2728
private DNSRecordCollector() {}
2829
private static final Logger logger = LogManager.getLogger(DNSRecordCollector.class);
29-
private static final String OPERATION_NAME = "java.net.InetAddress.getAllByName";
30+
private static final String INET_ADDRESS_OPERATION_NAME = "java.net.InetAddress.getAllByName";
31+
private static final String SOCKET_CHANNEL_OPERATION_NAME = "java.nio.channels.SocketChannel.connect";
32+
3033
public static void report(String hostname, InetAddress[] inetAddresses) {
34+
// Consume pending ports recorded by URLCollector for this hostname. InetAddress.getAllByName()
35+
// resolves every address for the hostname in one call, so removing them here ensures each
36+
// (hostname, port) pair is counted exactly once per DNS lookup.
37+
process(hostname, inetAddresses, PendingHostnamesStore.getAndRemove(hostname), INET_ADDRESS_OPERATION_NAME);
38+
}
39+
40+
// Used for clients that resolve their own DNS (bypassing InetAddress.getAllByName entirely,
41+
// e.g. Reactor Netty's async resolver used by default by Spring's WebClient) or that connect
42+
// straight to an IP literal without any resolution step. The resolved address is only known
43+
// at the point of the actual connection, one address at a time.
44+
//
45+
// A single outbound request can trigger multiple connect() calls to the same hostname (e.g.
46+
// trying the IPv4 then the IPv6 address of a dual-stack host). Unlike report(), this peeks the
47+
// pending port instead of consuming it, so every connect attempt for that hostname is still
48+
// checked; consuming on the first attempt would let a later attempt to a different resolved
49+
// address for the same hostname bypass SSRF/blocking entirely. The entry is cleared per
50+
// incoming request by WebRequestCollector instead of being consumed here.
51+
public static void reportConnect(String hostname, InetAddress resolvedAddress) {
52+
process(hostname, new InetAddress[]{resolvedAddress}, PendingHostnamesStore.getPorts(hostname), SOCKET_CHANNEL_OPERATION_NAME);
53+
}
54+
55+
private static void process(String hostname, InetAddress[] inetAddresses, Set<Integer> ports, String operationName) {
3156
try {
3257
logger.trace("DNSRecordCollector called with %s & inet addresses: %s", hostname, List.of(inetAddresses));
3358

3459
// store stats
35-
StatisticsStore.registerCall("java.net.InetAddress.getAllByName", OperationKind.OUTGOING_HTTP_OP);
36-
37-
// Consume pending ports recorded by URLCollector for this hostname.
38-
// Removing them here ensures each (hostname, port) pair is counted exactly once.
39-
Set<Integer> ports = PendingHostnamesStore.getAndRemove(hostname);
40-
if (!ports.isEmpty()) {
41-
for (int port : ports) {
42-
HostnamesStore.incrementHits(hostname, port);
43-
}
44-
} else {
45-
// We still need to report a hit to the hostname for outbound domain blocking
46-
HostnamesStore.incrementHits(hostname, 0);
47-
}
48-
49-
// Block if the hostname is in the blocked domains list
50-
if (ServiceConfigStore.shouldBlockOutgoingRequest(hostname)) {
51-
logger.debug("Blocking DNS lookup for domain: %s", hostname);
52-
throw BlockedOutboundException.get();
60+
StatisticsStore.registerCall(operationName, OperationKind.OUTGOING_HTTP_OP);
61+
62+
// No pending port + a private IP literal = infrastructure noise (Netty's resolver
63+
// bootstrap resolving nameserver/bind addresses, a client connecting directly by
64+
// IP, etc), not a real outbound request. SSRF checks below still run regardless.
65+
boolean isInfrastructureNoise = ports.isEmpty() && IsPrivateIP.isPrivateIp(hostname);
66+
if (!isInfrastructureNoise) {
67+
recordHitAndEnforceBlocklist(hostname, ports);
5368
}
5469

5570
// Convert inetAddresses array to a List of IP strings :
@@ -62,7 +77,7 @@ public static void report(String hostname, InetAddress[] inetAddresses) {
6277
for (int port : ports) {
6378
logger.debug("Hostname: %s, Port: %s, IPs: %s", hostname, port, ipAddresses);
6479

65-
Attack attack = SSRFDetector.run(hostname, port, ipAddresses, OPERATION_NAME);
80+
Attack attack = SSRFDetector.run(hostname, port, ipAddresses, operationName);
6681
if (attack == null) {
6782
continue;
6883
}
@@ -81,7 +96,7 @@ public static void report(String hostname, InetAddress[] inetAddresses) {
8196

8297
// We don't need the context object to check for stored ssrf, but we do want to run this after our other
8398
// SSRF checks, making sure if it's a normal ssrf attack it gets reported like that.
84-
Attack storedSsrfAttack = new StoredSSRFDetector().run(hostname, ipAddresses, OPERATION_NAME);
99+
Attack storedSsrfAttack = new StoredSSRFDetector().run(hostname, ipAddresses, operationName);
85100
if (storedSsrfAttack != null) {
86101
attackDetected(storedSsrfAttack, Context.get());
87102

@@ -97,4 +112,21 @@ public static void report(String hostname, InetAddress[] inetAddresses) {
97112
logger.trace(e);
98113
}
99114
}
115+
116+
// Throws BlockedOutboundException, propagated by process()'s catch clause above.
117+
private static void recordHitAndEnforceBlocklist(String hostname, Set<Integer> ports) {
118+
if (ports.isEmpty()) {
119+
// No pending port: still record a hit so lockdown mode can still block unknown domains.
120+
HostnamesStore.incrementHits(hostname, 0);
121+
} else {
122+
for (int port : ports) {
123+
HostnamesStore.incrementHits(hostname, port);
124+
}
125+
}
126+
127+
if (ServiceConfigStore.shouldBlockOutgoingRequest(hostname)) {
128+
logger.debug("Blocking DNS lookup for domain: %s", hostname);
129+
throw BlockedOutboundException.get();
130+
}
131+
}
100132
}

agent_api/src/main/java/dev/aikido/agent_api/storage/PendingHostnamesStore.java

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,30 @@
44

55
/**
66
* Thread-local bridge between URLCollector and DNSRecordCollector.
7-
* URLCollector records hostname+port here; DNSRecordCollector reads and removes the entry
8-
* so each (hostname, port) pair is processed exactly once per DNS lookup.
7+
* URLCollector records hostname+port here; DNSRecordCollector.report() (fed by
8+
* InetAddress.getAllByName(), which resolves everything in one call) reads and removes the
9+
* entry so each (hostname, port) pair is processed exactly once per DNS lookup.
10+
* DNSRecordCollector.reportConnect() (fed by SocketChannel.connect(), which fires once per
11+
* connect attempt) instead peeks the entry, since a single outbound request can trigger
12+
* multiple connect attempts to the same hostname (e.g. IPv4 then IPv6 for a dual-stack host).
13+
*
14+
* Entries are normally cleared per incoming request by WebRequestCollector, but a peeked
15+
* entry added outside any incoming-request context (e.g. a WebClient call from a @Scheduled
16+
* task) would never be cleared that way. Capped at MAX_ENTRIES per thread, evicting the least
17+
* recently used entry once exceeded, same bounded-LRU pattern as Hostnames.
918
*/
1019
public final class PendingHostnamesStore {
1120
private PendingHostnamesStore() {}
1221

22+
private static final int MAX_ENTRIES = 1000;
23+
1324
private static final ThreadLocal<Map<String, Set<Integer>>> store =
14-
ThreadLocal.withInitial(LinkedHashMap::new);
25+
ThreadLocal.withInitial(() -> new LinkedHashMap<>(16, 0.75f, true) {
26+
@Override
27+
protected boolean removeEldestEntry(Map.Entry<String, Set<Integer>> eldest) {
28+
return size() > MAX_ENTRIES;
29+
}
30+
});
1531

1632
public static void add(String hostname, int port) {
1733
Map<String, Set<Integer>> map = store.get();

0 commit comments

Comments
 (0)