From b6d958d3d0ecf8f86b785bd72416ca6ab0b33044 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Oct 2026 22:38:16 +0000 Subject: [PATCH] perf(backend): bound reverse DNS lookup time on relay details Run the PTR and forward lookups for the relay details host names on a small dedicated pool with an overall deadline of 2 s per IP. Host names that are not verified by then are returned as unverified. When all lookup threads are busy, the response has no host names and nothing is cached, so a later request tries again. Forward lookups now use dnsjava (A/AAAA) like the PTR lookup, with a 1 s query timeout instead of the system resolver defaults. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013mFejCibtUKMydqfjJdqag --- THREAT_MODEL.md | 4 +- .../service/RelayDetailsQueryService.kt | 8 +- .../tormap/service/ReverseDnsLookupService.kt | 121 +++++++++++++++--- .../service/RelayDetailsQueryServiceTest.kt | 23 ++++ .../ReverseDnsLookupServiceIntegrationTest.kt | 37 ++++++ .../service/ReverseDnsLookupServiceTest.kt | 84 +++++++++++- 6 files changed, 259 insertions(+), 18 deletions(-) diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index fabc81213..4f16af30b 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -88,7 +88,9 @@ Relay data is public by design, so confidentiality of relay data is not an asset 128 MB read per response, every line validated (strict date, `[a-z]{2}` country, bounded numbers, no future dates), bound parameters only. The API serves the stored rows for a validated `LocalDate`, cached per day with a bounded cache. - Reverse DNS: at most 10 PTR names verified per IP, forward results filtered to public addresses, results cached for - 6 hours with `sync = true`. + 6 hours with `sync = true`. PTR and forward (A/AAAA) lookups use dnsjava with a 1 s query timeout and run on at most + 8 dedicated threads with a 2 s deadline per IP. Names not verified by then are returned as unverified. When all + threads are busy, the request gets no host names and the result is not cached. - Backend container runs as non-root `1000:1000` from a digest-pinned base image. The frontend image (static files only) is based on the digest-pinned `nginxinc/nginx-unprivileged`, runs as non-root `101` and listens on port 8080. - Supply chain: Maven Central and npm registry only, Yarn install scripts disabled, 3-day npm age gate, Renovate with diff --git a/backend/src/main/kotlin/org/tormap/service/RelayDetailsQueryService.kt b/backend/src/main/kotlin/org/tormap/service/RelayDetailsQueryService.kt index 441e38f40..f933a3c21 100644 --- a/backend/src/main/kotlin/org/tormap/service/RelayDetailsQueryService.kt +++ b/backend/src/main/kotlin/org/tormap/service/RelayDetailsQueryService.kt @@ -32,10 +32,16 @@ class RelayDetailsQueryService( return RelayDetailsDto( details = details, confirmedFamilyMembers = details.confirmedFamilyMembers(), - reverseDnsLookupResult = reverseDnsLookupService.lookupHostNames(details.address), + reverseDnsLookupResult = lookupHostNames(details.address), ) } + private fun lookupHostNames(address: String): ReverseDnsLookupResult = try { + reverseDnsLookupService.lookupHostNames(address) + } catch (_: ReverseDnsLookupCapacityException) { + ReverseDnsLookupResult() + } + private fun RelayDetails.confirmedFamilyMembers(): List { val currentRelayId = id ?: return emptyList() val currentFamilyId = familyId ?: return emptyList() diff --git a/backend/src/main/kotlin/org/tormap/service/ReverseDnsLookupService.kt b/backend/src/main/kotlin/org/tormap/service/ReverseDnsLookupService.kt index fe176209e..aa5caf5c6 100644 --- a/backend/src/main/kotlin/org/tormap/service/ReverseDnsLookupService.kt +++ b/backend/src/main/kotlin/org/tormap/service/ReverseDnsLookupService.kt @@ -1,16 +1,30 @@ package org.tormap.service +import org.springframework.beans.factory.DisposableBean import org.springframework.cache.annotation.Cacheable import org.springframework.stereotype.Component import org.springframework.stereotype.Service import org.tormap.config.CacheConfig import org.tormap.util.logger +import org.xbill.DNS.AAAARecord +import org.xbill.DNS.ARecord +import org.xbill.DNS.ExtendedResolver import org.xbill.DNS.Lookup +import org.xbill.DNS.Name import org.xbill.DNS.PTRRecord +import org.xbill.DNS.Record import org.xbill.DNS.ReverseMap import org.xbill.DNS.Type import java.net.Inet6Address import java.net.InetAddress +import java.time.Duration +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ExecutionException +import java.util.concurrent.RejectedExecutionException +import java.util.concurrent.SynchronousQueue +import java.util.concurrent.ThreadPoolExecutor +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException data class ReverseDnsLookupResult( val verifiedHostNames: List = emptyList(), @@ -22,15 +36,26 @@ interface ReverseDnsResolver { fun lookupAddresses(hostName: String): List } +/** + * Thrown when all reverse DNS lookup slots are busy. Exceptions are not cached, so a later request retries the lookup. + */ +class ReverseDnsLookupCapacityException : RuntimeException("All reverse DNS lookup slots are busy") + @Component class DNSJavaReverseDnsResolver : ReverseDnsResolver { + companion object { + // Per query, across all configured name servers + private val QUERY_TIMEOUT: Duration = Duration.ofSeconds(1) + } + private val logger = logger() + private val dnsResolver = ExtendedResolver().apply { timeout = QUERY_TIMEOUT } override fun lookupPtrRecords(ipAddress: String): List { return try { - val records = Lookup(ReverseMap.fromAddress(ipAddress), Type.PTR).run() - ?: return emptyList() - records.filterIsInstance().map { it.target.toString().trimEnd('.') } + lookup(ReverseMap.fromAddress(ipAddress), Type.PTR) + .filterIsInstance() + .map { it.target.toString().trimEnd('.') } } catch (e: Exception) { logger.debug("PTR record lookup for IP {} failed! {}: {}", ipAddress, e.javaClass, e.message) emptyList() @@ -39,7 +64,17 @@ class DNSJavaReverseDnsResolver : ReverseDnsResolver { override fun lookupAddresses(hostName: String): List { return try { - InetAddress.getAllByName(hostName) + // Absolute name, so the search path of the host is not applied + val name = Name.fromString(hostName, Name.root) + listOf(Type.A, Type.AAAA) + .flatMap { lookup(name, it) } + .mapNotNull { + when (it) { + is ARecord -> it.address + is AAAARecord -> it.address + else -> null + } + } .filter { it.isPubliclyRoutable() } .map { it.hostAddress } } catch (e: Exception) { @@ -48,6 +83,9 @@ class DNSJavaReverseDnsResolver : ReverseDnsResolver { } } + private fun lookup(name: Name, type: Int): List = + Lookup(name, type).apply { setResolver(dnsResolver) }.run()?.toList() ?: emptyList() + private fun InetAddress.isULA(): Boolean = this is Inet6Address && (address[0].toInt() and 0xFE) == 0xFC // fc00::/7 @@ -68,29 +106,82 @@ class DNSJavaReverseDnsResolver : ReverseDnsResolver { @Service class ReverseDnsLookupService( private val reverseDnsResolver: ReverseDnsResolver, -) { + private val lookupTimeout: Duration = Duration.ofSeconds(2), + maxConcurrentLookups: Int = 8, +) : DisposableBean { companion object { private const val MAX_PTR_HOSTNAMES_TO_VERIFY = 10 } + + private val logger = logger() + + // Without a queue, so a lookup either starts right away or is rejected + private val lookupExecutor = ThreadPoolExecutor( + maxConcurrentLookups, + maxConcurrentLookups, + 0L, + TimeUnit.MILLISECONDS, + SynchronousQueue(), + Thread.ofPlatform().name("reverse-dns-", 0).daemon().factory(), + ) + + /** + * Looks up the PTR names of [ipAddress] and verifies them with forward lookups, within [lookupTimeout]. + * Host names that are not verified before the deadline are returned as unverified. + * + * @throws ReverseDnsLookupCapacityException if all lookup slots are busy + */ @Cacheable(CacheConfig.REVERSE_DNS_LOOKUPS, key = "#ipAddress", sync = true) fun lookupHostNames(ipAddress: String): ReverseDnsLookupResult { - val hostNames = reverseDnsResolver.lookupPtrRecords(ipAddress) + val deadline = System.nanoTime() + lookupTimeout.toNanos() + val progress = LookupProgress() + val task = try { + lookupExecutor.submit { lookupHostNames(ipAddress, deadline, progress) } + } catch (_: RejectedExecutionException) { + throw ReverseDnsLookupCapacityException() + } + try { + task.get(lookupTimeout.toNanos(), TimeUnit.NANOSECONDS) + } catch (_: TimeoutException) { + task.cancel(true) + logger.debug("Reverse DNS lookup for IP {} timed out after {}", ipAddress, lookupTimeout) + } catch (e: ExecutionException) { + logger.debug("Reverse DNS lookup for IP {} failed! {}: {}", ipAddress, e.cause?.javaClass, e.cause?.message) + } catch (_: InterruptedException) { + task.cancel(true) + Thread.currentThread().interrupt() + } + return progress.toResult() + } + + override fun destroy() { + lookupExecutor.shutdownNow() + } + + private fun lookupHostNames(ipAddress: String, deadline: Long, progress: LookupProgress) { + progress.hostNames = reverseDnsResolver.lookupPtrRecords(ipAddress) .map { it.trim().trimEnd('.') } .filter { it.isNotBlank() } .distinct() .take(MAX_PTR_HOSTNAMES_TO_VERIFY) - if (hostNames.isEmpty()) { - return ReverseDnsLookupResult() + for (hostName in progress.hostNames) { + if (Thread.currentThread().isInterrupted || System.nanoTime() - deadline >= 0) return + if (reverseDnsResolver.lookupAddresses(hostName).any { it == ipAddress }) { + progress.verifiedHostNames += hostName + } } + } - val (verifiedHostNames, unverifiedHostNames) = hostNames.partition { hostName -> - reverseDnsResolver.lookupAddresses(hostName).any { it == ipAddress } - } + /** Written by the lookup thread, read by the request thread, also after a timeout. */ + private class LookupProgress { + @Volatile + var hostNames: List = emptyList() + val verifiedHostNames: MutableSet = ConcurrentHashMap.newKeySet() - return ReverseDnsLookupResult( - verifiedHostNames = verifiedHostNames, - unverifiedHostNames = unverifiedHostNames, - ) + fun toResult(): ReverseDnsLookupResult { + val (verified, unverified) = hostNames.partition { it in verifiedHostNames } + return ReverseDnsLookupResult(verifiedHostNames = verified, unverifiedHostNames = unverified) + } } } diff --git a/backend/src/test/kotlin/org/tormap/service/RelayDetailsQueryServiceTest.kt b/backend/src/test/kotlin/org/tormap/service/RelayDetailsQueryServiceTest.kt index 4fffafe73..1382a8bd0 100644 --- a/backend/src/test/kotlin/org/tormap/service/RelayDetailsQueryServiceTest.kt +++ b/backend/src/test/kotlin/org/tormap/service/RelayDetailsQueryServiceTest.kt @@ -3,6 +3,8 @@ package org.tormap.service import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.StringSpec import io.kotest.matchers.shouldBe +import io.mockk.every +import io.mockk.mockk import org.tormap.adapter.controller.exception.RelayNotFoundException import org.tormap.database.entity.RelayDetails import org.tormap.mockRelayDetails @@ -63,6 +65,27 @@ class RelayDetailsQueryServiceTest : StringSpec({ response.confirmedFamilyMembers shouldBe emptyList() } + "getRelay returns empty reverse DNS names when all lookup slots are busy" { + val relay = mockRelayDetails('A').apply { + id = 1L + familyId = null + address = "1.2.3.4" + } + val reverseDnsLookupService = mockk { + every { lookupHostNames("1.2.3.4") } throws ReverseDnsLookupCapacityException() + } + + val relayDetailsQueryService = RelayDetailsQueryService( + relayDetailsLookup = FakeRelayDetailsLookup(relaysById = mapOf(1L to relay)), + reverseDnsLookupService = reverseDnsLookupService, + ) + + val response = relayDetailsQueryService.getRelay(1L) + + response.verifiedHostNames shouldBe emptyList() + response.unverifiedHostNames shouldBe emptyList() + } + "getRelay throws when relay does not exist" { val relayDetailsQueryService = RelayDetailsQueryService( relayDetailsLookup = FakeRelayDetailsLookup(), diff --git a/backend/src/test/kotlin/org/tormap/service/ReverseDnsLookupServiceIntegrationTest.kt b/backend/src/test/kotlin/org/tormap/service/ReverseDnsLookupServiceIntegrationTest.kt index dbe4e6a15..125828cb1 100644 --- a/backend/src/test/kotlin/org/tormap/service/ReverseDnsLookupServiceIntegrationTest.kt +++ b/backend/src/test/kotlin/org/tormap/service/ReverseDnsLookupServiceIntegrationTest.kt @@ -1,5 +1,6 @@ package org.tormap.service +import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.StringSpec import io.kotest.matchers.shouldBe import org.springframework.boot.test.context.SpringBootTest @@ -69,11 +70,47 @@ class ReverseDnsLookupServiceIntegrationTest( controllableResolver.lookupPtrRecordsCallCount.get() shouldBe 1 } + + "lookupHostNames rejects lookups when all slots are busy and does not cache the rejection" { + val busyLookupCount = 8 + controllableResolver.configure( + ptrRecords = listOf("host1.example."), + addresses = mapOf("host1.example" to listOf("1.2.3.4")), + delayMs = 1_000, + ) + + val executor = Executors.newFixedThreadPool(busyLookupCount) + try { + repeat(busyLookupCount) { + executor.submit { reverseDnsLookupService.lookupHostNames("10.0.0.${it + 1}") } + } + while (controllableResolver.lookupPtrRecordsCallCount.get() < busyLookupCount) Thread.sleep(10) + + shouldThrow { + reverseDnsLookupService.lookupHostNames("1.2.3.4") + } + + executor.shutdown() + executor.awaitTermination(10, TimeUnit.SECONDS) + } finally { + executor.shutdownNow() + } + + controllableResolver.configure( + ptrRecords = listOf("host1.example."), + addresses = mapOf("host1.example" to listOf("1.2.3.4")), + ) + reverseDnsLookupService.lookupHostNames("1.2.3.4").verifiedHostNames shouldBe listOf("host1.example") + controllableResolver.lookupPtrRecordsCallCount.get() shouldBe busyLookupCount + 1 + } }) class ControllableReverseDnsResolver : ReverseDnsResolver { + @Volatile private var ptrRecords: List = emptyList() + @Volatile private var addresses: Map> = emptyMap() + @Volatile private var delayMs: Long = 0 val lookupPtrRecordsCallCount = AtomicInteger(0) diff --git a/backend/src/test/kotlin/org/tormap/service/ReverseDnsLookupServiceTest.kt b/backend/src/test/kotlin/org/tormap/service/ReverseDnsLookupServiceTest.kt index 7484e10f2..d57065efc 100644 --- a/backend/src/test/kotlin/org/tormap/service/ReverseDnsLookupServiceTest.kt +++ b/backend/src/test/kotlin/org/tormap/service/ReverseDnsLookupServiceTest.kt @@ -1,7 +1,13 @@ package org.tormap.service +import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.StringSpec +import io.kotest.matchers.longs.shouldBeLessThan import io.kotest.matchers.shouldBe +import java.time.Duration +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import kotlin.concurrent.thread class ReverseDnsLookupServiceTest : StringSpec({ "lookupHostNames separates verified and unverified host names" { @@ -51,18 +57,94 @@ class ReverseDnsLookupServiceTest : StringSpec({ result.unverifiedHostNames.size shouldBe 10 reverseDnsResolver.lookupAddressesCallCount shouldBe 10 } + + "lookupHostNames returns host names not verified before the deadline as unverified" { + val reverseDnsResolver = LookupServiceFakeReverseDnsResolver( + ptrRecordsByIpAddress = mapOf( + "1.2.3.4" to listOf("fast.example.", "slow.example.", "later.example.") + ), + addressesByHostName = mapOf( + "fast.example" to listOf("1.2.3.4"), + "slow.example" to listOf("1.2.3.4"), + "later.example" to listOf("1.2.3.4"), + ), + addressLookupDelayByHostName = mapOf("slow.example" to Duration.ofSeconds(10)), + ) + val reverseDnsLookupService = ReverseDnsLookupService(reverseDnsResolver, Duration.ofMillis(200)) + + val startTime = System.nanoTime() + val result = reverseDnsLookupService.lookupHostNames("1.2.3.4") + + Duration.ofNanos(System.nanoTime() - startTime).toMillis() shouldBeLessThan 2_000 + result.verifiedHostNames shouldBe listOf("fast.example") + result.unverifiedHostNames shouldBe listOf("slow.example", "later.example") + reverseDnsLookupService.destroy() + } + + "lookupHostNames returns empty lists when the PTR lookup does not finish before the deadline" { + val reverseDnsResolver = LookupServiceFakeReverseDnsResolver( + ptrRecordsByIpAddress = mapOf("1.2.3.4" to listOf("host.example.")), + ptrLookupDelay = Duration.ofSeconds(10), + ) + val reverseDnsLookupService = ReverseDnsLookupService(reverseDnsResolver, Duration.ofMillis(200)) + + val startTime = System.nanoTime() + val result = reverseDnsLookupService.lookupHostNames("1.2.3.4") + + Duration.ofNanos(System.nanoTime() - startTime).toMillis() shouldBeLessThan 2_000 + result shouldBe ReverseDnsLookupResult() + reverseDnsLookupService.destroy() + } + + "lookupHostNames rejects lookups without waiting when all lookup slots are busy" { + val ptrLookupStarted = CountDownLatch(1) + val releasePtrLookup = CountDownLatch(1) + val reverseDnsResolver = object : ReverseDnsResolver { + override fun lookupPtrRecords(ipAddress: String): List { + ptrLookupStarted.countDown() + releasePtrLookup.await() + return emptyList() + } + + override fun lookupAddresses(hostName: String): List = emptyList() + } + val reverseDnsLookupService = ReverseDnsLookupService( + reverseDnsResolver = reverseDnsResolver, + lookupTimeout = Duration.ofSeconds(10), + maxConcurrentLookups = 1, + ) + val busyLookup = thread { reverseDnsLookupService.lookupHostNames("1.2.3.4") } + try { + ptrLookupStarted.await(5, TimeUnit.SECONDS) shouldBe true + + shouldThrow { + reverseDnsLookupService.lookupHostNames("5.6.7.8") + } + } finally { + releasePtrLookup.countDown() + busyLookup.join() + reverseDnsLookupService.destroy() + } + } }) private class LookupServiceFakeReverseDnsResolver( private val ptrRecordsByIpAddress: Map> = emptyMap(), private val addressesByHostName: Map> = emptyMap(), + private val ptrLookupDelay: Duration = Duration.ZERO, + private val addressLookupDelayByHostName: Map = emptyMap(), ) : ReverseDnsResolver { + @Volatile var lookupAddressesCallCount = 0 - override fun lookupPtrRecords(ipAddress: String): List = ptrRecordsByIpAddress[ipAddress] ?: emptyList() + override fun lookupPtrRecords(ipAddress: String): List { + Thread.sleep(ptrLookupDelay) + return ptrRecordsByIpAddress[ipAddress] ?: emptyList() + } override fun lookupAddresses(hostName: String): List { lookupAddressesCallCount++ + Thread.sleep(addressLookupDelayByHostName[hostName] ?: Duration.ZERO) return addressesByHostName[hostName] ?: emptyList() } }