From b79f86f256782be139d4ec09a74e0f75da52876a Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 4 Jun 2019 22:43:21 -0600 Subject: Fix bugs related to auto HTTPS and alternate port configurations --- modules/caddyhttp/caddyhttp.go | 31 +++++++++++++++++++++++++++++-- modules/caddytls/acmemanager.go | 40 ++++++++++++++++++++++++++-------------- modules/caddytls/tls.go | 30 +++++++++++++++--------------- 3 files changed, 70 insertions(+), 31 deletions(-) (limited to 'modules') diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go index 3ffc989..89ea58f 100644 --- a/modules/caddyhttp/caddyhttp.go +++ b/modules/caddyhttp/caddyhttp.go @@ -126,7 +126,8 @@ func (app *App) Start() error { return fmt.Errorf("%s: listening on %s: %v", network, addr, err) } - // enable HTTP/2 by default + // enable HTTP/2 (and support for solving the + // TLS-ALPN ACME challenge) by default for _, pol := range srv.TLSConnPolicies { if len(pol.ALPN) == 0 { pol.ALPN = append(pol.ALPN, defaultALPN...) @@ -219,13 +220,38 @@ func (app *App) automaticHTTPS() error { domains = append(domains, d) } + // ensure that these certificates are managed properly; + // for example, it's implied that the HTTPPort should also + // be the port the HTTP challenge is solved on, and so + // for HTTPS port and TLS-ALPN challenge also - we need + // to tell the TLS app to manage these certs by honoring + // those port configurations + acmeManager := &caddytls.ACMEManagerMaker{ + Challenges: caddytls.ChallengesConfig{ + HTTP: caddytls.HTTPChallengeConfig{ + AlternatePort: app.HTTPPort, + }, + TLSALPN: caddytls.TLSALPNChallengeConfig{ + AlternatePort: app.HTTPSPort, + }, + }, + } + acmeManager.SetDefaults() + tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, + caddytls.AutomationPolicy{ + Hosts: domains, + Management: acmeManager, + }) + // manage their certificates err := tlsApp.Manage(domains) if err != nil { return fmt.Errorf("%s: managing certificate for %s: %s", srvName, domains, err) } - // tell the server to use TLS + // tell the server to use TLS by specifying a TLS + // connection policy (which supports HTTP/2 and the + // TLS-ALPN ACME challenge as well) srv.TLSConnPolicies = caddytls.ConnectionPolicies{ {ALPN: defaultALPN}, } @@ -296,6 +322,7 @@ func (app *App) automaticHTTPS() error { Listen: lnAddrs, Routes: redirRoutes, DisableAutoHTTPS: true, + tlsApp: tlsApp, // required to solve HTTP challenge } } diff --git a/modules/caddytls/acmemanager.go b/modules/caddytls/acmemanager.go index 9352d75..9cbf349 100644 --- a/modules/caddytls/acmemanager.go +++ b/modules/caddytls/acmemanager.go @@ -15,13 +15,19 @@ import ( func init() { caddy2.RegisterModule(caddy2.Module{ Name: "tls.management.acme", - New: func() interface{} { return new(acmeManagerMaker) }, + New: func() interface{} { return new(ACMEManagerMaker) }, }) } -// acmeManagerMaker makes an ACME manager -// for managinig certificates using ACME. -type acmeManagerMaker struct { +// ACMEManagerMaker makes an ACME manager +// for managing certificates using ACME. +// If crafting one manually rather than +// through the config-unmarshal process +// (provisioning), be sure to call +// SetDefaults to ensure sane defaults +// after you have configured this struct +// to your liking. +type ACMEManagerMaker struct { CA string `json:"ca,omitempty"` Email string `json:"email,omitempty"` RenewAhead caddy2.Duration `json:"renew_ahead,omitempty"` @@ -36,19 +42,22 @@ type acmeManagerMaker struct { keyType certcrypto.KeyType } -func (m *acmeManagerMaker) newManager(interactive bool) (certmagic.Manager, error) { +// newManager is a no-op to satisfy the ManagerMaker interface, +// because this manager type is a special case. +func (m *ACMEManagerMaker) newManager(interactive bool) (certmagic.Manager, error) { return nil, nil } -func (m *acmeManagerMaker) Provision(ctx caddy2.Context) error { +// Provision sets up m. +func (m *ACMEManagerMaker) Provision(ctx caddy2.Context) error { // DNS providers if m.Challenges.DNS != nil { - val, err := ctx.LoadModuleInline("provider", "tls.dns", m.Challenges.DNS) + val, err := ctx.LoadModuleInline("provider", "tls.dns", m.Challenges.DNSRaw) if err != nil { return fmt.Errorf("loading TLS storage module: %s", err) } - m.Challenges.dns = val.(challenge.Provider) - m.Challenges.DNS = nil // allow GC to deallocate - TODO: Does this help? + m.Challenges.DNS = val.(challenge.Provider) + m.Challenges.DNSRaw = nil // allow GC to deallocate - TODO: Does this help? } // policy-specific storage implementation @@ -65,14 +74,14 @@ func (m *acmeManagerMaker) Provision(ctx caddy2.Context) error { m.Storage = nil // allow GC to deallocate - TODO: Does this help? } - m.setDefaults() + m.SetDefaults() return nil } -// setDefaults sets necessary values that are +// SetDefaults sets necessary values that are // currently empty to their default values. -func (m *acmeManagerMaker) setDefaults() { +func (m *ACMEManagerMaker) SetDefaults() { if m.CA == "" { m.CA = certmagic.LetsEncryptStagingCA // certmagic.Default.CA // TODO: When not testing, switch to production CA } @@ -93,7 +102,7 @@ func (m *acmeManagerMaker) setDefaults() { // makeCertMagicConfig converts m into a certmagic.Config, because // this is a special case where the default manager is the certmagic // Config and not a separate manager. -func (m *acmeManagerMaker) makeCertMagicConfig(ctx caddy2.Context) certmagic.Config { +func (m *ACMEManagerMaker) makeCertMagicConfig(ctx caddy2.Context) certmagic.Config { storage := m.storage if storage == nil { storage = ctx.Storage() @@ -115,7 +124,7 @@ func (m *acmeManagerMaker) makeCertMagicConfig(ctx caddy2.Context) certmagic.Con RenewDurationBefore: time.Duration(m.RenewAhead), AltHTTPPort: m.Challenges.HTTP.AlternatePort, AltTLSALPNPort: m.Challenges.TLSALPN.AlternatePort, - DNSProvider: m.Challenges.dns, + DNSProvider: m.Challenges.DNS, KeyType: supportedCertKeyTypes[m.KeyType], CertObtainTimeout: time.Duration(m.ACMETimeout), OnDemand: ond, @@ -133,3 +142,6 @@ var supportedCertKeyTypes = map[string]certcrypto.KeyType{ "P256": certcrypto.EC256, "P384": certcrypto.EC384, } + +// Interface guard +var _ managerMaker = (*ACMEManagerMaker)(nil) diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 38447ba..22749ae 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -45,12 +45,12 @@ func (t *TLS) Provision(ctx caddy2.Context) error { // automation/management policies for i, ap := range t.Automation.Policies { - val, err := ctx.LoadModuleInline("module", "tls.management", ap.Management) + val, err := ctx.LoadModuleInline("module", "tls.management", ap.ManagementRaw) if err != nil { return fmt.Errorf("loading TLS automation management module: %s", err) } - t.Automation.Policies[i].management = val.(ManagerMaker) - t.Automation.Policies[i].Management = nil // allow GC to deallocate - TODO: Does this help? + t.Automation.Policies[i].Management = val.(managerMaker) + t.Automation.Policies[i].ManagementRaw = nil // allow GC to deallocate - TODO: Does this help? } // certificate loaders @@ -164,9 +164,9 @@ func (t *TLS) getAutomationPolicyForName(name string) AutomationPolicy { } // default automation policy - mgmt := new(acmeManagerMaker) - mgmt.setDefaults() - return AutomationPolicy{management: mgmt} + mgmt := new(ACMEManagerMaker) + mgmt.SetDefaults() + return AutomationPolicy{Management: mgmt} } // CertificateLoader is a type that can load certificates. @@ -183,22 +183,22 @@ type AutomationConfig struct { // AutomationPolicy designates the policy for automating the // management of managed TLS certificates. type AutomationPolicy struct { - Hosts []string `json:"hosts,omitempty"` - Management json.RawMessage `json:"management"` + Hosts []string `json:"hosts,omitempty"` + ManagementRaw json.RawMessage `json:"management"` - management ManagerMaker + Management managerMaker `json:"-"` } func (ap AutomationPolicy) makeCertMagicConfig(ctx caddy2.Context) certmagic.Config { // default manager (ACME) is a special case because of how CertMagic is designed // TODO: refactor certmagic so that ACME manager is not a special case by extracting // its config fields out of the certmagic.Config struct, or something... - if acmeMgmt, ok := ap.management.(*acmeManagerMaker); ok { + if acmeMgmt, ok := ap.Management.(*ACMEManagerMaker); ok { return acmeMgmt.makeCertMagicConfig(ctx) } return certmagic.Config{ - NewManager: ap.management.newManager, + NewManager: ap.Management.newManager, } } @@ -206,9 +206,9 @@ func (ap AutomationPolicy) makeCertMagicConfig(ctx caddy2.Context) certmagic.Con type ChallengesConfig struct { HTTP HTTPChallengeConfig `json:"http"` TLSALPN TLSALPNChallengeConfig `json:"tls-alpn"` - DNS json.RawMessage `json:"dns,omitempty"` + DNSRaw json.RawMessage `json:"dns,omitempty"` - dns challenge.Provider + DNS challenge.Provider `json:"-"` } // HTTPChallengeConfig configures the ACME HTTP challenge. @@ -232,8 +232,8 @@ type OnDemandConfig struct { AskStarlark string `json:"ask_starlark,omitempty"` } -// ManagerMaker makes a certificate manager. -type ManagerMaker interface { +// managerMaker makes a certificate manager. +type managerMaker interface { newManager(interactive bool) (certmagic.Manager, error) } -- cgit v1.2.3