From 115b877e1a83527e4b5e1bf33bb2d9b62f3f3059 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sun, 15 Mar 2020 09:24:24 -0600 Subject: [PATCH] caddytls: Set Issuer properly on automation policies (fix #3150) When using the default automation policy specifically, ap.Issuer would be nil, so we'd end up overwriting the ap.magic.Issuer's default value (after New()) with nil; this instead sets Issuer on the template before New() is called, and no overwriting is done. --- modules/caddytls/tls.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 0b39c712..98e11641 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -479,6 +479,9 @@ type AutomationPolicy struct { // TODO: is this really necessary per-policy? why not a global setting... ManageSync bool `json:"manage_sync,omitempty"` + // Issuer stores the decoded issuer parameters. This is only + // used to populate an underlying certmagic.Config's Issuer + // field; it is not referenced thereafter. Issuer certmagic.Issuer `json:"-"` magic *certmagic.Config @@ -527,6 +530,14 @@ func (ap *AutomationPolicy) provision(tlsApp *TLS) error { } } + if ap.IssuerRaw != nil { + val, err := tlsApp.ctx.LoadModule(ap, "IssuerRaw") + if err != nil { + return fmt.Errorf("loading TLS automation management module: %s", err) + } + ap.Issuer = val.(certmagic.Issuer) + } + keySource := certmagic.StandardKeyGenerator{ KeyType: supportedCertKeyTypes[ap.KeyType], } @@ -542,17 +553,13 @@ func (ap *AutomationPolicy) provision(tlsApp *TLS) error { KeySource: keySource, OnDemand: ond, Storage: storage, + Issuer: ap.Issuer, // if nil, certmagic.New() will set default in returned Config + } + if rev, ok := ap.Issuer.(certmagic.Revoker); ok { + template.Revoker = rev } ap.magic = certmagic.New(tlsApp.certCache, template) - if ap.IssuerRaw != nil { - val, err := tlsApp.ctx.LoadModule(ap, "IssuerRaw") - if err != nil { - return fmt.Errorf("loading TLS automation management module: %s", err) - } - ap.Issuer = val.(certmagic.Issuer) - } - // sometimes issuers may need the parent certmagic.Config in // order to function properly (for example, ACMEIssuer needs // access to the correct storage and cache so it can solve @@ -562,11 +569,6 @@ func (ap *AutomationPolicy) provision(tlsApp *TLS) error { configger.SetConfig(ap.magic) } - ap.magic.Issuer = ap.Issuer - if rev, ok := ap.Issuer.(certmagic.Revoker); ok { - ap.magic.Revoker = rev - } - return nil }