From 6ea36133dca2f95930925bf260bde5e622a5fd6a Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 28 Feb 2025 11:01:54 -0700 Subject: [PATCH] Augment, don't overwrite, HTTPS records --- caddyconfig/httpcaddyfile/builtins.go | 31 ---- go.mod | 2 - go.sum | 4 - modules/caddytls/ech.go | 225 ++++++++++++++++++++++++-- modules/caddytls/ech_test.go | 129 +++++++++++++++ modules/caddytls/tls.go | 4 +- 6 files changed, 344 insertions(+), 51 deletions(-) create mode 100644 modules/caddytls/ech_test.go diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index fb8f6dcb9..45570d016 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/caddyconfig/httpcaddyfile/builtins.go @@ -112,9 +112,6 @@ func parseBind(h Helper) ([]ConfigValue, error) { // issuer [...] // get_certificate [...] // insecure_secrets_log -// ech { -// dns ... -// } // } func parseTLS(h Helper) ([]ConfigValue, error) { h.Next() // consume directive name @@ -464,34 +461,6 @@ func parseTLS(h Helper) ([]ConfigValue, error) { } cp.InsecureSecretsLog = h.Val() - // case "ech": - // if !h.NextArg() { - // return nil, h.ArgErr() - // } - // if cp.EncryptedClientHello == nil { - // cp.EncryptedClientHello = new(caddytls.ECH) - // } - // cp.EncryptedClientHello.PublicName = h.Val() - - // for nesting := h.Nesting(); h.NextBlock(nesting); { - // switch h.Val() { - // case "dns": - // if !h.Next() { - // return nil, h.ArgErr() - // } - // providerName := h.Val() - // modID := "dns.providers." + providerName - // unm, err := caddyfile.UnmarshalModule(h.Dispenser, modID) - // if err != nil { - // return nil, err - // } - // cp.EncryptedClientHello.DNSProviderRaw = caddyconfig.JSONModuleObject(unm, "name", providerName, h.warnings) - // default: - // return nil, h.Errf("ech: unrecognized subdirective '%s'", h.Val()) - // } - // } - // log.Println("CP:", cp.EncryptedClientHello) - default: return nil, h.Errf("unknown subdirective: %s", h.Val()) } diff --git a/go.mod b/go.mod index 1dabeaabb..2f5a9552a 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/Masterminds/sprig/v3 v3.3.0 github.com/alecthomas/chroma/v2 v2.14.0 github.com/aryann/difflib v0.0.0-20210328193216-ff5ff6dc229b - github.com/caddy-dns/cloudflare v0.0.0-20250214163716-188b4850c0f2 github.com/caddyserver/certmagic v0.21.8-0.20250220203412-a7894dd6992d github.com/caddyserver/zerossl v0.1.3 github.com/cloudflare/circl v1.3.3 @@ -63,7 +62,6 @@ require ( github.com/google/go-tspi v0.3.0 // indirect github.com/google/pprof v0.0.0-20231212022811-ec68065c825e // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.22.0 // indirect - github.com/libdns/cloudflare v0.1.2 // indirect github.com/onsi/ginkgo/v2 v2.13.2 // indirect github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect diff --git a/go.sum b/go.sum index d0f83807d..e36ed911b 100644 --- a/go.sum +++ b/go.sum @@ -91,8 +91,6 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bradfitz/go-smtpd v0.0.0-20170404230938-deb6d6237625/go.mod h1:HYsPBTaaSFSlLx/70C2HPIMNZpVV8+vt/A+FMnYP11g= github.com/buger/jsonparser v0.0.0-20181115193947-bf1c66bbce23/go.mod h1:bbYlZJ7hK1yFx9hf58LP0zeX7UjIGs20ufpu3evjr+s= -github.com/caddy-dns/cloudflare v0.0.0-20250214163716-188b4850c0f2 h1:xsbs1fVM3S4bpUNS6WgDY2+Y8x89QnBxbQlYRXVuGms= -github.com/caddy-dns/cloudflare v0.0.0-20250214163716-188b4850c0f2/go.mod h1:R1888jq72Vx2kW4kEaqwgZ2YlBkx9YK3g3w2n/PvZjI= github.com/caddyserver/certmagic v0.21.8-0.20250220203412-a7894dd6992d h1:9zdfQHH838+rS8pmJ73/RSjpbfHGAyxRX1E79F+1zso= github.com/caddyserver/certmagic v0.21.8-0.20250220203412-a7894dd6992d/go.mod h1:LCPG3WLxcnjVKl/xpjzM0gqh0knrKKKiO5WVttX2eEI= github.com/caddyserver/zerossl v0.1.3 h1:onS+pxp3M8HnHpN5MMbOMyNjmTheJyWRaZYwn+YTAyA= @@ -330,8 +328,6 @@ github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/lib/pq v1.10.2/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= -github.com/libdns/cloudflare v0.1.2 h1:RWUqBSojAFpg2O/jzS29DnkCP9oWQj3LmNEU8OulTLs= -github.com/libdns/cloudflare v0.1.2/go.mod h1:XbvSCSMcxspwpSialM3bq0LsS3/Houy9WYxW8Ok8b6M= github.com/libdns/libdns v0.2.3 h1:ba30K4ObwMGB/QTmqUxf3H4/GmUrCAIkMWejeGl12v8= github.com/libdns/libdns v0.2.3/go.mod h1:4Bj9+5CQiNMVGf87wjX4CY3HQJypUHRuLvlsfsZqLWQ= github.com/lunixbochs/vtclean v1.0.0/go.mod h1:pHhQNgMf3btfWnGBVipUOjRYhoOsdGqdm/+2c2E2WMI= diff --git a/modules/caddytls/ech.go b/modules/caddytls/ech.go index 10831572b..fd260e8c4 100644 --- a/modules/caddytls/ech.go +++ b/modules/caddytls/ech.go @@ -347,11 +347,17 @@ type ECHPublication struct { type ECHDNSPublisher struct { // The DNS provider module which will establish the HTTPS record(s). ProviderRaw json.RawMessage `json:"provider,omitempty" caddy:"namespace=dns.providers inline_key=name"` - provider libdns.RecordSetter + provider ECHDNSProvider logger *zap.Logger } +// ECHDNSProvider can service DNS entries for ECH purposes. +type ECHDNSProvider interface { + libdns.RecordGetter + libdns.RecordSetter +} + // ECHDNSPublisherList is a list of DNS publication configs, // so that different groups of domain names may have ECH configs // published across different DNS providers, if necessary. @@ -376,11 +382,11 @@ func (dnsPubList ECHDNSPublisherList) Provision(ctx caddy.Context) error { if err != nil { return fmt.Errorf("loading ECH DNS provider module: %v", err) } - recSet, ok := dnsProvMod.(libdns.RecordSetter) + prov, ok := dnsProvMod.(ECHDNSProvider) if !ok { - return fmt.Errorf("ECH DNS provider module is not a RecordSetter: %v", err) + return fmt.Errorf("ECH DNS provider module is not an ECH DNS Provider: %v", err) } - dnsPubList[i].provider = recSet + dnsPubList[i].provider = prov dnsPubList[i].logger = ctx.Logger() } return nil @@ -406,13 +412,47 @@ func (dnsPub *ECHDNSPublisher) PublishECHConfigList(ctx context.Context, innerNa zap.Error(err)) continue } + + // get any existing HTTPS record for this domain, and augment + // our ech SvcParamKey with any other existing SvcParams + recs, err := dnsPub.provider.GetRecords(ctx, zone) + if err != nil { + dnsPub.logger.Error("unable to get existing DNS records to publish ECH data to HTTPS DNS record", + zap.String("domain", domain), + zap.Error(err)) + continue + } + relName := libdns.RelativeName(domain+".", zone) + var httpsRec libdns.Record + for _, rec := range recs { + if rec.Name == relName && rec.Type == "HTTPS" && (rec.Target == "" || rec.Target == ".") { + httpsRec = rec + } + } + params := make(svcParams) + if httpsRec.Value != "" { + params, err = parseSvcParams(httpsRec.Value) + if err != nil { + dnsPub.logger.Error("unable to parse existing DNS record to publish ECH data to HTTPS DNS record", + zap.String("domain", domain), + zap.String("https_rec_value", httpsRec.Value), + zap.Error(err)) + continue + } + } + + // overwrite only the ech SvcParamKey + params["ech"] = []string{base64.StdEncoding.EncodeToString(configListBin)} + + // publish record _, err = dnsPub.provider.SetRecords(ctx, zone, []libdns.Record{ { + // HTTPS and SVCB RRs: RFC 9460 (https://www.rfc-editor.org/rfc/rfc9460) Type: "HTTPS", - Name: libdns.RelativeName(domain+".", zone), - Priority: 1, // allows a manual override with priority 0 + Name: relName, + Priority: 2, // allows a manual override with priority 1 Target: ".", - Value: echSvcParam(configListBin), + Value: params.String(), TTL: 1 * time.Minute, // TODO: for testing only }, }) @@ -427,11 +467,6 @@ func (dnsPub *ECHDNSPublisher) PublishECHConfigList(ctx context.Context, innerNa return nil } -// SvcParam syntax is defined in RFC 9460: https://www.rfc-editor.org/rfc/rfc9460#presentation -func echSvcParam(echConfigListBinary []byte) string { - return fmt.Sprintf(`ech=%q`, base64.StdEncoding.EncodeToString(echConfigListBinary)) -} - // echConfig represents an ECHConfig from the specification, // [draft-ietf-tls-esni-22](https://www.ietf.org/archive/id/draft-ietf-tls-esni-22.html). type echConfig struct { @@ -653,6 +688,172 @@ func newECHConfigID(ctx caddy.Context) (uint8, error) { return 0, fmt.Errorf("depleted attempts to find an available config_id") } +// svcParams represents SvcParamKey and SvcParamValue pairs as +// described in https://www.rfc-editor.org/rfc/rfc9460 (section 2.1). +type svcParams map[string][]string + +// parseSvcParams parses service parameters into a structured type +// for safer manipulation. +func parseSvcParams(input string) (svcParams, error) { + if len(input) > 4096 { + return nil, fmt.Errorf("input too long: %d", len(input)) + } + + params := make(svcParams) + input = strings.TrimSpace(input) + " " + + for cursor := 0; cursor < len(input); cursor++ { + var key, rawVal string + + keyValPair: + for i := cursor; i < len(input); i++ { + switch input[i] { + case '=': + key = strings.ToLower(strings.TrimSpace(input[cursor:i])) + i++ + cursor = i + + var quoted bool + if input[cursor] == '"' { + quoted = true + i++ + cursor = i + } + + var escaped bool + + for j := cursor; j < len(input); j++ { + switch input[j] { + case '"': + if !quoted { + return nil, fmt.Errorf("illegal DQUOTE at position %d", j) + } + if !escaped { + // end of quoted value + rawVal = input[cursor:j] + j++ + cursor = j + break keyValPair + } + case '\\': + escaped = true + case ' ', '\t', '\n', '\r': + if !quoted { + // end of unquoted value + rawVal = input[cursor:j] + cursor = j + break keyValPair + } + default: + escaped = false + } + } + + case ' ', '\t', '\n', '\r': + // key with no value (flag) + key = input[cursor:i] + params[key] = []string{} + cursor = i + break keyValPair + } + } + + if rawVal == "" { + continue + } + + var sb strings.Builder + + var escape int // start of escape sequence (after \, so 0 is never a valid start) + for i := 0; i < len(rawVal); i++ { + ch := rawVal[i] + if escape > 0 { + // validate escape sequence + // (RFC 9460 Appendix A) + // escaped: "\" ( non-digit / dec-octet ) + // non-digit: "%x21-2F / %x3A-7E" + // dec-octet: "0-255 as a 3-digit decimal number" + if ch >= '0' && ch <= '9' { + // advance to end of decimal octet, which must be 3 digits + i += 2 + if i > len(rawVal) { + return nil, fmt.Errorf("value ends with incomplete escape sequence: %s", rawVal[escape:]) + } + decOctet, err := strconv.Atoi(rawVal[escape : i+1]) + if err != nil { + return nil, err + } + if decOctet < 0 || decOctet > 255 { + return nil, fmt.Errorf("invalid decimal octet in escape sequence: %s (%d)", rawVal[escape:i], decOctet) + } + sb.WriteRune(rune(decOctet)) + escape = 0 + continue + } else if (ch < 0x21 || ch > 0x2F) && (ch < 0x3A && ch > 0x7E) { + return nil, fmt.Errorf("illegal escape sequence %s", rawVal[escape:i]) + } + } + switch ch { + case ';', '(', ')': + // RFC 9460 Appendix A: + // > contiguous = 1*( non-special / escaped ) + // > non-special is VCHAR minus DQUOTE, ";", "(", ")", and "\". + return nil, fmt.Errorf("illegal character in value %q at position %d: %s", rawVal, i, string(ch)) + case '\\': + escape = i + 1 + default: + sb.WriteByte(ch) + escape = 0 + } + } + + params[key] = strings.Split(sb.String(), ",") + } + + return params, nil +} + +// String serializes svcParams into zone presentation format. +func (params svcParams) String() string { + var sb strings.Builder + for key, vals := range params { + if sb.Len() > 0 { + sb.WriteRune(' ') + } + sb.WriteString(key) + var hasVal, needsQuotes bool + for _, val := range vals { + if len(val) > 0 { + hasVal = true + } + if strings.ContainsAny(val, `" `) { + needsQuotes = true + } + if hasVal && needsQuotes { + break + } + } + if hasVal { + sb.WriteRune('=') + } + if needsQuotes { + sb.WriteRune('"') + } + for i, val := range vals { + if i > 0 { + sb.WriteRune(',') + } + val = strings.ReplaceAll(val, `"`, `\"`) + val = strings.ReplaceAll(val, `,`, `\,`) + sb.WriteString(val) + } + if needsQuotes { + sb.WriteRune('"') + } + } + return sb.String() +} + // ECHPublisher is an interface for publishing ECHConfigList values // so that they can be used by clients. type ECHPublisher interface { diff --git a/modules/caddytls/ech_test.go b/modules/caddytls/ech_test.go new file mode 100644 index 000000000..b722d2fbf --- /dev/null +++ b/modules/caddytls/ech_test.go @@ -0,0 +1,129 @@ +package caddytls + +import ( + "reflect" + "testing" +) + +func TestParseSvcParams(t *testing.T) { + for i, test := range []struct { + input string + expect svcParams + shouldErr bool + }{ + { + input: `alpn="h2,h3" no-default-alpn ipv6hint=2001:db8::1 port=443`, + expect: svcParams{ + "alpn": {"h2", "h3"}, + "no-default-alpn": {}, + "ipv6hint": {"2001:db8::1"}, + "port": {"443"}, + }, + }, + { + input: `key=value quoted="some string" flag`, + expect: svcParams{ + "key": {"value"}, + "quoted": {"some string"}, + "flag": {}, + }, + }, + { + input: `key="nested \"quoted\" value,foobar"`, + expect: svcParams{ + "key": {`nested "quoted" value`, "foobar"}, + }, + }, + { + input: `alpn=h3,h2 tls-supported-groups=29,23 no-default-alpn ech="foobar"`, + expect: svcParams{ + "alpn": {"h3", "h2"}, + "tls-supported-groups": {"29", "23"}, + "no-default-alpn": {}, + "ech": {"foobar"}, + }, + }, + { + input: `escape=\097`, + expect: svcParams{ + "escape": {"a"}, + }, + }, + { + input: `escapes=\097\098c`, + expect: svcParams{ + "escapes": {"abc"}, + }, + }, + } { + actual, err := parseSvcParams(test.input) + if err != nil && !test.shouldErr { + t.Errorf("Test %d: Expected no error, but got: %v (input=%q)", i, err, test.input) + continue + } else if err == nil && test.shouldErr { + t.Errorf("Test %d: Expected an error, but got no error (input=%q)", i, test.input) + continue + } + if !reflect.DeepEqual(test.expect, actual) { + t.Errorf("Test %d: Expected %v, got %v (input=%q)", i, test.expect, actual, test.input) + continue + } + } +} + +func TestSvcParamsString(t *testing.T) { + // this test relies on the parser also working + // because we can't just compare string outputs + // since map iteration is unordered + for i, test := range []svcParams{ + + { + "alpn": {"h2", "h3"}, + "no-default-alpn": {}, + "ipv6hint": {"2001:db8::1"}, + "port": {"443"}, + }, + + { + "key": {"value"}, + "quoted": {"some string"}, + "flag": {}, + }, + { + "key": {`nested "quoted" value`, "foobar"}, + }, + { + "alpn": {"h3", "h2"}, + "tls-supported-groups": {"29", "23"}, + "no-default-alpn": {}, + "ech": {"foobar"}, + }, + } { + combined := test.String() + parsed, err := parseSvcParams(combined) + if err != nil { + t.Errorf("Test %d: Expected no error, but got: %v (input=%q)", i, err, test) + continue + } + if len(parsed) != len(test) { + t.Errorf("Test %d: Expected %d keys, but got %d", i, len(test), len(parsed)) + continue + } + for key, expectedVals := range test { + if expected, actual := len(expectedVals), len(parsed[key]); expected != actual { + t.Errorf("Test %d: Expected key %s to have %d values, but had %d", i, key, expected, actual) + continue + } + for j, expected := range expectedVals { + if actual := parsed[key][j]; actual != expected { + t.Errorf("Test %d key %q value %d: Expected '%s' but got '%s'", i, key, j, expected, actual) + continue + } + } + } + if !reflect.DeepEqual(parsed, test) { + t.Errorf("Test %d: Expected %#v, got %#v", i, test, combined) + continue + } + } +} diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 5bb994fa5..0b7c56f38 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -399,12 +399,12 @@ func (t *TLS) Start() error { if t.EncryptedClientHello != nil { publicationList := t.EncryptedClientHello.Publication if publicationList == nil { - if recordSetter, ok := t.dns.(libdns.RecordSetter); ok { + if dnsProv, ok := t.dns.(ECHDNSProvider); ok { publicationList = []*ECHPublication{ { publishers: []ECHPublisher{ &ECHDNSPublisher{ - provider: recordSetter, + provider: dnsProv, logger: t.logger, }, },