Merge pull request #311 from AikidoSec/revert/pr-308-dns-private-ip-literals

Revert "Merge pull request #308 from AikidoSec/fix/dns-collector-skip…

Author: hansott

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

@@ -12,7 +12,6 @@
 import dev.aikido.agent_api.vulnerabilities.ssrf.SSRFException;
 import dev.aikido.agent_api.helpers.logging.LogManager;
 import dev.aikido.agent_api.helpers.logging.Logger;
-import dev.aikido.agent_api.vulnerabilities.ssrf.IsPrivateIP;
 import dev.aikido.agent_api.vulnerabilities.ssrf.StoredSSRFDetector;
 import dev.aikido.agent_api.vulnerabilities.ssrf.StoredSSRFException;
 
@@ -38,23 +37,13 @@ public static void report(String hostname, InetAddress[] inetAddresses) {
             // Consume pending ports recorded by URLCollector for this hostname.
             // Removing them here ensures each (hostname, port) pair is counted exactly once.
             Set<Integer> ports = PendingHostnamesStore.getAndRemove(hostname);
-
-            // The outbound-domains list is meant for hostnames, not raw IP literals.
-            // A private/internal IP literal passed straight to getAllByName (DNS-resolver
-            // bootstrap, service discovery connecting by IP, libraries building a private-IP
-            // matcher, ...) is not an outbound domain and would otherwise flood the
-            // "new outbound connection" feature. Skip recording those; SSRF/stored-SSRF and
-            // outbound-domain blocking below are unaffected.
-            boolean isPrivateIpLiteral = IsPrivateIP.isPrivateIp(hostname);
-            if (!isPrivateIpLiteral) {
-                if (!ports.isEmpty()) {
-                    for (int port : ports) {
-                        HostnamesStore.incrementHits(hostname, port);
-                    }
-                } else {
-                    // We still need to report a hit to the hostname for outbound domain blocking
-                    HostnamesStore.incrementHits(hostname, 0);
+            if (!ports.isEmpty()) {
+                for (int port : ports) {
+                    HostnamesStore.incrementHits(hostname, port);
                 }
+            } else {
+                // We still need to report a hit to the hostname for outbound domain blocking
+                HostnamesStore.incrementHits(hostname, 0);
             }
 
             // Block if the hostname is in the blocked domains list

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

@@ -223,59 +223,4 @@ public void testStoredSSRFWithNoContext() throws InterruptedException {
             DNSRecordCollector.report("metadata.google.internal", new InetAddress[]{imdsAddress1, inetAddress2});
         });
     }
-
-    @Test
-    public void testPrivateIpLiteralNotRecordedAsOutboundHostname() throws UnknownHostException {
-        // Regression: a private/internal IP literal passed straight to getAllByName
-        // (e.g. Reactor Netty resolver bootstrap resolving nameserver/gateway addresses,
-        // service discovery connecting by IP, a library building a private-IP matcher)
-        // must NOT be recorded as an outbound hostname, otherwise it floods the
-        // "new outbound connection" feature on port 0.
-        Context.set(null);
-        DNSRecordCollector.report("10.0.0.0", new InetAddress[]{InetAddress.getByName("10.0.0.0")});
-        DNSRecordCollector.report("172.16.0.0", new InetAddress[]{InetAddress.getByName("172.16.0.0")});
-        DNSRecordCollector.report("192.168.0.0", new InetAddress[]{InetAddress.getByName("192.168.0.0")});
-        DNSRecordCollector.report("169.254.0.0", new InetAddress[]{InetAddress.getByName("169.254.0.0")});
-        DNSRecordCollector.report("10.20.11.143", new InetAddress[]{InetAddress.getByName("10.20.11.143")});
-        DNSRecordCollector.report("127.0.0.1", new InetAddress[]{inetAddress2});
-
-        assertEquals(0, HostnamesStore.getHostnamesAsList().length);
-    }
-
-    @Test
-    public void testPrivateIpLiteralWithPendingPortStillConsumedButNotRecorded() {
-        // The pending port must still be consumed (so it can't leak into a later
-        // lookup of a different hostname), but nothing is recorded for the IP literal.
-        PendingHostnamesStore.add("10.20.11.143", 443);
-        Context.set(mock(ContextObject.class));
-
-        DNSRecordCollector.report("10.20.11.143", new InetAddress[]{inetAddress2});
-
-        assertEquals(0, HostnamesStore.getHostnamesAsList().length);
-        assertTrue(PendingHostnamesStore.getPorts("10.20.11.143").isEmpty());
-    }
-
-    @Test
-    public void testHostnameResolvingToPrivateIpStillRecorded() {
-        // Only raw IP literals are filtered. A real DNS name that resolves to a private
-        // IP (the customer's internal services) is still recorded by NAME.
-        Context.set(null);
-        DNSRecordCollector.report("keycloak.internal.example.com", new InetAddress[]{inetAddress2});
-
-        Hostnames.HostnameEntry[] entries = HostnamesStore.getHostnamesAsList();
-        assertEquals(1, entries.length);
-        assertEquals("keycloak.internal.example.com", entries[0].getHostname());
-        assertEquals(0, entries[0].getPort());
-    }
-
-    @Test
-    public void testPublicIpLiteralStillRecorded() {
-        // Scope: only PRIVATE IP literals are dropped; public IP literals remain visible.
-        Context.set(null);
-        DNSRecordCollector.report("1.1.1.1", new InetAddress[]{inetAddress1});
-
-        Hostnames.HostnameEntry[] entries = HostnamesStore.getHostnamesAsList();
-        assertEquals(1, entries.length);
-        assertEquals("1.1.1.1", entries[0].getHostname());
-    }
 }