dot upstreams: per-upstream tls_name for sni and cert verification by dns name

This commit is contained in:
2026-08-01 14:38:45 +02:00
parent 70bff22d75
commit 3baf5d6581
13 changed files with 405 additions and 17 deletions
+10
View File
@@ -1011,3 +1011,13 @@ review rounds; findings went 9 → 3 → 0):
`OutOfMemory` is a local resource, not a peer fault. `OutOfMemory` is a local resource, not a peer fault.
- `transport.zig`: `Endpoint.parse` rejects userinfo/query/fragment delimiters - `transport.zig`: `Endpoint.parse` rejects userinfo/query/fragment delimiters
(`@`, `?`, `#`) in the authority, and `?`/`#` in a DoH path. (`@`, `?`, `#`) in the authority, and `?`/`#` in a DoH path.
- `dot_client.zig` (follow-up, tls_name commit): `DotClient.init` takes a
`tls_name`; it is the SNI and certificate-verification name, while the dial
target stays `endpoint.host`. Empty keeps the endpoint host, which is the
behavior described above. The same follow-up fixed a send bug this file
had from the start, invisible until a DoT handshake first succeeded:
`tls.Client.flush` only encrypts into the socket writer's buffer and never
flushes it, so the query never left the process and the peer eventually
closed the connection (`ReceiveFailed`/`EndOfStream`). `TlsStream.flush` now
does both flushes and `exchange` calls it; a hermetic loopback test in
`tls_client_integration_test.zig` covers it.
+15
View File
@@ -1920,3 +1920,18 @@ refinements (three review rounds; findings went 8 → 3 → 1 low):
stock 0.16 — the `-Dlive` DoT case fails with `CertificateHostMismatch` stock 0.16 — the `-Dlive` DoT case fails with `CertificateHostMismatch`
against 1.1.1.1 while live DoH passes. Planned follow-up (own commit): a against 1.1.1.1 while live DoH passes. Planned follow-up (own commit): a
per-upstream `tls_name` for SNI + verification while dialing the IP. per-upstream `tls_name` for SNI + verification while dialing the IP.
### Follow-up: per-upstream `tls_name`
The stdlib limitation above is now worked around the way stubby and unbound
do it. `UpstreamServer` (not the `upstream` timeout section) gains
`tls_name: []const u8 = ""`, migration step 2 adds
`upstreams.tls_name TEXT NOT NULL DEFAULT ''`, and the ZON round trip carries
it like every other field. `validate.zig` adds `BadTlsName` (must pass
`dns.name.fromText`) and `TlsNameOnNonTlsUpstream` (a `tls_name` on a DoH
upstream is a config error, because DoH verifies by its url host and would
ignore the field). `DotClient` uses it for SNI and verification while still
dialing `endpoint.host`, and `cli.zig`'s check probe passes it through. The
`-Dlive` DoT test now uses `1.1.1.1` with `one.one.one.one` and passes; making
it pass also uncovered and fixed a missing socket flush in the DoT send path
(see the milestone-3 note).
+1 -1
View File
@@ -647,7 +647,7 @@ fn probeUpstreams(r: Runner, cfg: model.Config) !usize {
break :doh doh.client(); break :doh doh.client();
}, },
.dot => dot: { .dot => dot: {
dot = dot_client.DotClient.init(endpoint, r.gpa, &bundle, &bundle_lock, .{ dot = dot_client.DotClient.init(endpoint, server.tls_name, r.gpa, &bundle, &bundle_lock, .{
.tls_read = tls_buffers[0..chunk], .tls_read = tls_buffers[0..chunk],
.tls_write = tls_buffers[chunk .. 2 * chunk], .tls_write = tls_buffers[chunk .. 2 * chunk],
.stream_read = tls_buffers[2 * chunk .. 3 * chunk], .stream_read = tls_buffers[2 * chunk .. 3 * chunk],
+6 -1
View File
@@ -161,7 +161,7 @@ const seed_source: [:0]const u8 =
\\ .groups = .{ .{ .name = "default" }, .{ .name = "kids", .safe_search = true } }, \\ .groups = .{ .{ .name = "default" }, .{ .name = "kids", .safe_search = true } },
\\ .upstreams = .{ \\ .upstreams = .{
\\ .{ .url = "https://dns.example/dns-query", .priority = 10 }, \\ .{ .url = "https://dns.example/dns-query", .priority = 10 },
\\ .{ .url = "tls://dot.example:853", .priority = 20, .enabled = false }, \\ .{ .url = "tls://dot.example:853", .priority = 20, .enabled = false, .tls_name = "dot.example" },
\\ }, \\ },
\\ .clients = .{ .{ .ip = "fd00::1", .name = "tablet", .group = "kids" } }, \\ .clients = .{ .{ .ip = "fd00::1", .name = "tablet", .group = "kids" } },
\\ .client_prefixes = .{ .{ .prefix = "192.168.1.0/24", .group = "kids", .priority = 50 } }, \\ .client_prefixes = .{ .{ .prefix = "192.168.1.0/24", .group = "kids", .priority = 50 } },
@@ -252,6 +252,11 @@ test "readConfig, writeConfig, import and readConfig again produce an equal conf
try testing.expectEqual(a.logging.level, b.logging.level); try testing.expectEqual(a.logging.level, b.logging.level);
try testing.expectEqualStrings(a.web.password_hash, b.web.password_hash); try testing.expectEqualStrings(a.web.password_hash, b.web.password_hash);
try testing.expectEqual(a.groups.len, b.groups.len); try testing.expectEqual(a.groups.len, b.groups.len);
try testing.expectEqual(a.upstreams.len, b.upstreams.len);
for (a.upstreams, b.upstreams) |left, right| {
try testing.expectEqualStrings(left.url, right.url);
try testing.expectEqualStrings(left.tls_name, right.tls_name);
}
try testing.expectEqual(a.rules.len, b.rules.len); try testing.expectEqual(a.rules.len, b.rules.len);
try testing.expectEqualStrings(a.clients[0].ip, b.clients[0].ip); try testing.expectEqualStrings(a.clients[0].ip, b.clients[0].ip);
try testing.expectEqualStrings(a.forward_zones[0].resolver, b.forward_zones[0].resolver); try testing.expectEqualStrings(a.forward_zones[0].resolver, b.forward_zones[0].resolver);
+9 -1
View File
@@ -388,7 +388,7 @@ const full_source: [:0]const u8 =
\\ .groups = .{ .{ .name = "default" }, .{ .name = "kids", .safe_search = true } }, \\ .groups = .{ .{ .name = "default" }, .{ .name = "kids", .safe_search = true } },
\\ .upstreams = .{ \\ .upstreams = .{
\\ .{ .url = "https://dns.example/dns-query", .priority = 10 }, \\ .{ .url = "https://dns.example/dns-query", .priority = 10 },
\\ .{ .url = "tls://dot.example:853", .priority = 20, .enabled = false }, \\ .{ .url = "tls://dot.example:853", .priority = 20, .enabled = false, .tls_name = "dot.example" },
\\ }, \\ },
\\ .clients = .{ .{ .ip = "FD00:0:0:0:0:0:0:1", .name = "tablet", .group = "kids" } }, \\ .clients = .{ .{ .ip = "FD00:0:0:0:0:0:0:1", .name = "tablet", .group = "kids" } },
\\ .client_prefixes = .{ .{ .prefix = "192.168.1.0/24", .group = "kids", .priority = 50 } }, \\ .client_prefixes = .{ .{ .prefix = "192.168.1.0/24", .group = "kids", .priority = 50 } },
@@ -449,6 +449,14 @@ test "importSource seeds a migrated database and group 'default' keeps id 1" {
try testing.expectEqual(@as(i64, 2), try database.queryInt("SELECT count(*) FROM upstreams")); try testing.expectEqual(@as(i64, 2), try database.queryInt("SELECT count(*) FROM upstreams"));
// The v6 client address was written in canonical form, not as typed. // The v6 client address was written in canonical form, not as typed.
try testing.expectEqual(@as(i64, 1), try database.queryInt("SELECT count(*) FROM clients WHERE ip = 'fd00::1'")); try testing.expectEqual(@as(i64, 1), try database.queryInt("SELECT count(*) FROM clients WHERE ip = 'fd00::1'"));
try testing.expectEqual(
@as(i64, 1),
try database.queryInt("SELECT count(*) FROM upstreams WHERE tls_name = 'dot.example'"),
);
try testing.expectEqual(
@as(i64, 1),
try database.queryInt("SELECT count(*) FROM upstreams WHERE tls_name = ''"),
);
} }
test "applyToDb without force refuses a configured database and changes nothing" { test "applyToDb without force refuses a configured database and changes nothing" {
+10 -1
View File
@@ -204,7 +204,16 @@ pub const BlocklistUpdate = struct { enabled: bool = true, interval_hours: u16 =
pub const Group = struct { name: []const u8, safe_search: bool = false }; pub const Group = struct { name: []const u8, safe_search: bool = false };
pub const UpstreamServer = struct { url: []const u8, priority: i32 = 100, enabled: bool = true }; pub const UpstreamServer = struct {
url: []const u8,
priority: i32 = 100,
enabled: bool = true,
/// DoT only. The DNS name used for SNI and certificate verification while
/// the connection still dials the URL's host. `std.crypto.Certificate`
/// matches dNSName SANs only, so a `tls://` upstream written as an IP
/// literal cannot verify without one. Empty means "verify by the URL host".
tls_name: []const u8 = "",
};
pub const Client = struct { ip: []const u8, name: []const u8 = "", group: []const u8 = "default" }; pub const Client = struct { ip: []const u8, name: []const u8 = "", group: []const u8 = "default" };
+74 -2
View File
@@ -31,6 +31,8 @@ pub const ValidateError = error{
NoUpstreams, NoUpstreams,
BadUpstreamUrl, BadUpstreamUrl,
DuplicateUpstreamUrl, DuplicateUpstreamUrl,
BadTlsName,
TlsNameOnNonTlsUpstream,
MissingDefaultGroup, MissingDefaultGroup,
DuplicateGroupName, DuplicateGroupName,
UnknownGroup, UnknownGroup,
@@ -400,6 +402,40 @@ fn canonical(scratch: Allocator, value: anytype) error{OutOfMemory}![]u8 {
return scratch.dupe(u8, w.buffered()); return scratch.dupe(u8, w.buffered());
} }
/// A `tls_name` overrides SNI and certificate verification for a DoT upstream,
/// which is the only transport that needs it: DoH verifies by the url host and
/// the http client would ignore this field, so a `tls_name` there is a config
/// error rather than a setting with no effect.
fn checkTlsName(
diags: *Diagnostics,
server: model.UpstreamServer,
scheme: transport.Scheme,
index: usize,
) error{OutOfMemory}!void {
if (server.tls_name.len == 0) return;
if (scheme != .dot) {
try diags.add(
error.TlsNameOnNonTlsUpstream,
"upstreams[{d}].tls_name",
.{index},
"tls_name is only for a tls:// upstream; '{s}' verifies by its url host",
.{server.url},
);
return;
}
_ = dns_name.fromText(server.tls_name) catch {
try diags.add(
error.BadTlsName,
"upstreams[{d}].tls_name",
.{index},
"'{s}' is not a valid domain name",
.{server.tls_name},
);
};
}
fn checkCollections(cfg: Config, diags: *Diagnostics, scratch: Allocator) error{OutOfMemory}!void { fn checkCollections(cfg: Config, diags: *Diagnostics, scratch: Allocator) error{OutOfMemory}!void {
var group_names: StringSet = .empty; var group_names: StringSet = .empty;
var has_default = false; var has_default = false;
@@ -430,7 +466,12 @@ fn checkCollections(cfg: Config, diags: *Diagnostics, scratch: Allocator) error{
var upstream_urls: StringSet = .empty; var upstream_urls: StringSet = .empty;
var enabled_upstreams: usize = 0; var enabled_upstreams: usize = 0;
for (cfg.upstreams, 0..) |server, i| { for (cfg.upstreams, 0..) |server, i| {
_ = transport.Endpoint.parse(server.url) catch { // The scheme decides whether `tls_name` is meaningful, so the parse
// result is kept rather than discarded. An unparseable url reports only
// `BadUpstreamUrl`: what its scheme would have been is unknown.
if (transport.Endpoint.parse(server.url)) |endpoint| {
try checkTlsName(diags, server, endpoint.scheme, i);
} else |_| {
try diags.add( try diags.add(
error.BadUpstreamUrl, error.BadUpstreamUrl,
"upstreams[{d}].url", "upstreams[{d}].url",
@@ -438,7 +479,7 @@ fn checkCollections(cfg: Config, diags: *Diagnostics, scratch: Allocator) error{
"'{s}' is not an https:// or tls:// endpoint", "'{s}' is not an https:// or tls:// endpoint",
.{server.url}, .{server.url},
); );
}; }
if (try markSeen(&upstream_urls, scratch, server.url)) { if (try markSeen(&upstream_urls, scratch, server.url)) {
try diags.add( try diags.add(
error.DuplicateUpstreamUrl, error.DuplicateUpstreamUrl,
@@ -905,6 +946,37 @@ test "error.DuplicateUpstreamUrl" {
try expectProblem(cfg, error.DuplicateUpstreamUrl, "upstreams[1].url"); try expectProblem(cfg, error.DuplicateUpstreamUrl, "upstreams[1].url");
} }
test "a tls_name on a tls:// upstream validates cleanly" {
var cfg = baseConfig();
cfg.upstreams = &.{.{ .url = "tls://1.1.1.1:853", .tls_name = "one.one.one.one" }};
try expectClean(cfg);
}
test "error.BadTlsName on a malformed name" {
var cfg = baseConfig();
cfg.upstreams = &.{.{ .url = "tls://1.1.1.1:853", .tls_name = "one..one.one" }};
try expectProblem(cfg, error.BadTlsName, "upstreams[0].tls_name");
var too_long = baseConfig();
too_long.upstreams = &.{.{ .url = "tls://1.1.1.1:853", .tls_name = "a" ** 64 ++ ".example" }};
try expectProblem(too_long, error.BadTlsName, "upstreams[0].tls_name");
}
test "error.TlsNameOnNonTlsUpstream on a DoH upstream" {
var cfg = baseConfig();
cfg.upstreams = &.{.{ .url = "https://dns.example/dns-query", .tls_name = "dns.example" }};
try expectProblem(cfg, error.TlsNameOnNonTlsUpstream, "upstreams[0].tls_name");
}
test "an empty tls_name is accepted on every scheme" {
var cfg = baseConfig();
cfg.upstreams = &.{
.{ .url = "https://dns.example/dns-query" },
.{ .url = "tls://9.9.9.9:853" },
};
try expectClean(cfg);
}
test "error.MissingDefaultGroup" { test "error.MissingDefaultGroup" {
var cfg = baseConfig(); var cfg = baseConfig();
cfg.groups = &.{.{ .name = "kids" }}; cfg.groups = &.{.{ .name = "kids" }};
+12
View File
@@ -233,6 +233,18 @@ pub const TlsStream = struct {
return &self.client.writer; return &self.client.writer;
} }
/// Pushes buffered plaintext all the way to the socket.
///
/// Both flushes are required. `tls.Client.flush` only encrypts what the
/// plaintext writer holds into the socket writer's buffer and calls
/// `advance` (crypto/tls/Client.zig:999); it never flushes that writer, so
/// on its own it leaves the record sitting in this process. A caller that
/// then waits for a reply waits until the peer gives up.
pub fn flush(self: *TlsStream) std.Io.Writer.Error!void {
try self.client.writer.flush();
try self.stream_writer.interface.flush();
}
/// Sends close_notify and flushes the socket. Does not close the underlying /// Sends close_notify and flushes the socket. Does not close the underlying
/// stream; the caller owns it. /// stream; the caller owns it.
pub fn close(self: *TlsStream) void { pub fn close(self: *TlsStream) void {
@@ -108,3 +108,120 @@ test "live DoT handshake against cloudflare-dns.com" {
}, },
} }
} }
// ---------------------------------------------------------------------------
// Hermetic: loopback only, no name resolution, no external host.
// ---------------------------------------------------------------------------
const tls_server = @import("tls_server.zig");
/// The fixture certificate carries `DNS:localhost`, so this is the only name a
/// `TlsStream` can verify against it.
const fixture_host = "localhost";
const echo_message = "nxdns tls flush probe";
/// Reads one message and echoes it back.
fn echoOnce(
gpa: std.mem.Allocator,
ctx: *tls_server.ServerContext,
io: std.Io,
server: *net.Server,
) anyerror!void {
var stream = try server.accept(io);
defer stream.close(io);
var read_buffer: [4096]u8 = undefined;
var write_buffer: [4096]u8 = undefined;
var server_stream: tls_server.ServerStream = undefined;
try server_stream.accept(gpa, ctx, io, &stream, &read_buffer, &write_buffer);
defer server_stream.close(gpa);
var received: [echo_message.len]u8 = undefined;
try server_stream.reader().readSliceAll(&received);
try server_stream.writer().writeAll(&received);
try server_stream.writer().flush();
}
/// Writes through `TlsStream.flush` and waits for the echo. With only
/// `client.writer.flush()` the record would never leave this process and this
/// read would block until the budget expired.
fn flushAndEcho(io: std.Io, gpa: std.mem.Allocator, address: net.IpAddress) anyerror!void {
var stream = try address.connect(io, .{ .mode = .stream });
defer stream.close(io);
var bundle: Certificate.Bundle = .empty;
defer bundle.deinit(gpa);
var bundle_lock: std.Io.RwLock = .init;
var read_buffer: [4096]u8 = undefined;
var write_buffer: [4096]u8 = undefined;
var stream_read_buffer: [tls.Client.min_buffer_len]u8 = undefined;
var stream_write_buffer: [tls.Client.min_buffer_len]u8 = undefined;
var client: tls_client.TlsStream = undefined;
// The fixture certificate is self-signed, so the chain cannot verify; the
// host name still must match, which is what this test needs it to do.
try client.init(io, &stream, &bundle, &bundle_lock, gpa, .{
.host = fixture_host,
.ca = .insecure_skip_verify,
.read_buffer = &read_buffer,
.write_buffer = &write_buffer,
.stream_read_buffer = &stream_read_buffer,
.stream_write_buffer = &stream_write_buffer,
});
defer client.close();
try client.writer().writeAll(echo_message);
try client.flush();
var echoed: [echo_message.len]u8 = undefined;
try client.reader().readSliceAll(&echoed);
try std.testing.expectEqualStrings(echo_message, &echoed);
}
test "TlsStream.flush puts the record on the wire" {
if (!build_options.integration) return error.SkipZigTest;
const fixtures = @import("test_fixtures");
const gpa = std.testing.allocator;
var threaded: std.Io.Threaded = .init(gpa, .{});
defer threaded.deinit();
const io = threaded.io();
var ctx = try tls_server.ServerContext.init(gpa, fixtures.cert_pem, fixtures.key_pem);
defer ctx.deinit(gpa);
const listen_address: net.IpAddress = .{ .ip4 = .loopback(0) };
var server = try listen_address.listen(io, .{ .reuse_address = true });
defer server.deinit(io);
var server_task = try io.concurrent(echoOnce, .{ gpa, &ctx, io, &server });
// Raced against the budget: the failure this test guards against is a
// record that never leaves the process, which without a deadline would
// hang the run instead of failing it.
var outcomes: [2]Outcome = undefined;
var race: std.Io.Select(Outcome) = .init(io, &outcomes);
defer race.cancelDiscard();
try race.concurrent(.exchange, flushAndEcho, .{ io, gpa, server.socket.address });
try race.concurrent(.expiry, expire, .{ io, budget });
const client_result: anyerror!void = switch (try race.await()) {
.exchange => |result| result,
.expiry => |result| blk: {
try result;
break :blk error.TlsEchoTimedOut;
},
};
// A client that never connects would leave the server blocked in `accept`.
const server_result = if (client_result) |_|
server_task.await(io)
else |_|
server_task.cancel(io);
try client_result;
try server_result;
}
+66
View File
@@ -18,10 +18,19 @@ const log = std.log.scoped(.migrations);
pub const Step = struct { version: u32, sql: [:0]const u8 }; pub const Step = struct { version: u32, sql: [:0]const u8 };
/// Append only. Editing a released step — or `config_schema.ddl_v1` — would make
/// a fresh database and an upgraded one disagree, and nothing would detect it.
pub const steps = [_]Step{ pub const steps = [_]Step{
.{ .version = 1, .sql = config_schema.ddl_v1 }, .{ .version = 1, .sql = config_schema.ddl_v1 },
.{ .version = 2, .sql = ddl_v2 },
}; };
/// The DoT verification name (`upstreams.tls_name`). Empty keeps the pre-step-2
/// behavior: verify the certificate against the url host.
const ddl_v2: [:0]const u8 =
\\ALTER TABLE upstreams ADD COLUMN tls_name TEXT NOT NULL DEFAULT '';
;
pub const target_version: u32 = steps[steps.len - 1].version; pub const target_version: u32 = steps[steps.len - 1].version;
comptime { comptime {
@@ -236,6 +245,63 @@ test "a stepwise upgrade applies only the new steps" {
try testing.expectEqual(@as(i64, 1), try database.queryInt("SELECT count(*) FROM groups")); try testing.expectEqual(@as(i64, 1), try database.queryInt("SELECT count(*) FROM groups"));
} }
fn columnExists(database: *db.Db, table: []const u8, column: []const u8) !bool {
var stmt = try database.prepare("SELECT count(*) FROM pragma_table_info(?1) WHERE name = ?2");
defer stmt.deinit();
try stmt.bindText(1, table);
try stmt.bindText(2, column);
if (!try stmt.step()) return error.SqliteError;
return stmt.columnInt(0) != 0;
}
test "a fresh database reaches version 2 with the tls_name column" {
var database = try openMigrated();
defer database.close();
try testing.expectEqual(@as(u32, 2), try migrate(&database));
try testing.expectEqual(@as(u32, 2), target_version);
try testing.expect(try columnExists(&database, "upstreams", "tls_name"));
}
test "a version 1 database upgrades to 2 and keeps its rows with an empty tls_name" {
var database = try openMigrated();
defer database.close();
const first = [_]Step{.{ .version = 1, .sql = config_schema.ddl_v1 }};
try testing.expectEqual(@as(u32, 1), try migrateSteps(&database, &first));
try testing.expect(!try columnExists(&database, "upstreams", "tls_name"));
try database.exec("INSERT INTO upstreams (url, priority, enabled) VALUES ('tls://1.1.1.1:853', 10, 1);");
try testing.expectEqual(@as(u32, 2), try migrate(&database));
try testing.expectEqual(@as(u32, 2), try readVersion(&database));
try testing.expect(try columnExists(&database, "upstreams", "tls_name"));
var stmt = try database.prepare("SELECT url, tls_name FROM upstreams");
defer stmt.deinit();
try testing.expect(try stmt.step());
try testing.expectEqualStrings("tls://1.1.1.1:853", stmt.columnText(0));
try testing.expectEqualStrings("", stmt.columnText(1));
}
test "a failing step after step 2 rolls back the whole upgrade from version 1" {
var database = try openMigrated();
defer database.close();
const first = [_]Step{.{ .version = 1, .sql = config_schema.ddl_v1 }};
_ = try migrateSteps(&database, &first);
const broken = [_]Step{
steps[0],
steps[1],
.{ .version = 3, .sql = "CREATE TABLE third (" },
};
try testing.expectError(error.Unexpected, migrateSteps(&database, &broken));
// One transaction: the ALTER TABLE of step 2 went back with step 3.
try testing.expect(!try columnExists(&database, "upstreams", "tls_name"));
try testing.expectEqual(@as(u32, 1), try readVersion(&database));
}
test "delete_order and content_tables name exactly the tables the schema creates" { test "delete_order and content_tables name exactly the tables the schema creates" {
var database = try openMigrated(); var database = try openMigrated();
defer database.close(); defer database.close();
+28 -5
View File
@@ -18,7 +18,9 @@ const InsertContext = context.InsertContext;
/// Every string in the result is a heap copy owned by `gpa`. /// Every string in the result is a heap copy owned by `gpa`.
pub fn listUpstreams(database: *db.Db, gpa: Allocator) db.Error!std.ArrayList(model.UpstreamServer) { pub fn listUpstreams(database: *db.Db, gpa: Allocator) db.Error!std.ArrayList(model.UpstreamServer) {
var stmt = try database.prepare("SELECT url, priority, enabled FROM upstreams ORDER BY priority, url"); var stmt = try database.prepare(
"SELECT url, priority, enabled, tls_name FROM upstreams ORDER BY priority, url",
);
defer stmt.deinit(); defer stmt.deinit();
var out: std.ArrayList(model.UpstreamServer) = .empty; var out: std.ArrayList(model.UpstreamServer) = .empty;
@@ -31,22 +33,35 @@ pub fn listUpstreams(database: *db.Db, gpa: Allocator) db.Error!std.ArrayList(mo
const url = try stmt.columnTextAlloc(gpa, 0); const url = try stmt.columnTextAlloc(gpa, 0);
errdefer gpa.free(url); errdefer gpa.free(url);
const priority = std.math.cast(i32, stmt.columnInt(1)) orelse return error.Mismatch; const priority = std.math.cast(i32, stmt.columnInt(1)) orelse return error.Mismatch;
try out.append(gpa, .{ .url = url, .priority = priority, .enabled = stmt.columnBool(2) }); const tls_name = try stmt.columnTextAlloc(gpa, 3);
errdefer gpa.free(tls_name);
try out.append(gpa, .{
.url = url,
.priority = priority,
.enabled = stmt.columnBool(2),
.tls_name = tls_name,
});
} }
return out; return out;
} }
pub fn freeUpstreams(gpa: Allocator, items: []const model.UpstreamServer) void { pub fn freeUpstreams(gpa: Allocator, items: []const model.UpstreamServer) void {
for (items) |item| gpa.free(item.url); for (items) |item| {
gpa.free(item.url);
gpa.free(item.tls_name);
}
} }
pub fn insertUpstream(database: *db.Db, item: model.UpstreamServer, ctx: InsertContext) db.Error!void { pub fn insertUpstream(database: *db.Db, item: model.UpstreamServer, ctx: InsertContext) db.Error!void {
_ = ctx; _ = ctx;
var stmt = try database.prepare("INSERT INTO upstreams (url, priority, enabled) VALUES (?1, ?2, ?3)"); var stmt = try database.prepare(
"INSERT INTO upstreams (url, priority, enabled, tls_name) VALUES (?1, ?2, ?3, ?4)",
);
defer stmt.deinit(); defer stmt.deinit();
try stmt.bindText(1, item.url); try stmt.bindText(1, item.url);
try stmt.bindInt(2, item.priority); try stmt.bindInt(2, item.priority);
try stmt.bindBool(3, item.enabled); try stmt.bindBool(3, item.enabled);
try stmt.bindText(4, item.tls_name);
try stmt.exec(); try stmt.exec();
} }
@@ -75,7 +90,12 @@ fn openMigrated() !db.Db {
fn seedUpstreams(database: *db.Db) !void { fn seedUpstreams(database: *db.Db) !void {
const ctx: InsertContext = .{}; const ctx: InsertContext = .{};
try insertUpstream(database, .{ .url = "https://dns.example/dns-query", .priority = 50 }, ctx); try insertUpstream(database, .{ .url = "https://dns.example/dns-query", .priority = 50 }, ctx);
try insertUpstream(database, .{ .url = "tls://1.1.1.1:853", .priority = 10, .enabled = false }, ctx); try insertUpstream(database, .{
.url = "tls://1.1.1.1:853",
.priority = 10,
.enabled = false,
.tls_name = "one.one.one.one",
}, ctx);
try insertUpstream(database, .{ .url = "https://a.example/dns-query", .priority = 50 }, ctx); try insertUpstream(database, .{ .url = "https://a.example/dns-query", .priority = 50 }, ctx);
} }
@@ -92,7 +112,10 @@ test "upstreams round-trip in priority then url order" {
try testing.expectEqualStrings("tls://1.1.1.1:853", items.items[0].url); try testing.expectEqualStrings("tls://1.1.1.1:853", items.items[0].url);
try testing.expectEqual(@as(i32, 10), items.items[0].priority); try testing.expectEqual(@as(i32, 10), items.items[0].priority);
try testing.expect(!items.items[0].enabled); try testing.expect(!items.items[0].enabled);
try testing.expectEqualStrings("one.one.one.one", items.items[0].tls_name);
try testing.expectEqualStrings("https://a.example/dns-query", items.items[1].url); try testing.expectEqualStrings("https://a.example/dns-query", items.items[1].url);
// An upstream inserted without one reads back as the empty column default.
try testing.expectEqualStrings("", items.items[1].tls_name);
try testing.expectEqual(@as(i32, 50), items.items[1].priority); try testing.expectEqual(@as(i32, 50), items.items[1].priority);
try testing.expect(items.items[1].enabled); try testing.expect(items.items[1].enabled);
try testing.expectEqualStrings("https://dns.example/dns-query", items.items[2].url); try testing.expectEqualStrings("https://dns.example/dns-query", items.items[2].url);
+50 -4
View File
@@ -45,6 +45,12 @@ pub fn resolveAddress(endpoint: transport.Endpoint) ResolveError!net.IpAddress {
pub const DotClient = struct { pub const DotClient = struct {
endpoint: transport.Endpoint, endpoint: transport.Endpoint,
/// SNI, and the name matched against the leaf certificate. The dial target
/// stays `endpoint.host`, so this is what lets an upstream configured as an
/// IP literal verify: `std.crypto.Certificate.Parsed.verifyHostName` matches
/// dNSName SANs only and never an IP SAN, so `tls://1.1.1.1:853` alone is
/// always `error.CertificateHostMismatch`. Borrowed, like `endpoint`.
verify_name: []const u8,
gpa: std.mem.Allocator, gpa: std.mem.Allocator,
/// Caller-owned, shared across endpoints. /// Caller-owned, shared across endpoints.
bundle: *Certificate.Bundle, bundle: *Certificate.Bundle,
@@ -66,8 +72,12 @@ pub const DotClient = struct {
/// A `.doh` endpoint or an undersized buffer is a wiring bug in this /// A `.doh` endpoint or an undersized buffer is a wiring bug in this
/// process, not a runtime condition, so both are assertions. /// process, not a runtime condition, so both are assertions.
///
/// An empty `tls_name` keeps the endpoint's own host as the verification
/// name, which is correct whenever the url already carries a DNS name.
pub fn init( pub fn init(
endpoint: transport.Endpoint, endpoint: transport.Endpoint,
tls_name: []const u8,
gpa: std.mem.Allocator, gpa: std.mem.Allocator,
bundle: *Certificate.Bundle, bundle: *Certificate.Bundle,
bundle_lock: *std.Io.RwLock, bundle_lock: *std.Io.RwLock,
@@ -80,6 +90,7 @@ pub const DotClient = struct {
std.debug.assert(buffers.stream_write.len >= tls.Client.min_buffer_len); std.debug.assert(buffers.stream_write.len >= tls.Client.min_buffer_len);
return .{ return .{
.endpoint = endpoint, .endpoint = endpoint,
.verify_name = if (tls_name.len == 0) endpoint.host else tls_name,
.gpa = gpa, .gpa = gpa,
.bundle = bundle, .bundle = bundle,
.bundle_lock = bundle_lock, .bundle_lock = bundle_lock,
@@ -149,7 +160,7 @@ pub const DotClient = struct {
tls_stream.stream_reader.err = null; tls_stream.stream_reader.err = null;
tls_stream.stream_writer.err = null; tls_stream.stream_writer.err = null;
tls_stream.init(io, &stream, self.bundle, self.bundle_lock, self.gpa, .{ tls_stream.init(io, &stream, self.bundle, self.bundle_lock, self.gpa, .{
.host = self.endpoint.host, .host = self.verify_name,
.ca = .system, .ca = .system,
.read_buffer = self.buffers.tls_read, .read_buffer = self.buffers.tls_read,
.write_buffer = self.buffers.tls_write, .write_buffer = self.buffers.tls_write,
@@ -157,8 +168,9 @@ pub const DotClient = struct {
.stream_write_buffer = self.buffers.stream_write, .stream_write_buffer = self.buffers.stream_write,
}) catch |err| { }) catch |err| {
const cause = concreteHandshake(&tls_stream, err); const cause = concreteHandshake(&tls_stream, err);
log.warn("dot upstream {s}: TLS handshake failed: {s} ({t})", .{ log.warn("dot upstream {s}: TLS handshake as \"{s}\" failed: {s} ({t})", .{
self.endpoint.url, self.endpoint.url,
self.verify_name,
@errorName(cause), @errorName(cause),
tls_client.classify(cause), tls_client.classify(cause),
}); });
@@ -170,7 +182,10 @@ pub const DotClient = struct {
const prefix = framePrefix(@intCast(query.len)); const prefix = framePrefix(@intCast(query.len));
writer.writeAll(&prefix) catch |err| return sendFailure(&tls_stream, err); writer.writeAll(&prefix) catch |err| return sendFailure(&tls_stream, err);
writer.writeAll(query) catch |err| return sendFailure(&tls_stream, err); writer.writeAll(query) catch |err| return sendFailure(&tls_stream, err);
writer.flush() catch |err| return sendFailure(&tls_stream, err); // `TlsStream.flush`, not `writer.flush`: the latter leaves the encrypted
// record in the socket writer's buffer and the query never leaves this
// process.
tls_stream.flush() catch |err| return sendFailure(&tls_stream, err);
const reader = tls_stream.reader(); const reader = tls_stream.reader();
var prefix_bytes: [prefix_len]u8 = undefined; var prefix_bytes: [prefix_len]u8 = undefined;
@@ -463,7 +478,7 @@ test "DotClient satisfies the Client interface" {
// `init` asserts `endpoint.scheme == .dot`; a `.doh` endpoint trips // `init` asserts `endpoint.scheme == .dot`; a `.doh` endpoint trips
// `std.debug.assert`, which a test cannot catch in-process. // `std.debug.assert`, which a test cannot catch in-process.
var dot: DotClient = .init(try .parse("tls://9.9.9.9:853"), gpa, &bundle, &bundle_lock, .{ var dot: DotClient = .init(try .parse("tls://9.9.9.9:853"), "", gpa, &bundle, &bundle_lock, .{
.tls_read = buffer[0..chunk], .tls_read = buffer[0..chunk],
.tls_write = buffer[chunk .. 2 * chunk], .tls_write = buffer[chunk .. 2 * chunk],
.stream_read = buffer[2 * chunk .. 3 * chunk], .stream_read = buffer[2 * chunk .. 3 * chunk],
@@ -476,3 +491,34 @@ test "DotClient satisfies the Client interface" {
const iface: transport.Client = dot.client(); const iface: transport.Client = dot.client();
try testing.expectEqual(@as(*anyopaque, @ptrCast(&dot)), iface.ptr); try testing.expectEqual(@as(*anyopaque, @ptrCast(&dot)), iface.ptr);
} }
test "a tls_name replaces the verification name and leaves the dial target alone" {
const gpa = testing.allocator;
const buffer = try gpa.alloc(u8, 4 * tls.Client.min_buffer_len);
defer gpa.free(buffer);
const chunk = tls.Client.min_buffer_len;
var bundle: Certificate.Bundle = .empty;
defer bundle.deinit(gpa);
var bundle_lock: std.Io.RwLock = .init;
const buffers: DotClient.Buffers = .{
.tls_read = buffer[0..chunk],
.tls_write = buffer[chunk .. 2 * chunk],
.stream_read = buffer[2 * chunk .. 3 * chunk],
.stream_write = buffer[3 * chunk ..],
};
const endpoint: transport.Endpoint = try .parse("tls://1.1.1.1:853");
const named: DotClient = .init(endpoint, "one.one.one.one", gpa, &bundle, &bundle_lock, buffers);
try testing.expectEqualStrings("one.one.one.one", named.verify_name);
try testing.expectEqualStrings("1.1.1.1", named.endpoint.host);
const address = try resolveAddress(named.endpoint);
try testing.expectEqualSlices(u8, &.{ 1, 1, 1, 1 }, &address.ip4.bytes);
try testing.expectEqual(@as(u16, 853), address.ip4.port);
const plain: DotClient = .init(endpoint, "", gpa, &bundle, &bundle_lock, buffers);
try testing.expectEqualStrings("1.1.1.1", plain.verify_name);
}
+7 -2
View File
@@ -26,10 +26,14 @@ const query_bytes =
"\x07example\x03com\x00\x00\x01\x00\x01"; "\x07example\x03com\x00\x00\x01\x00\x01";
/// This machine's IPv6 egress is dead and upstream name resolution is out of /// This machine's IPv6 egress is dead and upstream name resolution is out of
/// scope, so the documented anycast IPv4 literal is used. Cloudflare's /// scope, so the documented anycast IPv4 literal is used.
/// certificate carries 1.1.1.1 as an IP SAN, so full verification still applies.
const upstream_url = "tls://1.1.1.1:853"; const upstream_url = "tls://1.1.1.1:853";
/// The name Cloudflare publishes for this endpoint. Without it the handshake is
/// `error.CertificateHostMismatch`: `std.crypto.Certificate` matches dNSName
/// SANs only, so the IP SAN on the leaf certificate is never consulted.
const upstream_tls_name = "one.one.one.one";
const Outcome = union(enum) { const Outcome = union(enum) {
exchange: anyerror!usize, exchange: anyerror!usize,
expiry: std.Io.Cancelable!void, expiry: std.Io.Cancelable!void,
@@ -47,6 +51,7 @@ fn runExchange(io: std.Io, params: Params) anyerror!usize {
const endpoint: transport.Endpoint = try .parse(upstream_url); const endpoint: transport.Endpoint = try .parse(upstream_url);
var client: dot_client.DotClient = .init( var client: dot_client.DotClient = .init(
endpoint, endpoint,
upstream_tls_name,
params.gpa, params.gpa,
params.bundle, params.bundle,
params.bundle_lock, params.bundle_lock,