Repository: miekg/dns
Pull request #1341 (2022-02-20 19:46:27)

Update SVCB
The only breaking change worth mentioning is renaming SVCB_ECHCONFIG to SVCB_ECH. We can avoid this and use its legacy name internally if you wish.

I had chosen ( https://github.com/miekg/dns/pull/1067#issuecomment-699635130 ) to forbid IPv4 addresses in ipv6hint to be consistent with the rest of the library. I don't know why ( https://github.com/MikeBishop/dns-alt-svc/issues/361 ) but the SVCB spec requires otherwise.

@miekg @tmthrgd Would you like to review?

miekg: Breaking changes in this record I fine, it's still draft IIRC

> I had chosen ( https://github.com/miekg/dns/pull/1067#issuecomment-699635130 ) to forbid IPv4 addresses in ipv6hint to be consistent with the rest of the library. I don't know why ( https://github.com/MikeBishop/dns-alt-svc/issues/361 ) but the SVCB spec requires otherwise.

That sgtm, internal consistency is a good thing to have
DesWurstes: @miekg  Many thanks for your review. I have addresses your comments except "if you want this to make a list in godoc you need empty lines before and after. Maybe just make this more prose?" because I don't know how to turn this into a paragraph without making it unreadable.

> > I had chosen ( https://github.com/miekg/dns/pull/1067#issuecomment-699635130 ) to forbid IPv4 addresses in ipv6hint to be consistent with the rest of the library. I don't know why ( https://github.com/MikeBishop/dns-alt-svc/issues/361 ) but the SVCB spec requires otherwise.
>
> That sgtm, internal consistency is a good thing to have

On the SVCB side, they decided to update their example, so this library now achieves full compliance and consistency. Hence I updated the relevant code.

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/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 8c807501..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, 46}
+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/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`, ``},