Properly parse alpn values in SVCB (#1363)
* Modify the SVCBAlpn to properly parse/print * Remove debug * Change SVCB test from reflect to loop * Refactor SVCB code to reduce indentation * When stringifying SVCBAlpn, use strings.Builder for whole process * Update comment in svcb.go Co-authored-by: Miek Gieben <miek@miek.nl> * Describe why we use a specific size for the alpn buffer Co-authored-by: Miek Gieben <miek@miek.nl>
This commit is contained in:
Shane Kerr
GitHub
parent
0d2c95b99c
commit
feda877277
|
|
@ -1650,8 +1650,8 @@ func TestParseSVCB(t *testing.T) {
|
|||
`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"`,
|
||||
`example.com. SVCB 16 foo.example.org. alpn="f\\\\oo\\,bar,h2"`: `example.com. 3600 IN SVCB 16 foo.example.org. alpn="f\\\092oo\\\044bar,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\\\044bar,h2"`,
|
||||
// From draft-ietf-add-ddr-06
|
||||
`_dns.example.net. SVCB 1 example.net. alpn=h2 dohpath=/dns-query{?dns}`: `_dns.example.net. 3600 IN SVCB 1 example.net. alpn="h2" dohpath="/dns-query{?dns}"`,
|
||||
}
|
||||
|
|
@ -1704,6 +1704,10 @@ func TestParseBadSVCB(t *testing.T) {
|
|||
`1 . ipv4hint=`, // empty ipv4
|
||||
`1 . port=`, // empty port
|
||||
`1 . echconfig=YUd`, // bad base64
|
||||
`1 . alpn=h\`, // unterminated escape
|
||||
`1 . alpn=h2\\.h3`, // comma-separated list with bad character
|
||||
`1 . alpn=h2,,h3`, // empty protocol identifier
|
||||
`1 . alpn=h3,`, // final protocol identifier empty
|
||||
}
|
||||
for _, o := range evils {
|
||||
_, err := NewRR(header + o)
|
||||
|
|
|
|||
90
svcb.go
90
svcb.go
|
|
@ -342,13 +342,57 @@ func (s *SVCBMandatory) copy() SVCBKeyValue {
|
|||
// h.Hdr = dns.RR_Header{Name: ".", Rrtype: dns.TypeHTTPS, Class: dns.ClassINET}
|
||||
// e := new(dns.SVCBAlpn)
|
||||
// e.Alpn = []string{"h2", "http/1.1"}
|
||||
// h.Value = append(o.Value, e)
|
||||
// h.Value = append(h.Value, e)
|
||||
type SVCBAlpn struct {
|
||||
Alpn []string
|
||||
}
|
||||
|
||||
func (*SVCBAlpn) Key() SVCBKey { return SVCB_ALPN }
|
||||
func (s *SVCBAlpn) String() string { return strings.Join(s.Alpn, ",") }
|
||||
|
||||
func (s *SVCBAlpn) String() string {
|
||||
// An ALPN value is a comma-separated list of values, each of which can be
|
||||
// an arbitrary binary value. In order to allow parsing, the comma and
|
||||
// backslash characters are themselves excaped.
|
||||
//
|
||||
// However, this escaping is done in addition to the normal escaping which
|
||||
// happens in zone files, meaning that these values must be
|
||||
// double-escaped. This looks terrible, so if you see a never-ending
|
||||
// sequence of backslash in a zone file this may be why.
|
||||
//
|
||||
// https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-08#appendix-A.1
|
||||
var str strings.Builder
|
||||
for i, alpn := range s.Alpn {
|
||||
// 4*len(alpn) is the worst case where we escape every character in the alpn as \123, plus 1 byte for the ',' separating the alpn from others
|
||||
str.Grow(4*len(alpn) + 1)
|
||||
if i > 0 {
|
||||
str.WriteByte(',')
|
||||
}
|
||||
for j := 0; j < len(alpn); j++ {
|
||||
e := alpn[j]
|
||||
if ' ' > e || e > '~' {
|
||||
str.WriteString(escapeByte(e))
|
||||
continue
|
||||
}
|
||||
switch e {
|
||||
// We escape a few characters which may confuse humans or parsers.
|
||||
case '"', ';', ' ':
|
||||
str.WriteByte('\\')
|
||||
str.WriteByte(e)
|
||||
// The comma and backslash characters themselves must be
|
||||
// doubly-escaped. We use `\\` for the first backslash and
|
||||
// the escaped numeric value for the other value. We especially
|
||||
// don't want a comma in the output.
|
||||
case ',':
|
||||
str.WriteString(`\\\044`)
|
||||
case '\\':
|
||||
str.WriteString(`\\\092`)
|
||||
default:
|
||||
str.WriteByte(e)
|
||||
}
|
||||
}
|
||||
}
|
||||
return str.String()
|
||||
}
|
||||
|
||||
func (s *SVCBAlpn) pack() ([]byte, error) {
|
||||
// Liberally estimate the size of an alpn as 10 octets
|
||||
|
|
@ -383,7 +427,47 @@ func (s *SVCBAlpn) unpack(b []byte) error {
|
|||
}
|
||||
|
||||
func (s *SVCBAlpn) parse(b string) error {
|
||||
s.Alpn = strings.Split(b, ",")
|
||||
if len(b) == 0 {
|
||||
s.Alpn = []string{}
|
||||
return nil
|
||||
}
|
||||
|
||||
alpn := []string{}
|
||||
a := []byte{}
|
||||
for p := 0; p < len(b); {
|
||||
c, q := nextByte(b, p)
|
||||
if q == 0 {
|
||||
return errors.New("dns: svcbalpn: unterminated escape")
|
||||
}
|
||||
p += q
|
||||
// If we find a comma, we have finished reading an alpn.
|
||||
if c == ',' {
|
||||
if len(a) == 0 {
|
||||
return errors.New("dns: svcbalpn: empty protocol identifier")
|
||||
}
|
||||
alpn = append(alpn, string(a))
|
||||
a = []byte{}
|
||||
continue
|
||||
}
|
||||
// If it's a backslash, we need to handle a comma-separated list.
|
||||
if c == '\\' {
|
||||
dc, dq := nextByte(b, p)
|
||||
if dq == 0 {
|
||||
return errors.New("dns: svcbalpn: unterminated escape decoding comma-separated list")
|
||||
}
|
||||
if dc != '\\' && dc != ',' {
|
||||
return errors.New("dns: svcbalpn: bad escaped character decoding comma-separated list")
|
||||
}
|
||||
p += dq
|
||||
c = dc
|
||||
}
|
||||
a = append(a, c)
|
||||
}
|
||||
// Add the final alpn.
|
||||
if len(a) == 0 {
|
||||
return errors.New("dns: svcbalpn: last protocol identifier empty")
|
||||
}
|
||||
s.Alpn = append(alpn, string(a))
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
|
|||
42
svcb_test.go
42
svcb_test.go
|
|
@ -95,6 +95,48 @@ func TestDecodeBadSVCB(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestPresentationSVCBAlpn(t *testing.T) {
|
||||
tests := map[string]string{
|
||||
"h2": "h2",
|
||||
"http": "http",
|
||||
"\xfa": `\250`,
|
||||
"some\"other,chars": `some\"other\\\044chars`,
|
||||
}
|
||||
for input, want := range tests {
|
||||
e := new(SVCBAlpn)
|
||||
e.Alpn = []string{input}
|
||||
if e.String() != want {
|
||||
t.Errorf("improper conversion with String(), wanted %v got %v", want, e.String())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSVCBAlpn(t *testing.T) {
|
||||
tests := map[string][]string{
|
||||
`. 1 IN SVCB 10 one.test. alpn=h2`: {"h2"},
|
||||
`. 2 IN SVCB 20 two.test. alpn=h2,h3-19`: {"h2", "h3-19"},
|
||||
`. 3 IN SVCB 30 three.test. alpn="f\\\\oo\\,bar,h2"`: {`f\oo,bar`, "h2"},
|
||||
`. 4 IN SVCB 40 four.test. alpn="part1,part2,part3\\,part4\\\\"`: {"part1", "part2", `part3,part4\`},
|
||||
`. 5 IN SVCB 50 five.test. alpn=part1\,\p\a\r\t2\044part3\092,part4\092\\`: {"part1", "part2", `part3,part4\`},
|
||||
}
|
||||
for s, v := range tests {
|
||||
rr, err := NewRR(s)
|
||||
if err != nil {
|
||||
t.Error("failed to parse RR: ", err)
|
||||
continue
|
||||
}
|
||||
alpn := rr.(*SVCB).Value[0].(*SVCBAlpn).Alpn
|
||||
if len(v) != len(alpn) {
|
||||
t.Fatalf("parsing alpn failed, wanted %v got %v", v, alpn)
|
||||
}
|
||||
for i := range v {
|
||||
if v[i] != alpn[i] {
|
||||
t.Fatalf("parsing alpn failed, wanted %v got %v", v, alpn)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestCompareSVCB(t *testing.T) {
|
||||
val1 := []SVCBKeyValue{
|
||||
&SVCBPort{
|
||||
|
|
|
|||
Loading…
Reference in New Issue