Augment, don't overwrite, HTTPS records

caddyconfig/httpcaddyfile/builtins.go

@@ -112,9 +112,6 @@ func parseBind(h Helper) ([]ConfigValue, error) {
 //	    issuer                        <module_name> [...]
 //	    get_certificate               <module_name> [...]
 //	    insecure_secrets_log          <log_file>
-//	    ech <public_name> {
-//	        dns <provider> ...
-//	    }
 //	}
 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())
 		}

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

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=

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),
+				Name:     relName,
-				Priority: 1, // allows a manual override with priority 0
+				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 {

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
+		}
+	}
+}

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,
 							},
 						},