From 41eb524d3e31aca20284af0d2254c89ae358a8fa Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:10:10 +0200 Subject: [PATCH] fix(dns): compare IPv6 addresses by value --- app/Actions/Shared/CheckDomainDns.php | 5 +- .../Concerns/InteractsWithDnsProviders.php | 6 +- app/Services/Dns/CloudflareDnsProvider.php | 7 +- app/Support/DnsRecordHints.php | 19 +++++ bootstrap/helpers/shared.php | 3 +- tests/Feature/DnsRecordHintsTest.php | 13 +++ tests/Feature/DnsValidationTest.php | 79 +++++++++++++++++++ .../Feature/ManagedDnsRecordOwnershipTest.php | 28 +++++++ 8 files changed, 153 insertions(+), 7 deletions(-) diff --git a/app/Actions/Shared/CheckDomainDns.php b/app/Actions/Shared/CheckDomainDns.php index 2a34c34a1d..d52ba50142 100644 --- a/app/Actions/Shared/CheckDomainDns.php +++ b/app/Actions/Shared/CheckDomainDns.php @@ -3,6 +3,7 @@ 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; @@ -92,7 +93,7 @@ class CheckDomainDns } $receivedAddressRecord = true; - if (isCloudflareIp($record->getData()) || ($expectedIp && $record->getData() === $expectedIp)) { + if (isCloudflareIp($record->getData()) || ($expectedIp && DnsRecordHints::sameAddress($record->getData(), $expectedIp))) { return $this->result('ok', $this->successMessage($server, $expectedIp), $expectedIp); } } @@ -103,7 +104,7 @@ class CheckDomainDns if (! $receivedAddressRecord && hrtime(true) < $deadline) { foreach ($this->resolveWithSystemDns($host, $type) as $resolvedIp) { - if (isCloudflareIp($resolvedIp) || ($expectedIp && $resolvedIp === $expectedIp)) { + if (isCloudflareIp($resolvedIp) || ($expectedIp && DnsRecordHints::sameAddress($resolvedIp, $expectedIp))) { return $this->result('ok', $this->successMessage($server, $expectedIp), $expectedIp); } } diff --git a/app/Livewire/Concerns/InteractsWithDnsProviders.php b/app/Livewire/Concerns/InteractsWithDnsProviders.php index bf5cd4c609..f5e8e103c5 100644 --- a/app/Livewire/Concerns/InteractsWithDnsProviders.php +++ b/app/Livewire/Concerns/InteractsWithDnsProviders.php @@ -392,7 +392,11 @@ trait InteractsWithDnsProviders { $addresses = $this->publicServerIpsForDnsProvider(); - return $requested === null ? ($addresses[0] ?? null) : (in_array($requested, $addresses, true) ? $requested : null); + if ($requested === null) { + return $addresses[0] ?? null; + } + + return collect($addresses)->first(fn (string $address): bool => DnsRecordHints::sameAddress($address, $requested)); } abstract protected function persistDomainDnsStatuses(): void; diff --git a/app/Services/Dns/CloudflareDnsProvider.php b/app/Services/Dns/CloudflareDnsProvider.php index 99a6e853a4..c4864900f1 100644 --- a/app/Services/Dns/CloudflareDnsProvider.php +++ b/app/Services/Dns/CloudflareDnsProvider.php @@ -7,6 +7,7 @@ use App\Exceptions\DnsRecordConflictException; use App\Models\DnsProviderZone; use App\Models\IntegrationToken; use App\Models\ManagedDnsRecord; +use App\Support\DnsRecordHints; use Illuminate\Contracts\Cache\LockTimeoutException; use Illuminate\Database\Eloquent\Model; use Illuminate\Http\Client\PendingRequest; @@ -132,7 +133,7 @@ class CloudflareDnsProvider { $type = $this->recordType($content); $remoteRecords = $this->findRecords($zone, $hostname, $type); - $matching = collect($remoteRecords)->first(fn (array $remote): bool => $remote['content'] === $content); + $matching = collect($remoteRecords)->first(fn (array $remote): bool => DnsRecordHints::sameAddress($remote['content'], $content)); if ($matching !== null) { if ($matching['id'] === '') { throw new RuntimeException('Cloudflare DNS records could not be checked.'); @@ -222,7 +223,7 @@ class CloudflareDnsProvider if ($remote === null || $remote['id'] === '' || $remote['id'] !== $recordId - || ($expectedCurrent !== null && $remote['content'] !== $expectedCurrent) + || ($expectedCurrent !== null && ! DnsRecordHints::sameAddress($remote['content'], $expectedCurrent)) || $remote['name'] !== $hostname) { throw new RuntimeException('The DNS conflict is no longer available. Check the record again.'); } @@ -265,7 +266,7 @@ class CloudflareDnsProvider } if (($remote['type'] ?? null) !== $record->type || strtolower((string) ($remote['name'] ?? '')) !== $record->name - || ($remote['content'] ?? null) !== $record->content + || ! DnsRecordHints::sameAddress(is_string($remote['content'] ?? null) ? $remote['content'] : null, $record->content) || ($remote['comment'] ?? null) !== $record->ownershipComment()) { return ManagedDnsDeletionResult::ChangedExternally; } diff --git a/app/Support/DnsRecordHints.php b/app/Support/DnsRecordHints.php index ced4f1c948..bfe64c50c1 100644 --- a/app/Support/DnsRecordHints.php +++ b/app/Support/DnsRecordHints.php @@ -39,6 +39,25 @@ class DnsRecordHints return ord($binary[0]) !== 0xFF; } + /** + * Whether two DNS record values name the same address. IP addresses are compared in binary form, + * so equivalent IPv6 notations (2001:db8::1 and 2001:0DB8:0:0::1) match; other values must be identical. + */ + public static function sameAddress(?string $first, ?string $second): bool + { + if ($first === null || $second === null) { + return false; + } + + $firstBinary = @inet_pton(trim($first)); + $secondBinary = @inet_pton(trim($second)); + if ($firstBinary !== false && $secondBinary !== false) { + return $firstBinary === $secondBinary; + } + + return $first === $second; + } + /** * Build A/AAAA entries for every hostname (deduped). * diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 7d16b816d5..bf8624ee16 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -32,6 +32,7 @@ use App\Models\StandaloneSqlite; use App\Models\SwarmDocker; use App\Models\Team; use App\Models\User; +use App\Support\DnsRecordHints; use Carbon\CarbonImmutable; use DanHarrin\LivewireRateLimiting\Exceptions\TooManyRequestsException; use Illuminate\Database\Eloquent\ModelNotFoundException; @@ -2323,7 +2324,7 @@ function validateDNSEntry(string $fqdn, Server $server) $found_matching_ip = true; break 2; } - if ($ip && $result->getData() === $ip) { + if ($ip && DnsRecordHints::sameAddress($result->getData(), $ip)) { $found_matching_ip = true; break 2; } diff --git a/tests/Feature/DnsRecordHintsTest.php b/tests/Feature/DnsRecordHintsTest.php index 9f9269a570..921e255276 100644 --- a/tests/Feature/DnsRecordHintsTest.php +++ b/tests/Feature/DnsRecordHintsTest.php @@ -74,3 +74,16 @@ it('treats only publicly routable addresses as public dns record targets', funct 'not an ip' => ['app.example.com', false], 'empty' => ['', false], ]); + +it('compares IP addresses independent of their notation', function (string $first, string $second, bool $same) { + expect(DnsRecordHints::sameAddress($first, $second))->toBe($same); +})->with([ + 'compressed vs expanded IPv6' => ['2001:db8::1', '2001:0db8:0:0::1', true], + 'upper vs lower case IPv6' => ['2001:DB8::A', '2001:db8::a', true], + 'fully expanded IPv6' => ['2001:0db8:0000:0000:0000:0000:0000:0001', '2001:db8::1', true], + 'different IPv6' => ['2001:db8::1', '2001:db8::2', false], + 'same IPv4' => ['203.0.113.10', '203.0.113.10', true], + 'different IPv4' => ['203.0.113.10', '203.0.113.11', false], + 'IPv4 vs IPv6' => ['203.0.113.10', '::ffff:203.0.113.10', false], + 'non-IP values' => ['target.example.com', 'target.example.com', true], +]); diff --git a/tests/Feature/DnsValidationTest.php b/tests/Feature/DnsValidationTest.php index 1e03332b7f..d505ea9359 100644 --- a/tests/Feature/DnsValidationTest.php +++ b/tests/Feature/DnsValidationTest.php @@ -188,3 +188,82 @@ it('does not use the system resolver after a custom dns server returns an addres expect($result['example']['status'])->toBe('failed'); }); + +it('treats equivalent IPv6 notations as the same address', function (string $resolvedIp, string $expectedIp) { + InstanceSettings::unguarded(fn () => InstanceSettings::query()->updateOrCreate( + ['id' => 0], + ['is_dns_validation_enabled' => true, 'custom_dns_servers' => '192.0.2.1'], + )); + + app()->bind(DNSQuery::class, fn () => new class('192.0.2.1', $resolvedIp) extends DNSQuery + { + public function __construct(string $server, private readonly string $resolvedIp) + { + parent::__construct($server); + } + + public function query(string $question, string $typeName = DNSTypes::NAME_A) + { + return [new DNSResult($typeName, 1, 'IN', 60, $this->resolvedIp, $question, '', [])]; + } + + public function hasError(): bool + { + return false; + } + }); + + $server = new Server(['ip' => $expectedIp]); + $server->id = 1; + + $result = CheckDomainDns::run(['example' => 'https://example.com'], $server, $expectedIp); + + expect($result['example']['status'])->toBe('ok'); +})->with([ + 'compressed record, expanded expectation' => ['2001:db8::1', '2001:0db8:0:0::1'], + 'expanded record, compressed expectation' => ['2001:0DB8:0000:0000:0000:0000:0000:0001', '2001:db8::1'], +]); + +it('treats equivalent IPv6 notations from the system resolver as the same address', function () { + InstanceSettings::unguarded(fn () => InstanceSettings::query()->updateOrCreate( + ['id' => 0], + ['is_dns_validation_enabled' => true, 'custom_dns_servers' => ''], + )); + + $action = new class extends CheckDomainDns + { + protected function resolveWithSystemDns(string $host, string $type): array + { + return ['2001:db8::1']; + } + }; + $server = new Server(['ip' => '2001:0db8:0:0::1']); + $server->id = 1; + + expect($action->handle(['example' => 'https://example.com'], $server, '2001:0db8:0:0::1')['example']['status'])->toBe('ok'); +}); + +it('validateDNSEntry treats equivalent IPv6 notations as the same address', function () { + InstanceSettings::unguarded(fn () => InstanceSettings::query()->updateOrCreate( + ['id' => 0], + ['is_dns_validation_enabled' => true, 'custom_dns_servers' => '192.0.2.1'], + )); + + app()->bind(DNSQuery::class, fn () => new class('192.0.2.1') extends DNSQuery + { + public function query(string $question, string $typeName = DNSTypes::NAME_A) + { + return [new DNSResult($typeName, 1, 'IN', 60, '2001:db8::1', $question, '', [])]; + } + + public function hasError(): bool + { + return false; + } + }); + + $server = new Server(['ip' => '2001:0db8:0:0::1']); + $server->id = 1; + + expect(validateDNSEntry('https://example.com', $server))->toBeTrue(); +}); diff --git a/tests/Feature/ManagedDnsRecordOwnershipTest.php b/tests/Feature/ManagedDnsRecordOwnershipTest.php index 5b892c692e..bfae930a42 100644 --- a/tests/Feature/ManagedDnsRecordOwnershipTest.php +++ b/tests/Feature/ManagedDnsRecordOwnershipTest.php @@ -1,5 +1,6 @@ and($foreignRecord->fresh())->not->toBeNull() ->and($foreignRecord->references()->count())->toBe(1); }); + +test('an existing AAAA record written in another IPv6 notation satisfies the request', function () { + fakeCloudflareDns([ + ['id' => 'record-1', 'type' => 'AAAA', 'name' => 'app.example.com', 'content' => '2001:db8::1', 'comment' => null], + ]); + $application = createDnsTestApplication($this, 'https://app.example.com'); + + $record = app(CloudflareDnsProvider::class)->createRecord($this->zone, 'app.example.com', '2001:0DB8:0:0::1', $application); + + expect($record->provider_record_id)->toBe('record-1')->and($record->owned)->toBeFalse(); + expect(sentDnsRequests('POST'))->toBe(0); +}); + +test('an owned AAAA record is deleted when the provider returns it in canonical IPv6 notation', function () { + $cloudflare = fakeCloudflareDns(); + $application = createDnsTestApplication($this, 'https://app.example.com'); + $record = app(CloudflareDnsProvider::class)->createRecord($this->zone, 'app.example.com', '2001:0db8:0000:0000:0000:0000:0000:0001', $application); + + $records = $cloudflare['records']; + $records[$record->provider_record_id]['content'] = '2001:db8::1'; + $cloudflare['records'] = $records; + + $result = app(CloudflareDnsProvider::class)->deleteRecord($record->fresh(), $application); + + expect($result)->toBe(ManagedDnsDeletionResult::Deleted) + ->and($cloudflare['records'])->not->toHaveKey($record->provider_record_id); +});