From abf0999fee64071e7e494d77a0acb866a155109d Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Sun, 20 Feb 2022 14:37:09 +0000 Subject: [PATCH 01/10] Rename ECH, bump draft number --- svcb.go | 48 ++++++++++++++++++++++++------------------------ svcb_test.go | 2 +- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/svcb.go b/svcb.go index 3344253c2..d1a396e1d 100644 --- a/svcb.go +++ b/svcb.go @@ -13,14 +13,14 @@ 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_ECH SVCBKey = 5 SVCB_IPV6HINT SVCBKey = 6 svcb_RESERVED SVCBKey = 65535 ) @@ -31,7 +31,7 @@ var svcbKeyToStringMap = map[SVCBKey]string{ SVCB_NO_DEFAULT_ALPN: "no-default-alpn", SVCB_PORT: "port", SVCB_IPV4HINT: "ipv4hint", - SVCB_ECHCONFIG: "echconfig", + SVCB_ECH: "ech", SVCB_IPV6HINT: "ipv6hint", } @@ -187,8 +187,8 @@ func makeSVCBKeyValue(key SVCBKey) SVCBKeyValue { return new(SVCBPort) case SVCB_IPV4HINT: return new(SVCBIPv4Hint) - case SVCB_ECHCONFIG: - return new(SVCBECHConfig) + case SVCB_ECH: + return new(SVCBECH) case SVCB_IPV6HINT: return new(SVCBIPv6Hint) case svcb_RESERVED: @@ -200,7 +200,7 @@ 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 @@ -522,42 +522,42 @@ 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: +// SVCBECH pair contains the ECHConfig structure defined in draft-ietf-tls-esni [RFC xxxx]. +// Basic use pattern for creating an ech option: // // h := new(dns.HTTPS) // h.Hdr = dns.RR_Header{Name: ".", Rrtype: dns.TypeHTTPS, Class: dns.ClassINET} -// e := new(dns.SVCBECHConfig) +// e := new(dns.SVCBECH) // e.ECH = []byte{0xfe, 0x08, ...} // h.Value = append(h.Value, e) -type SVCBECHConfig struct { - ECH []byte +type SVCBECH struct { + ECHConfigList []byte // includes the redundant length prefix } -func (*SVCBECHConfig) Key() SVCBKey { return SVCB_ECHCONFIG } -func (s *SVCBECHConfig) String() string { return toBase64(s.ECH) } -func (s *SVCBECHConfig) len() int { return len(s.ECH) } +func (*SVCBECH) Key() SVCBKey { return SVCB_ECH } +func (s *SVCBECH) String() string { return toBase64(s.ECHConfigList) } +func (s *SVCBECH) len() int { return len(s.ECHConfigList) } -func (s *SVCBECHConfig) pack() ([]byte, error) { - return append([]byte(nil), s.ECH...), nil +func (s *SVCBECH) pack() ([]byte, error) { + return append([]byte(nil), s.ECHConfigList...), nil } -func (s *SVCBECHConfig) copy() SVCBKeyValue { - return &SVCBECHConfig{ - append([]byte(nil), s.ECH...), +func (s *SVCBECH) copy() SVCBKeyValue { + return &SVCBECH{ + append([]byte(nil), s.ECHConfigList...), } } -func (s *SVCBECHConfig) unpack(b []byte) error { - s.ECH = append([]byte(nil), b...) +func (s *SVCBECH) unpack(b []byte) error { + s.ECHConfigList = append([]byte(nil), b...) return nil } -func (s *SVCBECHConfig) parse(b string) error { +func (s *SVCBECH) 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 + s.ECHConfigList = x return nil } diff --git a/svcb_test.go b/svcb_test.go index 6994aca59..254949954 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`, ``}, From 22417486d5af189213233c25aa18646142e26592 Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Sun, 20 Feb 2022 16:50:01 +0000 Subject: [PATCH 02/10] AliasForm new treatment --- parse_test.go | 6 +++++- svcb.go | 10 ++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/parse_test.go b/parse_test.go index 6c55a74ad..49397b601 100644 --- a/parse_test.go +++ b/parse_test.go @@ -1639,6 +1639,11 @@ 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="\ "`, + // "In AliasMode, records SHOULD NOT include any SvcParams, and recipients MUST + // ignore any SvcParams that are present." + // and what we do is to pass it on, which is the correct behavior to encourage + // fixing the source of this + `example.com. 3600 IN SVCB 0 no-default-alpn`: `example.com. 3600 IN SVCB 0 no-default-alpn.`, } for s, o := range svcbs { rr, err := NewRR(s) @@ -1655,7 +1660,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 diff --git a/svcb.go b/svcb.go index d1a396e1d..eaeb17c59 100644 --- a/svcb.go +++ b/svcb.go @@ -167,10 +167,10 @@ func (rr *SVCB) parse(c *zlexer, o string) *ParseError { } l, _ = c.Next() } + + // Don't check rr.Priority == 0 && len(xs) > 0 + // as explained in parse_test.go rr.Value = xs - if rr.Priority == 0 && len(xs) > 0 { - return &ParseError{l.token, "SVCB aliasform can't have values", l} - } return nil } @@ -205,7 +205,9 @@ type SVCB struct { Hdr RR_Header Priority uint16 Target string `dns:"domain-name"` - Value []SVCBKeyValue `dns:"pairs"` // Value must be empty if Priority is zero. + Value []SVCBKeyValue `dns:"pairs"` + // If Priority is zero, Value should be empty. + // If it is not, when receiving, Value must be considered empty. } // HTTPS RR. Everything valid for SVCB applies to HTTPS as well. From 6f736b7762b8ec6ea0249798863283bec0a04dd7 Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Sun, 20 Feb 2022 18:25:45 +0000 Subject: [PATCH 03/10] alpn is no longer mandatory by default --- svcb.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/svcb.go b/svcb.go index eaeb17c59..23507309f 100644 --- a/svcb.go +++ b/svcb.go @@ -237,7 +237,16 @@ 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: +// - "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 +// // Basic use pattern for creating a mandatory option: // // s := &dns.SVCB{Hdr: dns.RR_Header{Name: ".", Rrtype: dns.TypeSVCB, Class: dns.ClassINET}} @@ -245,7 +254,7 @@ type SVCBKeyValue interface { // e.Code = []uint16{65403} // s.Value = append(s.Value, e) type SVCBMandatory struct { - Code []SVCBKey // Must not include mandatory + Code []SVCBKey } func (*SVCBMandatory) Key() SVCBKey { return SVCB_MANDATORY } @@ -372,6 +381,7 @@ 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}} @@ -387,14 +397,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 } From f0320fc477c510b3addc852efbd681237c03aa34 Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Sun, 20 Feb 2022 18:56:20 +0000 Subject: [PATCH 04/10] Document the non-empty value requirement --- parse_test.go | 4 +++- svcb.go | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/parse_test.go b/parse_test.go index 49397b601..37c2c8b6c 100644 --- a/parse_test.go +++ b/parse_test.go @@ -1642,7 +1642,7 @@ func TestParseSVCB(t *testing.T) { // "In AliasMode, records SHOULD NOT include any SvcParams, and recipients MUST // ignore any SvcParams that are present." // and what we do is to pass it on, which is the correct behavior to encourage - // fixing the source of this + // fixing its cause `example.com. 3600 IN SVCB 0 no-default-alpn`: `example.com. 3600 IN SVCB 0 no-default-alpn.`, } for s, o := range svcbs { @@ -1686,10 +1686,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/svcb.go b/svcb.go index 23507309f..a402afc30 100644 --- a/svcb.go +++ b/svcb.go @@ -241,11 +241,13 @@ type SVCBKeyValue interface { // "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: +// 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: // @@ -313,6 +315,7 @@ func (s *SVCBMandatory) copy() SVCBKeyValue { } // SVCBAlpn pair is used to list supported connection protocols. +// 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: From 0858508b1e78d279e5bdf6d9a26fd01719cbaddc Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Sun, 20 Feb 2022 19:25:32 +0000 Subject: [PATCH 05/10] new test cases --- parse_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/parse_test.go b/parse_test.go index 37c2c8b6c..52ed1244d 100644 --- a/parse_test.go +++ b/parse_test.go @@ -1644,6 +1644,11 @@ func TestParseSVCB(t *testing.T) { // and what we do is to pass it on, which is the correct behavior to encourage // fixing its cause `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"`, } for s, o := range svcbs { rr, err := NewRR(s) From 330adef8a80ea7639a2754b6895ec16a342a6d3d Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Sun, 20 Feb 2022 19:36:08 +0000 Subject: [PATCH 06/10] more test cases --- parse_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/parse_test.go b/parse_test.go index 52ed1244d..6c43e01ce 100644 --- a/parse_test.go +++ b/parse_test.go @@ -1649,6 +1649,12 @@ func TestParseSVCB(t *testing.T) { `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="::ffff:198.51.100.100"`: ``, + `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) From b18e32ecb62f19cb2a183d2c812dd27bbe6e4052 Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Mon, 21 Feb 2022 18:09:11 +0000 Subject: [PATCH 07/10] Continue forbidding v4-map-v6 but not v4-embed-v6 https://github.com/miekg/dns/pull/1067#discussion_r495556735 and https://github.com/MikeBishop/dns-alt-svc/issues/361 --- parse_test.go | 2 +- svcb.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/parse_test.go b/parse_test.go index 6c43e01ce..f6bc373ca 100644 --- a/parse_test.go +++ b/parse_test.go @@ -1651,7 +1651,7 @@ func TestParseSVCB(t *testing.T) { `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="::ffff:198.51.100.100"`: ``, + `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"`, diff --git a/svcb.go b/svcb.go index a402afc30..531ee1c41 100644 --- a/svcb.go +++ b/svcb.go @@ -633,9 +633,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 { @@ -643,6 +640,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 From 38baee21c1854fc53b324d022cb0803f843a436e Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Mon, 21 Feb 2022 18:28:51 +0000 Subject: [PATCH 08/10] Update documentation --- svcb.go | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/svcb.go b/svcb.go index 531ee1c41..8f9383dcc 100644 --- a/svcb.go +++ b/svcb.go @@ -15,14 +15,15 @@ type SVCBKey uint16 // 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_ECH 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_ECH + SVCB_IPV6HINT + + svcb_RESERVED SVCBKey = 65535 ) var svcbKeyToStringMap = map[SVCBKey]string{ @@ -203,11 +204,9 @@ func makeSVCBKeyValue(key SVCBKey) SVCBKeyValue { // 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 / discarded Target string `dns:"domain-name"` Value []SVCBKeyValue `dns:"pairs"` - // If Priority is zero, Value should be empty. - // If it is not, when receiving, Value must be considered empty. } // HTTPS RR. Everything valid for SVCB applies to HTTPS as well. @@ -253,8 +252,11 @@ type SVCBKeyValue interface { // // 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 } @@ -316,7 +318,7 @@ func (s *SVCBMandatory) copy() SVCBKeyValue { // SVCBAlpn pair is used to list supported connection protocols. // The user of this library must ensure that at least one protocol is listed when alpn is present. -// Protocol ids can be found at: +// 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: // @@ -388,6 +390,9 @@ func (s *SVCBAlpn) copy() SVCBKeyValue { // 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{} @@ -543,7 +548,7 @@ func (s *SVCBIPv4Hint) copy() SVCBKeyValue { // h := new(dns.HTTPS) // h.Hdr = dns.RR_Header{Name: ".", Rrtype: dns.TypeHTTPS, Class: dns.ClassINET} // e := new(dns.SVCBECH) -// e.ECH = []byte{0xfe, 0x08, ...} +// e.ECHConfigList = []byte{0xfe, 0x08, ...} // h.Value = append(h.Value, e) type SVCBECH struct { ECHConfigList []byte // includes the redundant length prefix From b2858a8c763bb1165e0b71d9021b22e180d186d3 Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Sat, 12 Mar 2022 09:32:14 +0000 Subject: [PATCH 09/10] revert rename ech --- svcb.go | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/svcb.go b/svcb.go index 8f9383dcc..0e733b156 100644 --- a/svcb.go +++ b/svcb.go @@ -20,7 +20,7 @@ const ( SVCB_NO_DEFAULT_ALPN SVCB_PORT SVCB_IPV4HINT - SVCB_ECH + SVCB_ECHCONFIG SVCB_IPV6HINT svcb_RESERVED SVCBKey = 65535 @@ -32,7 +32,7 @@ var svcbKeyToStringMap = map[SVCBKey]string{ SVCB_NO_DEFAULT_ALPN: "no-default-alpn", SVCB_PORT: "port", SVCB_IPV4HINT: "ipv4hint", - SVCB_ECH: "ech", + SVCB_ECHCONFIG: "ech", SVCB_IPV6HINT: "ipv6hint", } @@ -188,8 +188,8 @@ func makeSVCBKeyValue(key SVCBKey) SVCBKeyValue { return new(SVCBPort) case SVCB_IPV4HINT: return new(SVCBIPv4Hint) - case SVCB_ECH: - return new(SVCBECH) + case SVCB_ECHCONFIG: + return new(SVCBECHConfig) case SVCB_IPV6HINT: return new(SVCBIPv6Hint) case svcb_RESERVED: @@ -542,42 +542,42 @@ func (s *SVCBIPv4Hint) copy() SVCBKeyValue { } } -// SVCBECH pair contains the ECHConfig structure defined in draft-ietf-tls-esni [RFC xxxx]. +// SVCBECHConfig pair contains the ECHConfig structure defined in draft-ietf-tls-esni [RFC xxxx]. // Basic use pattern for creating an ech option: // // h := new(dns.HTTPS) // h.Hdr = dns.RR_Header{Name: ".", Rrtype: dns.TypeHTTPS, Class: dns.ClassINET} -// e := new(dns.SVCBECH) -// e.ECHConfigList = []byte{0xfe, 0x08, ...} +// e := new(dns.SVCBECHConfig) +// e.ECH = []byte{0xfe, 0x08, ...} // h.Value = append(h.Value, e) -type SVCBECH struct { - ECHConfigList []byte // includes the redundant length prefix +type SVCBECHConfig struct { + ECH []byte // Specifically ECHConfigList including the redundant length prefix } -func (*SVCBECH) Key() SVCBKey { return SVCB_ECH } -func (s *SVCBECH) String() string { return toBase64(s.ECHConfigList) } -func (s *SVCBECH) len() int { return len(s.ECHConfigList) } +func (*SVCBECHConfig) Key() SVCBKey { return SVCB_ECHCONFIG } +func (s *SVCBECHConfig) String() string { return toBase64(s.ECH) } +func (s *SVCBECHConfig) len() int { return len(s.ECH) } -func (s *SVCBECH) pack() ([]byte, error) { - return append([]byte(nil), s.ECHConfigList...), nil +func (s *SVCBECHConfig) pack() ([]byte, error) { + return append([]byte(nil), s.ECH...), nil } -func (s *SVCBECH) copy() SVCBKeyValue { - return &SVCBECH{ - append([]byte(nil), s.ECHConfigList...), +func (s *SVCBECHConfig) copy() SVCBKeyValue { + return &SVCBECHConfig{ + append([]byte(nil), s.ECH...), } } -func (s *SVCBECH) unpack(b []byte) error { - s.ECHConfigList = append([]byte(nil), b...) +func (s *SVCBECHConfig) unpack(b []byte) error { + s.ECH = append([]byte(nil), b...) return nil } -func (s *SVCBECH) parse(b string) error { +func (s *SVCBECHConfig) parse(b string) error { x, err := fromBase64([]byte(b)) if err != nil { return errors.New("dns: svcbech: bad base64 ech") } - s.ECHConfigList = x + s.ECH = x return nil } From 8c07a31a71e95f89b50f55c2dc1988e85a36875f Mon Sep 17 00:00:00 2001 From: DesWurstes Date: Sat, 12 Mar 2022 10:20:30 +0000 Subject: [PATCH 10/10] Reword AliasMode with key=value pairs --- parse_test.go | 5 +---- svcb.go | 10 +++++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/parse_test.go b/parse_test.go index f6bc373ca..444ac1e5a 100644 --- a/parse_test.go +++ b/parse_test.go @@ -1639,10 +1639,7 @@ 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="\ "`, - // "In AliasMode, records SHOULD NOT include any SvcParams, and recipients MUST - // ignore any SvcParams that are present." - // and what we do is to pass it on, which is the correct behavior to encourage - // fixing its cause + // 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.`, diff --git a/svcb.go b/svcb.go index 0e733b156..ff5e01086 100644 --- a/svcb.go +++ b/svcb.go @@ -169,8 +169,12 @@ func (rr *SVCB) parse(c *zlexer, o string) *ParseError { l, _ = c.Next() } - // Don't check rr.Priority == 0 && len(xs) > 0 - // as explained in parse_test.go + // "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 return nil } @@ -204,7 +208,7 @@ func makeSVCBKeyValue(key SVCBKey) SVCBKeyValue { // SVCB RR. See RFC xxxx (https://tools.ietf.org/html/draft-ietf-dnsop-svcb-https-08). type SVCB struct { Hdr RR_Header - Priority uint16 // If zero, Value must be empty / discarded + 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"` }