Use Sendable DNS types. by jglogan · Pull Request #1269 · apple/container

jglogan · 2026-02-26T00:11:46Z

Closes [Request]: Eliminate external DNS library dependencies. #1268.
The types we were using weren't very usable with Swift 6 structured concurrency.
Implements just the subset of records that we use.
Use notImplemented instead of formatError for unknown record types.
Use pure actor for LocalhostDNSHandler now that we have sendable types.
Use DNSName as key for table lookups in LocalhostDNSHandler and HostTableResolver.
Utilize dot-suffixed domain names everywhere in the lookup chain.

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Testing

Tested locally
Added/updated tests
Added/updated docs

manojmahapatra · 2026-02-26T22:09:48Z

+
+    /// The fully-qualified domain name with trailing dot.
+    public var description: String {
+        labels.isEmpty ? "." : labels.joined(separator: ".") + "."


Question: some lookup paths appear to use plain string (no normalization), will this change cause any issue? For example in AttachmentAllocator;

// store func allocate(hostname: String).. // lookup func lookup(hostname: String) async throws -> UInt32? { hostnames[hostname] }

@manojmahapatra Thanks for looking at this! I converted it to draft.

This one was done by telling Claude to create the types we needed using the bit-fiddling code that ContainerizationNetlink uses, and I pushed it primarily to back the change up and get something started for the issue.

I still need to review the work myself and sanity check it, but I'll bet I would have missed this!

I'll have a look through the relevant RFCs as I review this myself. This isn't an area I'm super expert in so if you've got experience and suggestions I'd welcome them!

I'm not an expert in this area either, but I reviewed the PR in detail against current behavior. Overall this is moving in the right direction.

I see two gaps worth addressing here:

Hostname normalization consistency (e.g. foo vs foo.) across parse/store/lookup paths. (additional focused unit tests will be nice)

Compression-pointer safety in DNSName decode to avoid malformed packet loops. Something like this maybe;

// Calculate pointer offset from message start let pointerLocation = offset let pointer = Int(length & 0x3F) << 8 | Int(buffer[offset + 1]) let pointerTarget = messageStart + pointer guard pointerTarget < pointerLocation else { throw DNSBindError.unmarshalFailure(type: "DNSName", field: "compression pointer not prior") } offset = pointerTarget

- Closes apple#1268. - The types we were using weren't very usable with Swift 6 structured concurrency. - Use notImplemented instead of formatError for unknown record types. - Use pure actor for LocalhostDNSHandler now that we have sendable types.

- Require all input domain names to carry trailing dot.

- Fold labels to lowercase. - Reject compression pointer recursion.

jglogan · 2026-03-13T00:46:12Z

@manojmahapatra I've addressed what you called out, and also we're now folding to lowercase when creating domain name records.

The larger change (requiring a trailing dot on all the DNS APIs) is in the first follow-up commit, and the smaller one for case folding and compression pointer recursion preventions is in the second one.

Finally got myself out from under some other obligations which've kept me busy elsewhere. I'll look over your apple/containerization#577 early tomorrow. Thank you again for your scrutiny on this PR!

manojmahapatra · 2026-03-13T05:06:52Z

+    ///
+    /// - Parameter dnsHostname: A DNS-format hostname, optionally with a trailing dot
+    ///   (e.g. `"example.com."` or `"example.com"`). The trailing dot is stripped before lookup.
+    public func lookup(dnsHostname: String) async throws -> Attachment? {


Question: Looks like it's a public API break due to the function rename? Do we need a shim?

Ah, my bad. With the docc description we don't really need to change the parameter name.

manojmahapatra · 2026-03-13T05:07:43Z

 /// Handler that uses table lookup to resolve hostnames.
+///
+/// All keys in `hosts4` must be canonical DNS names — fully-qualified with a
+/// trailing dot (e.g. `"example.com."`). This matches the canonical form used


manojmahapatra · 2026-03-13T05:25:42Z

+    public init(_ string: String) {
+        // Remove trailing dot if present, then split
+        let normalized = string.hasSuffix(".") ? String(string.dropLast()) : string
+        self.labels = normalized.isEmpty ? [] : normalized.split(separator: ".").map { String($0).lowercased() }


Question: So here labels are lowercases when parsing names. However, HostTableResolver does not normalize keys on init, which I think might be an issue. can you please confirm?

Thanks for catching that. We're not using HostTableResolver yet, but it would have been broken. I reworked the case folding and the HostTableResolver a little to do things correctly, and a little more cleanly.

manojmahapatra · 2026-03-13T05:26:30Z

@jglogan overall the changes looks good, just two questions.

manojmahapatra · 2026-03-13T05:29:12Z

+    ///
+    /// - Parameter hosts4: A dictionary mapping fully-qualified domain names (with trailing dot)
+    ///   to IPv4 addresses. Keys without a trailing dot will not match wire-decoded queries.
+    /// - Parameter ttl: The TTL in seconds to set on answer records.


nitpick: The TTL in seconds to set on answer records. Defaults to 300.

katiewasnothere · 2026-03-17T06:32:39Z

+            let endOffset = try record.appendBuffer(&buffer, offset: 0)
+
+            // Should have written: name + type(2) + class(2) + ttl(4) + rdlen(2) + rdata(4)
+            #expect(endOffset > 0)


Can we check that the end offset is > 4 instead?

katiewasnothere

Couple test requests, otherwise LGTM

manojmahapatra

LGTM

katiewasnothere

LGTM

- Removes the Bindable prototype as we're not using it.

- Fix a double-optional expression, use flatMap. - Validate the DNS name extracted from the resolv.conf option, and then use the lowercased name as the DNS dict key. - Return NODATA on AAAA query if an A record exists, which matches HostTableResolver and ContainerDNSHandler behavior.

- Lookup now matches HostTableResolver.

jglogan · 2026-03-19T15:47:05Z

+/// are case-insensitive and trailing dots are optional.
 public struct HostTableResolver: DNSHandler {
-    public let hosts4: [String: IPv4]
+    public let hosts4: [DNSName: IPv4Address]


summary:

Replace IPv4 with our IPv4Address everywhere.

Use DNSName for keys to validate and normalize hostnames for lookup.

jglogan · 2026-03-19T15:51:50Z


    public func answer(query: Message) async throws -> Message? {
-        let question = query.questions[0]
+        guard let question = query.questions.first else {


summary: updated all handlers to ensure questions[0] is present, and if not, defer to the next handler

This would result in a NXDOMAIN when the last handler returns nil, when a .formatError would be more appropriate. The typical usage, though, is to put a StandardQueryValidator at the front of the chain which ensures that exactly one question is present and returns a .formatError otherwise.

jglogan · 2026-03-19T15:57:24Z

+        guard let question = query.questions.first else {
+            return nil
+        }
+        let n = question.name.hasSuffix(".") ? String(question.name.dropLast()) : question.name


summary: for the DNS names that come in on the wire, we don't perform hostname validation - we just split into labels and form a DNSName, and look them up. If they have weird characters or are otherwise invalid hostnames, they'll just miss as L35 ensures that our keys are always valid hostnames.

jglogan · 2026-03-19T15:58:43Z

            }
            // If hostname doesn't exist, return nil which will become NXDOMAIN
            return nil
-        case ResourceRecordType.nameServer,


summary: all unhandled question types in handlers should return .notImplemented

jglogan · 2026-03-19T15:59:43Z

+//===----------------------------------------------------------------------===//
+
+/// Errors that can occur during DNS message serialization/deserialization.
+public enum DNSBindError: Error, CustomStringConvertible {


summary: extracted types from Bindable.swift into individual files

jglogan · 2026-03-19T16:00:40Z

+    public var labels: [String]
+
+    /// Creates a DNS name representing the root (empty label list).
+    public init() {


summary: used to building DNS names label by label from wire data

- Ensures labels are normalized.

jglogan · 2026-03-19T16:38:44Z

+// We can look at refining this later to see if we can use some common
+// bit fiddling code everywhere.
+
+extension [UInt8] {


summary: removed Bindable.swift, extracting types into individual files, this code is brought over from containerization

jglogan · 2026-03-19T16:39:31Z

+        // EDNS0 (RFC 6891), and this server only resolves host A/AAAA queries, so a
+        // legitimate query will never approach this limit. Reject oversized packets
+        // before reading to avoid allocating memory for malformed or malicious datagrams.
+        let maxPacketSize = 512


summary: added for safety

jglogan · 2026-03-19T16:41:44Z


            self.log?.debug("serializing response")
            responseData = try response.serialize()
+        } catch let error as DNSBindError {


summary: we now differentiate between structural errors in DNS queries (.formatError) and unsupported query values (.notImplemented)

jglogan · 2026-03-19T16:42:19Z

+            responseData = try response.serialize()
        } catch {
-            self.log?.error("error processing message from \(query): \(error)")
+            let rawId = data.count >= 2 ? data[0..<2].withUnsafeBytes { $0.load(as: UInt16.self) } : 0


summary: our "500" path

jglogan · 2026-03-19T16:43:47Z

    private let watcher: DirectoryWatcher

-    private let dns: Mutex<[String: IPv4]>
+    private var dns: [DNSName: IPv4Address]


summary: no need for Mutex, actor isolation is fine here, and use DNSName keys to ensure hostname validity

jglogan · 2026-03-19T16:45:47Z


                if let match = content.firstMatch(of: regex),
-                    let ipv4 = (match[1].substring.map { String($0) })
+                    let ipv4 = (match[1].substring.flatMap { try? IPv4Address(String($0)) })


summary: flatMap should be used here since substring produces an optional

jglogan · 2026-03-19T16:47:13Z

+        guard let question = query.questions.first else {
+            return nil
+        }
+        let n = question.name.hasSuffix(".") ? String(question.name.dropLast()) : question.name


Same pattern as HostTableHandler, don't need to do hostname validation on the wire data.

jglogan · 2026-03-19T16:47:39Z

-            ? ""
-            : HostDNSResolver.localhostOptionsRegex.replacingOccurrences(
-                of: #"\((.*?)\)"#, with: localhost!.description, options: .regularExpression)
+            localhost.map {


summary: remove force-unwrap

manojmahapatra reviewed Feb 26, 2026

View reviewed changes

jglogan marked this pull request as draft March 4, 2026 03:33

jglogan force-pushed the dns-records branch 3 times, most recently from 0d90e59 to 32eac7e Compare March 12, 2026 21:17

jglogan added 3 commits March 12, 2026 17:33

Use Sendable DNS types.

4de2a15

- Closes apple#1268. - The types we were using weren't very usable with Swift 6 structured concurrency. - Use notImplemented instead of formatError for unknown record types. - Use pure actor for LocalhostDNSHandler now that we have sendable types.

Additional PR feedback - domain name handling refinement.

f716800

- Require all input domain names to carry trailing dot.

Additional PR feedback.

c778d70

- Fold labels to lowercase. - Reject compression pointer recursion.

jglogan force-pushed the dns-records branch from 9daa836 to c778d70 Compare March 13, 2026 00:37

jglogan marked this pull request as ready for review March 13, 2026 00:45

manojmahapatra reviewed Mar 13, 2026

View reviewed changes

jglogan added 3 commits March 16, 2026 08:52

Rename API parameter to avoid breaking change.

a1b6325

Add default ttl to docc comment.

ea1e492

Fix case folding in HostTableResolver.

a108c64

katiewasnothere reviewed Mar 17, 2026

View reviewed changes

Comment thread Tests/DNSServerTests/RecordsTests.swift

katiewasnothere reviewed Mar 17, 2026

View reviewed changes

Comment thread Tests/DNSServerTests/RecordsTests.swift

katiewasnothere approved these changes Mar 17, 2026

View reviewed changes

Test all record processing offsets, add compression ptr test.

5ac4c31

manojmahapatra approved these changes Mar 18, 2026

View reviewed changes

Merge branch 'main' into dns-records

f13d26e

jglogan modified the milestone: 2026-03 Mar 18, 2026

katiewasnothere approved these changes Mar 18, 2026

View reviewed changes

Validate domain names in DNSName init.

e8a75b5

jglogan force-pushed the dns-records branch from dfe6242 to 600183b Compare March 19, 2026 00:03

jglogan added 8 commits March 18, 2026 17:19

Split Bindable.swift into separate files.

0046f87

- Removes the Bindable prototype as we're not using it.

security: bound packet rx size

044ca99

Prevent crash if handlers not protected by the validating handler.

5547f78

Log failure to send response on NIO channel.

a5429fd

Eliminate a force-unwrap.

3e6a760

Tighten the link compression pointer check.

6d4ac74

Key LocalhostDNSHandler on DNSName.

7cb521b

- Lookup now matches HostTableResolver.

jglogan force-pushed the dns-records branch from 292c57f to 7cb521b Compare March 19, 2026 03:33

Merge remote-tracking branch 'upstream/main' into dns-records

282440a

jglogan commented Mar 19, 2026

View reviewed changes

Prevent direct writes to DNSName label list.

69ad281

- Ensures labels are normalized.

jglogan commented Mar 19, 2026

View reviewed changes

katiewasnothere approved these changes Mar 19, 2026

View reviewed changes

jglogan merged commit 4a8a8a6 into apple:main Mar 19, 2026
3 checks passed

jglogan deleted the dns-records branch March 19, 2026 17:58

BrewTestBot mentioned this pull request Mar 31, 2026

container 0.11.0 Homebrew/homebrew-core#275415

Merged

Conversation

jglogan commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of Change

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jglogan commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jglogan Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manojmahapatra commented Mar 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

katiewasnothere left a comment

Choose a reason for hiding this comment

Uh oh!

manojmahapatra left a comment

Choose a reason for hiding this comment

Uh oh!

katiewasnothere left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jglogan Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jglogan commented Feb 26, 2026 •

edited

Loading

jglogan commented Mar 13, 2026 •

edited

Loading

jglogan Mar 17, 2026 •

edited

Loading

jglogan Mar 19, 2026 •

edited

Loading