Add Oblivious DNS-over-HTTPS types and serialization #43

yigityazicilar · 2025-08-22T13:06:24Z

Add Oblivious DNS-over-HTTPS types and serialization

Motivation:

This change adds the types and wire format serialization for
Oblivious DNS-over-HTTPS (ODoH) as specified in RFC 9230, providing the data
structures needed for privacy-preserving DNS resolution through HPKE encryption
and proxy routing.

Modifications:

Added ObliviousDoH target to Package.swift with dependencies on
ObliviousXHelpers
Created comprehensive type system with Configuration, MessagePlaintext, and
Message structs
Implemented ODoH.Codable protocol for bidirectional wire format conversion
Added detailed error handling with ObliviousDoHError types for protocol
violations
Added test suite covering configuration parsing, message serialization, and
error cases

Result:

The library provides complete ODoH type infrastructure with
RFC-compliant wire format serialization, enabling future implementation of the
encryption/decryption routine.

glbrntt · 2025-08-22T14:17:44Z

Sources/ObliviousDoH/Errors.swift

+    @inline(never)
+    public static func unsupportedHPKEParameters() -> ObliviousDoHError {
+        Self.init(backing: .unsupportedHPKEParameters)
+    }
+
+    @inline(never)
+    public static func invalidODoHData() -> ObliviousDoHError {
+        Self.init(backing: .invalidODoHData)
+    }


These two can be computed static vars, no need for them to be functions.

glbrntt · 2025-08-22T14:19:44Z

Sources/ObliviousDoH/ODoHRoutine.swift

+        /// HPKE sender context required for response decryption
+        public let context: HPKE.Sender
+
+        internal init(encryptedQuery: Data, context: HPKE.Sender) {


Why are all the inits internal?

QueryEncryptionResult and QueryDecryptionResult should only be created by encrypting or decrypting a query. So only the ODoH HPKE logic will create these.

glbrntt · 2025-08-22T14:24:52Z

Sources/ObliviousDoH/ODoHRoutine.swift

+    /// Provides bidirectional conversion between Swift types and their network representation
+    /// as specified in RFC 9230. All ODoH message types implement this protocol to enable
+    /// consistent serialization and parsing across the protocol stack.
+    public protocol Codable {


I don't think this name is quite right, it's quite different to Swift.Codable.

glbrntt · 2025-08-22T14:26:51Z

Sources/ObliviousDoH/ODoHRoutine.swift

+        /// Initialize from wire format bytes, consuming data as it parses
+        /// - Parameter bytes: The raw network data to parse (consumed during parsing)
+        /// - Returns: `nil` if parsing fails or data is invalid
+        init?(_ bytes: inout Data)


This should probably have a parameter label, maybe something like init?(decoding data: inout Data)

glbrntt · 2025-08-22T14:28:22Z

Sources/ObliviousDoH/ODoHRoutine.swift

+    public protocol Codable {
+        /// Initialize from wire format bytes, consuming data as it parses
+        /// - Parameter bytes: The raw network data to parse (consumed during parsing)
+        /// - Returns: `nil` if parsing fails or data is invalid


Do we know prior to decoding if we'll have enough bytes?

There's also some asymmetry here: encode() throws on failure but here we just return nil.

In NIO-land nil means "not enough bytes" and throws signals parsing failure. (If nil is returned no bytes would've been consumed so it's safe to wait for more bytes and try again.) Is there any reason to not do the same model here?

Yes, we do know prior to decoding if we'll have enough bytes. The ODoH
configuration format includes explicit length fields at every level:

Configurations level: totalLength (2 bytes)

Each Configuration: length (2 bytes) for contents size

ConfigurationContents: keyLength (2 bytes)

This is exactly what we do at
ODoHRoutine.swift:352:

let totalLength = data.popUInt16(), var configsData = data.popFirst(Int(totalLength))

ODoHRoutine.swift:470:

let length = bytes.popUInt16(), var contentsBytes = bytes.popFirst(Int(length)) // Pop the entire object

ODoHRoutine.swift:552:

let keyLength = bytes.popUInt16(), let key = bytes.popFirst(Int(keyLength)),

So, we only return nil when we actually don't have enough data as the only error we cannot recover from is invalidODoHData. The rest of the errors correctly consume the number of bytes so we can continue parsing the Configurations object until we have a valid Configuration we can handle.

Then we check for this error which gets throws if we don't have enough data.
ODoHRoutine.swift:372:

case .failure(let error): if error == ObliviousDoHError.invalidODoHData() { break }

Thanks for clarifying. In that case the method should be documented to say something to that effect.

Also, why nil vs. throws?

The parsing strategy prioritizes returning usable configurations even when some fail:

If we parse 3 configurations and the first 2 are valid but the 3rd fails with unsupportedHPKEParameters, we still return the first 2 valid ones

When we get invalidODoHData, it means we're at the end of readable bytes and cannot read more, so everything else is a failure. But we can still return the configurations parsed previously.

We only return nil when there are zero usable configurations

This differs from NIO's typical "nil = need more bytes" pattern because we're parsing a collection where partial success is valuable. In ODoH, having some working configurations is better than failing completely due to one bad entry or hitting the end of available data.

You've described the behaviour of the caller, which is equally achievable with catching errors as it is with checking for nil.

The general guidance is that you should only return nil from inits where the failure is trivial (e.g. Int("not-an-int") returning nil is fine because there's only one path to failure: the string doesn't represent an integer).

Anything more complicated should throw an error. This gives the caller more control as they can inspect different error cases and act appropriately. This could just be doing let x = try? SomeFailableInit().

I think the docs here are outdated since this isn't a failable init anymore

glbrntt · 2025-08-22T14:33:04Z

Sources/ObliviousDoH/ODoHRoutine.swift

+        /// Creates a new ODoH configuration with the specified contents backing.
+        ///
+        /// - Parameter contentsBacking: The version-specific contents backing
+        internal init(contentsBacking: ContentsBacking) {


Any reason this shouldn't be private?

This is private because it is internally using an enum. Currently ODoH has one version however each new version could potentially have a new contents structure.

The helper function (ODoH.Configuration.v1()) below to create a v1 configuration should be used for the server side and the client should only get the configuration from the server side.

I know, I'm asking why the init is currently internal and not private?

glbrntt · 2025-08-22T14:34:02Z

Sources/ObliviousDoH/ODoHRoutine.swift

+            public static func query() -> Self {
+                Self(rawValue: 1)
+            }
+
+            public static func response() -> Self {
+                Self(rawValue: 2)
+            }


These can be computed static vars

glbrntt · 2025-08-22T14:35:14Z

Sources/ObliviousDoH/ODoHRoutine.swift

+            let totalLength = data.popUInt16(),
+            var configsData = data.popFirst(Int(totalLength))
+        else {
+            return ODoH.ConfigurationParsingResult(


Should data be re-instated with fullData here?

If it fails there the entire Configurations structure is invalid so I am not sure if we should reset it.

and this is based on the expectation that the data contains enough bytes? If so this should be documented.

glbrntt · 2025-08-22T14:38:50Z

Sources/ObliviousDoH/ODoHRoutine.swift

+        data.append(bigEndianBytes: UInt16(self.dnsMessage.count))  // 2 bytes: DNS length
+        data.append(self.dnsMessage)  // Variable: DNS data
+        data.append(bigEndianBytes: UInt16(self.paddingLength))  // 2 bytes: padding length
+        data.append(contentsOf: .init(repeating: 0, count: self.paddingLength))


What are we .init-ing here to append? Just want to make sure we're not having an avoidable temporary allocation.

It is just 0 bytes repeated paddingLength times. There might be a better way to add that to Data.

what type are we .init ing? I suspect its Data/Array.

Either way it'd be a temporary allocation we can avoid with:

data.append(contentsOf: repeatElement(0, count: self.paddingLength))

glbrntt · 2025-08-22T14:39:59Z

Sources/ObliviousDoH/ODoHRoutine.swift

+/// Used to derive the key identifier from the target's public key configuration
+private let ODoHKeyIDInfo = Data("odoh key id".utf8)
+
+/// Used as HPKE info parameter when setting up encryption context for DNS queries
+private let ODoHQueryInfo = Data("odoh query".utf8)
+
+/// Used when exporting secrets from HPKE context for response encryption
+private let ODoHResponseInfo = Data("odoh response".utf8)
+
+/// Used to derive the AEAD key for encrypting/decrypting DNS responses
+private let ODoHKeyInfo = Data("odoh key".utf8)
+
+/// Used to derive the AEAD nonce for encrypting/decrypting DNS responses
+private let ODoHNonceInfo = Data("odoh nonce".utf8)


Top-level constants are generally other be avoided. Better to extend Data instead:

extension Data { fileprivate static let oDoHKeyIDInfo = Self(...) // ... }

glbrntt · 2025-08-26T09:53:59Z

Sources/ObliviousDoH/ODoHRoutine.swift

+    public protocol Codable {
+        /// Initialize from wire format bytes, consuming data as it parses
+        /// - Parameter bytes: The raw network data to parse (consumed during parsing)
+        /// - Returns: `nil` if parsing fails or data is invalid


Thanks for clarifying. In that case the method should be documented to say something to that effect.

Also, why nil vs. throws?

glbrntt · 2025-08-26T09:55:04Z

Sources/ObliviousDoH/ODoHRoutine.swift

+        public var version: Int {
+            contentsBacking.version
+        }
+        // length prefix (UInt16)


Okay. Code comments should provide enough context for any reader/maintainer, not just enough for the author.

glbrntt · 2025-08-26T09:56:27Z

Sources/ObliviousDoH/ODoHRoutine.swift

+        /// Creates a new ODoH configuration with the specified contents backing.
+        ///
+        /// - Parameter contentsBacking: The version-specific contents backing
+        internal init(contentsBacking: ContentsBacking) {


I know, I'm asking why the init is currently internal and not private?

glbrntt · 2025-08-26T09:57:47Z

Sources/ObliviousDoH/ODoHRoutine.swift

+            let totalLength = data.popUInt16(),
+            var configsData = data.popFirst(Int(totalLength))
+        else {
+            return ODoH.ConfigurationParsingResult(


and this is based on the expectation that the data contains enough bytes? If so this should be documented.

glbrntt · 2025-08-26T09:58:44Z

Sources/ObliviousDoH/ODoHRoutine.swift

+        data.append(bigEndianBytes: UInt16(self.dnsMessage.count))  // 2 bytes: DNS length
+        data.append(self.dnsMessage)  // Variable: DNS data
+        data.append(bigEndianBytes: UInt16(self.paddingLength))  // 2 bytes: padding length
+        data.append(contentsOf: .init(repeating: 0, count: self.paddingLength))


what type are we .init ing? I suspect its Data/Array.

Either way it'd be a temporary allocation we can avoid with:

data.append(contentsOf: repeatElement(0, count: self.paddingLength))

glbrntt · 2025-08-26T10:02:25Z

Sources/ObliviousDoH/ODoHRoutine.swift

+    fileprivate static let ODoHKeyIDInfo = Self("odoh key id".utf8)
+
+    /// Used as HPKE info parameter when setting up encryption context for DNS queries
+    fileprivate static let ODoHQueryInfo = Self("odoh query".utf8)
+
+    /// Used when exporting secrets from HPKE context for response encryption
+    fileprivate static let ODoHResponseInfo = Self("odoh response".utf8)
+
+    /// Used to derive the AEAD key for encrypting/decrypting DNS responses
+    fileprivate static let ODoHKeyInfo = Self("odoh key".utf8)
+
+    /// Used to derive the AEAD nonce for encrypting/decrypting DNS responses
+    fileprivate static let ODoHNonceInfo = Self("odoh nonce".utf8)


As properties these should all start with a lowercase: oDoH...

Tests/ObliviousDoHTests/ObliviousDoHTests.swift

gjcairo · 2025-09-05T15:28:39Z

Sources/ObliviousDoH/ODoHRoutine.swift

+    public protocol Codable {
+        /// Initialize from wire format bytes, consuming data as it parses
+        /// - Parameter bytes: The raw network data to parse (consumed during parsing)
+        /// - Returns: `nil` if parsing fails or data is invalid


I think the docs here are outdated since this isn't a failable init anymore

Sources/ObliviousDoH/ODoHRoutine.swift

Sources/ObliviousDoH/Errors.swift

Co-authored-by: Gus Cairo <[email protected]>

feat: add types and serialization for Oblivious DNS-over-HTTPS

706ef32

glbrntt requested changes Aug 22, 2025

View reviewed changes

fix: apply review feedback to ODoH types and API

7b5e59d

yigityazicilar requested a review from glbrntt August 26, 2025 09:56

glbrntt reviewed Aug 26, 2025

View reviewed changes

yigityazicilar added 2 commits August 26, 2025 15:02

refactor: improve ODoH decoding API with proper error handling

e31d644

fix DocC comments as function signatures changed

1ccea7a

yigityazicilar requested a review from glbrntt August 26, 2025 14:27

glbrntt approved these changes Sep 5, 2025

View reviewed changes

gjcairo reviewed Sep 5, 2025

View reviewed changes

docs: add DocC comments to public API.

bc93496

glbrntt enabled auto-merge (squash) September 5, 2025 16:01

glbrntt disabled auto-merge September 5, 2025 16:02

yigityazicilar and others added 2 commits September 5, 2025 17:03

feat: add protocol conformance to errors

67085b6

Update Tests/ObliviousDoHTests/ObliviousDoHTests.swift

7ca45fe

Co-authored-by: Gus Cairo <[email protected]>

gjcairo approved these changes Sep 7, 2025

View reviewed changes

feat: add RawRepresentable conformance to MessageType

be0245c

glbrntt added the 🆕 semver/minor Adds new public API. label Sep 9, 2025

glbrntt enabled auto-merge (squash) September 9, 2025 11:52

glbrntt merged commit b4e1b44 into apple:main Sep 9, 2025
34 of 35 checks passed

Add Oblivious DNS-over-HTTPS types and serialization #43

Add Oblivious DNS-over-HTTPS types and serialization #43

Uh oh!

Conversation

yigityazicilar commented Aug 22, 2025

Motivation:

Modifications:

Result:

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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development