caddytls: Don't publish ECH configs if other records don't exist

Publishing a DNS record for a name that doesn't have any could make wildcards ineffective, which would be surprising for site owners and could lead to downtime.

modules/caddyhttp/autohttps.go

@@ -163,6 +163,7 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er
 			}
 		}
 
+		// trim the list of domains covered by wildcards, if configured
 		if srv.AutoHTTPS.PreferWildcard {
 			wildcards := make(map[string]struct{})
 			for d := range serverDomainSet {
@@ -184,6 +185,17 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er
 			}
 		}
 
+		// build the list of domains that could be used with ECH (if enabled)
+		// so the TLS app can know to publish ECH configs for them; we do this
+		// after trimming domains covered by wildcards because, presumably,
+		// if the user wants to use wildcard certs, they also want to use the
+		// wildcard for ECH, rather than individual subdomains
+		echDomains := make([]string, 0, len(serverDomainSet))
+		for d := range serverDomainSet {
+			echDomains = append(echDomains, d)
+		}
+		app.tlsApp.RegisterServerNames(echDomains)
+
 		// nothing more to do here if there are no domains that qualify for
 		// automatic HTTPS and there are no explicit TLS connection policies:
 		// if there is at least one domain but no TLS conn policy (F&&T), we'll
@@ -205,7 +217,6 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er
 		// for all the hostnames we found, filter them so we have
 		// a deduplicated list of names for which to obtain certs
 		// (only if cert management not disabled for this server)
-		var echDomains []string
 		if srv.AutoHTTPS.DisableCerts {
 			logger.Warn("skipping automated certificate management for server because it is disabled", zap.String("server_name", srvName))
 		} else {
@@ -232,14 +243,10 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er
 					}
 
 					uniqueDomainsForCerts[d] = struct{}{}
-					echDomains = append(echDomains, d)
 				}
 			}
 		}
 
-		// let the TLS server know we have some hostnames that could be protected behind ECH
-		app.tlsApp.RegisterServerNames(echDomains)
-
 		// tell the server to use TLS if it is not already doing so
 		if srv.TLSConnPolicies == nil {
 			srv.TLSConnPolicies = caddytls.ConnectionPolicies{new(caddytls.ConnectionPolicy)}

modules/caddytls/ech.go

@@ -639,8 +639,16 @@ func (dnsPub *ECHDNSPublisher) PublishECHConfigList(ctx context.Context, innerNa
 			continue
 		}
 
-		// get any existing HTTPS record for this domain, and augment
+		relName := libdns.RelativeName(domain+".", zone)
-		// our ech SvcParamKey with any other existing SvcParams
+		// TODO: libdns.RelativeName should probably return "@" instead of "". (The latest commits of libdns do this, so remove this logic once upgraded.)
+		if relName == "" {
+			relName = "@"
+		}
+
+		// get existing records for this domain; we need to make sure another
+		// record exists for it so we don't accidentally trample a wildcard; we
+		// also want to get any HTTPS record that may already exist for it so
+		// we can augment the 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",
@@ -648,17 +656,29 @@ func (dnsPub *ECHDNSPublisher) PublishECHConfigList(ctx context.Context, innerNa
 				zap.Error(err))
 			continue
 		}
-		relName := libdns.RelativeName(domain+".", zone)
-		// TODO: libdns.RelativeName should probably return "@" instead of "".
-		if relName == "" {
-			relName = "@"
-		}
 		var httpsRec libdns.Record
+		var nameHasExistingRecord bool
 		for _, rec := range recs {
-			if rec.Name == relName && rec.Type == "HTTPS" && (rec.Target == "" || rec.Target == ".") {
+			if rec.Name == relName {
+				nameHasExistingRecord = true
+				if rec.Type == "HTTPS" && (rec.Target == "" || rec.Target == ".") {
 					httpsRec = rec
+					break
 				}
 			}
+		}
+		if !nameHasExistingRecord {
+			// Turns out if you publish a DNS record for a name that doesn't have any DNS record yet,
+			// any wildcard records won't apply for the name anymore, meaning if a wildcard A/AAAA record
+			// is used to resolve the domain to a server, publishing an HTTPS record could break resolution!
+			// In theory, this should be a non-issue, at least for A/AAAA records, if the HTTPS record
+			// includes ipv[4|6]hint SvcParamKeys,
+			dnsPub.logger.Warn("domain does not have any existing records, so skipping publication of HTTPS record",
+				zap.String("domain", domain),
+				zap.String("relative_name", relName),
+				zap.String("zone", zone))
+			continue
+		}
 		params := make(svcParams)
 		if httpsRec.Value != "" {
 			params, err = parseSvcParams(httpsRec.Value)

modules/caddytls/tls.go

@@ -563,7 +563,7 @@ func (t *TLS) Manage(names []string) error {
 // (keeping only the hotsname) and filters IP addresses, which can't be
 // used with ECH.
 //
-// EXPERIMENTAL: This function and its behavior are subject to change.
+// EXPERIMENTAL: This function and its semantics/behavior are subject to change.
 func (t *TLS) RegisterServerNames(dnsNames []string) {
 	t.serverNamesMu.Lock()
 	for _, name := range dnsNames {