Merge pull request #4023 from fosrl/dev

Match wildcard domains properly

server/private/lib/dns/customRecordMatch.test.ts

@@ -0,0 +1,172 @@
+import { matchCustomRecords } from "./customRecordMatch";
+import { assertEquals } from "@test/assert";
+
+const zone = "example.com";
+
+function record(fullDomain: string, type = "A") {
+    return {
+        fullDomain,
+        type,
+        values: ["192.0.2.1"],
+        ttl: 300,
+        priority: null,
+        weight: null,
+        port: null
+    };
+}
+
+const zoneRecords = [
+    record("example.com"),
+    record("example.com", "MX"),
+    record("coder.example.com"),
+    record("*.coder.example.com"),
+    record("fleet.example.com", "NS"),
+    record("_imaps._tcp.example.com", "SRV"),
+    record("_submission._tcp.example.com", "SRV"),
+    record("app.nested.coder.example.com"),
+    record("*.svc.example.com"),
+    record("*.x.svc.example.com", "TXT")
+];
+
+function names(name: string) {
+    return matchCustomRecords(zoneRecords, name, zone)
+        .records.map((r) => `${r.fullDomain}/${r.type}`)
+        .sort()
+        .join(",");
+}
+
+function exists(name: string) {
+    return matchCustomRecords(zoneRecords, name, zone).nameExists;
+}
+
+function testExactMatches() {
+    console.log("Running exact match tests...");
+
+    assertEquals(
+        names("example.com"),
+        "example.com/A,example.com/MX",
+        "apex returns all its records"
+    );
+    assertEquals(
+        names("coder.example.com"),
+        "coder.example.com/A",
+        "an exact name is never answered by its child wildcard"
+    );
+    assertEquals(
+        names("Coder.Example.Com."),
+        "coder.example.com/A",
+        "names are matched case-insensitively and without a trailing dot"
+    );
+    assertEquals(
+        names("*.coder.example.com"),
+        "*.coder.example.com/A",
+        "the wildcard owner name itself matches literally"
+    );
+}
+
+function testWildcards() {
+    console.log("Running wildcard tests...");
+
+    assertEquals(
+        names("foo.coder.example.com"),
+        "*.coder.example.com/A",
+        "single-label name below a wildcard"
+    );
+    assertEquals(
+        names("bar.baz.coder.example.com"),
+        "*.coder.example.com/A",
+        "multi-label name below a wildcard (RFC 4592)"
+    );
+    assertEquals(
+        names("a.b.c.coder.example.com"),
+        "*.coder.example.com/A",
+        "deeply nested name below a wildcard"
+    );
+    assertEquals(
+        names("other.nested.coder.example.com"),
+        "",
+        "an existing intermediate name blocks the wildcard above it"
+    );
+    assertEquals(
+        exists("other.nested.coder.example.com"),
+        false,
+        "a blocked name is NXDOMAIN"
+    );
+    assertEquals(
+        names("a.b.x.svc.example.com"),
+        "*.x.svc.example.com/TXT",
+        "the closest wildcard wins over one further up"
+    );
+    assertEquals(
+        names("q.svc.example.com"),
+        "*.svc.example.com/A",
+        "a sibling of a nested wildcard uses the outer one"
+    );
+    assertEquals(
+        names("unknown.example.com"),
+        "",
+        "no wildcard at the apex means nothing matches"
+    );
+    assertEquals(
+        exists("unknown.example.com"),
+        false,
+        "an unknown name is NXDOMAIN"
+    );
+}
+
+function testEmptyNonTerminals() {
+    console.log("Running empty non-terminal tests...");
+
+    assertEquals(
+        names("_tcp.example.com"),
+        "",
+        "an empty non-terminal has no records"
+    );
+    assertEquals(
+        exists("_tcp.example.com"),
+        true,
+        "an empty non-terminal exists (NODATA, not NXDOMAIN)"
+    );
+    assertEquals(
+        names("nested.coder.example.com"),
+        "",
+        "an empty non-terminal is not answered by the wildcard above it"
+    );
+    assertEquals(
+        exists("nested.coder.example.com"),
+        true,
+        "an empty non-terminal under a wildcard still exists"
+    );
+    assertEquals(
+        exists("svc.example.com"),
+        true,
+        "a wildcard's parent exists"
+    );
+    assertEquals(
+        exists("x.svc.example.com"),
+        true,
+        "a nested wildcard's parent exists"
+    );
+}
+
+function testOutsideZone() {
+    console.log("Running out-of-zone tests...");
+
+    assertEquals(
+        names("example.org"),
+        "",
+        "names outside the zone never match"
+    );
+    assertEquals(
+        names("notexample.com"),
+        "",
+        "a name that merely ends with the zone string does not match"
+    );
+    assertEquals(exists("com"), false, "the zone's parent does not exist");
+}
+
+testExactMatches();
+testWildcards();
+testEmptyNonTerminals();
+testOutsideZone();
+console.log("All custom record match tests passed");

server/private/lib/dns/customRecordMatch.ts

@@ -0,0 +1,76 @@
+/*
+ * This file is part of a proprietary work.
+ *
+ * Copyright (c) 2025-2026 Fossorial, Inc.
+ * All rights reserved.
+ *
+ * This file is licensed under the Fossorial Commercial License.
+ * You may not use this file except in compliance with the License.
+ * Unauthorized use, copying, modification, or distribution is strictly prohibited.
+ *
+ * This file is not licensed under the AGPLv3.
+ */
+
+export type CustomZoneRecord = {
+    fullDomain: string;
+    type: string;
+    values: string[];
+    ttl: number;
+    priority: number | null;
+    weight: number | null;
+    port: number | null;
+};
+
+export type CustomRecordMatch<T> = {
+    // The records that answer for the name: its own, or those of the
+    // wildcard that covers it
+    records: T[];
+    // Whether the name exists in the zone's custom records, including as an
+    // empty non-terminal (e.g. _tcp.example.com when only
+    // _imaps._tcp.example.com has records) - for NODATA vs NXDOMAIN
+    nameExists: boolean;
+};
+
+// Picks which of a zone's custom records answer for `name`, following
+// RFC 4592: a name with records of its own (or with records beneath it)
+// is never answered by a wildcard. Otherwise the closest existing ancestor
+// (the "closest encloser") is found and its wildcard, if any, answers -
+// so *.example.com covers a.b.example.com unless b.example.com exists.
+// Only custom record names are considered when deciding what exists.
+export function matchCustomRecords<T extends { fullDomain: string }>(
+    zoneRecords: T[],
+    name: string,
+    zone: string
+): CustomRecordMatch<T> {
+    const normalized = name.replace(/\.$/, "").toLowerCase();
+    if (normalized !== zone && !normalized.endsWith(`.${zone}`)) {
+        return { records: [], nameExists: false };
+    }
+
+    const existsAtOrBelow = (candidate: string) =>
+        zoneRecords.some(
+            (r) =>
+                r.fullDomain === candidate ||
+                r.fullDomain.endsWith(`.${candidate}`)
+        );
+
+    const exact = zoneRecords.filter((r) => r.fullDomain === normalized);
+    if (exact.length > 0) {
+        return { records: exact, nameExists: true };
+    }
+    if (normalized === zone || existsAtOrBelow(normalized)) {
+        return { records: [], nameExists: true };
+    }
+
+    // Walk up to the closest encloser; the zone apex always exists, so the
+    // walk never leaves the zone
+    let encloser = normalized;
+    do {
+        encloser = encloser.slice(encloser.indexOf(".") + 1);
+    } while (encloser !== zone && !existsAtOrBelow(encloser));
+
+    const wildcard = zoneRecords.filter(
+        (r) => r.fullDomain === `*.${encloser}`
+    );
+    return { records: wildcard, nameExists: wildcard.length > 0 };
+}

server/private/lib/dns/server.ts

@@ -38,6 +38,11 @@ import * as dnsResolver from "dns";
 import logger from "@server/logger";
 import { listExitNodes } from "../exitNodes";
 import { parseCustomDnsRecordValues } from "@server/routers/customDnsRecord/types";
+import {
+    matchCustomRecords,
+    type CustomRecordMatch,
+    type CustomZoneRecord
+} from "./customRecordMatch";
 import { rateLimitService } from "../rateLimit";
 import license from "#private/license/license";
 
@@ -499,9 +504,10 @@ export class AuthoritativeDNSServer {
 
         // User-defined records come after everything Pangolin manages itself,
         // so they can never shadow a resource, login page or redirect.
-        const customRecords = isCustomerZone
+        const customMatch: CustomRecordMatch<CustomRecord> = isCustomerZone
             ? await this.getCustomRecords(queryName, baseDomain!)
-            : [];
+            : { records: [], nameExists: false };
+        const customRecords = customMatch.records;
 
         if (customRecords.length > 0) {
             let matchingRecords = customRecords.filter(
@@ -532,7 +538,7 @@ export class AuthoritativeDNSServer {
             this.domainExists(queryName, baseDomain!),
             this.getSOARecord(baseDomain!)
         ]);
-        const domainExists = resourceExists || customRecords.length > 0;
+        const domainExists = resourceExists || customMatch.nameExists;
 
         if (!domainExists) {
             // NXDOMAIN: Name does not exist
@@ -1222,80 +1228,87 @@ export class AuthoritativeDNSServer {
         }
     }
 
-    // User-defined records for `name`. Only served when the record's domain
-    // is the zone this query resolved to and is a verified NS delegation, so
-    // records can never surface under another org's zone or a domain whose
-    // ownership hasn't been proven. An exact name match takes precedence over
-    // a wildcard (RFC 4592).
+    // User-defined records for `name`, plus whether the name exists among
+    // them (see matchCustomRecords for how wildcards apply).
     private async getCustomRecords(
         name: string,
         zone: string
-    ): Promise<CustomRecord[]> {
-        const normalized = name.replace(/\.$/, "").toLowerCase();
-        const cacheKey = `customRecords:${zone}:${normalized}`;
+    ): Promise<CustomRecordMatch<CustomRecord>> {
+        const match = matchCustomRecords(
+            await this.getCustomZoneRecords(zone),
+            name,
+            zone
+        );
+        return {
+            // Each stored value is served as its own resource record
+            records: match.records.flatMap(
+                ({ fullDomain, values, ...record }) =>
+                    values.map((value) => ({ ...record, value }))
+            ),
+            nameExists: match.nameExists
+        };
+    }
 
-        const cached = this.cache.get<CustomRecord[]>(cacheKey);
+    // All enabled custom records of `zone`. Only loaded when the zone is a
+    // verified NS delegation, so records can never surface under another
+    // org's zone or a domain whose ownership hasn't been proven. Loaded
+    // whole (a domain has at most 100) since wildcard matching needs to know
+    // every name that exists in the zone.
+    private async getCustomZoneRecords(
+        zone: string
+    ): Promise<CustomZoneRecord[]> {
+        const cacheKey = `customZoneRecords:${zone}`;
+
+        const cached = this.cache.get<CustomZoneRecord[]>(cacheKey);
         if (cached !== undefined) {
             return cached;
         }
 
-        return this.withInFlightDedup<CustomRecord[]>(cacheKey, async () => {
-            try {
-                const wildcardPattern = this.getWildcardPattern(normalized);
-                const names =
-                    wildcardPattern && wildcardPattern !== normalized
-                        ? [normalized, wildcardPattern]
-                        : [normalized];
-
-                const rows = await db
-                    .select({
-                        fullDomain: customDnsRecords.fullDomain,
-                        type: customDnsRecords.type,
-                        values: customDnsRecords.values,
-                        ttl: customDnsRecords.ttl,
-                        priority: customDnsRecords.priority,
-                        weight: customDnsRecords.weight,
-                        port: customDnsRecords.port
-                    })
-                    .from(customDnsRecords)
-                    .innerJoin(
-                        domains,
-                        eq(domains.domainId, customDnsRecords.domainId)
-                    )
-                    .where(
-                        and(
-                            inArray(customDnsRecords.fullDomain, names),
-                            eq(customDnsRecords.enabled, true),
-                            eq(domains.baseDomain, zone),
-                            eq(domains.type, "ns"),
-                            eq(domains.verified, true)
+        return this.withInFlightDedup<CustomZoneRecord[]>(
+            cacheKey,
+            async () => {
+                try {
+                    const rows = await db
+                        .select({
+                            fullDomain: customDnsRecords.fullDomain,
+                            type: customDnsRecords.type,
+                            values: customDnsRecords.values,
+                            ttl: customDnsRecords.ttl,
+                            priority: customDnsRecords.priority,
+                            weight: customDnsRecords.weight,
+                            port: customDnsRecords.port
+                        })
+                        .from(customDnsRecords)
+                        .innerJoin(
+                            domains,
+                            eq(domains.domainId, customDnsRecords.domainId)
                         )
+                        .where(
+                            and(
+                                eq(customDnsRecords.enabled, true),
+                                eq(domains.baseDomain, zone),
+                                eq(domains.type, "ns"),
+                                eq(domains.verified, true)
+                            )
+                        );
+
+                    const result: CustomZoneRecord[] = rows.map((row) => ({
+                        ...row,
+                        values: parseCustomDnsRecordValues(row.values)
+                    }));
+
+                    this.cache.set(
+                        cacheKey,
+                        result,
+                        CUSTOM_RECORD_CACHE_TTL_SECONDS
                     );
-
-                const exactRows = rows.filter(
-                    (row) => row.fullDomain === normalized
-                );
-                // Each stored value is served as its own resource record
-                const result: CustomRecord[] = (
-                    exactRows.length > 0 ? exactRows : rows
-                ).flatMap(({ fullDomain, values, ...record }) =>
-                    parseCustomDnsRecordValues(values).map((value) => ({
-                        ...record,
-                        value
-                    }))
-                );
-
-                this.cache.set(
-                    cacheKey,
-                    result,
-                    CUSTOM_RECORD_CACHE_TTL_SECONDS
-                );
-                return result;
-            } catch (error) {
-                logger.error("Error querying custom DNS records:", error);
-                return [];
+                    return result;
+                } catch (error) {
+                    logger.error("Error querying custom DNS records:", error);
+                    return [];
+                }
             }
-        });
+        );
     }
 
     // Finds the user-defined delegation (NS records on a subdomain) that
@@ -1414,7 +1427,9 @@ export class AuthoritativeDNSServer {
             ) {
                 continue;
             }
-            const glue = (await this.getCustomRecords(target, zone)).filter(
+            const glue = (
+                await this.getCustomRecords(target, zone)
+            ).records.filter(
                 (record) => record.type === "A" || record.type === "AAAA"
             );
             additionals.push(