Skip to content

Commit bcfa853

Browse files
committed
Revert "Merge pull request #308 from AikidoSec/fix/dns-collector-skip-private-ip-literals"
This reverts commit 8987c88, reversing changes made to 65ba7fc.
1 parent f394d23 commit bcfa853

2 files changed

Lines changed: 6 additions & 72 deletions

File tree

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

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
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;
1615
import dev.aikido.agent_api.vulnerabilities.ssrf.StoredSSRFDetector;
1716
import dev.aikido.agent_api.vulnerabilities.ssrf.StoredSSRFException;
1817

@@ -38,23 +37,13 @@ public static void report(String hostname, InetAddress[] inetAddresses) {
3837
// Consume pending ports recorded by URLCollector for this hostname.
3938
// Removing them here ensures each (hostname, port) pair is counted exactly once.
4039
Set<Integer> ports = PendingHostnamesStore.getAndRemove(hostname);
41-
42-
// The outbound-domains list is meant for hostnames, not raw IP literals.
43-
// A private/internal IP literal passed straight to getAllByName (DNS-resolver
44-
// bootstrap, service discovery connecting by IP, libraries building a private-IP
45-
// matcher, ...) is not an outbound domain and would otherwise flood the
46-
// "new outbound connection" feature. Skip recording those; SSRF/stored-SSRF and
47-
// outbound-domain blocking below are unaffected.
48-
boolean isPrivateIpLiteral = IsPrivateIP.isPrivateIp(hostname);
49-
if (!isPrivateIpLiteral) {
50-
if (!ports.isEmpty()) {
51-
for (int port : ports) {
52-
HostnamesStore.incrementHits(hostname, port);
53-
}
54-
} else {
55-
// We still need to report a hit to the hostname for outbound domain blocking
56-
HostnamesStore.incrementHits(hostname, 0);
40+
if (!ports.isEmpty()) {
41+
for (int port : ports) {
42+
HostnamesStore.incrementHits(hostname, port);
5743
}
44+
} else {
45+
// We still need to report a hit to the hostname for outbound domain blocking
46+
HostnamesStore.incrementHits(hostname, 0);
5847
}
5948

6049
// Block if the hostname is in the blocked domains list

agent_api/src/test/java/collectors/DNSRecordCollectorTest.java

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -223,59 +223,4 @@ public void testStoredSSRFWithNoContext() throws InterruptedException {
223223
DNSRecordCollector.report("metadata.google.internal", new InetAddress[]{imdsAddress1, inetAddress2});
224224
});
225225
}
226-
227-
@Test
228-
public void testPrivateIpLiteralNotRecordedAsOutboundHostname() throws UnknownHostException {
229-
// Regression: a private/internal IP literal passed straight to getAllByName
230-
// (e.g. Reactor Netty resolver bootstrap resolving nameserver/gateway addresses,
231-
// service discovery connecting by IP, a library building a private-IP matcher)
232-
// must NOT be recorded as an outbound hostname, otherwise it floods the
233-
// "new outbound connection" feature on port 0.
234-
Context.set(null);
235-
DNSRecordCollector.report("10.0.0.0", new InetAddress[]{InetAddress.getByName("10.0.0.0")});
236-
DNSRecordCollector.report("172.16.0.0", new InetAddress[]{InetAddress.getByName("172.16.0.0")});
237-
DNSRecordCollector.report("192.168.0.0", new InetAddress[]{InetAddress.getByName("192.168.0.0")});
238-
DNSRecordCollector.report("169.254.0.0", new InetAddress[]{InetAddress.getByName("169.254.0.0")});
239-
DNSRecordCollector.report("10.20.11.143", new InetAddress[]{InetAddress.getByName("10.20.11.143")});
240-
DNSRecordCollector.report("127.0.0.1", new InetAddress[]{inetAddress2});
241-
242-
assertEquals(0, HostnamesStore.getHostnamesAsList().length);
243-
}
244-
245-
@Test
246-
public void testPrivateIpLiteralWithPendingPortStillConsumedButNotRecorded() {
247-
// The pending port must still be consumed (so it can't leak into a later
248-
// lookup of a different hostname), but nothing is recorded for the IP literal.
249-
PendingHostnamesStore.add("10.20.11.143", 443);
250-
Context.set(mock(ContextObject.class));
251-
252-
DNSRecordCollector.report("10.20.11.143", new InetAddress[]{inetAddress2});
253-
254-
assertEquals(0, HostnamesStore.getHostnamesAsList().length);
255-
assertTrue(PendingHostnamesStore.getPorts("10.20.11.143").isEmpty());
256-
}
257-
258-
@Test
259-
public void testHostnameResolvingToPrivateIpStillRecorded() {
260-
// Only raw IP literals are filtered. A real DNS name that resolves to a private
261-
// IP (the customer's internal services) is still recorded by NAME.
262-
Context.set(null);
263-
DNSRecordCollector.report("keycloak.internal.example.com", new InetAddress[]{inetAddress2});
264-
265-
Hostnames.HostnameEntry[] entries = HostnamesStore.getHostnamesAsList();
266-
assertEquals(1, entries.length);
267-
assertEquals("keycloak.internal.example.com", entries[0].getHostname());
268-
assertEquals(0, entries[0].getPort());
269-
}
270-
271-
@Test
272-
public void testPublicIpLiteralStillRecorded() {
273-
// Scope: only PRIVATE IP literals are dropped; public IP literals remain visible.
274-
Context.set(null);
275-
DNSRecordCollector.report("1.1.1.1", new InetAddress[]{inetAddress1});
276-
277-
Hostnames.HostnameEntry[] entries = HostnamesStore.getHostnamesAsList();
278-
assertEquals(1, entries.length);
279-
assertEquals("1.1.1.1", entries[0].getHostname());
280-
}
281226
}

0 commit comments

Comments
 (0)