Repository: miekg/dns
Pull request #1338 (2022-02-06 04:15:56)

Invalid NSEC/3 bitmap on non-zero buffer
If the PackBuffer is used to encode an NSEC/3 record, the bitmap is
xored with the content of the buffer instead of being zeroed first.

The algorithm has been changed so it is able zero bytes without
losing too much performance (around 2x slower).

tmthrgd: What’s the actual issue here? Is it observable using the packages public API?

Patch:
diff --git a/dnssec.go b/dnssec.go
index 8539aae6..e95f0377 100644
--- a/dnssec.go
+++ b/dnssec.go
@@ -65,6 +65,9 @@ var AlgorithmToString = map[uint8]string{
 }
 
 // AlgorithmToHash is a map of algorithm crypto hash IDs to crypto.Hash's.
+// For newer algorithm that do their own hashing (i.e. ED25519) the returned value
+// is 0, implying no (external) hashing should occur. The non-exported identityHash is then
+// used.
 var AlgorithmToHash = map[uint8]crypto.Hash{
 	RSAMD5:           crypto.MD5, // Deprecated in RFC 6725
 	DSA:              crypto.SHA1,
@@ -74,7 +77,7 @@ var AlgorithmToHash = map[uint8]crypto.Hash{
 	ECDSAP256SHA256:  crypto.SHA256,
 	ECDSAP384SHA384:  crypto.SHA384,
 	RSASHA512:        crypto.SHA512,
-	ED25519:          crypto.Hash(0),
+	ED25519:          0,
 }
 
 // DNSSEC hashing algorithm codes.
@@ -296,35 +299,20 @@ func (rr *RRSIG) Sign(k crypto.Signer, rrset []RR) error {
 		return err
 	}
 
-	hash, ok := AlgorithmToHash[rr.Algorithm]
-	if !ok {
-		return ErrAlg
+	h, cryptohash, err := hashFromAlgorithm(rr.Algorithm)
+	if err != nil {
+		return err
 	}
 
 	switch rr.Algorithm {
-	case ED25519:
-		// ed25519 signs the raw message and performs hashing internally.
-		// All other supported signature schemes operate over the pre-hashed
-		// message, and thus ed25519 must be handled separately here.
-		//
-		// The raw message is passed directly into sign and crypto.Hash(0) is
-		// used to signal to the crypto.Signer that the data has not been hashed.
-		signature, err := sign(k, append(signdata, wire...), crypto.Hash(0), rr.Algorithm)
-		if err != nil {
-			return err
-		}
-
-		rr.Signature = toBase64(signature)
-		return nil
 	case RSAMD5, DSA, DSANSEC3SHA1:
 		// See RFC 6944.
 		return ErrAlg
 	default:
-		h := hash.New()
 		h.Write(signdata)
 		h.Write(wire)
 
-		signature, err := sign(k, h.Sum(nil), hash, rr.Algorithm)
+		signature, err := sign(k, h.Sum(nil), cryptohash, rr.Algorithm)
 		if err != nil {
 			return err
 		}
@@ -341,7 +329,7 @@ func sign(k crypto.Signer, hashed []byte, hash crypto.Hash, alg uint8) ([]byte,
 	}
 
 	switch alg {
-	case RSASHA1, RSASHA1NSEC3SHA1, RSASHA256, RSASHA512:
+	case RSASHA1, RSASHA1NSEC3SHA1, RSASHA256, RSASHA512, ED25519:
 		return signature, nil
 	case ECDSAP256SHA256, ECDSAP384SHA384:
 		ecdsaSignature := &struct {
@@ -362,8 +350,6 @@ func sign(k crypto.Signer, hashed []byte, hash crypto.Hash, alg uint8) ([]byte,
 		signature := intToBytes(ecdsaSignature.R, intlen)
 		signature = append(signature, intToBytes(ecdsaSignature.S, intlen)...)
 		return signature, nil
-	case ED25519:
-		return signature, nil
 	default:
 		return nil, ErrAlg
 	}
@@ -437,9 +423,9 @@ func (rr *RRSIG) Verify(k *DNSKEY, rrset []RR) error {
 		// remove the domain name and assume its ours?
 	}
 
-	hash, ok := AlgorithmToHash[rr.Algorithm]
-	if !ok {
-		return ErrAlg
+	h, cryptohash, err := hashFromAlgorithm(rr.Algorithm)
+	if err != nil {
+		return err
 	}
 
 	switch rr.Algorithm {
@@ -450,10 +436,9 @@ func (rr *RRSIG) Verify(k *DNSKEY, rrset []RR) error {
 			return ErrKey
 		}
 
-		h := hash.New()
 		h.Write(signeddata)
 		h.Write(wire)
-		return rsa.VerifyPKCS1v15(pubkey, hash, h.Sum(nil), sigbuf)
+		return rsa.VerifyPKCS1v15(pubkey, cryptohash, h.Sum(nil), sigbuf)
 
 	case ECDSAP256SHA256, ECDSAP384SHA384:
 		pubkey := k.publicKeyECDSA()
@@ -465,7 +450,6 @@ func (rr *RRSIG) Verify(k *DNSKEY, rrset []RR) error {
 		r := new(big.Int).SetBytes(sigbuf[:len(sigbuf)/2])
 		s := new(big.Int).SetBytes(sigbuf[len(sigbuf)/2:])
 
-		h := hash.New()
 		h.Write(signeddata)
 		h.Write(wire)
 		if ecdsa.Verify(pubkey, h.Sum(nil), r, s) {
diff --git a/edns.go b/edns.go
index 862e5343..14568c2e 100644
--- a/edns.go
+++ b/edns.go
@@ -584,14 +584,17 @@ func (e *EDNS0_N3U) copy() EDNS0 { return &EDNS0_N3U{e.Code, e.AlgCode} }
 type EDNS0_EXPIRE struct {
 	Code   uint16 // Always EDNS0EXPIRE
 	Expire uint32
+	Empty  bool // Empty is used to signal an empty Expire option in a backwards compatible way, it's not used on the wire.
 }
 
 // Option implements the EDNS0 interface.
 func (e *EDNS0_EXPIRE) Option() uint16 { return EDNS0EXPIRE }
-func (e *EDNS0_EXPIRE) String() string { return strconv.FormatUint(uint64(e.Expire), 10) }
-func (e *EDNS0_EXPIRE) copy() EDNS0    { return &EDNS0_EXPIRE{e.Code, e.Expire} }
+func (e *EDNS0_EXPIRE) copy() EDNS0    { return &EDNS0_EXPIRE{e.Code, e.Expire, e.Empty} }
 
 func (e *EDNS0_EXPIRE) pack() ([]byte, error) {
+	if e.Empty {
+		return []byte{}, nil
+	}
 	b := make([]byte, 4)
 	binary.BigEndian.PutUint32(b, e.Expire)
 	return b, nil
@@ -600,15 +603,24 @@ func (e *EDNS0_EXPIRE) pack() ([]byte, error) {
 func (e *EDNS0_EXPIRE) unpack(b []byte) error {
 	if len(b) == 0 {
 		// zero-length EXPIRE query, see RFC 7314 Section 2
+		e.Empty = true
 		return nil
 	}
 	if len(b) < 4 {
 		return ErrBuf
 	}
 	e.Expire = binary.BigEndian.Uint32(b)
+	e.Empty = false
 	return nil
 }
 
+func (e *EDNS0_EXPIRE) String() (s string) {
+	if e.Empty {
+		return ""
+	}
+	return strconv.FormatUint(uint64(e.Expire), 10)
+}
+
 // The EDNS0_LOCAL option is used for local/experimental purposes. The option
 // code is recommended to be within the range [EDNS0LOCALSTART, EDNS0LOCALEND]
 // (RFC6891), although any unassigned code can actually be used.  The content of
diff --git a/hash.go b/hash.go
new file mode 100644
index 00000000..7d4183e0
--- /dev/null
+++ b/hash.go
@@ -0,0 +1,31 @@
+package dns
+
+import (
+	"bytes"
+	"crypto"
+	"hash"
+)
+
+// identityHash will not hash, it only buffers the data written into it and returns it as-is.
+type identityHash struct {
+	b *bytes.Buffer
+}
+
+// Implement the hash.Hash interface.
+
+func (i identityHash) Write(b []byte) (int, error) { return i.b.Write(b) }
+func (i identityHash) Size() int                   { return i.b.Len() }
+func (i identityHash) BlockSize() int              { return 1024 }
+func (i identityHash) Reset()                      { i.b.Reset() }
+func (i identityHash) Sum(b []byte) []byte         { return append(b, i.b.Bytes()...) }
+
+func hashFromAlgorithm(alg uint8) (hash.Hash, crypto.Hash, error) {
+	hashnumber, ok := AlgorithmToHash[alg]
+	if !ok {
+		return nil, 0, ErrAlg
+	}
+	if hashnumber == 0 {
+		return identityHash{b: &bytes.Buffer{}}, hashnumber, nil
+	}
+	return hashnumber.New(), hashnumber, nil
+}
diff --git a/msg_helpers.go b/msg_helpers.go
index 10754c8b..b049028b 100644
--- a/msg_helpers.go
+++ b/msg_helpers.go
@@ -558,6 +558,16 @@ func packDataNsec(bitmap []uint16, msg []byte, off int) (int, error) {
 	if len(bitmap) == 0 {
 		return off, nil
 	}
+	if off > len(msg) {
+		return off, &Error{err: "overflow packing nsec"}
+	}
+	toZero := msg[off:]
+	if maxLen := typeBitMapLen(bitmap); maxLen < len(toZero) {
+		toZero = toZero[:maxLen]
+	}
+	for i := range toZero {
+		toZero[i] = 0
+	}
 	var lastwindow, lastlength uint16
 	for _, t := range bitmap {
 		window := t / 256
diff --git a/sig0.go b/sig0.go
index e781c9bb..2c4b1035 100644
--- a/sig0.go
+++ b/sig0.go
@@ -3,6 +3,7 @@ package dns
 import (
 	"crypto"
 	"crypto/ecdsa"
+	"crypto/ed25519"
 	"crypto/rsa"
 	"encoding/binary"
 	"math/big"
@@ -38,18 +39,17 @@ func (rr *SIG) Sign(k crypto.Signer, m *Msg) ([]byte, error) {
 	}
 	buf = buf[:off:cap(buf)]
 
-	hash, ok := AlgorithmToHash[rr.Algorithm]
-	if !ok {
-		return nil, ErrAlg
+	h, cryptohash, err := hashFromAlgorithm(rr.Algorithm)
+	if err != nil {
+		return nil, err
 	}
 
-	hasher := hash.New()
 	// Write SIG rdata
-	hasher.Write(buf[len(mbuf)+1+2+2+4+2:])
+	h.Write(buf[len(mbuf)+1+2+2+4+2:])
 	// Write message
-	hasher.Write(buf[:len(mbuf)])
+	h.Write(buf[:len(mbuf)])
 
-	signature, err := sign(k, hasher.Sum(nil), hash, rr.Algorithm)
+	signature, err := sign(k, h.Sum(nil), cryptohash, rr.Algorithm)
 	if err != nil {
 		return nil, err
 	}
@@ -82,20 +82,10 @@ func (rr *SIG) Verify(k *KEY, buf []byte) error {
 		return ErrKey
 	}
 
-	var hash crypto.Hash
-	switch rr.Algorithm {
-	case RSASHA1:
-		hash = crypto.SHA1
-	case RSASHA256, ECDSAP256SHA256:
-		hash = crypto.SHA256
-	case ECDSAP384SHA384:
-		hash = crypto.SHA384
-	case RSASHA512:
-		hash = crypto.SHA512
-	default:
-		return ErrAlg
-	}
-	hasher := hash.New()
+	h, cryptohash, err := hashFromAlgorithm(rr.Algorithm)
+	if err != nil {
+		return err
+	}
 
 	buflen := len(buf)
 	qdc := binary.BigEndian.Uint16(buf[4:])
@@ -103,7 +93,6 @@ func (rr *SIG) Verify(k *KEY, buf []byte) error {
 	auc := binary.BigEndian.Uint16(buf[8:])
 	adc := binary.BigEndian.Uint16(buf[10:])
 	offset := headerSize
-	var err error
 	for i := uint16(0); i < qdc && offset < buflen; i++ {
 		_, offset, err = UnpackDomainName(buf, offset)
 		if err != nil {
@@ -166,21 +155,21 @@ func (rr *SIG) Verify(k *KEY, buf []byte) error {
 		return &Error{err: "signer name doesn't match key name"}
 	}
 	sigend := offset
-	hasher.Write(buf[sigstart:sigend])
-	hasher.Write(buf[:10])
-	hasher.Write([]byte{
+	h.Write(buf[sigstart:sigend])
+	h.Write(buf[:10])
+	h.Write([]byte{
 		byte((adc - 1) << 8),
 		byte(adc - 1),
 	})
-	hasher.Write(buf[12:bodyend])
+	h.Write(buf[12:bodyend])
 
-	hashed := hasher.Sum(nil)
+	hashed := h.Sum(nil)
 	sig := buf[sigend:]
 	switch k.Algorithm {
 	case RSASHA1, RSASHA256, RSASHA512:
 		pk := k.publicKeyRSA()
 		if pk != nil {
-			return rsa.VerifyPKCS1v15(pk, hash, hashed, sig)
+			return rsa.VerifyPKCS1v15(pk, cryptohash, hashed, sig)
 		}
 	case ECDSAP256SHA256, ECDSAP384SHA384:
 		pk := k.publicKeyECDSA()
@@ -192,6 +181,14 @@ func (rr *SIG) Verify(k *KEY, buf []byte) error {
 			}
 			return ErrSig
 		}
+	case ED25519:
+		pk := k.publicKeyED25519()
+		if pk != nil {
+			if ed25519.Verify(pk, hashed, sig) {
+				return nil
+			}
+			return ErrSig
+		}
 	}
 	return ErrKeyAlg
 }
diff --git a/svcb.go b/svcb.go
index 3344253c..ff5e0108 100644
--- a/svcb.go
+++ b/svcb.go
@@ -13,16 +13,17 @@ import (
 // SVCBKey is the type of the keys used in the SVCB RR.
 type SVCBKey uint16
 
-// Keys defined in draft-ietf-dnsop-svcb-https-01 Section 12.3.2.
+// Keys defined in draft-ietf-dnsop-svcb-https-08 Section 14.3.2.
 const (
-	SVCB_MANDATORY       SVCBKey = 0
-	SVCB_ALPN            SVCBKey = 1
-	SVCB_NO_DEFAULT_ALPN SVCBKey = 2
-	SVCB_PORT            SVCBKey = 3
-	SVCB_IPV4HINT        SVCBKey = 4
-	SVCB_ECHCONFIG       SVCBKey = 5
-	SVCB_IPV6HINT        SVCBKey = 6
-	svcb_RESERVED        SVCBKey = 65535
+	SVCB_MANDATORY SVCBKey = iota
+	SVCB_ALPN
+	SVCB_NO_DEFAULT_ALPN
+	SVCB_PORT
+	SVCB_IPV4HINT
+	SVCB_ECHCONFIG
+	SVCB_IPV6HINT
+
+	svcb_RESERVED SVCBKey = 65535
 )
 
 var svcbKeyToStringMap = map[SVCBKey]string{
@@ -31,7 +32,7 @@ var svcbKeyToStringMap = map[SVCBKey]string{
 	SVCB_NO_DEFAULT_ALPN: "no-default-alpn",
 	SVCB_PORT:            "port",
 	SVCB_IPV4HINT:        "ipv4hint",
-	SVCB_ECHCONFIG:       "echconfig",
+	SVCB_ECHCONFIG:       "ech",
 	SVCB_IPV6HINT:        "ipv6hint",
 }
 
@@ -167,10 +168,14 @@ func (rr *SVCB) parse(c *zlexer, o string) *ParseError {
 		}
 		l, _ = c.Next()
 	}
+
+	// "In AliasMode, records SHOULD NOT include any SvcParams, and recipients MUST
+	// ignore any SvcParams that are present."
+	// However, we don't check rr.Priority == 0 && len(xs) > 0 here
+	// It is the responsibility of the user of the library to check this.
+	// This is to encourage the fixing of the source of this error.
+
 	rr.Value = xs
-	if rr.Priority == 0 && len(xs) > 0 {
-		return &ParseError{l.token, "SVCB aliasform can't have values", l}
-	}
 	return nil
 }
 
@@ -200,12 +205,12 @@ func makeSVCBKeyValue(key SVCBKey) SVCBKeyValue {
 	}
 }
 
-// SVCB RR. See RFC xxxx (https://tools.ietf.org/html/draft-ietf-dnsop-svcb-https-01).
+// SVCB RR. See RFC xxxx (https://tools.ietf.org/html/draft-ietf-dnsop-svcb-https-08).
 type SVCB struct {
 	Hdr      RR_Header
-	Priority uint16
+	Priority uint16         // If zero, Value must be empty or discarded by the user of this library
 	Target   string         `dns:"domain-name"`
-	Value    []SVCBKeyValue `dns:"pairs"` // Value must be empty if Priority is zero.
+	Value    []SVCBKeyValue `dns:"pairs"`
 }
 
 // HTTPS RR. Everything valid for SVCB applies to HTTPS as well.
@@ -235,15 +240,29 @@ type SVCBKeyValue interface {
 }
 
 // SVCBMandatory pair adds to required keys that must be interpreted for the RR
-// to be functional.
+// to be functional. If ignored, the whole RRSet must be ignored.
+// "port" and "no-default-alpn" are mandatory by default if present,
+// so they shouldn't be included here.
+//
+// It is incumbent upon the user of this library to reject the RRSet if
+// or avoid constructing such an RRSet that:
+// - "mandatory" is included as one of the keys of mandatory
+// - no key is listed multiple times in mandatory
+// - all keys listed in mandatory are present
+// - escape sequences are not used in mandatory
+// - mandatory, when present, lists at least one key
+//
 // Basic use pattern for creating a mandatory option:
 //
 //	s := &dns.SVCB{Hdr: dns.RR_Header{Name: ".", Rrtype: dns.TypeSVCB, Class: dns.ClassINET}}
 //	e := new(dns.SVCBMandatory)
-//	e.Code = []uint16{65403}
+//	e.Code = []uint16{dns.SVCB_ALPN}
 //	s.Value = append(s.Value, e)
+//	t := new(dns.SVCBAlpn)
+//	t.Alpn = []string{"xmpp-client"}
+//	s.Value = append(s.Value, t)
 type SVCBMandatory struct {
-	Code []SVCBKey // Must not include mandatory
+	Code []SVCBKey
 }
 
 func (*SVCBMandatory) Key() SVCBKey { return SVCB_MANDATORY }
@@ -302,7 +321,8 @@ func (s *SVCBMandatory) copy() SVCBKeyValue {
 }
 
 // SVCBAlpn pair is used to list supported connection protocols.
-// Protocol ids can be found at:
+// The user of this library must ensure that at least one protocol is listed when alpn is present.
+// Protocol IDs can be found at:
 // https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids
 // Basic use pattern for creating an alpn option:
 //
@@ -370,9 +390,13 @@ func (s *SVCBAlpn) copy() SVCBKeyValue {
 }
 
 // SVCBNoDefaultAlpn pair signifies no support for default connection protocols.
+// Should be used in conjunction with alpn.
 // Basic use pattern for creating a no-default-alpn option:
 //
 //	s := &dns.SVCB{Hdr: dns.RR_Header{Name: ".", Rrtype: dns.TypeSVCB, Class: dns.ClassINET}}
+//	t := new(dns.SVCBAlpn)
+//	t.Alpn = []string{"xmpp-client"}
+//	s.Value = append(s.Value, t)
 //	e := new(dns.SVCBNoDefaultAlpn)
 //	s.Value = append(s.Value, e)
 type SVCBNoDefaultAlpn struct{}
@@ -385,14 +409,14 @@ func (*SVCBNoDefaultAlpn) len() int              { return 0 }
 
 func (*SVCBNoDefaultAlpn) unpack(b []byte) error {
 	if len(b) != 0 {
-		return errors.New("dns: svcbnodefaultalpn: no_default_alpn must have no value")
+		return errors.New("dns: svcbnodefaultalpn: no-default-alpn must have no value")
 	}
 	return nil
 }
 
 func (*SVCBNoDefaultAlpn) parse(b string) error {
 	if b != "" {
-		return errors.New("dns: svcbnodefaultalpn: no_default_alpn must have no value")
+		return errors.New("dns: svcbnodefaultalpn: no-default-alpn must have no value")
 	}
 	return nil
 }
@@ -523,7 +547,7 @@ func (s *SVCBIPv4Hint) copy() SVCBKeyValue {
 }
 
 // SVCBECHConfig pair contains the ECHConfig structure defined in draft-ietf-tls-esni [RFC xxxx].
-// Basic use pattern for creating an echconfig option:
+// Basic use pattern for creating an ech option:
 //
 //	h := new(dns.HTTPS)
 //	h.Hdr = dns.RR_Header{Name: ".", Rrtype: dns.TypeHTTPS, Class: dns.ClassINET}
@@ -531,7 +555,7 @@ func (s *SVCBIPv4Hint) copy() SVCBKeyValue {
 //	e.ECH = []byte{0xfe, 0x08, ...}
 //	h.Value = append(h.Value, e)
 type SVCBECHConfig struct {
-	ECH []byte
+	ECH []byte // Specifically ECHConfigList including the redundant length prefix
 }
 
 func (*SVCBECHConfig) Key() SVCBKey     { return SVCB_ECHCONFIG }
@@ -555,7 +579,7 @@ func (s *SVCBECHConfig) unpack(b []byte) error {
 func (s *SVCBECHConfig) parse(b string) error {
 	x, err := fromBase64([]byte(b))
 	if err != nil {
-		return errors.New("dns: svcbechconfig: bad base64 echconfig")
+		return errors.New("dns: svcbech: bad base64 ech")
 	}
 	s.ECH = x
 	return nil
@@ -618,9 +642,6 @@ func (s *SVCBIPv6Hint) String() string {
 }
 
 func (s *SVCBIPv6Hint) parse(b string) error {
-	if strings.Contains(b, ".") {
-		return errors.New("dns: svcbipv6hint: expected ipv6, got ipv4")
-	}
 	str := strings.Split(b, ",")
 	dst := make([]net.IP, len(str))
 	for i, e := range str {
@@ -628,6 +649,9 @@ func (s *SVCBIPv6Hint) parse(b string) error {
 		if ip == nil {
 			return errors.New("dns: svcbipv6hint: bad ip")
 		}
+		if ip.To4() != nil {
+			return errors.New("dns: svcbipv6hint: expected ipv6, got ipv4-mapped-ipv6")
+		}
 		dst[i] = ip
 	}
 	s.Hint = dst
diff --git a/version.go b/version.go
index 5094af7e..4b2ba901 100644
--- a/version.go
+++ b/version.go
@@ -3,7 +3,7 @@ package dns
 import "fmt"
 
 // Version is current version of this library.
-var Version = v{1, 1, 45}
+var Version = v{1, 1, 47}
 
 // v holds the version of this library.
 type v struct {


Test patch:
diff --git a/client_test.go b/client_test.go
index 71c61770..83d5af70 100644
--- a/client_test.go
+++ b/client_test.go
@@ -257,16 +257,14 @@ func TestClientSyncBadID(t *testing.T) {
 	m := new(Msg)
 	m.SetQuestion("miek.nl.", TypeSOA)
 
+	// Test with client.Exchange, the plain Exchange function is just a wrapper, so
+	// we don't need to test that separately.
 	c := &Client{
-		Timeout: 50 * time.Millisecond,
+		Timeout: 10 * time.Millisecond,
 	}
 	if _, _, err := c.Exchange(m, addrstr); err == nil || !isNetworkTimeout(err) {
 		t.Errorf("query did not time out")
 	}
-	// And now with plain Exchange().
-	if _, err = Exchange(m, addrstr); err == nil || !isNetworkTimeout(err) {
-		t.Errorf("query did not time out")
-	}
 }
 
 func TestClientSyncBadThenGoodID(t *testing.T) {
@@ -290,14 +288,6 @@ func TestClientSyncBadThenGoodID(t *testing.T) {
 	if r.Id != m.Id {
 		t.Errorf("failed to get response with expected Id")
 	}
-	// And now with plain Exchange().
-	r, err = Exchange(m, addrstr)
-	if err != nil {
-		t.Errorf("failed to exchange: %v", err)
-	}
-	if r.Id != m.Id {
-		t.Errorf("failed to get response with expected Id")
-	}
 }
 
 func TestClientSyncTCPBadID(t *testing.T) {
diff --git a/msg_helpers_test.go b/msg_helpers_test.go
index bd41e245..1f17c57c 100644
--- a/msg_helpers_test.go
+++ b/msg_helpers_test.go
@@ -19,7 +19,8 @@ func TestPackDataNsec(t *testing.T) {
 	tests := []struct {
 		name       string
 		args       args
-		want       int
+		wantOff    int
+		wantBytes  []byte
 		wantErr    bool
 		wantErrMsg string
 	}{
@@ -44,14 +45,14 @@ func TestPackDataNsec(t *testing.T) {
 			},
 			wantErr:    true,
 			wantErrMsg: "dns: overflow packing nsec",
-			want:       31,
+			wantOff:    48,
 		},
 		{
 			name: "disordered nsec bits",
 			args: args{
 				bitmap: []uint16{
 					8962,
-					0,
+					1,
 				},
 				msg: []byte{
 					48, 48, 48, 48, 0, 0, 0, 1, 0, 0, 0, 0,
@@ -72,13 +73,13 @@ func TestPackDataNsec(t *testing.T) {
 			},
 			wantErr:    true,
 			wantErrMsg: "dns: nsec bits out of order",
-			want:       155,
+			wantOff:    155,
 		},
 		{
 			name: "simple message with only one window",
 			args: args{
 				bitmap: []uint16{
-					0,
+					1,
 				},
 				msg: []byte{
 					48, 48, 48, 48, 0, 0,
@@ -89,13 +90,33 @@ func TestPackDataNsec(t *testing.T) {
 				},
 				off: 0,
 			},
-			wantErr: false,
-			want:    3,
+			wantErr:   false,
+			wantOff:   3,
+			wantBytes: []byte{0, 1, 64},
+		},
+		{
+			name: "multiple types",
+			args: args{
+				bitmap: []uint16{
+					TypeNS, TypeSOA, TypeRRSIG, TypeDNSKEY, TypeNSEC3PARAM,
+				},
+				msg: []byte{
+					48, 48, 48, 48, 0, 0,
+					0, 1, 0, 0, 0, 0,
+					0, 0, 50, 48, 48, 48,
+					48, 48, 48, 0, 54, 48,
+					48, 48, 48, 0, 19, 48, 48,
+				},
+				off: 0,
+			},
+			wantErr:   false,
+			wantOff:   9,
+			wantBytes: []byte{0, 7, 34, 0, 0, 0, 0, 2, 144},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := packDataNsec(tt.args.bitmap, tt.args.msg, tt.args.off)
+			gotOff, err := packDataNsec(tt.args.bitmap, tt.args.msg, tt.args.off)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("packDataNsec() error = %v, wantErr %v", err, tt.wantErr)
 				return
@@ -104,13 +125,49 @@ func TestPackDataNsec(t *testing.T) {
 				t.Errorf("packDataNsec() error msg = %v, wantErrMsg %v", err.Error(), tt.wantErrMsg)
 				return
 			}
-			if got != tt.want {
-				t.Errorf("packDataNsec() = %v, want %v", got, tt.want)
+			if gotOff != tt.wantOff {
+				t.Errorf("packDataNsec() = %v, want off %v", gotOff, tt.wantOff)
+			}
+			if err == nil && tt.args.off < len(tt.args.msg) && gotOff < len(tt.args.msg) {
+				if want, got := tt.wantBytes, tt.args.msg[tt.args.off:gotOff]; !bytes.Equal(got, want) {
+					t.Errorf("packDataNsec() = %v, want bytes %v", got, want)
+				}
 			}
 		})
 	}
 }
 
+func TestPackDataNsecDirtyBuffer(t *testing.T) {
+	zeroBuf := []byte{0, 0, 0, 0, 0, 0, 0, 0, 0}
+	dirtyBuf := []byte{1, 2, 3, 4, 5, 6, 7, 8, 9}
+	off1, _ := packDataNsec([]uint16{TypeNS, TypeSOA, TypeRRSIG}, zeroBuf, 0)
+	off2, _ := packDataNsec([]uint16{TypeNS, TypeSOA, TypeRRSIG}, dirtyBuf, 0)
+	if off1 != off2 {
+		t.Errorf("off1 %v != off2 %v", off1, off2)
+	}
+	if !bytes.Equal(zeroBuf[:off1], dirtyBuf[:off2]) {
+		t.Errorf("dirty buffer differs from zero buffer: %v, %v", zeroBuf[:off1], dirtyBuf[:off2])
+	}
+}
+
+func BenchmarkPackDataNsec(b *testing.B) {
+	benches := []struct {
+		name  string
+		types []uint16
+	}{
+		{"empty", nil},
+		{"typical", []uint16{TypeNS, TypeSOA, TypeRRSIG, TypeDNSKEY, TypeNSEC3PARAM}},
+		{"multiple_windows", []uint16{1, 300, 350, 10000, 20000}},
+	}
+	for _, bb := range benches {
+		b.Run(bb.name, func(b *testing.B) {
+			buf := make([]byte, 100)
+			for n := 0; n < b.N; n++ {
+				packDataNsec(bb.types, buf, 0)
+			}
+		})
+	}
+}
 func TestUnpackString(t *testing.T) {
 	msg := []byte("\x00abcdef\x0f\\\"ghi\x04mmm\x7f")
 	msg[0] = byte(len(msg) - 1)
diff --git a/parse_test.go b/parse_test.go
index 6c55a74a..444ac1e5 100644
--- a/parse_test.go
+++ b/parse_test.go
@@ -1639,6 +1639,19 @@ func TestParseSVCB(t *testing.T) {
 		`example.com. 3600 IN SVCB 0 cloudflare.com.`: `example.com.	3600	IN	SVCB	0 cloudflare.com.`,
 		`example.com. 3600 IN SVCB 65000 cloudflare.com. alpn=h2 ipv4hint=3.4.3.2`: `example.com.	3600	IN	SVCB	65000 cloudflare.com. alpn="h2" ipv4hint="3.4.3.2"`,
 		`example.com. 3600 IN SVCB 65000 cloudflare.com. key65000=4\ 3 key65001="\" " key65002 key65003= key65004="" key65005== key65006==\"\" key65007=\254 key65008=\032`: `example.com.	3600	IN	SVCB	65000 cloudflare.com. key65000="4\ 3" key65001="\"\ " key65002="" key65003="" key65004="" key65005="=" key65006="=\"\"" key65007="\254" key65008="\ "`,
+		// Explained in svcb.go "In AliasMode, records SHOULD NOT include any SvcParams,"
+		`example.com. 3600 IN SVCB 0 no-default-alpn`: `example.com.	3600	IN	SVCB	0 no-default-alpn.`,
+		// From the specification
+		`example.com.   HTTPS   0 foo.example.com.`: `example.com.	3600	IN	HTTPS	0 foo.example.com.`,
+		`example.com.   SVCB   1 .`: `example.com.	3600	IN	SVCB	1 .`,
+		`example.com.   SVCB   16 foo.example.com. port=53`: `example.com.	3600	IN	SVCB	16 foo.example.com. port="53"`,
+		`example.com.   SVCB   1 foo.example.com. key667=hello`: `example.com.	3600	IN	SVCB	1 foo.example.com. key667="hello"`,
+		`example.com.   SVCB   1 foo.example.com. key667="hello\210qoo"`: `example.com.	3600	IN	SVCB	1 foo.example.com. key667="hello\210qoo"`,
+		`example.com.   SVCB   1 foo.example.com. ipv6hint="2001:db8::1,2001:db8::53:1"`: `example.com.	3600	IN	SVCB	1 foo.example.com. ipv6hint="2001:db8::1,2001:db8::53:1"`,
+		`example.com.   SVCB   1 example.com. ipv6hint="2001:db8::198.51.100.100"`: `example.com.	3600	IN	SVCB	1 example.com. ipv6hint="2001:db8::c633:6464"`,
+		`example.com.   SVCB   16 foo.example.org. alpn=h2,h3-19 mandatory=ipv4hint,alpn ipv4hint=192.0.2.1`: `example.com.	3600	IN	SVCB	16 foo.example.org. alpn="h2,h3-19" mandatory="ipv4hint,alpn" ipv4hint="192.0.2.1"`,
+		`example.com.   SVCB   16 foo.example.org. alpn="f\\\\oo\\,bar,h2"`: `example.com.	3600	IN	SVCB	16 foo.example.org. alpn="f\\\\oo\\,bar,h2"`,
+		`example.com.   SVCB   16 foo.example.org. alpn=f\\\092oo\092,bar,h2`: `example.com.	3600	IN	SVCB	16 foo.example.org. alpn="f\\\092oo\092,bar,h2"`,
 	}
 	for s, o := range svcbs {
 		rr, err := NewRR(s)
@@ -1655,7 +1668,6 @@ func TestParseSVCB(t *testing.T) {
 func TestParseBadSVCB(t *testing.T) {
 	header := `example.com. 3600 IN HTTPS `
 	evils := []string{
-		`0 . no-default-alpn`,     // aliasform
 		`65536 . no-default-alpn`, // bad priority
 		`1 ..`,                    // bad domain
 		`1 . no-default-alpn=1`,   // value illegal
@@ -1682,10 +1694,12 @@ func TestParseBadSVCB(t *testing.T) {
 		`1 . ipv6hint=1.1.1.1`,    // not ipv6
 		`1 . ipv6hint=1:1:1:1`,    // not ipv6
 		`1 . ipv6hint=a`,          // not ipv6
+		`1 . ipv6hint=`,           // empty ipv6
 		`1 . ipv4hint=1.1.1.1.1`,  // not ipv4
 		`1 . ipv4hint=::fc`,       // not ipv4
 		`1 . ipv4hint=..11`,       // not ipv4
 		`1 . ipv4hint=a`,          // not ipv4
+		`1 . ipv4hint=`,           // empty ipv4
 		`1 . port=`,               // empty port
 		`1 . echconfig=YUd`,       // bad base64
 	}
diff --git a/sig0_test.go b/sig0_test.go
index 1abdc4af..5b991d22 100644
--- a/sig0_test.go
+++ b/sig0_test.go
@@ -12,7 +12,7 @@ func TestSIG0(t *testing.T) {
 	}
 	m := new(Msg)
 	m.SetQuestion("example.org.", TypeSOA)
-	for _, alg := range []uint8{ECDSAP256SHA256, ECDSAP384SHA384, RSASHA1, RSASHA256, RSASHA512} {
+	for _, alg := range []uint8{ECDSAP256SHA256, ECDSAP384SHA384, RSASHA1, RSASHA256, RSASHA512, ED25519} {
 		algstr := AlgorithmToString[alg]
 		keyrr := new(KEY)
 		keyrr.Hdr.Name = algstr + "."
@@ -21,7 +21,7 @@ func TestSIG0(t *testing.T) {
 		keyrr.Algorithm = alg
 		keysize := 512
 		switch alg {
-		case ECDSAP256SHA256:
+		case ECDSAP256SHA256, ED25519:
 			keysize = 256
 		case ECDSAP384SHA384:
 			keysize = 384
@@ -30,7 +30,7 @@ func TestSIG0(t *testing.T) {
 		}
 		pk, err := keyrr.Generate(keysize)
 		if err != nil {
-			t.Errorf("failed to generate key for “%s”: %v", algstr, err)
+			t.Errorf("failed to generate key for %q: %v", algstr, err)
 			continue
 		}
 		now := uint32(time.Now().Unix())
@@ -45,16 +45,16 @@ func TestSIG0(t *testing.T) {
 		sigrr.SignerName = keyrr.Hdr.Name
 		mb, err := sigrr.Sign(pk.(crypto.Signer), m)
 		if err != nil {
-			t.Errorf("failed to sign message using “%s”: %v", algstr, err)
+			t.Errorf("failed to sign message using %q: %v", algstr, err)
 			continue
 		}
 		m := new(Msg)
 		if err := m.Unpack(mb); err != nil {
-			t.Errorf("failed to unpack message signed using “%s”: %v", algstr, err)
+			t.Errorf("failed to unpack message signed using %q: %v", algstr, err)
 			continue
 		}
 		if len(m.Extra) != 1 {
-			t.Errorf("missing SIG for message signed using “%s”", algstr)
+			t.Errorf("missing SIG for message signed using %q", algstr)
 			continue
 		}
 		var sigrrwire *SIG
@@ -71,20 +71,20 @@ func TestSIG0(t *testing.T) {
 				id = "sigrrwire"
 			}
 			if err := rr.Verify(keyrr, mb); err != nil {
-				t.Errorf("failed to verify “%s” signed SIG(%s): %v", algstr, id, err)
+				t.Errorf("failed to verify %q signed SIG(%s): %v", algstr, id, err)
 				continue
 			}
 		}
 		mb[13]++
 		if err := sigrr.Verify(keyrr, mb); err == nil {
-			t.Errorf("verify succeeded on an altered message using “%s”", algstr)
+			t.Errorf("verify succeeded on an altered message using %q", algstr)
 			continue
 		}
 		sigrr.Expiration = 2
 		sigrr.Inception = 1
 		mb, _ = sigrr.Sign(pk.(crypto.Signer), m)
 		if err := sigrr.Verify(keyrr, mb); err == nil {
-			t.Errorf("verify succeeded on an expired message using “%s”", algstr)
+			t.Errorf("verify succeeded on an expired message using %q", algstr)
 			continue
 		}
 	}
diff --git a/svcb_test.go b/svcb_test.go
index 6994aca5..25494995 100644
--- a/svcb_test.go
+++ b/svcb_test.go
@@ -17,7 +17,7 @@ func TestSVCB(t *testing.T) {
 		{`ipv4hint`, `3.4.3.2,1.1.1.1`},
 		{`no-default-alpn`, ``},
 		{`ipv6hint`, `1::4:4:4:4,1::3:3:3:3`},
-		{`echconfig`, `YUdWc2JHOD0=`},
+		{`ech`, `YUdWc2JHOD0=`},
 		{`key65000`, `4\ 3`},
 		{`key65001`, `\"\ `},
 		{`key65002`, ``},