laantungir/amethyst
1
0
Fork 0
mirror of https://github.com/vitorpamplona/amethyst.git synced 2026-08-08 23:54:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2936 from vitorpamplona/claude/fix-dns-loopback-caching-ntjbN

Browse Source 
Filter DNS poison (loopback/any-local) from non-loopback hosts
This commit is contained in:
Vitor Pamplona
2026-05-16 09:42:03 -04:00
committed by GitHub
parent a68ce75313 c36c9ccf43
commit bbeba28f0b
2 changed files with 183 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDns.kt
+61 -7
View File
@@ -139,7 +139,15 @@ class SurgeDns(
private fun lookupAndCache(host: String): List<InetAddress> =
try {
val addresses = delegate.lookup(host).ifEmpty { throw UnknownHostException(host) }
val raw = delegate.lookup(host).ifEmpty { throw UnknownHostException(host) }
val addresses =
if (isLoopbackHostname(host)) {
raw
} else {
raw
.filterNot { it.isLoopbackAddress || it.isAnyLocalAddress }
.ifEmpty { throw UnknownHostException(host) }
}
putPositive(host, addresses)
addresses
} catch (e: UnknownHostException) {
@@ -233,22 +241,35 @@ class SurgeDns(
/**
* Restore from a previously taken snapshot. Entries already expired on disk are dropped.
* Existing in-memory entries are NOT overwritten (so a fresh lookup that completes before
* restore lands keeps its newer answer).
* restore lands keeps its newer answer). Loopback / any-local addresses (127/8, ::1, 0.0.0.0)
* are filtered out for non-loopback hostnames — they're poison left over from a captive
* portal, ad-blocker DNS, or VPN hiccup at snapshot time. When such addresses are dropped
* the cache is marked dirty so the next persist re-writes the snapshot without them.
*/
fun restore(records: List<DnsCacheRecord>) {
val now = System.currentTimeMillis()
var droppedPoisoned = false
for (record in records) {
if (record.expiresAtMillis <= now) continue
val addresses =
record.addresses.mapNotNull { literal ->
// getByName on a numeric literal parses without doing DNS.
runCatching { InetAddress.getByName(literal) }.getOrNull()
val key = record.hostname.lowercase(Locale.ROOT)
val allowLoopback = isLoopbackHostname(key)
val addresses = ArrayList<InetAddress>(record.addresses.size)
for (literal in record.addresses) {
// getByName on a numeric literal parses without doing DNS.
val addr = runCatching { InetAddress.getByName(literal) }.getOrNull() ?: continue
if (!allowLoopback && (addr.isLoopbackAddress || addr.isAnyLocalAddress)) {
droppedPoisoned = true
continue
}
addresses += addr
}
if (addresses.isNotEmpty()) {
val key = record.hostname.lowercase(Locale.ROOT)
cache.putIfAbsent(key, Entry(addresses, record.expiresAtMillis))
}
}
// Dirty the cache so the next save rewrites the snapshot without the poisoned entries
// — otherwise they'd be re-restored on every cold start.
if (droppedPoisoned) dirty.set(true)
}
/** True if the cache has changed since the last [tryClearDirty] / [markDirty] / construction. */
@@ -267,6 +288,39 @@ class SurgeDns(
dirty.set(true)
}
/**
* True if [host] is itself a loopback identifier — the user-configured `localhost`-style relay
* case where 127.0.0.1 / ::1 in the answer is legitimate and must not be filtered as poison.
* Accepts the DNS name `localhost` and any `.localhost` subdomain (RFC 6761), and IP literals
* that parse to a loopback or any-local address (127.0.0.0/8, ::1, 0.0.0.0). [host] is assumed
* already lowercased.
*/
private fun isLoopbackHostname(host: String): Boolean {
// RFC 1034: a trailing dot is the FQDN form (e.g. `localhost.`), still the same name.
val name = host.trimEnd('.')
if (name == "localhost" || name.endsWith(".localhost")) return true
val literal = parseIpLiteral(name) ?: return false
return literal.isLoopbackAddress || literal.isAnyLocalAddress
}
/**
* Parses [host] as a numeric IP literal without ever triggering a DNS lookup. Returns null for
* anything that doesn't structurally look like an IPv4 or IPv6 literal — we must not hand a
* plain hostname to [InetAddress.getByName] from inside the resolver.
*/
private fun parseIpLiteral(host: String): InetAddress? {
val candidate =
if (host.startsWith("[") && host.endsWith("]")) {
host.substring(1, host.length - 1)
} else {
host
}
val looksLikeIpv4 = candidate.isNotEmpty() && candidate.all { it.isDigit() || it == '.' } && candidate.contains('.')
val looksLikeIpv6 = candidate.contains(':')
if (!looksLikeIpv4 && !looksLikeIpv6) return null
return runCatching { InetAddress.getByName(candidate) }.getOrNull()
}
private class Entry(
val addresses: List<InetAddress>,
val expiresAtMillis: Long,
amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsTest.kt
+122
View File
@@ -497,6 +497,128 @@ class SurgeDnsTest {
assertThrows(UnknownHostException::class.java) { dns.lookup("a.example") }
}
@Test
fun `loopback address from upstream is rejected for non-loopback host`() {
val upstream = CountingDns(mapOf("relay.example" to listOf(ip("127.0.0.1"))))
val dns = SurgeDns(delegate = upstream)
assertThrows(UnknownHostException::class.java) { dns.lookup("relay.example") }
assertFalse("Rejected answer must not be cached as positive", dns.isDirty())
}
@Test
fun `any-local address from upstream is rejected for non-loopback host`() {
val upstream = CountingDns(mapOf("relay.example" to listOf(ip("0.0.0.0"))))
val dns = SurgeDns(delegate = upstream)
assertThrows(UnknownHostException::class.java) { dns.lookup("relay.example") }
}
@Test
fun `loopback addresses are stripped but routable addresses are kept`() {
val upstream =
CountingDns(mapOf("relay.example" to listOf(ip("127.0.0.1"), ip("5.6.7.8"))))
val dns = SurgeDns(delegate = upstream)
val result = dns.lookup("relay.example")
assertEquals(listOf(ip("5.6.7.8")), result)
}
@Test
fun `localhost hostname keeps loopback answers`() {
val upstream = CountingDns(mapOf("localhost" to listOf(ip("127.0.0.1"))))
val dns = SurgeDns(delegate = upstream)
assertEquals(listOf(ip("127.0.0.1")), dns.lookup("localhost"))
}
@Test
fun `dot-localhost subdomain keeps loopback answers`() {
val upstream = CountingDns(mapOf("relay.localhost" to listOf(ip("127.0.0.1"))))
val dns = SurgeDns(delegate = upstream)
assertEquals(listOf(ip("127.0.0.1")), dns.lookup("relay.localhost"))
}
@Test
fun `trailing-dot FQDN form of localhost keeps loopback answers`() {
// RFC 1034: `localhost.` is the same name as `localhost`, just in FQDN form.
val upstream = CountingDns(mapOf("localhost." to listOf(ip("127.0.0.1"))))
val dns = SurgeDns(delegate = upstream)
assertEquals(listOf(ip("127.0.0.1")), dns.lookup("localhost."))
}
@Test
fun `ipv4 loopback literal keeps loopback answers`() {
val upstream = CountingDns(mapOf("127.0.0.1" to listOf(ip("127.0.0.1"))))
val dns = SurgeDns(delegate = upstream)
assertEquals(listOf(ip("127.0.0.1")), dns.lookup("127.0.0.1"))
}
@Test
fun `ipv6 loopback literal keeps loopback answers`() {
val upstream = CountingDns(mapOf("::1" to listOf(ip("::1"))))
val dns = SurgeDns(delegate = upstream)
assertEquals(listOf(ip("::1")), dns.lookup("::1"))
}
@Test
fun `restore filters loopback poison and marks cache dirty`() {
val upstream = CountingDns(mapOf("relay.example" to listOf(ip("5.6.7.8"))))
val dns = SurgeDns(delegate = upstream)
val expiresAt = System.currentTimeMillis() + 60_000
dns.restore(listOf(DnsCacheRecord("relay.example", listOf("127.0.0.1"), expiresAt)))
assertTrue("Dropping poison must dirty the cache so the snapshot is rewritten", dns.isDirty())
// Cache should not hold the poisoned entry, so the next lookup hits upstream.
assertEquals(listOf(ip("5.6.7.8")), dns.lookup("relay.example"))
assertEquals(1, upstream.calls("relay.example"))
}
@Test
fun `restore keeps non-loopback addresses when poison is mixed in`() {
val upstream = CountingDns(mapOf("relay.example" to listOf(ip("9.9.9.9"))))
val dns = SurgeDns(delegate = upstream)
val expiresAt = System.currentTimeMillis() + 60_000
dns.restore(
listOf(
DnsCacheRecord("relay.example", listOf("127.0.0.1", "5.6.7.8"), expiresAt),
),
)
assertTrue("Dropping one poisoned address still dirties the cache", dns.isDirty())
assertEquals(listOf(ip("5.6.7.8")), dns.lookup("relay.example"))
assertEquals("Restored survivors should serve without upstream", 0, upstream.calls("relay.example"))
}
@Test
fun `restore keeps loopback for localhost host`() {
val upstream = CountingDns(emptyMap())
val dns = SurgeDns(delegate = upstream)
val expiresAt = System.currentTimeMillis() + 60_000
dns.restore(listOf(DnsCacheRecord("localhost", listOf("127.0.0.1"), expiresAt)))
assertFalse("Loopback entry for localhost is legitimate, not poison", dns.isDirty())
assertEquals(listOf(ip("127.0.0.1")), dns.lookup("localhost"))
}
@Test
fun `restore of clean snapshot does not dirty the cache`() {
val upstream = CountingDns(emptyMap())
val dns = SurgeDns(delegate = upstream)
val expiresAt = System.currentTimeMillis() + 60_000
dns.restore(listOf(DnsCacheRecord("relay.example", listOf("5.6.7.8"), expiresAt)))
assertFalse("Clean restore must not dirty the cache", dns.isDirty())
}
@Test
fun `refresh executor rejection cleans up the inflight slot`() {
val upstream = CountingDns(mapOf("a.example" to listOf(ip("1.2.3.4"))))