Repository: miekg/dns
Pull request #1567 (2024-05-18 12:33:26)

Add RFC 9540 to SVCBs to indicate Oblivious HTTP is available
Hi folks!

The IETF just recently published a new [RFC 9540](https://datatracker.ietf.org/doc/rfc9540/). It introduces a new SVCB parameter `ohttp` to a service binding DNS RR to indicate [Oblivious HTTP](https://datatracker.ietf.org/doc/rfc9458/) is available on the target host. Note that the value is always empty. It can be either used in the HTTPS SVCB or in the SVCBs for DDR (Disovery of Designated Resolvers) (see [RFC 9462](https://www.rfc-editor.org/rfc/rfc9462.html)). 

I've added the new SVCB type and added also the tests from the different RFCs. 


Patch:
diff --git a/go.mod b/go.mod
index bd188208..2f8e569b 100644
--- a/go.mod
+++ b/go.mod
@@ -3,10 +3,10 @@ module github.com/miekg/dns
 go 1.19
 
 require (
-	golang.org/x/net v0.22.0
-	golang.org/x/sync v0.6.0
-	golang.org/x/sys v0.18.0
-	golang.org/x/tools v0.19.0
+	golang.org/x/net v0.26.0
+	golang.org/x/sync v0.7.0
+	golang.org/x/sys v0.21.0
+	golang.org/x/tools v0.22.0
 )
 
-require golang.org/x/mod v0.16.0 // indirect
+require golang.org/x/mod v0.18.0 // indirect
diff --git a/go.sum b/go.sum
index d0f87415..1e3f4ba0 100644
--- a/go.sum
+++ b/go.sum
@@ -1,10 +1,10 @@
-golang.org/x/mod v0.16.0 h1:QX4fJ0Rr5cPQCF7O9lh9Se4pmwfwskqZfq5moyldzic=
-golang.org/x/mod v0.16.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
-golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc=
-golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
-golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
-golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
-golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
-golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
-golang.org/x/tools v0.19.0 h1:tfGCXNR1OsFG+sVdLAitlpjAvD/I6dHDKnYrpEZUHkw=
-golang.org/x/tools v0.19.0/go.mod h1:qoJWxmGSIBmAeriMx19ogtrEPrGtDbPK634QFIcLAhc=
+golang.org/x/mod v0.18.0 h1:5+9lSbEzPSdWkH32vYPBwEpX8KwDbM52Ud9xBUvNlb0=
+golang.org/x/mod v0.18.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
+golang.org/x/net v0.26.0 h1:soB7SVo0PWrY4vPW/+ay0jKDNScG2X9wFeYlXIvJsOQ=
+golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE=
+golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
+golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws=
+golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/tools v0.22.0 h1:gqSGLZqv+AI9lIQzniJ0nZDRG5GBPsSi+DRNHWNz6yA=
+golang.org/x/tools v0.22.0/go.mod h1:aCwcsjqvq7Yqt6TNyX7QMU2enbQ/Gt0bo6krSeEri+c=
diff --git a/scan_rr.go b/scan_rr.go
index 7d1ade7d..c1a76995 100644
--- a/scan_rr.go
+++ b/scan_rr.go
@@ -55,7 +55,10 @@ func endingToTxtSlice(c *zlexer, errstr string) ([]string, *ParseError) {
 			sx := []string{}
 			p := 0
 			for {
-				i := escapedStringOffset(l.token[p:], 255)
+				i, ok := escapedStringOffset(l.token[p:], 255)
+				if !ok {
+					return nil, &ParseError{err: errstr, lex: l}
+				}
 				if i != -1 && p+i != len(l.token) {
 					sx = append(sx, l.token[p:p+i])
 				} else {
@@ -1919,29 +1922,36 @@ func (rr *APL) parse(c *zlexer, o string) *ParseError {
 
 // escapedStringOffset finds the offset within a string (which may contain escape
 // sequences) that corresponds to a certain byte offset. If the input offset is
-// out of bounds, -1 is returned.
-func escapedStringOffset(s string, byteOffset int) int {
-	if byteOffset == 0 {
-		return 0
+// out of bounds, -1 is returned (which is *not* considered an error).
+func escapedStringOffset(s string, desiredByteOffset int) (int, bool) {
+	if desiredByteOffset == 0 {
+		return 0, true
 	}
 
-	offset := 0
-	for i := 0; i < len(s); i++ {
-		offset += 1
+	currentByteOffset, i := 0, 0
+
+	for i < len(s) {
+		currentByteOffset += 1
 
 		// Skip escape sequences
 		if s[i] != '\\' {
-			// Not an escape sequence; nothing to do.
+			// Single plain byte, not an escape sequence.
+			i++
 		} else if isDDD(s[i+1:]) {
-			i += 3
+			// Skip backslash and DDD.
+			i += 4
+		} else if len(s[i+1:]) < 1 {
+			// No character following the backslash; that's an error.
+			return 0, false
 		} else {
-			i++
+			// Skip backslash and following byte.
+			i += 2
 		}
 
-		if offset >= byteOffset {
-			return i + 1
+		if currentByteOffset >= desiredByteOffset {
+			return i, true
 		}
 	}
 
-	return -1
+	return -1, true
 }
diff --git a/server.go b/server.go
index 0207d6da..81580d1e 100644
--- a/server.go
+++ b/server.go
@@ -188,6 +188,14 @@ type DecorateReader func(Reader) Reader
 // Implementations should never return a nil Writer.
 type DecorateWriter func(Writer) Writer
 
+// MsgInvalidFunc is a listener hook for observing incoming messages that were discarded
+// because they could not be parsed.
+// Every message that is read by a Reader will eventually be provided to the Handler,
+// rejected (or ignored) by the MsgAcceptFunc, or passed to this function.
+type MsgInvalidFunc func(m []byte, err error)
+
+func DefaultMsgInvalidFunc(m []byte, err error) {}
+
 // A Server defines parameters for running an DNS server.
 type Server struct {
 	// Address to listen on, ":dns" if empty.
@@ -233,6 +241,8 @@ type Server struct {
 	// AcceptMsgFunc will check the incoming message and will reject it early in the process.
 	// By default DefaultMsgAcceptFunc will be used.
 	MsgAcceptFunc MsgAcceptFunc
+	// MsgInvalidFunc is optional, will be called if a message is received but cannot be parsed.
+	MsgInvalidFunc MsgInvalidFunc
 
 	// Shutdown handling
 	lock     sync.RWMutex
@@ -277,6 +287,9 @@ func (srv *Server) init() {
 	if srv.MsgAcceptFunc == nil {
 		srv.MsgAcceptFunc = DefaultMsgAcceptFunc
 	}
+	if srv.MsgInvalidFunc == nil {
+		srv.MsgInvalidFunc = DefaultMsgInvalidFunc
+	}
 	if srv.Handler == nil {
 		srv.Handler = DefaultServeMux
 	}
@@ -531,6 +544,7 @@ func (srv *Server) serveUDP(l net.PacketConn) error {
 			if cap(m) == srv.UDPSize {
 				srv.udpPool.Put(m[:srv.UDPSize])
 			}
+			srv.MsgInvalidFunc(m, ErrShortRead)
 			continue
 		}
 		wg.Add(1)
@@ -611,6 +625,7 @@ func (srv *Server) serveUDPPacket(wg *sync.WaitGroup, m []byte, u net.PacketConn
 func (srv *Server) serveDNS(m []byte, w *response) {
 	dh, off, err := unpackMsgHdr(m, 0)
 	if err != nil {
+		srv.MsgInvalidFunc(m, err)
 		// Let client hang, they are sending crap; any reply can be used to amplify.
 		return
 	}
@@ -620,10 +635,12 @@ func (srv *Server) serveDNS(m []byte, w *response) {
 
 	switch action := srv.MsgAcceptFunc(dh); action {
 	case MsgAccept:
-		if req.unpack(dh, m, off) == nil {
+		err := req.unpack(dh, m, off)
+		if err == nil {
 			break
 		}
 
+		srv.MsgInvalidFunc(m, err)
 		fallthrough
 	case MsgReject, MsgRejectNotImplemented:
 		opcode := req.Opcode
diff --git a/svcb.go b/svcb.go
index c1a740b6..310c7d11 100644
--- a/svcb.go
+++ b/svcb.go
@@ -14,7 +14,7 @@ import (
 // SVCBKey is the type of the keys used in the SVCB RR.
 type SVCBKey uint16
 
-// Keys defined in draft-ietf-dnsop-svcb-https-08 Section 14.3.2.
+// Keys defined in rfc9460
 const (
 	SVCB_MANDATORY SVCBKey = iota
 	SVCB_ALPN
@@ -23,7 +23,8 @@ const (
 	SVCB_IPV4HINT
 	SVCB_ECHCONFIG
 	SVCB_IPV6HINT
-	SVCB_DOHPATH // draft-ietf-add-svcb-dns-02 Section 9
+	SVCB_DOHPATH // rfc9461 Section 5
+	SVCB_OHTTP   // rfc9540 Section 8
 
 	svcb_RESERVED SVCBKey = 65535
 )
@@ -37,6 +38,7 @@ var svcbKeyToStringMap = map[SVCBKey]string{
 	SVCB_ECHCONFIG:       "ech",
 	SVCB_IPV6HINT:        "ipv6hint",
 	SVCB_DOHPATH:         "dohpath",
+	SVCB_OHTTP:           "ohttp",
 }
 
 var svcbStringToKeyMap = reverseSVCBKeyMap(svcbKeyToStringMap)
@@ -201,6 +203,8 @@ func makeSVCBKeyValue(key SVCBKey) SVCBKeyValue {
 		return new(SVCBIPv6Hint)
 	case SVCB_DOHPATH:
 		return new(SVCBDoHPath)
+	case SVCB_OHTTP:
+		return new(SVCBOhttp)
 	case svcb_RESERVED:
 		return nil
 	default:
@@ -771,8 +775,8 @@ func (s *SVCBIPv6Hint) copy() SVCBKeyValue {
 // SVCBDoHPath pair is used to indicate the URI template that the
 // clients may use to construct a DNS over HTTPS URI.
 //
-// See RFC xxxx (https://datatracker.ietf.org/doc/html/draft-ietf-add-svcb-dns-02)
-// and RFC yyyy (https://datatracker.ietf.org/doc/html/draft-ietf-add-ddr-06).
+// See RFC 9461 (https://datatracker.ietf.org/doc/html/rfc9461)
+// and RFC 9462 (https://datatracker.ietf.org/doc/html/rfc9462).
 //
 // A basic example of using the dohpath option together with the alpn
 // option to indicate support for DNS over HTTPS on a certain path:
@@ -816,6 +820,44 @@ func (s *SVCBDoHPath) copy() SVCBKeyValue {
 	}
 }
 
+// The "ohttp" SvcParamKey is used to indicate that a service described in a SVCB RR
+// can be accessed as a target using an associated gateway.
+// Both the presentation and wire-format values for the "ohttp" parameter MUST be empty.
+//
+// See RFC 9460 (https://datatracker.ietf.org/doc/html/rfc9460/)
+// and RFC 9230 (https://datatracker.ietf.org/doc/html/rfc9230/)
+//
+// A basic example of using the dohpath option together with the alpn
+// option to indicate support for DNS over HTTPS on a certain path:
+//
+//	s := new(dns.SVCB)
+//	s.Hdr = dns.RR_Header{Name: ".", Rrtype: dns.TypeSVCB, Class: dns.ClassINET}
+//	e := new(dns.SVCBAlpn)
+//	e.Alpn = []string{"h2", "h3"}
+//	p := new(dns.SVCBOhttp)
+//	s.Value = append(s.Value, e, p)
+type SVCBOhttp struct{}
+
+func (*SVCBOhttp) Key() SVCBKey          { return SVCB_OHTTP }
+func (*SVCBOhttp) copy() SVCBKeyValue    { return &SVCBOhttp{} }
+func (*SVCBOhttp) pack() ([]byte, error) { return []byte{}, nil }
+func (*SVCBOhttp) String() string        { return "" }
+func (*SVCBOhttp) len() int              { return 0 }
+
+func (*SVCBOhttp) unpack(b []byte) error {
+	if len(b) != 0 {
+		return errors.New("dns: svcbotthp: svcbotthp must have no value")
+	}
+	return nil
+}
+
+func (*SVCBOhttp) parse(b string) error {
+	if b != "" {
+		return errors.New("dns: svcbotthp: svcbotthp must have no value")
+	}
+	return nil
+}
+
 // SVCBLocal pair is intended for experimental/private use. The key is recommended
 // to be in the range [SVCB_PRIVATE_LOWER, SVCB_PRIVATE_UPPER].
 // Basic use pattern for creating a keyNNNNN option:
diff --git a/xfr.go b/xfr.go
index 2187c456..5cfbb516 100644
--- a/xfr.go
+++ b/xfr.go
@@ -209,6 +209,7 @@ func (t *Transfer) inIxfr(q *Msg, c chan *Envelope) {
 //	ch := make(chan *dns.Envelope)
 //	tr := new(dns.Transfer)
 //	var wg sync.WaitGroup
+//	wg.Add(1)
 //	go func() {
 //		tr.Out(w, r, ch)
 //		wg.Done()


Test patch:
diff --git a/acceptfunc_test.go b/acceptfunc_test.go
index d40d4e4c..868154d4 100644
--- a/acceptfunc_test.go
+++ b/acceptfunc_test.go
@@ -1,6 +1,8 @@
 package dns
 
 import (
+	"encoding/binary"
+	"net"
 	"testing"
 )
 
@@ -33,3 +35,86 @@ func handleNotify(w ResponseWriter, req *Msg) {
 	m.SetReply(req)
 	w.WriteMsg(m)
 }
+
+func TestInvalidMsg(t *testing.T) {
+	HandleFunc("example.org.", func(ResponseWriter, *Msg) {
+		t.Fatal("the handler must not be called in any of these tests")
+	})
+	s, addrstr, _, err := RunLocalTCPServer(":0")
+	if err != nil {
+		t.Fatalf("unable to run test server: %v", err)
+	}
+	defer s.Shutdown()
+
+	s.MsgAcceptFunc = func(dh Header) MsgAcceptAction {
+		switch dh.Id {
+		case 0x0001:
+			return MsgAccept
+		case 0x0002:
+			return MsgReject
+		case 0x0003:
+			return MsgIgnore
+		case 0x0004:
+			return MsgRejectNotImplemented
+		default:
+			t.Errorf("unexpected ID %x", dh.Id)
+			return -1
+		}
+	}
+
+	invalidErrors := make(chan error)
+	s.MsgInvalidFunc = func(m []byte, err error) {
+		invalidErrors <- err
+	}
+
+	c, err := net.Dial("tcp", addrstr)
+	if err != nil {
+		t.Fatalf("cannot connect to test server: %v", err)
+	}
+
+	write := func(m []byte) {
+		var length [2]byte
+		binary.BigEndian.PutUint16(length[:], uint16(len(m)))
+		_, err := c.Write(length[:])
+		if err != nil {
+			t.Fatalf("length write failed: %v", err)
+		}
+		_, err = c.Write(m)
+		if err != nil {
+			t.Fatalf("content write failed: %v", err)
+		}
+	}
+
+	/* Message is too short, so there is no header to accept or reject. */
+
+	tooShortMessage := make([]byte, 11)
+	tooShortMessage[1] = 0x3 // ID = 3, would be ignored if it were parsable.
+
+	write(tooShortMessage)
+	// Expect an error to be reported.
+	<-invalidErrors
+
+	/* Message is accepted but is actually invalid. */
+
+	badMessage := make([]byte, 13)
+	badMessage[1] = 0x1 // ID = 1, Accept.
+	badMessage[5] = 1   // QDCOUNT = 1
+	badMessage[12] = 99 // Bad question section.  Invalid!
+
+	write(badMessage)
+	// Expect an error to be reported.
+	<-invalidErrors
+
+	/* Message is rejected before it can be determined to be invalid. */
+
+	close(invalidErrors) // A call to InvalidMsgFunc would panic due to the closed chan.
+
+	badMessage[1] = 0x2 // ID = 2, Reject
+	write(badMessage)
+
+	badMessage[1] = 0x3 // ID = 3, Ignore
+	write(badMessage)
+
+	badMessage[1] = 0x4 // ID = 4, RejectNotImplemented
+	write(badMessage)
+}
diff --git a/parse_test.go b/parse_test.go
index da94cc38..f0d68a44 100644
--- a/parse_test.go
+++ b/parse_test.go
@@ -1609,7 +1609,18 @@ func TestParseSVCB(t *testing.T) {
 		// From draft-ietf-add-ddr-06
 		`_dns.example.net. SVCB 1 example.net. alpn=h2 dohpath=/dns-query{?dns}`:     `_dns.example.net.	3600	IN	SVCB	1 example.net. alpn="h2" dohpath="/dns-query{?dns}"`,
 		`_dns.example.net. SVCB 1 example.net. alpn=h2 dohpath=/dns\045query{\?dns}`: `_dns.example.net.	3600	IN	SVCB	1 example.net. alpn="h2" dohpath="/dns-query{?dns}"`,
+		// From RFC9461 Section 7 (https://datatracker.ietf.org/doc/html/rfc9461#section-7)
+		`_dns.simple.example. 7200 IN SVCB 1 simple.example. alpn=dot`:                                 `_dns.simple.example.	7200	IN	SVCB	1 simple.example. alpn="dot"`,
+		`_dns.doh.example. 7200 IN SVCB 1 doh.example. alpn=h2 dohpath=/dns-query{?dns}`:               `_dns.doh.example.	7200	IN	SVCB	1 doh.example. alpn="h2" dohpath="/dns-query{?dns}"`,
+		`_dns.resolver.example.  7200 IN SVCB 1 resolver.example. alpn=dot,doq,h2,h3 dohpath=/q{?dns}`: `_dns.resolver.example.	7200	IN	SVCB	1 resolver.example. alpn="dot,doq,h2,h3" dohpath="/q{?dns}"`,
+		`_dns.resolver.example.  7200 IN SVCB 2 resolver.example. alpn=dot port=8530`:                  `_dns.resolver.example.	7200	IN	SVCB	2 resolver.example. alpn="dot" port="8530"`,
+		// From RFC 9540 Section 4.2.1 (https://www.rfc-editor.org/rfc/rfc9540.html#name-the-ohttp-svcparamkey)
+		`_dns.resolver.arpa  7200  IN SVCB 1 doh.example.net alpn=h2 dohpath=/dns-query{?dns} ohttp`: `_dns.resolver.arpa.	7200	IN	SVCB	1 doh.example.net. alpn="h2" dohpath="/dns-query{?dns}" ohttp=""`,
+		// From RFC 9540 Section 4.1 (HTTPS RR) (https://www.rfc-editor.org/rfc/rfc9540.html#name-use-in-https-service-rrs)
+		`svc.example.com. 7200  IN HTTPS 1 . alpn=h2 ohttp`:         `svc.example.com.	7200	IN	HTTPS	1 . alpn="h2" ohttp=""`,
+		`svc.example.com. 7200  IN HTTPS 1 . mandatory=ohttp ohttp`: `svc.example.com.	7200	IN	HTTPS	1 . mandatory="ohttp" ohttp=""`,
 	}
+
 	for s, o := range svcbs {
 		rr, err := NewRR(s)
 		if err != nil {
diff --git a/scan_test.go b/scan_test.go
index 580236ea..c4f7e7f4 100644
--- a/scan_test.go
+++ b/scan_test.go
@@ -433,25 +433,37 @@ func TestEscapedStringOffset(t *testing.T) {
 		input          string
 		inputOffset    int
 		expectedOffset int
+		expectedOK     bool
 	}{
-		{"simple string with no escape sequences", 20, 20},
-		{"simple string with no escape sequences", 500, -1},
-		{`\;\088\\\;\120\\`, 0, 0},
-		{`\;\088\\\;\120\\`, 1, 2},
-		{`\;\088\\\;\120\\`, 2, 6},
-		{`\;\088\\\;\120\\`, 3, 8},
-		{`\;\088\\\;\120\\`, 4, 10},
-		{`\;\088\\\;\120\\`, 5, 14},
-		{`\;\088\\\;\120\\`, 6, 16},
-		{`\;\088\\\;\120\\`, 7, -1},
+		{"simple string with no escape sequences", 20, 20, true},
+		{"simple string with no escape sequences", 500, -1, true},
+		{`\;\088\\\;\120\\`, 0, 0, true},
+		{`\;\088\\\;\120\\`, 1, 2, true},
+		{`\;\088\\\;\120\\`, 2, 6, true},
+		{`\;\088\\\;\120\\`, 3, 8, true},
+		{`\;\088\\\;\120\\`, 4, 10, true},
+		{`\;\088\\\;\120\\`, 5, 14, true},
+		{`\;\088\\\;\120\\`, 6, 16, true},
+		{`\;\088\\\;\120\\`, 7, -1, true},
+		{`\`, 3, 0, false},
+		{`a\`, 3, 0, false},
+		{`aa\`, 3, 0, false},
+		{`aaa\`, 3, 3, true},
+		{`aaaa\`, 3, 3, true},
 	}
 	for i, test := range cases {
-		outputOffset := escapedStringOffset(test.input, test.inputOffset)
+		outputOffset, outputOK := escapedStringOffset(test.input, test.inputOffset)
 		if outputOffset != test.expectedOffset {
 			t.Errorf(
 				"Test %d (input %#q offset %d) returned offset %d but expected %d",
 				i, test.input, test.inputOffset, outputOffset, test.expectedOffset,
 			)
 		}
+		if outputOK != test.expectedOK {
+			t.Errorf(
+				"Test %d (input %#q offset %d) returned ok=%t but expected %t",
+				i, test.input, test.inputOffset, outputOK, test.expectedOK,
+			)
+		}
 	}
 }
diff --git a/svcb_test.go b/svcb_test.go
index 63a40102..a96a344b 100644
--- a/svcb_test.go
+++ b/svcb_test.go
@@ -24,6 +24,7 @@ func TestSVCB(t *testing.T) {
 		{`key65002`, ``},
 		{`key65003`, `=\"\"`},
 		{`key65004`, `\254\ \ \030\000`},
+		{`ohttp`, ``},
 	}
 
 	for _, o := range svcbs {
@@ -86,6 +87,10 @@ func TestDecodeBadSVCB(t *testing.T) {
 			key:  SVCB_IPV6HINT,
 			data: []byte{0, 0, 0},
 		},
+		{
+			key:  SVCB_OHTTP,
+			data: []byte{0},
+		},
 	}
 	for _, o := range svcbs {
 		err := makeSVCBKeyValue(SVCBKey(o.key)).unpack(o.data)