Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement DNS over QUIC #18

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 132 additions & 0 deletions doquic.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
//
// SPDX-License-Identifier: GPL-3.0-or-later
//
// DNS-over-QUIC implementation
//

// DNS over Dedicated QUIC Connections
// RFC 9250
// https://datatracker.ietf.org/doc/rfc9250/

package dnscore

import (
"context"
"crypto/tls"
"io"
"net"
"time"

"github.com/miekg/dns"
"github.com/quic-go/quic-go"
)

func (t *Transport) sendQueryQUIC(ctx context.Context, addr *ServerAddr,
query *dns.Msg) (stream quic.Stream, t0 time.Time, rawQuery []byte, err error) {

udpAddr, err := net.ResolveUDPAddr("udp", addr.Address)
if err != nil {
return
}

udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4zero, Port: 0})
if err != nil {
return
}
Comment on lines +27 to +35
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible here to use net.ListenConfig.ListenPacket instead? My thinking is that, like we do for TCP, I would like (not in this PR) to abstract away the establishment of a connection, which is useful for (a) testing and (b) measurements (by wrapping the returned conn). In light of this, it seems to me net.ListenConfig.ListenPacket is a better API to aim for, given that it combines DNS lookup and connection creation in a single call.


tr := &quic.Transport{
Conn: udpConn,
}

// 1. Fill in a default TLS config and QUIC config
hostname, _, err := net.SplitHostPort(addr.Address)
if err != nil {
return
}
tlsConfig := &tls.Config{
NextProtos: []string{"doq"},
ServerName: hostname,
}
quicConfig := &quic.Config{}

// 2. Use the context deadline to limit the query lifetime
// as documented in the [*Transport.Query] function.
if deadline, ok := ctx.Deadline(); ok {
_ = udpConn.SetDeadline(deadline)
}

// RFC 9250
// 4.2.1. DNS Message IDs
// When sending queries over a QUIC connection, the DNS Message ID MUST
// be set to 0.
query.Id = 0
Copy link
Member

@bassosimone bassosimone Dec 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should extend this comment to mention that we're modifying the original query, which means that, after additional refactoring, this code could cause a data race if it is changed so that we rewrite the query.Id in a background goroutine. I see this as documenting and making it explicit what could go wrong with refactoring.

Likewise, I would like a corresponding comment to be present in the *Transport.Query method. The comment should read something like "This function MAY modify the provided query in place. This happens, e.g., with DoH and DoQ, where we set the query.Id to zero. However, we guarantee that this modification will always happen in the same goroutine that called this function, thus avoiding data races."

(The actual comment does not need to be exactly like this, but I wrote this ~quick explainer to clarify what I mean and why I think it's important to document expectations with respect to mutability.)

rawQuery, err = query.Pack()
if err != nil {
return
}

t0 = t.maybeLogQuery(ctx, addr, rawQuery)

quicConn, err := tr.Dial(ctx, udpAddr, tlsConfig, quicConfig)
if err != nil {
return
}

stream, err = quicConn.OpenStream()
if err != nil {
return
}
stream.Write(rawQuery)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what we are doing here is not fully compliant with the RFC and, in particular, with this requirement laid out in Sect. 4.2.:

In order for multiple responses to be parsed, a 2-octet length field is used in exactly the same way as the 2-octet length field defined for DNS over TCP [RFC1035]. The practical result of this is that the content of each QUIC stream is exactly the same as the content of a TCP connection that would manage exactly one query.

All DNS messages (queries and responses) sent over DoQ connections MUST be encoded as a 2-octet length field followed by the message content as specified in [RFC1035].

We already have code to implement this kind of exchange with the server, in dotcp.go and we're using it to implement DNS over TCP and DNS over TLS. In fact, DNS over TLS is just a tiny function that setups the TLS connection and then invokes the *Transport.queryStream function.

In light of this, I was wondering whether it could be sensible to attempt to use the same code for DNS over QUIC as well. I think doing this would reduce the amount of duplicate code, but there will be some required changes (I am mostly guessing here, since I did not try this out systematically, just skimmed through the code):

  1. We may want to change the queryStream function signature to take a specific interface (maybe named dnsStream?) rather than a net.Conn, and this interface should only specify the functions we need inside queryStream (and using a different interface would also communicate that queryStream accepts data types beyond the classical net.Conn and indeed could also accept a QUIC stream).

  2. The queryStream function should have a way to only close the stream/conn when we're doing DoQ, which means DoQ requires us to complicate the original algorithm by a single if, which seems ~fine.

  3. It seems to me quic.Stream does not have the LocalAddr and RemoteAddr method, so we should probably play composition by creating a new ad-hoc type that uses the quic.Conn or the quic.Stream to implement the required methods of dnsStream (or however we want to call this interface) to ensure the combination of quic.Stream and quic.Conn meets the requirements of the queryStream method.


// RFC 9250
// 4.2. Stream Mapping and Usage
// The client MUST send the DNS query over the selected stream and MUST
// indicate through the STREAM FIN mechanism that no further data will
// be sent on that stream.
_ = stream.Close()

return
}

// recvResponseUDP reads and parses the response from the server and
// possibly logs the response. It returns the parsed response or an error.
func (t *Transport) recvResponseQUIC(ctx context.Context, addr *ServerAddr, stream quic.Stream,
t0 time.Time, query *dns.Msg, rawQuery []byte) (*dns.Msg, error) {
// 1. Read the corresponding raw response
buffer := make([]byte, 1024)
io.ReadFull(stream, buffer)

// 2. Parse the raw response and possibly log that we received it.
resp := &dns.Msg{}
if err := resp.Unpack(buffer); err != nil {
return nil, err
}

// t.maybeLogResponseConn(ctx, addr, t0, rawQuery, buffer, conn)

return resp, nil
}

func (t *Transport) queryQUIC(ctx context.Context, addr *ServerAddr, query *dns.Msg) (*dns.Msg, error) {
// 0. immediately fail if the context is already done, which
// is useful to write unit tests
if ctx.Err() != nil {
return nil, ctx.Err()
}

// Send the query and log the query if needed.
stream, t0, rawQuery, err := t.sendQueryQUIC(ctx, addr, query)
if err != nil {
return nil, err
}

// ctx, cancel := context.WithCancel(ctx)
// defer cancel()
// go func() {
// defer stream.Close()
// <-ctx.Done()
// }()

// Read and parse the response and log it if needed.
return t.recvResponseQUIC(ctx, addr, stream, t0, query, rawQuery)
}
7 changes: 7 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@ require (

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 // indirect
github.com/onsi/ginkgo/v2 v2.9.5 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/quic-go/quic-go v0.48.2 // indirect
go.uber.org/mock v0.4.0 // indirect
golang.org/x/crypto v0.31.0 // indirect
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
golang.org/x/mod v0.22.0 // indirect
golang.org/x/sync v0.10.0 // indirect
golang.org/x/sys v0.28.0 // indirect
Expand Down
23 changes: 23 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,21 +1,43 @@
github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI=
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 h1:yAJXTCF9TqKcTiHJAE8dj7HMvPfh66eeA2JYW7eFpSE=
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/miekg/dns v1.1.62 h1:cN8OuEF1/x5Rq6Np+h1epln8OiyPWV+lROx9LxcGgIQ=
github.com/miekg/dns v1.1.62/go.mod h1:mvDlcItzm+br7MToIKqkglaGhlFMHJ9DTNNWONWXbNQ=
github.com/onsi/ginkgo/v2 v2.9.5 h1:+6Hr4uxzP4XIUyAkg61dWBw8lb/gc4/X5luuxN/EC+Q=
github.com/onsi/ginkgo/v2 v2.9.5/go.mod h1:tvAoo1QUJwNEU2ITftXTpR7R1RbCzoZUOs3RonqW57k=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/quic-go/quic-go v0.48.2 h1:wsKXZPeGWpMpCGSWqOcqpW2wZYic/8T3aqiOID0/KWE=
github.com/quic-go/quic-go v0.48.2/go.mod h1:yBgs3rWBOADpga7F+jJsb6Ybg1LSYiQvwWlLX+/6HMs=
github.com/rbmk-project/common v0.16.0 h1:DLqmpggmLo3ep44sBrzxytO6UMdc9R2YjHyXno0aDU8=
github.com/rbmk-project/common v0.16.0/go.mod h1:4rOJcJZuqPk9qm/0ysoSlfEUP6nExcnNPy3fq/CKnHo=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
go.uber.org/mock v0.4.0 h1:VcM4ZOtdbR4f6VXfiOpwpVJDL6lCReaZ6mw31wqh7KU=
go.uber.org/mock v0.4.0/go.mod h1:a6FSlNadKUHUa9IP5Vyt1zh4fC7uAwxMutEAscFbkZc=
golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 h1:vr/HnozRka3pE4EsMEg1lgkXJkTFJCVUX+S/ZT6wYzM=
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842/go.mod h1:XtvwrStGgqGPLc4cjQfWqZHG1YFdYs6swckp8vpsjnc=
golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I=
golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
Expand All @@ -24,5 +46,6 @@ golang.org/x/tools v0.28.0 h1:WuB6qZ4RPCQo5aP3WdKZS7i595EdWqWR8vqJTlwTVK8=
golang.org/x/tools v0.28.0/go.mod h1:dcIOrVd3mfQKTgrDVQHqCPMWy6lnhfhtX3hLXYVLfRw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
6 changes: 6 additions & 0 deletions internal/cmd/transport/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ func Test_main(t *testing.T) {
main()
})

t.Run("DNS-over-QUIC", func(t *testing.T) {
*serverAddr = "dns0.eu:853"
*protocol = "doq"
main()
})

t.Run("AAAA query", func(t *testing.T) {
*serverAddr = "8.8.8.8:53"
*protocol = "udp"
Expand Down
3 changes: 3 additions & 0 deletions serveraddr.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ const (

// ProtocolDoH is DNS over HTTPS.
ProtocolDoH = Protocol("doh")

// ProtocolDoQ is DNS over QUIC.
ProtocolDoQ = Protocol("doq")
)

// ServerAddr is a DNS server address.
Expand Down
3 changes: 3 additions & 0 deletions transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ func (t *Transport) Query(ctx context.Context,
case ProtocolDoH:
return t.queryHTTPS(ctx, addr, query)

case ProtocolDoQ:
return t.queryQUIC(ctx, addr, query)

default:
return nil, fmt.Errorf("%w: %s", ErrNoSuchTransportProtocol, addr.Protocol)
}
Expand Down
Loading