Skip to content

Commit db3e19b

Browse files
authored
caddytls: fix permission requirement with AutomationPolicy (#6328)
Certificate automation has permission modules that are designed to prevent inappropriate issuance of unbounded or wildcard certificates. When an explicit cert manager is used, no additional permission should be necessary. For example, this should be a valid caddyfile: https:// { tls { get_certificate tailscale } respond OK } This is accomplished when provisioning an AutomationPolicy by tracking whether there were explicit managers configured directly on the policy (in the ManagersRaw field). Only when a number of potentially unsafe conditions are present AND no explicit cert managers are configured is an error returned. The problem arises from the fact that ctx.LoadModule deletes the raw bytes after loading in order to save memory. The first time an AutomationPolicy is provisioned, the ManagersRaw field is populated, and everything is fine. An AutomationPolicy with no subjects is treated as a special "catch-all" policy. App.createAutomationPolicies ensures that this catch-all policy has an ACME issuer, and then calls its Provision method again because it may have changed. This second time Provision is called, ManagesRaw is no longer populated, and the permission check fails because it appears as though the policy has no explicit managers. Address this by storing a new boolean on AutomationPolicy recording whether it had explicit cert managers configured on it. Also fix an inverted boolean check on this value when setting failClosed. Updates #6060 Updates #6229 Updates #6327 Signed-off-by: Will Norris <[email protected]>
1 parent 1fc151f commit db3e19b

File tree

1 file changed

+6
-3
lines changed

1 file changed

+6
-3
lines changed

modules/caddytls/automation.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,9 @@ type AutomationPolicy struct {
170170
subjects []string
171171
magic *certmagic.Config
172172
storage certmagic.Storage
173+
174+
// Whether this policy had explicit managers configured directly on it.
175+
hadExplicitManagers bool
173176
}
174177

175178
// Provision sets up ap and builds its underlying CertMagic config.
@@ -201,8 +204,8 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
201204
// store them on the policy before putting it on the config
202205

203206
// load and provision any cert manager modules
204-
hadExplicitManagers := len(ap.ManagersRaw) > 0
205207
if ap.ManagersRaw != nil {
208+
ap.hadExplicitManagers = true
206209
vals, err := tlsApp.ctx.LoadModule(ap, "ManagersRaw")
207210
if err != nil {
208211
return fmt.Errorf("loading external certificate manager modules: %v", err)
@@ -262,9 +265,9 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
262265
// prevent issuance from Issuers (when Managers don't provide a certificate) if there's no
263266
// permission module configured
264267
noProtections := ap.isWildcardOrDefault() && !ap.onlyInternalIssuer() && (tlsApp.Automation == nil || tlsApp.Automation.OnDemand == nil || tlsApp.Automation.OnDemand.permission == nil)
265-
failClosed := noProtections && hadExplicitManagers // don't allow on-demand issuance (other than implicit managers) if no managers have been explicitly configured
268+
failClosed := noProtections && !ap.hadExplicitManagers // don't allow on-demand issuance (other than implicit managers) if no managers have been explicitly configured
266269
if noProtections {
267-
if !hadExplicitManagers {
270+
if !ap.hadExplicitManagers {
268271
// no managers, no explicitly-configured permission module, this is a config error
269272
return fmt.Errorf("on-demand TLS cannot be enabled without a permission module to prevent abuse; please refer to documentation for details")
270273
}

0 commit comments

Comments
 (0)