diff --git a/app/Actions/Shared/CheckDomainDns.php b/app/Actions/Shared/CheckDomainDns.php index e8a8f5a265..de56a4e33b 100644 --- a/app/Actions/Shared/CheckDomainDns.php +++ b/app/Actions/Shared/CheckDomainDns.php @@ -5,7 +5,6 @@ namespace App\Actions\Shared; use App\Models\Server; use App\Support\DnsRecordHints; use Lorisleiva\Actions\Concerns\AsAction; -use PurplePixie\PhpDns\DNSQuery; use PurplePixie\PhpDns\DNSTypes; use Spatie\Url\Url; @@ -76,14 +75,9 @@ class CheckDomainDns } try { - $query = app()->make(DNSQuery::class, [ - 'server' => formatHostForUrl($dnsServer), - 'port' => 53, - 'timeout' => min(5, (int) floor($remainingNanoseconds / 1_000_000_000)), - ]); - $records = $query->query($host, $type); + $records = queryDnsServer($dnsServer, $host, $type, min(5, (int) floor($remainingNanoseconds / 1_000_000_000))); - if ($records === false || $query->hasError()) { + if ($records === false) { continue; } diff --git a/app/Rules/SafeWebhookUrl.php b/app/Rules/SafeWebhookUrl.php index f7350da027..c872dde82b 100644 --- a/app/Rules/SafeWebhookUrl.php +++ b/app/Rules/SafeWebhookUrl.php @@ -361,10 +361,9 @@ class SafeWebhookUrl implements ValidationRule foreach ($dnsServers as $dnsServer) { foreach ([DNSTypes::NAME_A, DNSTypes::NAME_AAAA] as $type) { try { - $query = createDnsQuery($dnsServer); - $records = $query->query($host, $type); + $records = queryDnsServer($dnsServer, $host, $type); - if ($records === false || $query->hasError()) { + if ($records === false) { continue; } diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 242ed14c91..137964d43c 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -72,6 +72,7 @@ use phpseclib3\Crypt\EC; use phpseclib3\Crypt\RSA; use Poliander\Cron\CronExpression; use PurplePixie\PhpDns\DNSQuery; +use PurplePixie\PhpDns\DNSResult; use PurplePixie\PhpDns\DNSTypes; use Spatie\Url\Url; use Symfony\Component\Yaml\Yaml; @@ -2342,10 +2343,8 @@ function validateDNSEntry(string $fqdn, Server $server) $type = dnsRecordTypeForIp($ip) === 'AAAA' ? DNSTypes::NAME_AAAA : DNSTypes::NAME_A; foreach ($dns_servers as $dns_server) { try { - $query = createDnsQuery($dns_server); - $results = $query->query($host, $type); - if ($results === false || $query->hasError()) { - } else { + $results = queryDnsServer($dns_server, $host, $type); + if ($results !== false) { foreach ($results as $result) { if ($result->getType() == $type) { if (isCloudflareIp($result->getData())) { @@ -2366,15 +2365,40 @@ function validateDNSEntry(string $fqdn, Server $server) return $found_matching_ip; } -function createDnsQuery(string $dnsServer): DNSQuery +function createDnsQuery(string $dnsServer, int $timeout = 5, bool $udp = true): DNSQuery { return app()->make(DNSQuery::class, [ 'server' => formatHostForUrl($dnsServer), 'port' => 53, - 'timeout' => 5, + 'timeout' => $timeout, + 'udp' => $udp, ]); } +/** + * Query one DNS server for one record type. When the UDP answer is truncated (TC bit), + * retry over TCP: resolvers such as 1.1.1.1 can truncate even small answers, and the + * records would otherwise be silently dropped. + * + * @return iterable|false + */ +function queryDnsServer(string $dnsServer, string $host, string $type, int $timeout = 5): iterable|false +{ + $query = createDnsQuery($dnsServer, $timeout); + $records = $query->query($host, $type); + + if ($records === false && str_contains($query->getLasterror(), 'too big for UDP')) { + $query = createDnsQuery($dnsServer, $timeout, udp: false); + $records = $query->query($host, $type); + } + + if ($records === false || $query->hasError()) { + return false; + } + + return $records; +} + function isCloudflareIp(string $ip): bool { // https://www.cloudflare.com/ips/ diff --git a/tests/Unit/SafeWebhookUrlTest.php b/tests/Unit/SafeWebhookUrlTest.php index b4b678a247..83d8c9557c 100644 --- a/tests/Unit/SafeWebhookUrlTest.php +++ b/tests/Unit/SafeWebhookUrlTest.php @@ -4,7 +4,9 @@ use App\Models\InstanceSettings; use App\Rules\SafeWebhookUrl; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Validator; +use PurplePixie\PhpDns\DNSAnswer; use PurplePixie\PhpDns\DNSQuery; +use PurplePixie\PhpDns\DNSResult; use PurplePixie\PhpDns\DNSTypes; use Tests\TestCase; @@ -485,3 +487,81 @@ it('accepts bracketed IPv6 custom dns servers from instance settings', function expect($method->invoke(new SafeWebhookUrl))->toBe(['1.1.1.1', '2606:4700:4700::1111']); }); + +/** + * Fakes a resolver like 1.1.1.1 that sets the TC bit on the UDP answer for A records. + */ +function bindTruncatingDnsResolver(ArrayObject $queries, string $truncatedError = 'Response too big for UDP, retry with TCP'): void +{ + app()->bind(DNSQuery::class, function ($app, array $parameters) use ($queries, $truncatedError) { + return new class($parameters['udp'], $queries, $truncatedError) extends DNSQuery + { + private string $fakeError = ''; + + public function __construct( + private readonly bool $useUdp, + private readonly ArrayObject $queries, + private readonly string $truncatedError, + ) { + parent::__construct('192.0.2.1'); + } + + public function query(string $question, string $typeName = DNSTypes::NAME_A) + { + $this->queries->append([$typeName, $this->useUdp ? 'udp' : 'tcp']); + $this->fakeError = ''; + + if ($typeName === DNSTypes::NAME_A && $this->useUdp) { + $this->fakeError = $this->truncatedError; + + return false; + } + + $answer = new DNSAnswer; + $ip = $typeName === DNSTypes::NAME_A ? '172.64.66.1' : '2606:4700:113::1'; + $answer->addResult(new DNSResult($typeName, 0, 'IN', 300, $ip, $question, '', [])); + + return $answer; + } + + public function hasError(): bool + { + return $this->fakeError !== ''; + } + + public function getLasterror(): string + { + return $this->fakeError; + } + }; + }); +} + +it('retries custom dns lookups over TCP when the UDP answer is truncated', function () { + $queries = new ArrayObject; + bindTruncatingDnsResolver($queries); + + $method = new ReflectionMethod(SafeWebhookUrl::class, 'resolveHostWithCustomDnsServers'); + + expect($method->invoke(new SafeWebhookUrl, 'bucket.r2.cloudflarestorage.com', ['1.1.1.1'])) + ->toBe(['172.64.66.1', '2606:4700:113::1']) + ->and($queries->getArrayCopy())->toBe([ + [DNSTypes::NAME_A, 'udp'], + [DNSTypes::NAME_A, 'tcp'], + [DNSTypes::NAME_AAAA, 'udp'], + ]); +}); + +it('does not retry custom dns lookups over TCP for other UDP failures', function () { + $queries = new ArrayObject; + bindTruncatingDnsResolver($queries, 'Failed to read data buffer'); + + $method = new ReflectionMethod(SafeWebhookUrl::class, 'resolveHostWithCustomDnsServers'); + + expect($method->invoke(new SafeWebhookUrl, 'bucket.r2.cloudflarestorage.com', ['1.1.1.1'])) + ->toBe(['2606:4700:113::1']) + ->and($queries->getArrayCopy())->toBe([ + [DNSTypes::NAME_A, 'udp'], + [DNSTypes::NAME_AAAA, 'udp'], + ]); +});