From 43961b542b077f99f78d64629348b9300d3cd4a3 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 26 Apr 2019 12:35:39 -0600 Subject: [PATCH] General cleanup and more godocs --- modules.go | 39 ++++++++++++++-- modules/caddytls/acmemanager.go | 81 +++++++++++++++++++++++++++------ modules/caddytls/matchers.go | 20 ++++++-- modules/caddytls/tls.go | 59 ++++-------------------- storage.go | 16 ++----- 5 files changed, 129 insertions(+), 86 deletions(-) diff --git a/modules.go b/modules.go index 0fd898e9d..5a4e69fc0 100644 --- a/modules.go +++ b/modules.go @@ -11,15 +11,44 @@ import ( // Module represents a Caddy module. type Module struct { - Name string - New func() (interface{}, error) - OnLoad func(instances []interface{}, priorState interface{}) (newState interface{}, err error) + // Name is the full name of the module. It + // must be unique and properly namespaced. + Name string + + // New returns a new, empty instance of + // the module's type. The host module + // which loads this module will likely + // invoke methods on the returned value. + // It must return a pointer; if not, it + // is converted into one. + New func() (interface{}, error) + + // OnLoad is invoked after all module + // instances ave been loaded. It receives + // pointers to each instance of this + // module, and any state from a previous + // running configuration, which may be + // nil. + // + // If this module is to carry "global" + // state between all instances through + // reloads, you might find it helpful + // to return it. + // TODO: Is this really better/safer than a global variable? + OnLoad func(instances []interface{}, priorState interface{}) (newState interface{}, err error) + + // OnUnload is called after all module + // instances have been stopped, possibly + // in favor of a new configuration. It + // receives the state given by OnLoad (if + // any). OnUnload func(state interface{}) error } func (m Module) String() string { return m.Name } -// RegisterModule registers a module. +// RegisterModule registers a module. Modules must call +// this function in the init phase of runtime. func RegisterModule(mod Module) error { if mod.Name == "caddy" { return fmt.Errorf("modules cannot be named 'caddy'") @@ -35,7 +64,7 @@ func RegisterModule(mod Module) error { return nil } -// GetModule returns a module by name. +// GetModule returns a module by its full name. func GetModule(name string) (Module, error) { modulesMu.Lock() defer modulesMu.Unlock() diff --git a/modules/caddytls/acmemanager.go b/modules/caddytls/acmemanager.go index a7a460a26..40e2d248c 100644 --- a/modules/caddytls/acmemanager.go +++ b/modules/caddytls/acmemanager.go @@ -3,6 +3,7 @@ package caddytls import ( "encoding/json" "fmt" + "time" "github.com/go-acme/lego/certcrypto" @@ -18,11 +19,6 @@ func init() { }) } -// ManagerMaker TODO: WIP... -type ManagerMaker interface { - newManager(interactive bool) (certmagic.Manager, error) -} - // acmeManagerMaker makes an ACME manager // for managinig certificates using ACME. type acmeManagerMaker struct { @@ -40,9 +36,11 @@ type acmeManagerMaker struct { keyType certcrypto.KeyType } -func (m *acmeManagerMaker) Provision() error { - m.setDefaults() +func (m *acmeManagerMaker) newManager(interactive bool) (certmagic.Manager, error) { + return nil, nil +} +func (m *acmeManagerMaker) Provision() error { // DNS providers if m.Challenges.DNS != nil { val, err := caddy2.LoadModuleInline("provider", "tls.dns", m.Challenges.DNS) @@ -67,18 +65,71 @@ func (m *acmeManagerMaker) Provision() error { m.Storage = nil // allow GC to deallocate - TODO: Does this help? } + m.setDefaults() + return nil } -// setDefaults indiscriminately sets all the default values in m. +// setDefaults sets necessary values that are +// currently empty to their default values. func (m *acmeManagerMaker) setDefaults() { - m.CA = certmagic.LetsEncryptStagingCA // certmagic.Default.CA // TODO: When not testing, switch to production CA - m.Email = certmagic.Default.Email - m.RenewAhead = caddy2.Duration(certmagic.Default.RenewDurationBefore) - m.keyType = certmagic.Default.KeyType - m.storage = certmagic.Default.Storage + if m.CA == "" { + m.CA = certmagic.LetsEncryptStagingCA // certmagic.Default.CA // TODO: When not testing, switch to production CA + } + if m.Email == "" { + m.Email = certmagic.Default.Email + } + if m.RenewAhead == 0 { + m.RenewAhead = caddy2.Duration(certmagic.Default.RenewDurationBefore) + } + if m.keyType == "" { + m.keyType = certmagic.Default.KeyType + } + if m.storage == nil { + m.storage = certmagic.Default.Storage + } } -func (m *acmeManagerMaker) newManager(interactive bool) (certmagic.Manager, error) { - return nil, nil +// 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() certmagic.Config { + storage := m.storage + if storage == nil { + storage = caddy2.GetStorage() + } + + var ond *certmagic.OnDemandConfig + if m.OnDemand != nil { + ond = &certmagic.OnDemandConfig{ + // TODO: fill this out + } + } + + return certmagic.Config{ + CA: certmagic.LetsEncryptStagingCA, //ap.CA, // TODO: Restore true value + Email: m.Email, + Agreed: true, + DisableHTTPChallenge: m.Challenges.HTTP.Disabled, + DisableTLSALPNChallenge: m.Challenges.TLSALPN.Disabled, + RenewDurationBefore: time.Duration(m.RenewAhead), + AltHTTPPort: m.Challenges.HTTP.AlternatePort, + AltTLSALPNPort: m.Challenges.TLSALPN.AlternatePort, + DNSProvider: m.Challenges.dns, + KeyType: supportedCertKeyTypes[m.KeyType], + CertObtainTimeout: time.Duration(m.ACMETimeout), + OnDemand: ond, + MustStaple: m.MustStaple, + Storage: storage, + // TODO: listenHost + } +} + +// supportedCertKeyTypes is all the key types that are supported +// for certificates that are obtained through ACME. +var supportedCertKeyTypes = map[string]certcrypto.KeyType{ + "RSA2048": certcrypto.RSA2048, + "RSA4096": certcrypto.RSA4096, + "P256": certcrypto.EC256, + "P384": certcrypto.EC384, } diff --git a/modules/caddytls/matchers.go b/modules/caddytls/matchers.go index c376f8744..b308bd0f7 100644 --- a/modules/caddytls/matchers.go +++ b/modules/caddytls/matchers.go @@ -7,13 +7,22 @@ import ( ) type ( + // MatchServerName matches based on SNI. MatchServerName []string // TODO: these others should be enterprise-only, probably - MatchProtocol []string // TODO: version or protocol? + + // MatchProtocol matches based on protocol. + MatchProtocol []string // TODO: Protocol or version? + + // MatchClientCert matches based on client certificate / client auth? MatchClientCert struct{} // TODO: client certificate options - MatchRemote []string - MatchStarlark string + + // MatchRemote matches based on the remote address of the connection. + MatchRemote []string + + // MatchStarlark matches based on a Starlark script. + MatchStarlark string ) func init() { @@ -39,6 +48,7 @@ func init() { }) } +// Match matches hello based on SNI. func (m MatchServerName) Match(hello *tls.ClientHelloInfo) bool { for _, name := range m { // TODO: support wildcards (and regex?) @@ -49,21 +59,25 @@ func (m MatchServerName) Match(hello *tls.ClientHelloInfo) bool { return false } +// Match matches hello based on protocol version. func (m MatchProtocol) Match(hello *tls.ClientHelloInfo) bool { // TODO: not implemented return false } +// Match matches hello based on client certificate. func (m MatchClientCert) Match(hello *tls.ClientHelloInfo) bool { // TODO: not implemented return false } +// Match matches hello based on remote address. func (m MatchRemote) Match(hello *tls.ClientHelloInfo) bool { // TODO: not implemented return false } +// Match matches hello based on a Starlark script. func (m MatchStarlark) Match(hello *tls.ClientHelloInfo) bool { // TODO: not implemented return false diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 43ad957f1..fbc850cd0 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -5,10 +5,8 @@ import ( "encoding/json" "fmt" "net/http" - "time" "bitbucket.org/lightcodelabs/caddy2" - "github.com/go-acme/lego/certcrypto" "github.com/go-acme/lego/challenge" "github.com/klauspost/cpuid" "github.com/mholt/certmagic" @@ -30,8 +28,7 @@ type TLS struct { certCache *certmagic.Cache } -// TODO: Finish stubbing out this two-phase setup process: prepare, then start... - +// Provision sets up the configuration for the TLS app. func (t *TLS) Provision() error { // set up the certificate cache // TODO: this makes a new cache every time; better to only make a new @@ -97,15 +94,6 @@ func (t *TLS) Start(handle caddy2.Handle) error { if err != nil { return fmt.Errorf("automate: managing %v: %v", names, err) } - // for _, name := range names { - // t.Manage([]string{name) - // ap := t.getAutomationPolicyForName(name) - // magic := certmagic.New(t.certCache, ap.makeCertMagicConfig()) - // err := magic.Manage([]string{name}) - // if err != nil { - // return fmt.Errorf("automate: manage %s: %v", name, err) - // } - // } } t.Certificates = nil // allow GC to deallocate - TODO: Does this help? @@ -191,38 +179,11 @@ type AutomationPolicy struct { } func (ap AutomationPolicy) makeCertMagicConfig() 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 { - // default, which is management via ACME - - storage := acmeMgmt.storage - if storage == nil { - storage = caddy2.GetStorage() - } - - var ond *certmagic.OnDemandConfig - if acmeMgmt.OnDemand != nil { - ond = &certmagic.OnDemandConfig{ - // TODO: fill this out - } - } - - return certmagic.Config{ - CA: certmagic.LetsEncryptStagingCA, //ap.CA, // TODO: Restore true value - Email: acmeMgmt.Email, - Agreed: true, - DisableHTTPChallenge: acmeMgmt.Challenges.HTTP.Disabled, - DisableTLSALPNChallenge: acmeMgmt.Challenges.TLSALPN.Disabled, - RenewDurationBefore: time.Duration(acmeMgmt.RenewAhead), - AltHTTPPort: acmeMgmt.Challenges.HTTP.AlternatePort, - AltTLSALPNPort: acmeMgmt.Challenges.TLSALPN.AlternatePort, - DNSProvider: acmeMgmt.Challenges.dns, - KeyType: supportedCertKeyTypes[acmeMgmt.KeyType], - CertObtainTimeout: time.Duration(acmeMgmt.ACMETimeout), - OnDemand: ond, - MustStaple: acmeMgmt.MustStaple, - Storage: storage, - // TODO: listenHost - } + return acmeMgmt.makeCertMagicConfig() } return certmagic.Config{ @@ -260,13 +221,9 @@ type OnDemandConfig struct { AskStarlark string `json:"ask_starlark,omitempty"` } -// supportedCertKeyTypes is all the key types that are supported -// for certificates that are obtained through ACME. -var supportedCertKeyTypes = map[string]certcrypto.KeyType{ - "RSA2048": certcrypto.RSA2048, - "RSA4096": certcrypto.RSA4096, - "P256": certcrypto.EC256, - "P384": certcrypto.EC384, +// ManagerMaker makes a certificate manager. +type ManagerMaker interface { + newManager(interactive bool) (certmagic.Manager, error) } // supportedCipherSuites is the unordered map of cipher suite diff --git a/storage.go b/storage.go index cb93f5981..2b77851a2 100644 --- a/storage.go +++ b/storage.go @@ -16,22 +16,14 @@ func init() { } // StorageConverter is a type that can convert itself -// to a valid, usable certmagic.Storage value. The -// value might be short-lived. +// to a valid, usable certmagic.Storage value. (The +// value might be short-lived.) This interface allows +// us to adapt any CertMagic storage implementation +// into a consistent API for Caddy configuration. type StorageConverter interface { CertMagicStorage() (certmagic.Storage, error) } -// TODO: Wrappers other than file_system should be enterprise-only. - -// It may seem trivial to wrap these, but the benefits are: -// 1. We don't need to change the actual CertMagic storage implementions -// to a structure that is operable with Caddy's config (including JSON -// tags), and -// 2. We don't need to rely on rely on maintainers of third-party -// certmagic.Storage implementations. We can make any certmagic.Storage -// work with Caddy this way. - // fileStorage is a certmagic.Storage wrapper for certmagic.FileStorage. type fileStorage struct { Root string `json:"root"`