From 39262f86632401ae4915600b042ef5a28141d3d5 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 11 Mar 2025 08:12:42 -0600 Subject: [PATCH] caddytls: Minor fixes for ECH --- modules/caddytls/acmeissuer.go | 15 ++++---- modules/caddytls/connpolicy.go | 15 +++----- modules/caddytls/ech.go | 62 +++++++++++++++++++++------------- 3 files changed, 50 insertions(+), 42 deletions(-) diff --git a/modules/caddytls/acmeissuer.go b/modules/caddytls/acmeissuer.go index f0965855a..a07a34c10 100644 --- a/modules/caddytls/acmeissuer.go +++ b/modules/caddytls/acmeissuer.go @@ -507,21 +507,20 @@ func (iss *ACMEIssuer) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { iss.TrustedRootsPEMFiles = d.RemainingArgs() case "dns": - if !d.NextArg() { - return d.ArgErr() - } - provName := d.Val() if iss.Challenges == nil { iss.Challenges = new(ChallengesConfig) } if iss.Challenges.DNS == nil { iss.Challenges.DNS = new(DNSChallengeConfig) } - unm, err := caddyfile.UnmarshalModule(d, "dns.providers."+provName) - if err != nil { - return err + if d.NextArg() { + provName := d.Val() + unm, err := caddyfile.UnmarshalModule(d, "dns.providers."+provName) + if err != nil { + return err + } + iss.Challenges.DNS.ProviderRaw = caddyconfig.JSONModuleObject(unm, "name", provName, nil) } - iss.Challenges.DNS.ProviderRaw = caddyconfig.JSONModuleObject(unm, "name", provName, nil) case "propagation_delay": if !d.NextArg() { diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go index cf0ef1e0a..fb9588c91 100644 --- a/modules/caddytls/connpolicy.go +++ b/modules/caddytls/connpolicy.go @@ -940,17 +940,10 @@ func setDefaultTLSParams(cfg *tls.Config) { cfg.CurvePreferences = defaultCurves } - if cfg.MinVersion == 0 { - // crypto/tls docs: - // "If EncryptedClientHelloKeys is set, MinVersion, if set, must be VersionTLS13." - if cfg.EncryptedClientHelloKeys == nil { - cfg.MinVersion = tls.VersionTLS12 - } else { - cfg.MinVersion = tls.VersionTLS13 - } - } - if cfg.MaxVersion == 0 { - cfg.MaxVersion = tls.VersionTLS13 + // crypto/tls docs: + // "If EncryptedClientHelloKeys is set, MinVersion, if set, must be VersionTLS13." + if cfg.EncryptedClientHelloKeys != nil && cfg.MinVersion != 0 && cfg.MinVersion < tls.VersionTLS13 { + cfg.MinVersion = tls.VersionTLS13 } } diff --git a/modules/caddytls/ech.go b/modules/caddytls/ech.go index 10f325565..63b872260 100644 --- a/modules/caddytls/ech.go +++ b/modules/caddytls/ech.go @@ -44,6 +44,10 @@ func init() { // each individual publication config object. (Requires a custom build with a // DNS provider module.) // +// ECH requires at least TLS 1.3, so any TLS connection policies with ECH +// applied will automatically upgrade the minimum TLS version to 1.3, even if +// configured to a lower version. +// // Note that, as of Caddy 2.10.0 (~March 2025), ECH keys are not automatically // rotated due to a limitation in the Go standard library (see // https://github.com/golang/go/issues/71920). This should be resolved when @@ -294,31 +298,36 @@ func (t *TLS) publishECHConfigs() error { // publish this ECH config list with this publisher pubTime := time.Now() err := publisher.PublishECHConfigList(t.ctx, dnsNamesToPublish, echCfgListBin) - if err != nil { - t.logger.Error("publishing ECH configuration list", - zap.Strings("for_domains", publication.Domains), + if err == nil { + t.logger.Info("published ECH configuration list", + zap.Strings("domains", publication.Domains), + zap.Uint8s("config_ids", configIDs), zap.Error(err)) - } - - // update publication history, so that we don't unnecessarily republish every time - for _, cfg := range echCfgList { - if cfg.meta.Publications == nil { - cfg.meta.Publications = make(publicationHistory) - } - if _, ok := cfg.meta.Publications[publisherKey]; !ok { - cfg.meta.Publications[publisherKey] = make(map[string]time.Time) - } - for _, name := range dnsNamesToPublish { - cfg.meta.Publications[publisherKey][name] = pubTime - } - metaBytes, err := json.Marshal(cfg.meta) - if err != nil { - return fmt.Errorf("marshaling ECH config metadata: %v", err) - } - metaKey := path.Join(echConfigsKey, strconv.Itoa(int(cfg.ConfigID)), "meta.json") - if err := t.ctx.Storage().Store(t.ctx, metaKey, metaBytes); err != nil { - return fmt.Errorf("storing updated ECH config metadata: %v", err) + // update publication history, so that we don't unnecessarily republish every time + for _, cfg := range echCfgList { + if cfg.meta.Publications == nil { + cfg.meta.Publications = make(publicationHistory) + } + if _, ok := cfg.meta.Publications[publisherKey]; !ok { + cfg.meta.Publications[publisherKey] = make(map[string]time.Time) + } + for _, name := range dnsNamesToPublish { + cfg.meta.Publications[publisherKey][name] = pubTime + } + metaBytes, err := json.Marshal(cfg.meta) + if err != nil { + return fmt.Errorf("marshaling ECH config metadata: %v", err) + } + metaKey := path.Join(echConfigsKey, strconv.Itoa(int(cfg.ConfigID)), "meta.json") + if err := t.ctx.Storage().Store(t.ctx, metaKey, metaBytes); err != nil { + return fmt.Errorf("storing updated ECH config metadata: %v", err) + } } + } else { + t.logger.Error("publishing ECH configuration list", + zap.Strings("domains", publication.Domains), + zap.Uint8s("config_ids", configIDs), + zap.Error(err)) } } } @@ -640,6 +649,10 @@ func (dnsPub *ECHDNSPublisher) PublishECHConfigList(ctx context.Context, innerNa continue } relName := libdns.RelativeName(domain+".", zone) + // TODO: libdns.RelativeName should probably return "@" instead of "". + if relName == "" { + relName = "@" + } var httpsRec libdns.Record for _, rec := range recs { if rec.Name == relName && rec.Type == "HTTPS" && (rec.Target == "" || rec.Target == ".") { @@ -674,8 +687,11 @@ func (dnsPub *ECHDNSPublisher) PublishECHConfigList(ctx context.Context, innerNa }, }) if err != nil { + // TODO: Maybe this should just stop and return the error... dnsPub.logger.Error("unable to publish ECH data to HTTPS DNS record", zap.String("domain", domain), + zap.String("zone", zone), + zap.String("dns_record_name", relName), zap.Error(err)) continue }