Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update SVCB #1341

Merged
merged 10 commits into from
Apr 1, 2022
+68 −30
Merged

Update SVCB #1341

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
abf0999
Rename ECH, bump draft number
DesWurstes Feb 20, 2022
2241748
AliasForm new treatment
DesWurstes Feb 20, 2022
6f736b7
alpn is no longer mandatory by default
DesWurstes Feb 20, 2022
f0320fc
Document the non-empty value requirement
DesWurstes Feb 20, 2022
0858508
new test cases
DesWurstes Feb 20, 2022
330adef
more test cases
DesWurstes Feb 20, 2022
b18e32e
Continue forbidding v4-map-v6 but not v4-embed-v6
DesWurstes Feb 21, 2022
38baee2
Update documentation
DesWurstes Feb 21, 2022
b2858a8
revert rename ech
DesWurstes Mar 12, 2022
8c07a31
Reword AliasMode with key=value pairs
DesWurstes Mar 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion parse_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1639,6 +1639,22 @@ 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
`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="::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)
Expand All @@ -1655,7 +1671,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
Expand All @@ -1682,10 +1697,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
}
Expand Down
79 changes: 47 additions & 32 deletions svcb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
// Keys defined in draft-ietf-dnsop-svcb-https-08 Section 14.3.2.
const (
SVCB_MANDATORY SVCBKey = 0
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
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
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
svcb_RESERVED SVCBKey = 65535
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
)
Expand All @@ -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",
}

Expand Down Expand Up @@ -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
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
rr.Value = xs
if rr.Priority == 0 && len(xs) > 0 {
return &ParseError{l.token, "SVCB aliasform can't have values", l}
}
return nil
}

Expand All @@ -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:
Expand All @@ -200,12 +200,14 @@ 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
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.
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
// If it is not, when receiving, Value must be considered empty.
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
}

// HTTPS RR. Everything valid for SVCB applies to HTTPS as well.
Expand Down Expand Up @@ -235,15 +237,26 @@ 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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want this to make a list in godoc you need empty lines before and after. Maybe just make this more prose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't turn this list into a paragraph; it would be unreadable.

// - 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}
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
// s.Value = append(s.Value, e)
type SVCBMandatory struct {
Code []SVCBKey // Must not include mandatory
Code []SVCBKey
}

func (*SVCBMandatory) Key() SVCBKey { return SVCB_MANDATORY }
Expand Down Expand Up @@ -302,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:
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
// https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids
// Basic use pattern for creating an alpn option:
Expand Down Expand Up @@ -370,6 +384,7 @@ func (s *SVCBAlpn) copy() SVCBKeyValue {
}

// SVCBNoDefaultAlpn pair signifies no support for default connection protocols.
// Should be used in conjunction with alpn.
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
// Basic use pattern for creating a no-default-alpn option:
//
// s := &dns.SVCB{Hdr: dns.RR_Header{Name: ".", Rrtype: dns.TypeSVCB, Class: dns.ClassINET}}
Expand All @@ -385,14 +400,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
}
Expand Down Expand Up @@ -522,42 +537,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, ...}
DesWurstes marked this conversation as resolved.
Show resolved Hide resolved
// h.Value = append(h.Value, e)
type SVCBECHConfig struct {
ECH []byte
type SVCBECH struct {
ECHConfigList []byte // includes the redundant length prefix
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is too long. Just Config? What does the draft use?

Copy link
Contributor Author

@DesWurstes DesWurstes Feb 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The draft uses ECHConfigList which is taken from the RFC draft of ECH/ESNI. Makes sense to keep the original name so that an ESNI dev can easily see what this represents.

}

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
}

Expand Down
2 changes: 1 addition & 1 deletion svcb_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`, ``},
Expand Down