Repository: miekg/dns
Pull request #1532 (2024-01-21 11:11:55)

IsDomainName: check for escape as last character
Keep track if the escape, if still true when returning isDomainName
should return false.

TODO:
- Should still be done in packDomainName as well.
- And that should be tested
- Some tests now fail

There are multiple other places that supposedly also check for this, but
they are not called in the parsing.

Fixes: #1528

Signed-off-by: Miek Gieben <miek@miek.nl>


miekg: testing in my tree, yields:
~~~
--- FAIL: TestUpdate2NoRdataUnpack (0.00s)
    update2_test.go:72: failed to unpack RR with zero rdata: SOA: dns: overflow unpacking uint32
    update2_test.go:72: failed to unpack RR with zero rdata: TSIG: dns: overflow unpacking uint16
    update2_test.go:72: failed to unpack RR with zero rdata: TKEY: dns: overflow unpacking uint16
FAIL
~~~

janik-cloudflare: I've applied this to our DNS API repository which has a fairly extensive test suite, and everything looks good!
janik-cloudflare: > Should still be done in packDomainName as well.

Maybe also on the lexer level so that `. 1 IN TXT hello\` is also rejected? (I'd be happy to work on a PR for that, but it might take another two weeks or so.)

Patch:
diff --git a/README.md b/README.md
index e57d86af..bca1a55a 100644
--- a/README.md
+++ b/README.md
@@ -83,6 +83,7 @@ A not-so-up-to-date-list-that-may-be-actually-current:
 * https://github.com/egbakou/domainverifier
 * https://github.com/semihalev/sdns
 * https://github.com/wintbiit/NineDNS
+* https://linuxcontainers.org/incus/
 
 
 Send pull request if you want to be listed here.
diff --git a/defaults.go b/defaults.go
index 02d9199a..68e766c6 100644
--- a/defaults.go
+++ b/defaults.go
@@ -198,10 +198,12 @@ func IsDomainName(s string) (labels int, ok bool) {
 		off    int
 		begin  int
 		wasDot bool
+		escape bool
 	)
 	for i := 0; i < len(s); i++ {
 		switch s[i] {
 		case '\\':
+			escape = !escape
 			if off+1 > lenmsg {
 				return labels, false
 			}
@@ -217,6 +219,7 @@ func IsDomainName(s string) (labels int, ok bool) {
 
 			wasDot = false
 		case '.':
+			escape = false
 			if i == 0 && len(s) > 1 {
 				// leading dots are not legal except for the root zone
 				return labels, false
@@ -243,10 +246,13 @@ func IsDomainName(s string) (labels int, ok bool) {
 			labels++
 			begin = i + 1
 		default:
+			escape = false
 			wasDot = false
 		}
 	}
-
+	if escape {
+		return labels, false
+	}
 	return labels, true
 }
 
diff --git a/xfr.go b/xfr.go
index 05b3c5ad..91080109 100644
--- a/xfr.go
+++ b/xfr.go
@@ -1,6 +1,7 @@
 package dns
 
 import (
+	"crypto/tls"
 	"fmt"
 	"time"
 )
@@ -20,6 +21,7 @@ type Transfer struct {
 	TsigProvider   TsigProvider      // An implementation of the TsigProvider interface. If defined it replaces TsigSecret and is used for all TSIG operations.
 	TsigSecret     map[string]string // Secret(s) for Tsig map[<zonename>]<base64 secret>, zonename must be in canonical form (lowercase, fqdn, see RFC 4034 Section 6.2)
 	tsigTimersOnly bool
+	TLS            *tls.Config // TLS config. If Xfr over TLS will be attempted
 }
 
 func (t *Transfer) tsigProvider() TsigProvider {
@@ -57,7 +59,11 @@ func (t *Transfer) In(q *Msg, a string) (env chan *Envelope, err error) {
 	}
 
 	if t.Conn == nil {
-		t.Conn, err = DialTimeout("tcp", a, timeout)
+		if t.TLS != nil {
+			t.Conn, err = DialTimeoutWithTLS("tcp-tls", a, t.TLS, timeout)
+		} else {
+			t.Conn, err = DialTimeout("tcp", a, timeout)
+		}
 		if err != nil {
 			return nil, err
 		}


Test patch:
diff --git a/scan_test.go b/scan_test.go
index 3332c82d..207748b6 100644
--- a/scan_test.go
+++ b/scan_test.go
@@ -359,6 +359,15 @@ func TestParseKnownRRAsRFC3597(t *testing.T) {
 	})
 }
 
+func TestParseOpenEscape(t *testing.T) {
+	if _, err := NewRR("example.net IN CNAME example.net."); err != nil {
+		t.Fatalf("expected no error, but got: %s", err)
+	}
+	if _, err := NewRR("example.net IN CNAME example.org\\"); err == nil {
+		t.Fatalf("expected an error, but got none")
+	}
+}
+
 func BenchmarkNewRR(b *testing.B) {
 	const name1 = "12345678901234567890123456789012345.12345678.123."
 	const s = name1 + " 3600 IN MX 10 " + name1
diff --git a/xfr_test.go b/xfr_test.go
index f6c5e98c..04801a2e 100644
--- a/xfr_test.go
+++ b/xfr_test.go
@@ -1,6 +1,7 @@
 package dns
 
 import (
+	"crypto/tls"
 	"testing"
 	"time"
 )
@@ -87,6 +88,27 @@ func TestSingleEnvelopeXfr(t *testing.T) {
 	axfrTestingSuite(t, addrstr)
 }
 
+func TestSingleEnvelopeXfrTLS(t *testing.T) {
+	HandleFunc("miek.nl.", SingleEnvelopeXfrServer)
+	defer HandleRemove("miek.nl.")
+
+	cert, err := tls.X509KeyPair(CertPEMBlock, KeyPEMBlock)
+	if err != nil {
+		t.Fatalf("unable to build certificate: %v", err)
+	}
+
+	tlsConfig := tls.Config{
+		Certificates: []tls.Certificate{cert},
+	}
+	s, addrstr, _, err := RunLocalTLSServer(":0", &tlsConfig)
+	if err != nil {
+		t.Fatalf("unable to run test server: %s", err)
+	}
+	defer s.Shutdown()
+
+	axfrTestingSuiteTLS(t, addrstr)
+}
+
 func TestMultiEnvelopeXfr(t *testing.T) {
 	HandleFunc("miek.nl.", MultipleEnvelopeXfrServer)
 	defer HandleRemove("miek.nl.")
@@ -131,6 +153,38 @@ func axfrTestingSuite(t *testing.T, addrstr string) {
 	}
 }
 
+func axfrTestingSuiteTLS(t *testing.T, addrstr string) {
+	tr := new(Transfer)
+	m := new(Msg)
+	m.SetAxfr("miek.nl.")
+
+	tr.TLS = &tls.Config{
+		InsecureSkipVerify: true,
+	}
+	c, err := tr.In(m, addrstr)
+	if err != nil {
+		t.Fatal("failed to zone transfer in", err)
+	}
+
+	var records []RR
+	for msg := range c {
+		if msg.Error != nil {
+			t.Fatal(msg.Error)
+		}
+		records = append(records, msg.RR...)
+	}
+
+	if len(records) != len(xfrTestData) {
+		t.Fatalf("bad axfr: expected %v, got %v", records, xfrTestData)
+	}
+
+	for i, rr := range records {
+		if !IsDuplicate(rr, xfrTestData[i]) {
+			t.Fatalf("bad axfr: expected %v, got %v", records, xfrTestData)
+		}
+	}
+}
+
 func axfrTestingSuiteWithCustomTsig(t *testing.T, addrstr string, provider TsigProvider) {
 	tr := new(Transfer)
 	m := new(Msg)