Implement SVCB (#1067)

* Implement SVCB

* Fix serialization and deserialization of double quotes

* More effort (?)

4 months old commit

* DEBUG

* _

* Presentation format serialization/deserialization

* _

Remove generated

* Progress on presentation format parse & write

* _

* Finish parsing presentation format

* Regenerate

* Pack unpack

* Move to svcb.go

Scan_rr.go and types.go should be untouched now

* 🐛

Thanks ghedo

* Definitions

* TypeHTTPSSVC

* Generated

and isDuplicate

* Goodbye lenient functions

Now private key=value pairs have to be defined as structs too. They are no longer automatically named as KeyNNNNN

* Encode/decode

* Experimental svc

* Read method

* Implement some of the methods, use trick...

to  report where the error is while reading it. This should be applied to EDNS too. Todo: Find if case can only contain e := new(SVC_ALPN) and rest moved out

Also fix two compile errors

* Add SVC_LOCAL methods, reorder, remove alpn value, bugs

* Errors

* Alpn, make it build

* Correct testsuite

* Fully implement parser

Change from keeping a state variable to reading in one iteration until the key=value pair is fully consumed

* Simplify and document

EDNS should be simplified too

* Attempt to fix fuzzer

And Alpn bug

* A bug and change type values to match @ghedo's implementation

* IP bug

Also there are two ip duplicating patterns, one with copy, one with append. Maybe change it to be consistent.

* Check for strictly increasing keys as required

* Don't panic on invalid alpn

* Redundant check, don't modify original array

* Size calculation

* Fix the fuzzer, match the style

* 65535 is reserved too, don't delay errors

* Check keyNNN, check for aliasform having values

* IPvNHint is an array

* Fix ipvNHint

* Rename everything

* Unrecognized keys according to the updated specification

* Skip zero-length structs in generators. Fix CI

* Doc cleanup

* Off by one

* Add parse tests

* Check if private key doesn't collide with known key, invalid tests

* Disallow IPv4 as IPv6. More tests.

Related #1107

* Style fixes

* More consistency, more tests

* 🐛 Deep copy as in the documentation

	a := make([]net.IP, 1)
	a[0] = net.ParseIP("1.1.1.1").To4()
	b := append(make([]net.IP, 0, 1), a...)
	b[0] = net.ParseIP("3.1.1.1").To4()
	fmt.Println(a[0][0])

* Make tests readable

* Move valid parse tests to different file

* 🐛 One of previous commits not fully committed

* Test binary single value encoding/decoding and full encode/decode

* Add worst-case grows to builders, 🐛 Wrong visible character range, redundant tests

* Testing improvements

And don't convert to IPv4 twice

* Doc update only

* Document worst case allocations

and ipv6 can be at most of length 39, not 40

* Redundant IP copy, consistent IPv6 behavior, fix deep copy

* isDuplicate for SVCB

* Optimizations

* echoconfig

* Svc => SVCB

* Fix CI

* Regenerate after REBASE (2)

Rebased twice on 15th and 20th May

* Rename svc, use escapeByte.

* Fix parsing whitespaces between quotes, rename ECHOHOConfig

* resolve

Remove svcbFieldLen
Use reverseInt
Uppercase SVCB
Rename key_value
"invalid" => bad
Alpn comments
> 65535 check
Unneeded slices

* a little more

read => parse
IP array meaning
Force pushed because forgot to change read in svcb_test.go

* HTTPSSVC -> HTTPS

* Use new values

* mandatory code

MikeBishop/dns-alt-svc#205

* Resolve comments

Rename svcb-pairs
Remove SVCB_PRIVATE ranges
Comment on SVCB_KEY65535
ParseError return l.token
rename svcbKeyToString and svcbStringToKey
privatize SVCBKeyToString, SVCBStringToKey

* Refactor 1

Rename sorted, originalPairs
Use append instead of copy
Use svcb_RESERVED instead of 65535, with it now being private
"type SVCBKey uint16"

* Refactor 2

svcbKeyToString as method
svcbStringToKey updated after key 0
:bug: mandatory has missing key
Rename str
idx < 0

* Refactor 3

Use l.token as z
var key, value string
Comment wrap
0:
Sentences with '.'
keyValue => kv

* Refactor 4

* Refactor 5

len() int

* Refactor 6

* Refactor 7

* Test remove parsing

* Error messages

* Rewrite two estimate comments

* parse shouldn't modify original array :bug:

* Remove two unneeded comments

* Address review comments

Push 2 because can't build fuzzer python
Push 3 to try again

* Simplify argument duplication as per tmthrgd's suggestion

And add the relevant test
Force push edit: Make sorting code fit into one line

* Rewrite ECHConfig and address the review

* Remove the optional tab

* Add To4() Check

* More cleanup and fix mandatory not sorting bug

duplicate_test.go

@@ -34,6 +34,39 @@ func TestDuplicateTXT(t *testing.T) {
 	}
 }
 
+func TestDuplicateSVCB(t *testing.T) {
+	a1, _ := NewRR(`example.com. 3600 IN SVCB 1 . ipv6hint=1::3:3:3:3 key65300=\254\032\030\000\ \043,\;`)
+	a2, _ := NewRR(`example.com. 3600 IN SVCB 1 . ipv6hint=1:0::3:3:3:3 key65300="\254\ \030\000 +\,;"`)
+
+	if !IsDuplicate(a1, a2) {
+		t.Errorf("expected %s/%s to be duplicates, but got false", a1.String(), a2.String())
+	}
+
+	a2, _ = NewRR(`example.com. 3600 IN SVCB 1 . ipv6hint=1::3:3:3:3 key65300="\255\ \030\000 +\,;"`)
+
+	if IsDuplicate(a1, a2) {
+		t.Errorf("expected %s/%s not to be duplicates, but got true", a1.String(), a2.String())
+	}
+
+	a1, _ = NewRR(`example.com. 3600 IN SVCB 1 . ipv6hint=1::3:3:3:3`)
+
+	if IsDuplicate(a1, a2) {
+		t.Errorf("expected %s/%s not to be duplicates, but got true", a1.String(), a2.String())
+	}
+
+	a2, _ = NewRR(`example.com. 3600 IN SVCB 1 . ipv4hint=1.1.1.1`)
+
+	if IsDuplicate(a1, a2) {
+		t.Errorf("expected %s/%s not to be duplicates, but got true", a1.String(), a2.String())
+	}
+
+	a1, _ = NewRR(`example.com. 3600 IN SVCB 1 . ipv4hint=1.1.1.1,1.0.2.1`)
+
+	if IsDuplicate(a1, a2) {
+		t.Errorf("expected %s/%s not to be duplicates, but got true", a1.String(), a2.String())
+	}
+}
+
 func TestDuplicateOwner(t *testing.T) {
 	a1, _ := NewRR("www.example.org. IN A 127.0.0.1")
 	a2, _ := NewRR("www.example.org. IN A 127.0.0.1")

msg_helpers.go

@@ -6,6 +6,7 @@ import (
 	"encoding/binary"
 	"encoding/hex"
 	"net"
+	"sort"
 	"strings"
 )
 
@@ -612,6 +613,65 @@ func packDataNsec(bitmap []uint16, msg []byte, off int) (int, error) {
 	return off, nil
 }
 
+func unpackDataSVCB(msg []byte, off int) ([]SVCBKeyValue, int, error) {
+	var xs []SVCBKeyValue
+	var code uint16
+	var length uint16
+	var err error
+	for off < len(msg) {
+		code, off, err = unpackUint16(msg, off)
+		if err != nil {
+			return nil, len(msg), &Error{err: "overflow unpacking SVCB"}
+		}
+		length, off, err = unpackUint16(msg, off)
+		if err != nil || off+int(length) > len(msg) {
+			return nil, len(msg), &Error{err: "overflow unpacking SVCB"}
+		}
+		e := makeSVCBKeyValue(SVCBKey(code))
+		if e == nil {
+			return nil, len(msg), &Error{err: "bad SVCB key"}
+		}
+		if err := e.unpack(msg[off : off+int(length)]); err != nil {
+			return nil, len(msg), err
+		}
+		if len(xs) > 0 && e.Key() <= xs[len(xs)-1].Key() {
+			return nil, len(msg), &Error{err: "SVCB keys not in strictly increasing order"}
+		}
+		xs = append(xs, e)
+		off += int(length)
+	}
+	return xs, off, nil
+}
+
+func packDataSVCB(pairs []SVCBKeyValue, msg []byte, off int) (int, error) {
+	pairs = append([]SVCBKeyValue(nil), pairs...)
+	sort.Slice(pairs, func(i, j int) bool {
+		return pairs[i].Key() < pairs[j].Key()
+	})
+	prev := svcb_RESERVED
+	for _, el := range pairs {
+		if el.Key() == prev {
+			return len(msg), &Error{err: "repeated SVCB keys are not allowed"}
+		}
+		prev = el.Key()
+		packed, err := el.pack()
+		if err != nil {
+			return len(msg), err
+		}
+		off, err = packUint16(uint16(el.Key()), msg, off)
+		if err != nil {
+			return len(msg), &Error{err: "overflow packing SVCB"}
+		}
+		off, err = packUint16(uint16(len(packed)), msg, off)
+		if err != nil || off+len(packed) > len(msg) {
+			return len(msg), &Error{err: "overflow packing SVCB"}
+		}
+		copy(msg[off:off+len(packed)], packed)
+		off += len(packed)
+	}
+	return off, nil
+}
+
 func unpackDataDomainNames(msg []byte, off, end int) ([]string, int, error) {
 	var (
 		servers []string

parse_test.go

@@ -1633,6 +1633,70 @@ func TestParseCSYNC(t *testing.T) {
 	}
 }
 
+func TestParseSVCB(t *testing.T) {
+	svcbs := map[string]string{
+		`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="\ "`,
+	}
+	for s, o := range svcbs {
+		rr, err := NewRR(s)
+		if err != nil {
+			t.Error("failed to parse RR: ", err)
+			continue
+		}
+		if rr.String() != o {
+			t.Errorf("`%s' should be equal to\n`%s', but is     `%s'", s, o, rr.String())
+		}
+	}
+}
+
+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
+		`1 . key`,                 // invalid key
+		`1 . key=`,                // invalid key
+		`1 . =`,                   // invalid key
+		`1 . ==`,                  // invalid key
+		`1 . =a`,                  // invalid key
+		`1 . ""`,                  // invalid key
+		`1 . ""=`,                 // invalid key
+		`1 . "a"`,                 // invalid key
+		`1 . "a"=`,                // invalid key
+		`1 . key1=`,               // we know that key
+		`1 . key65535`,            // key reserved
+		`1 . key065534`,           // key can't be padded
+		`1 . key65534="f`,         // unterminated value
+		`1 . key65534="`,          // unterminated value
+		`1 . key65534=\2`,         // invalid numberic escape
+		`1 . key65534=\24`,        // invalid numberic escape
+		`1 . key65534=\256`,       // invalid numberic escape
+		`1 . key65534=\`,          // invalid numberic escape
+		`1 . key65534=""alpn`,     // zQuote ending needs whitespace
+		`1 . key65534="a"alpn`,    // zQuote ending needs whitespace
+		`1 . ipv6hint=1.1.1.1`,    // not ipv6
+		`1 . ipv6hint=1:1:1:1`,    // not ipv6
+		`1 . ipv6hint=a`,          // not 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 . port=`,               // empty port
+		`1 . echconfig=YUd`,       // bad base64
+	}
+	for _, o := range evils {
+		_, err := NewRR(header + o)
+		if err == nil {
+			t.Error("failed to reject invalid RR: ", header+o)
+			continue
+		}
+	}
+}
+
 func TestParseBadNAPTR(t *testing.T) {
 	// Should look like: mplus.ims.vodafone.com.	3600	IN	NAPTR	10 100 "S" "SIP+D2U" "" _sip._udp.mplus.ims.vodafone.com.
 	naptr := `mplus.ims.vodafone.com.	3600	IN	NAPTR	10 100 S SIP+D2U  _sip._udp.mplus.ims.vodafone.com.`