DNS over QUIC

[bluejekyll]

This is an implementation of DNS over QUIC (DoQ) using the quinn library.

This is a draft right now, I still need to add a test harness for the full end-to-end configuration, but the stream implementation is done and working.

closes: #670

[codecov]

Codecov Report

Merging #1673 (a7d3612) into main (88fb317) will decrease coverage by 0.05%.
The diff coverage is 70.57%.

@@            Coverage Diff             @@
##             main    #1673      +/-   ##
==========================================
- Coverage   79.88%   79.84%   -0.05%     
==========================================
  Files         177      183       +6     
  Lines       18264    18602     +338     
==========================================
+ Hits        14590    14851     +261     
- Misses       3674     3751      +77     

[djc]

This is exciting! cc @Ralith

[djc]

If you have any feedback on Quinn, that would be interesting too!

[bluejekyll]

If you have any feedback on Quinn, that would be interesting too!

So I ended up using the higher-level crate, quinn, initially I was going to try and use quinn-proto but there was a significant amount of work to do that. The reasons being, is that I am trying to have us support more than one runtime, quinn has a few embedded calls to tokio::spawn that make that hard. So I wonder if there is a way to generify quinn a bit more so that it's not such a huge lift to go from the proto crate to the full implementation.

Otherwise, I think it's working pretty well. The QuicStream impl I made for DNS appears to work fine in my tests. I have a bug in the server impl somewhere at the moment where the trust-dns server isn't responding to requests in the integration tests. So I'm trying to figure that one out.

Anyway, overall, I think I like the higher-level interfaces, I was able to make progress relatively quickly, so I think it's really good.

[Ralith]

So I wonder if there is a way to generify quinn a bit more so that it's not such a huge lift to go from the proto crate to the full implementation.

This should be feasible. Quinn presently needs only two things from a runtime: the ability to wait on readiness of a file descriptor (or Windows equivalent), and the ability to spawn a task. Neither of these are fundamentally hard problems, they just need someone who cares enough to see them abstracted. There's some incidental helpers (e.g. channels) we currently pull from tokio as well, but I believe those are self-contained and should safely work on any runtime, or at worst replaced with independent or vendored code.

Anyway, overall, I think I like the higher-level interfaces, I was able to make progress relatively quickly, so I think it's really good.

Great to hear!

[bluejekyll]

@djc, am I using write_chunks and read_exact correctly? I'm seeing some odd behavior where the first read for message length happens, then I do a second read, and it appears that there is a bit of extra data at the front of the second read:

https://github.com/bluejekyll/trust-dns/blob/04f3efca330f09533caadb0ebaa68a5f62ead461/crates/proto/src/quic/quic_stream.rs#L146

This data is sent: b"\0\0\x01\0\0\x01\0\0\0\0\0\x01\x03www\x07example\x03com\0\0\x01\0\x01\0\0)\x04\xd0\0\0\0\0\0\0"

https://github.com/bluejekyll/trust-dns/blob/04f3efca330f09533caadb0ebaa68a5f62ead461/crates/proto/src/quic/quic_stream.rs#L173-L182

But on that second read exact this is received: b"\0\0\0\0\0\0\0\0\x01\0\0\x01\0\0\0\0\0\x01\x03www\x07example\x03com\0\0\x01\0\x01\0\0)\x04\xd0"

It appears that these first 6 bytes are being prepended to the stream: b\0\0\0\0\0\0\. Can you think of what I might be doing wrong?

[Ralith]

This is writing a usize worth of bytes:
https://github.com/bluejekyll/trust-dns/blob/04f3efca330f09533caadb0ebaa68a5f62ead461/crates/proto/src/quic/quic_stream.rs#L143-L146

This is reading exactly two bytes, leaving exactly 6 left over if the sender is 64-bit:
https://github.com/bluejekyll/trust-dns/blob/04f3efca330f09533caadb0ebaa68a5f62ead461/crates/proto/src/quic/quic_stream.rs#L172-L173

You probably meant to convert to u16 somewhere on the send side.

[bluejekyll]

Thank you, @Ralith, I was not seeing that, that resolved my issue.

There's an additional check needed there too.

[bluejekyll]

Quinn presently needs only two things from a runtime: the ability to wait on readiness of a file descriptor (or Windows equivalent), and the ability to spawn a task. Neither of these are fundamentally hard problems, they just need someone who cares enough to see them abstracted.

This is pretty much what we've done:

https://github.com/bluejekyll/trust-dns/blob/04f3efca330f09533caadb0ebaa68a5f62ead461/crates/resolver/src/name_server/connection_provider.rs#L74-L94

I wonder if we should build an abstraction over these executors that others can use? I don't know exactly what that would look like other than a library with a bunch of traits in it.

[Ralith]

I wonder if we should build an abstraction over these executors that others can use?

This is difficult (though not necessarily impossible) due to the diversity of requirements. For example, the interface you quoted above is not sufficient for Quinn, because we rely on obscure platform-specific I/O primitives that don't fit a general purpose UdpSocket interface, necessary for both performance and correctness; hence the specific requirement of file descriptor readiness notifications, which are more flexible.

Library (e.g. Quinn) specific traits would allow us to move forwards with runtime portability without getting bogged down in trying to find One True Abstraction, and a survey of such traits across the ecosystem would be a strong foundation for future work to establish a shared interop crate.

[djc]

Maybe there's some work to be shared on abstracting over async runtimes: launchbadge/sqlx#1669 (comment).

[bluejekyll]

Ok, I think this is ready. There are probably some loose ends to tie up in regards to the RFC, but I think they're minor. I'm assuming there are bugs here to work through. So we should assume this is experimental at this point.

[djc]

I quickly skimmed this, seems okay.