Repository: miekg/dns (go)

Issue:
DNSSEC RRSIG Verify Function Fails to Enforce RFC4035 Constraints
### Description

The `Verify` function does not fully comply with **RFC 4035 Section 5.3.1**, leading to improper validation of RRSIG records. Specifically, the following constraints are not enforced:

1. **Owner Name Mismatch**:
   - The RRSIG owner name (`rr.Hdr.Name`) must match the RRset owner name. Currently, this validation is missing.
   
2. **Signer Name Validation**:
   - The RRSIG `SignerName` must correspond to the zone containing the RRset. The current implementation only checks against the `DNSKEY` name, which is insufficient.

3. **Labels Field Validation**:
   - The number of labels in the RRset owner name must be greater than or equal to the `Labels` field in the RRSIG. This validation is absent.

---

### Steps to Reproduce

Modify `TestSignVerify` in `dnssec_test.go` as one of follows, then run the test:
   1. Change `soa.Hdr.Name` to `example.com.`.
   2. Change `sig.Hdr.Name` to `example.com.`.
   3. Change `soa.Hdr.Name` to `nl.`

### Expected Behavior

The test should fail due to:
1. Owner name mismatch.
2. Invalid `SignerName`.
3. Labels field mismatch.

### Actual Behavior

The test passes incorrectly, indicating the constraints are not being enforced.

---

### References

- **RFC 4035 Section 5.3.1**: [Link](https://www.rfc-editor.org/rfc/rfc4035#section-5.3.1)

Pull request description:
fix: input validation for RRSIG.Verify()
Maybe not a perfect fix, but this attempts to resolve #1617.

Also introduces tests to ensure compliance with RFC 4035.

Patch:
diff --git a/README.md b/README.md
index 8d5a2a47..9831c37b 100644
--- a/README.md
+++ b/README.md
@@ -85,6 +85,7 @@ A not-so-up-to-date-list-that-may-be-actually-current:
 * https://github.com/wintbiit/NineDNS
 * https://linuxcontainers.org/incus/
 * https://ifconfig.es
+* https://github.com/zmap/zdns
 
 
 Send pull request if you want to be listed here.
diff --git a/dnssec.go b/dnssec.go
index 1be87eae..ffdafceb 100644
--- a/dnssec.go
+++ b/dnssec.go
@@ -250,14 +250,6 @@ func (d *DS) ToCDS() *CDS {
 // zero, it is used as-is, otherwise the TTL of the RRset is used as the
 // OrigTTL.
 func (rr *RRSIG) Sign(k crypto.Signer, rrset []RR) error {
-	if k == nil {
-		return ErrPrivKey
-	}
-	// s.Inception and s.Expiration may be 0 (rollover etc.), the rest must be set
-	if rr.KeyTag == 0 || len(rr.SignerName) == 0 || rr.Algorithm == 0 {
-		return ErrKey
-	}
-
 	h0 := rrset[0].Header()
 	rr.Hdr.Rrtype = TypeRRSIG
 	rr.Hdr.Name = h0.Name
@@ -272,6 +264,18 @@ func (rr *RRSIG) Sign(k crypto.Signer, rrset []RR) error {
 		rr.Labels-- // wildcard, remove from label count
 	}
 
+	return rr.signAsIs(k, rrset)
+}
+
+func (rr *RRSIG) signAsIs(k crypto.Signer, rrset []RR) error {
+	if k == nil {
+		return ErrPrivKey
+	}
+	// s.Inception and s.Expiration may be 0 (rollover etc.), the rest must be set
+	if rr.KeyTag == 0 || len(rr.SignerName) == 0 || rr.Algorithm == 0 {
+		return ErrKey
+	}
+
 	sigwire := new(rrsigWireFmt)
 	sigwire.TypeCovered = rr.TypeCovered
 	sigwire.Algorithm = rr.Algorithm
@@ -370,9 +374,12 @@ func (rr *RRSIG) Verify(k *DNSKEY, rrset []RR) error {
 	if rr.Algorithm != k.Algorithm {
 		return ErrKey
 	}
-	if !strings.EqualFold(rr.SignerName, k.Hdr.Name) {
+
+	signerName := CanonicalName(rr.SignerName)
+	if !equal(signerName, k.Hdr.Name) {
 		return ErrKey
 	}
+
 	if k.Protocol != 3 {
 		return ErrKey
 	}
@@ -384,9 +391,18 @@ func (rr *RRSIG) Verify(k *DNSKEY, rrset []RR) error {
 	}
 
 	// IsRRset checked that we have at least one RR and that the RRs in
-	// the set have consistent type, class, and name. Also check that type and
-	// class matches the RRSIG record.
-	if h0 := rrset[0].Header(); h0.Class != rr.Hdr.Class || h0.Rrtype != rr.TypeCovered {
+	// the set have consistent type, class, and name. Also check that type,
+	// class and name matches the RRSIG record.
+	// Also checks RFC 4035 5.3.1 the number of labels in the RRset owner
+	// name MUST be greater than or equal to the value in the RRSIG RR's Labels field.
+	// RFC 4035 5.3.1 Signer's Name MUST be the name of the zone that [contains the RRset].
+	// Since we don't have SOA info, checking suffix may be the best we can do...?
+	if h0 := rrset[0].Header(); h0.Class != rr.Hdr.Class ||
+		h0.Rrtype != rr.TypeCovered ||
+		uint8(CountLabel(h0.Name)) < rr.Labels ||
+		!equal(h0.Name, rr.Hdr.Name) ||
+		!strings.HasSuffix(CanonicalName(h0.Name), signerName) {
+
 		return ErrRRset
 	}
 
@@ -400,7 +416,7 @@ func (rr *RRSIG) Verify(k *DNSKEY, rrset []RR) error {
 	sigwire.Expiration = rr.Expiration
 	sigwire.Inception = rr.Inception
 	sigwire.KeyTag = rr.KeyTag
-	sigwire.SignerName = CanonicalName(rr.SignerName)
+	sigwire.SignerName = signerName
 	// Create the desired binary blob
 	signeddata := make([]byte, DefaultMsgSize)
 	n, err := packSigWire(sigwire, signeddata)
diff --git a/listen_no_reuseport.go b/listen_no_socket_options.go
similarity index 61%
rename from listen_no_reuseport.go
rename to listen_no_socket_options.go
index 8cebb2f1..9e4010bd 100644
--- a/listen_no_reuseport.go
+++ b/listen_no_socket_options.go
@@ -3,9 +3,15 @@
 
 package dns
 
-import "net"
+import (
+	"fmt"
+	"net"
+)
 
-const supportsReusePort = false
+const (
+	supportsReusePort = false
+	supportsReuseAddr = false
+)
 
 func listenTCP(network, addr string, reuseport, reuseaddr bool) (net.Listener, error) {
 	if reuseport || reuseaddr {
@@ -15,8 +21,6 @@ func listenTCP(network, addr string, reuseport, reuseaddr bool) (net.Listener, e
 	return net.Listen(network, addr)
 }
 
-const supportsReuseAddr = false
-
 func listenUDP(network, addr string, reuseport, reuseaddr bool) (net.PacketConn, error) {
 	if reuseport || reuseaddr {
 		// TODO(tmthrgd): return an error?
@@ -24,3 +28,13 @@ func listenUDP(network, addr string, reuseport, reuseaddr bool) (net.PacketConn,
 
 	return net.ListenPacket(network, addr)
 }
+
+// this is just for test compatibility
+func checkReuseport(fd uintptr) (bool, error) {
+	return false, fmt.Errorf("not supported")
+}
+
+// this is just for test compatibility
+func checkReuseaddr(fd uintptr) (bool, error) {
+	return false, fmt.Errorf("not supported")
+}
diff --git a/listen_reuseport.go b/listen_socket_options.go
similarity index 66%
rename from listen_reuseport.go
rename to listen_socket_options.go
index 41326f20..35dfc949 100644
--- a/listen_reuseport.go
+++ b/listen_socket_options.go
@@ -39,10 +39,40 @@ func reuseaddrControl(network, address string, c syscall.RawConn) error {
 	return opErr
 }
 
+func reuseaddrandportControl(network, address string, c syscall.RawConn) error {
+	err := reuseaddrControl(network, address, c)
+	if err != nil {
+		return err
+	}
+
+	return reuseportControl(network, address, c)
+}
+
+// this is just for test compatibility
+func checkReuseport(fd uintptr) (bool, error) {
+	v, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEPORT)
+	if err != nil {
+		return false, err
+	}
+
+	return v == 1, nil
+}
+
+// this is just for test compatibility
+func checkReuseaddr(fd uintptr) (bool, error) {
+	v, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEADDR)
+	if err != nil {
+		return false, err
+	}
+
+	return v == 1, nil
+}
+
 func listenTCP(network, addr string, reuseport, reuseaddr bool) (net.Listener, error) {
 	var lc net.ListenConfig
 	switch {
 	case reuseaddr && reuseport:
+		lc.Control = reuseaddrandportControl
 	case reuseport:
 		lc.Control = reuseportControl
 	case reuseaddr:
@@ -56,6 +86,7 @@ func listenUDP(network, addr string, reuseport, reuseaddr bool) (net.PacketConn,
 	var lc net.ListenConfig
 	switch {
 	case reuseaddr && reuseport:
+		lc.Control = reuseaddrandportControl
 	case reuseport:
 		lc.Control = reuseportControl
 	case reuseaddr:
diff --git a/sig0.go b/sig0.go
index 2c4b1035..057bb578 100644
--- a/sig0.go
+++ b/sig0.go
@@ -7,7 +7,6 @@ import (
 	"crypto/rsa"
 	"encoding/binary"
 	"math/big"
-	"strings"
 	"time"
 )
 
@@ -151,7 +150,7 @@ func (rr *SIG) Verify(k *KEY, buf []byte) error {
 	}
 	// If key has come from the DNS name compression might
 	// have mangled the case of the name
-	if !strings.EqualFold(signername, k.Header().Name) {
+	if !equal(signername, k.Header().Name) {
 		return &Error{err: "signer name doesn't match key name"}
 	}
 	sigend := offset


Test patch:
diff --git a/dnssec_test.go b/dnssec_test.go
index 1511278c..4f04e85f 100644
--- a/dnssec_test.go
+++ b/dnssec_test.go
@@ -155,6 +155,117 @@ func TestSignVerify(t *testing.T) {
 	}
 }
 
+// Test if RRSIG.Verify() conforms to RFC 4035 Section 5.3.1
+func TestShouldNotVerifyInvalidSig(t *testing.T) {
+	// The RRSIG RR and the RRset MUST have the same owner name
+	rrNameMismatch := getSoa()
+	rrNameMismatch.Hdr.Name = "example.com."
+
+	// ... and the same class
+	rrClassMismatch := getSoa()
+	rrClassMismatch.Hdr.Class = ClassCHAOS
+
+	// The RRSIG RR's Type Covered field MUST equal the RRset's type.
+	rrTypeMismatch := getSoa()
+	rrTypeMismatch.Hdr.Rrtype = TypeA
+
+	// The number of labels in the RRset owner name MUST be greater than
+	// or equal to the value in the RRSIG RR's Labels field.
+	rrLabelLessThan := getSoa()
+	rrLabelLessThan.Hdr.Name = "nl."
+
+	// Time checks are done in ValidityPeriod
+
+	// With this key
+	key := new(DNSKEY)
+	key.Hdr.Rrtype = TypeDNSKEY
+	key.Hdr.Name = "miek.nl."
+	key.Hdr.Class = ClassINET
+	key.Hdr.Ttl = 14400
+	key.Flags = 256
+	key.Protocol = 3
+	key.Algorithm = RSASHA256
+	privkey, _ := key.Generate(512)
+
+	normalSoa := getSoa()
+
+	// Fill in the normal values of the Sig, before signing
+	sig := new(RRSIG)
+	sig.Hdr = RR_Header{"miek.nl.", TypeRRSIG, ClassINET, 14400, 0}
+	sig.TypeCovered = TypeSOA
+	sig.Labels = uint8(CountLabel(normalSoa.Hdr.Name))
+	sig.OrigTtl = normalSoa.Hdr.Ttl
+	sig.Expiration = 1296534305 // date -u '+%s' -d"2011-02-01 04:25:05"
+	sig.Inception = 1293942305  // date -u '+%s' -d"2011-01-02 04:25:05"
+	sig.KeyTag = key.KeyTag()   // Get the keyfrom the Key
+	sig.SignerName = key.Hdr.Name
+	sig.Algorithm = RSASHA256
+
+	for i, rr := range []RR{rrNameMismatch, rrClassMismatch, rrTypeMismatch, rrLabelLessThan} {
+		if i != 0 { // Just for the rrNameMismatch case, we need the name to mismatch
+			sig := sig.copy().(*RRSIG)
+			sig.SignerName = rr.Header().Name
+			sig.Hdr.Name = rr.Header().Name
+			key := key.copy().(*DNSKEY)
+			key.Hdr.Name = rr.Header().Name
+		}
+
+		if err := sig.signAsIs(privkey.(*rsa.PrivateKey), []RR{rr}); err != nil {
+			t.Error("failure to sign the record:", err)
+			continue
+		}
+
+		if err := sig.Verify(key, []RR{rr}); err == nil {
+			t.Error("should not validate: ", rr)
+			continue
+		} else {
+			t.Logf("expected failure: %v for RR name %s, class %d, type %d, rrsig labels %d", err, rr.Header().Name, rr.Header().Class, rr.Header().Rrtype, CountLabel(rr.Header().Name))
+		}
+	}
+
+	// The RRSIG RR's Signer's Name field MUST be the name of the zone that contains the RRset.
+	// The RRSIG RR's Signer's Name, Algorithm, and Key Tag fields MUST match the owner name,
+	// algorithm, and key tag for some DNSKEY RR in the zone's apex DNSKEY RRset.
+	sigMismatchName := sig.copy().(*RRSIG)
+	sigMismatchName.SignerName = "example.com."
+	soaMismatchName := getSoa()
+	soaMismatchName.Hdr.Name = "example.com."
+	keyMismatchName := key.copy().(*DNSKEY)
+	keyMismatchName.Hdr.Name = "example.com."
+	if err := sigMismatchName.signAsIs(privkey.(*rsa.PrivateKey), []RR{soaMismatchName}); err != nil {
+		t.Error("failure to sign the record:", err)
+	} else if err := sigMismatchName.Verify(keyMismatchName, []RR{soaMismatchName}); err == nil {
+		t.Error("should not validate: ", soaMismatchName, ", RRSIG's signer's name does not match the owner name")
+	} else {
+		t.Logf("expected failure: %v for signer %s and owner %s", err, sigMismatchName.SignerName, sigMismatchName.Hdr.Name)
+	}
+
+	sigMismatchAlgo := sig.copy().(*RRSIG)
+	sigMismatchAlgo.Algorithm = RSASHA1
+	sigMismatchKeyTag := sig.copy().(*RRSIG)
+	sigMismatchKeyTag.KeyTag = 12345
+	for _, sigMismatch := range []*RRSIG{sigMismatchAlgo, sigMismatchKeyTag} {
+		if err := sigMismatch.Sign(privkey.(*rsa.PrivateKey), []RR{normalSoa}); err != nil {
+			t.Error("failure to sign the record:", err)
+		} else if err := sigMismatch.Verify(key, []RR{normalSoa}); err == nil {
+			t.Error("should not validate: ", normalSoa)
+		} else {
+			t.Logf("expected failure: %v for signer %s algo %d keytag %d", err, sigMismatch.SignerName, sigMismatch.Algorithm, sigMismatch.KeyTag)
+		}
+	}
+
+	// The matching DNSKEY RR MUST have the Zone Flag bit (DNSKEY RDATA Flag bit 7) set.
+	keyZoneBitWrong := key.copy().(*DNSKEY)
+	keyZoneBitWrong.Flags = key.Flags &^ ZONE
+	if err := sig.Sign(privkey.(*rsa.PrivateKey), []RR{normalSoa}); err != nil {
+		t.Error("failure to sign the record:", err)
+	} else if err := sig.Verify(keyZoneBitWrong, []RR{normalSoa}); err == nil {
+		t.Error("should not validate: ", normalSoa)
+	} else {
+		t.Logf("expected failure: %v for key flags %d", err, keyZoneBitWrong.Flags)
+	}
+}
+
 func Test65534(t *testing.T) {
 	t6 := new(RFC3597)
 	t6.Hdr = RR_Header{"miek.nl.", 65534, ClassINET, 14400, 0}
diff --git a/server_test.go b/server_test.go
index 4fc2af32..a9e2339b 100644
--- a/server_test.go
+++ b/server_test.go
@@ -996,6 +996,101 @@ func TestServerStartStopRace(t *testing.T) {
 	wg.Wait()
 }
 
+func TestSocketOptions(t *testing.T) {
+	if !supportsReuseAddr || !supportsReusePort {
+		t.Skip("reuseaddr or reuseport is not supported")
+	}
+
+	testSocketOptions := func(t *testing.T, reuseAddr bool, reusePort bool) {
+		wait := make(chan struct{})
+
+		srv := &Server{
+			Net:       "udp",
+			Addr:      ":0",
+			ReuseAddr: reuseAddr,
+			ReusePort: reusePort,
+		}
+
+		srv.NotifyStartedFunc = func() {
+			defer close(wait)
+
+			conn, ok := srv.PacketConn.(*net.UDPConn)
+			if !ok {
+				t.Errorf("unexpected conn type: %T", srv.PacketConn)
+				return
+			}
+
+			syscallConn, err := conn.SyscallConn()
+			if err != nil {
+				t.Errorf("cannot cast UDP conn to syscall conn: %v", err)
+				return
+
+			}
+
+			err = syscallConn.Control(func(fd uintptr) {
+				actualReusePort, err := checkReuseport(fd)
+				if err != nil {
+					t.Errorf("cannot get SO_REUSEPORT socket option: %v", err)
+					return
+				}
+
+				if actualReusePort != reusePort {
+					t.Errorf("SO_REUSEPORT is %v instead of %v", actualReusePort, reusePort)
+				}
+
+				actualReuseAddr, err := checkReuseaddr(fd)
+				if err != nil {
+					t.Errorf("cannot get SO_REUSEADDR socket option: %v", err)
+					return
+				}
+
+				if actualReuseAddr != reuseAddr {
+					t.Errorf("SO_REUSEADDR is %v instead of %v", actualReuseAddr, reusePort)
+				}
+			})
+			if err != nil {
+				t.Errorf("cannot check socket options: %v", err)
+			}
+		}
+
+		fin := make(chan error, 1)
+		go func() {
+			fin <- srv.ListenAndServe()
+		}()
+
+		select {
+		case <-wait:
+			err := srv.Shutdown()
+			if err != nil {
+				t.Fatalf("cannot shutdown server: %v", err)
+			}
+
+			err = <-fin
+			if err != nil {
+				t.Fatalf("listen adn serve: %v", err)
+			}
+		case err := <-fin:
+			t.Fatalf("listen adn serve: %v", err)
+		}
+	}
+
+	t.Run("no socket options", func(t *testing.T) {
+		testSocketOptions(t, false, false)
+	})
+
+	t.Run("SO_REUSEPORT", func(t *testing.T) {
+		testSocketOptions(t, false, true)
+	})
+
+	t.Run("SO_REUSEADDR", func(t *testing.T) {
+		testSocketOptions(t, true, false)
+	})
+
+	t.Run("SO_REUSEADDR and SO_REUSEPORT", func(t *testing.T) {
+		testSocketOptions(t, true, true)
+	})
+}
+
 func TestServerReuseport(t *testing.T) {
 	if !supportsReusePort {
 		t.Skip("reuseport is not supported")
@@ -1329,7 +1424,6 @@ func TestResponseWriteSinglePacket(t *testing.T) {
 	m.SetQuestion("miek.nl.", TypeTXT)
 	m.Response = true
 	err := rw.WriteMsg(m)
-
 	if err != nil {
 		t.Fatalf("failed to write: %v", err)
 	}