-
Notifications
You must be signed in to change notification settings - Fork 1
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
} | ||
|
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Likewise, I would like a corresponding comment to be present in the (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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.:
We already have code to implement this kind of exchange with the server, in 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):
|
||
|
||
// 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) | ||
} |
There was a problem hiding this comment.
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.