Infrastructure/caddy
1
Fork 0
mirror of https://github.com/caddyserver/caddy.git synced 2024-12-16 21:56:40 -05:00
Code Issues Activity

caddytls: fix permission requirement with AutomationPolicy (#6328)

Browse source 
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 <will@tailscale.com>
This commit is contained in:
Will Norris 2024-05-20 08:48:59 -07:00 committed by GitHub
parent 1fc151faec
commit db3e19b7b5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 6 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
modules/caddytls/automation.go
View file

@ -170,6 +170,9 @@ type AutomationPolicy struct {
subjects []string subjects []string
magic *certmagic.Config magic *certmagic.Config
storage certmagic.Storage storage certmagic.Storage
// Whether this policy had explicit managers configured directly on it.
hadExplicitManagers bool
} }
// Provision sets up ap and builds its underlying CertMagic config. // Provision sets up ap and builds its underlying CertMagic config.
@ -201,8 +204,8 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
// store them on the policy before putting it on the config // store them on the policy before putting it on the config
// load and provision any cert manager modules // load and provision any cert manager modules
hadExplicitManagers := len(ap.ManagersRaw) > 0
if ap.ManagersRaw != nil { if ap.ManagersRaw != nil {
ap.hadExplicitManagers = true
vals, err := tlsApp.ctx.LoadModule(ap, "ManagersRaw") vals, err := tlsApp.ctx.LoadModule(ap, "ManagersRaw")
if err != nil { if err != nil {
return fmt.Errorf("loading external certificate manager modules: %v", err) return fmt.Errorf("loading external certificate manager modules: %v", err)
@ -262,9 +265,9 @@ func (ap *AutomationPolicy) Provision(tlsApp *TLS) error {
// prevent issuance from Issuers (when Managers don't provide a certificate) if there's no // prevent issuance from Issuers (when Managers don't provide a certificate) if there's no
// permission module configured // permission module configured
noProtections := ap.isWildcardOrDefault() && !ap.onlyInternalIssuer() && (tlsApp.Automation == nil || tlsApp.Automation.OnDemand == nil || tlsApp.Automation.OnDemand.permission == nil) noProtections := ap.isWildcardOrDefault() && !ap.onlyInternalIssuer() && (tlsApp.Automation == nil || tlsApp.Automation.OnDemand == nil || tlsApp.Automation.OnDemand.permission == nil)
failClosed := noProtections && hadExplicitManagers // don't allow on-demand issuance (other than implicit managers) if no managers have been explicitly configured failClosed := noProtections && !ap.hadExplicitManagers // don't allow on-demand issuance (other than implicit managers) if no managers have been explicitly configured
if noProtections { if noProtections {
if !hadExplicitManagers { if !ap.hadExplicitManagers {
// no managers, no explicitly-configured permission module, this is a config error // no managers, no explicitly-configured permission module, this is a config error
return fmt.Errorf("on-demand TLS cannot be enabled without a permission module to prevent abuse; please refer to documentation for details") return fmt.Errorf("on-demand TLS cannot be enabled without a permission module to prevent abuse; please refer to documentation for details")
} }