diff options
| -rw-r--r-- | caddyconfig/httpcaddyfile/httptype.go | 20 | ||||
| -rw-r--r-- | go.mod | 2 | ||||
| -rw-r--r-- | go.sum | 4 | ||||
| -rw-r--r-- | modules/caddyhttp/autohttps.go | 59 | ||||
| -rw-r--r-- | modules/caddyhttp/caddyhttp.go | 16 | ||||
| -rw-r--r-- | modules/caddytls/acmeissuer.go | 6 | ||||
| -rw-r--r-- | modules/caddytls/connpolicy.go | 7 | ||||
| -rw-r--r-- | modules/caddytls/tls.go | 19 |
8 files changed, 68 insertions, 65 deletions
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index a8df28c18..c37b5f22d 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -84,12 +84,11 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock, "{method}", "{http.request.method}", "{path}", "{http.request.uri.path}", "{query}", "{http.request.uri.query}", + "{remote}", "{http.request.remote}", "{remote_host}", "{http.request.remote.host}", "{remote_port}", "{http.request.remote.port}", - "{remote}", "{http.request.remote}", "{scheme}", "{http.request.scheme}", "{uri}", "{http.request.uri}", - "{tls_cipher}", "{http.request.tls.cipher_suite}", "{tls_version}", "{http.request.tls.version}", "{tls_client_fingerprint}", "{http.request.tls.client.fingerprint}", @@ -173,10 +172,9 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock, // now that each server is configured, make the HTTP app httpApp := caddyhttp.App{ - HTTPPort: tryInt(options["http_port"], &warnings), - HTTPSPort: tryInt(options["https_port"], &warnings), - DefaultSNI: tryString(options["default_sni"], &warnings), - Servers: servers, + HTTPPort: tryInt(options["http_port"], &warnings), + HTTPSPort: tryInt(options["https_port"], &warnings), + Servers: servers, } // now for the TLS app! (TODO: refactor into own func) @@ -449,7 +447,6 @@ func (st *ServerType) serversFromPairings( groupCounter counter, ) (map[string]*caddyhttp.Server, error) { servers := make(map[string]*caddyhttp.Server) - defaultSNI := tryString(options["default_sni"], warnings) for i, p := range pairings { @@ -538,12 +535,7 @@ func (st *ServerType) serversFromPairings( srv.TLSConnPolicies = append(srv.TLSConnPolicies, cp) } - // TODO: consolidate equal conn policies - } else if defaultSNI != "" { - hasCatchAllTLSConnPolicy = true - srv.TLSConnPolicies = append(srv.TLSConnPolicies, &caddytls.ConnectionPolicy{ - DefaultSNI: defaultSNI, - }) + // TODO: consolidate equal conn policies? } // exclude any hosts that were defined explicitly with @@ -614,7 +606,7 @@ func (st *ServerType) serversFromPairings( // catch-all/default policy if there isn't one already (it's // important that it goes at the end) - see issue #3004: // https://github.com/caddyserver/caddy/issues/3004 - if len(srv.TLSConnPolicies) > 0 && !hasCatchAllTLSConnPolicy { + if !hasCatchAllTLSConnPolicy && (len(srv.TLSConnPolicies) > 0 || defaultSNI != "") { srv.TLSConnPolicies = append(srv.TLSConnPolicies, &caddytls.ConnectionPolicy{DefaultSNI: defaultSNI}) } @@ -6,7 +6,7 @@ require ( github.com/Masterminds/sprig/v3 v3.0.2 github.com/alecthomas/chroma v0.7.2-0.20200305040604-4f3623dce67a github.com/andybalholm/brotli v1.0.0 - github.com/caddyserver/certmagic v0.10.1 + github.com/caddyserver/certmagic v0.10.2 github.com/dustin/go-humanize v1.0.1-0.20200219035652-afde56e7acac github.com/go-acme/lego/v3 v3.4.0 github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e @@ -108,8 +108,8 @@ github.com/bombsimon/wsl/v2 v2.0.0/go.mod h1:mf25kr/SqFEPhhcxW1+7pxzGlW+hIl/hYTK github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= github.com/bradfitz/go-smtpd v0.0.0-20170404230938-deb6d6237625/go.mod h1:HYsPBTaaSFSlLx/70C2HPIMNZpVV8+vt/A+FMnYP11g= github.com/buger/jsonparser v0.0.0-20181115193947-bf1c66bbce23/go.mod h1:bbYlZJ7hK1yFx9hf58LP0zeX7UjIGs20ufpu3evjr+s= -github.com/caddyserver/certmagic v0.10.1 h1:k9E+C4b8WM3sTs3PSfmWIAwxtO9cXtr0bDHX2Bc0RIM= -github.com/caddyserver/certmagic v0.10.1/go.mod h1:Y8jcUBctgk/IhpAzlHKfimZNyXCkfGgRTC0orl8gROQ= +github.com/caddyserver/certmagic v0.10.2 h1:uz+QTSZULEN9iW2woZYf01ipR2O88iLx/bkIAoEXYbo= +github.com/caddyserver/certmagic v0.10.2/go.mod h1:Y8jcUBctgk/IhpAzlHKfimZNyXCkfGgRTC0orl8gROQ= github.com/cenkalti/backoff/v4 v4.0.0 h1:6VeaLF9aI+MAUQ95106HwWzYZgJJpZ4stumjj6RFYAU= github.com/cenkalti/backoff/v4 v4.0.0/go.mod h1:eEew/i+1Q6OrCDZh3WiXYv3+nJwBASZ8Bog/87DQnVg= github.com/census-instrumentation/opencensus-proto v0.2.0/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index 6a23ca0a8..39ca0f3f5 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -101,10 +101,6 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er continue } - defaultConnPolicies := caddytls.ConnectionPolicies{ - &caddytls.ConnectionPolicy{ALPN: defaultALPN}, - } - // if all listeners are on the HTTPS port, make sure // there is at least one TLS connection policy; it // should be obvious that they want to use TLS without @@ -115,7 +111,7 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er zap.String("server_name", srvName), zap.Int("https_port", app.httpsPort()), ) - srv.TLSConnPolicies = defaultConnPolicies + srv.TLSConnPolicies = caddytls.ConnectionPolicies{new(caddytls.ConnectionPolicy)} } // find all qualifying domain names (deduplicated) in this server @@ -149,7 +145,8 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er // for all the hostnames we found, filter them so we have // a deduplicated list of names for which to obtain certs for d := range serverDomainSet { - if !srv.AutoHTTPS.Skipped(d, srv.AutoHTTPS.SkipCerts) { + if certmagic.SubjectQualifiesForCert(d) && + !srv.AutoHTTPS.Skipped(d, srv.AutoHTTPS.SkipCerts) { // if a certificate for this name is already loaded, // don't obtain another one for it, unless we are // supposed to ignore loaded certificates @@ -176,7 +173,7 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er // tell the server to use TLS if it is not already doing so if srv.TLSConnPolicies == nil { - srv.TLSConnPolicies = defaultConnPolicies + srv.TLSConnPolicies = caddytls.ConnectionPolicies{new(caddytls.ConnectionPolicy)} } // nothing left to do if auto redirects are disabled @@ -212,13 +209,33 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er // turn the set into a slice so that phase 2 can use it app.allCertDomains = make([]string, 0, len(uniqueDomainsForCerts)) var internal, external []string +uniqueDomainsLoop: for d := range uniqueDomainsForCerts { + // whether or not there is already an automation policy for this + // name, we should add it to the list to manage a cert for it + app.allCertDomains = append(app.allCertDomains, d) + + // some names we've found might already have automation policies + // explicitly specified for them; we should exclude those from + // our hidden/implicit policy, since applying a name to more than + // one automation policy would be confusing and an error + if app.tlsApp.Automation != nil { + for _, ap := range app.tlsApp.Automation.Policies { + for _, apHost := range ap.Hosts { + if apHost == d { + continue uniqueDomainsLoop + } + } + } + } + + // if no automation policy exists for the name yet, we + // will associate it with an implicit one if certmagic.SubjectQualifiesForPublicCert(d) { external = append(external, d) } else { internal = append(internal, d) } - app.allCertDomains = append(app.allCertDomains, d) } // ensure there is an automation policy to handle these certs @@ -263,7 +280,7 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er return err } redirTo := "https://{http.request.host}" - if addr.StartPort != DefaultHTTPSPort { + if addr.StartPort != uint(app.httpsPort()) { redirTo += ":" + strconv.Itoa(int(addr.StartPort)) } redirTo += "{http.request.uri}" @@ -384,23 +401,23 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, publicNames, interna // start by finding a base policy that the user may have defined // which should, in theory, apply to any policies derived from it; // typically this would be a "catch-all" policy with no host filter - var matchingPolicy *caddytls.AutomationPolicy + var basePolicy *caddytls.AutomationPolicy if app.tlsApp.Automation != nil { - // if an existing policy matches (specifically, a catch-all policy), - // we should inherit from it, because that is what the user expects; - // this is very common for user setting a default issuer, with a - // custom CA endpoint, for example - whichever one we choose must - // have a host list that is a superset of the policy we make... - // the policy with no host filter is guaranteed to qualify for _, ap := range app.tlsApp.Automation.Policies { + // if an existing policy matches (specifically, a catch-all policy), + // we should inherit from it, because that is what the user expects; + // this is very common for user setting a default issuer, with a + // custom CA endpoint, for example - whichever one we choose must + // have a host list that is a superset of the policy we make... + // the policy with no host filter is guaranteed to qualify if len(ap.Hosts) == 0 { - matchingPolicy = ap + basePolicy = ap break } } } - if matchingPolicy == nil { - matchingPolicy = new(caddytls.AutomationPolicy) + if basePolicy == nil { + basePolicy = new(caddytls.AutomationPolicy) } // addPolicy adds an automation policy that uses issuer for hosts. @@ -408,7 +425,7 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, publicNames, interna // shallow-copy the matching policy; we want to inherit // from it, not replace it... this takes two lines to // overrule compiler optimizations - policyCopy := *matchingPolicy + policyCopy := *basePolicy newPolicy := &policyCopy // very important to provision it, since we are @@ -429,7 +446,7 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, publicNames, interna var acmeIssuer *caddytls.ACMEIssuer // if it has an ACME issuer, maybe we can just use that // TODO: we might need a deep copy here, like a Clone() method on ACMEIssuer... - acmeIssuer, _ = matchingPolicy.Issuer.(*caddytls.ACMEIssuer) + acmeIssuer, _ = basePolicy.Issuer.(*caddytls.ACMEIssuer) if acmeIssuer == nil { acmeIssuer = new(caddytls.ACMEIssuer) } diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go index 06719b51e..99f215f18 100644 --- a/modules/caddyhttp/caddyhttp.go +++ b/modules/caddyhttp/caddyhttp.go @@ -29,7 +29,6 @@ import ( "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/modules/caddytls" - "github.com/caddyserver/certmagic" "github.com/lucas-clemente/quic-go/http3" "go.uber.org/zap" ) @@ -113,10 +112,6 @@ type App struct { // affect functionality. Servers map[string]*Server `json:"servers,omitempty"` - // DefaultSNI if set configures all certificate lookups to fallback to use - // this SNI name if a more specific certificate could not be found - DefaultSNI string `json:"default_sni,omitempty"` - servers []*http.Server h3servers []*http3.Server h3listeners []net.PacketConn @@ -150,8 +145,6 @@ func (app *App) Provision(ctx caddy.Context) error { repl := caddy.NewReplacer() - certmagic.Default.DefaultServerName = app.DefaultSNI - // this provisions the matchers for each route, // and prepares auto HTTP->HTTPS redirects, and // is required before we provision each server @@ -278,13 +271,6 @@ func (app *App) Start() error { return fmt.Errorf("%s: listening on %s: %v", listenAddr.Network, hostport, err) } - // enable HTTP/2 by default - for _, pol := range srv.TLSConnPolicies { - if len(pol.ALPN) == 0 { - pol.ALPN = append(pol.ALPN, defaultALPN...) - } - } - // enable TLS if there is a policy and if this is not the HTTP port useTLS := len(srv.TLSConnPolicies) > 0 && int(listenAddr.StartPort+portOffset) != app.httpPort() if useTLS { @@ -397,8 +383,6 @@ func (app *App) httpsPort() int { return app.HTTPSPort } -var defaultALPN = []string{"h2", "http/1.1"} - // RequestMatcher is a type that can match to a request. // A route matcher MUST NOT modify the request, with the // only exception being its context. diff --git a/modules/caddytls/acmeissuer.go b/modules/caddytls/acmeissuer.go index f108d721e..53638fe55 100644 --- a/modules/caddytls/acmeissuer.go +++ b/modules/caddytls/acmeissuer.go @@ -144,6 +144,10 @@ func (m *ACMEIssuer) SetConfig(cfg *certmagic.Config) { m.magic = cfg } +// TODO: I kind of hate how each call to these methods needs to +// make a new ACME manager to fill in defaults before using; can +// we find the right place to do that just once and then re-use? + // PreCheck implements the certmagic.PreChecker interface. func (m *ACMEIssuer) PreCheck(names []string, interactive bool) error { return certmagic.NewACMEManager(m.magic, m.template).PreCheck(names, interactive) @@ -156,7 +160,7 @@ func (m *ACMEIssuer) Issue(ctx context.Context, csr *x509.CertificateRequest) (* // IssuerKey returns the unique issuer key for the configured CA endpoint. func (m *ACMEIssuer) IssuerKey() string { - return m.template.IssuerKey() // does not need storage and cache + return certmagic.NewACMEManager(m.magic, m.template).IssuerKey() } // Revoke revokes the given certificate. diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go index 5b830f982..7618db4f7 100644 --- a/modules/caddytls/connpolicy.go +++ b/modules/caddytls/connpolicy.go @@ -55,6 +55,11 @@ func (cp ConnectionPolicies) Provision(ctx caddy.Context) error { cp[i].certSelector = val.(certmagic.CertificateSelector) } + // enable HTTP/2 by default + if len(pol.ALPN) == 0 { + pol.ALPN = append(pol.ALPN, defaultALPN...) + } + // pre-build standard TLS config so we don't have to at handshake-time err = pol.buildStandardTLSConfig(ctx) if err != nil { @@ -452,3 +457,5 @@ func (a *PublicKeyAlgorithm) UnmarshalJSON(b []byte) error { type ConnectionMatcher interface { Match(*tls.ClientHelloInfo) bool } + +var defaultALPN = []string{"h2", "http/1.1"} diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index f91229f4c..0b39c712d 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -181,7 +181,6 @@ func (t *TLS) Validate() error { // ensure that host aren't repeated; since only the first // automation policy is used, repeating a host in the lists // isn't useful and is probably a mistake - // TODO: test this hostSet := make(map[string]int) for i, ap := range t.Automation.Policies { for _, h := range ap.Hosts { @@ -279,8 +278,8 @@ func (t *TLS) HandleHTTPChallenge(w http.ResponseWriter, r *http.Request) bool { if ap.magic.Issuer == nil { return false } - if am, ok := ap.magic.Issuer.(*certmagic.ACMEManager); ok { - return am.HandleHTTPChallenge(w, r) + if am, ok := ap.magic.Issuer.(*ACMEIssuer); ok { + return certmagic.NewACMEManager(am.magic, am.template).HandleHTTPChallenge(w, r) } return false } @@ -709,7 +708,7 @@ const automateKey = "automate" // (beta 16 changed the storage path for certificates), // after which this function can be deleted func (t *TLS) moveCertificates() error { - log := t.logger.Named("automigrate") + logger := t.logger.Named("automigrate") baseDir := caddy.AppDataDir() @@ -760,7 +759,7 @@ func (t *TLS) moveCertificates() error { } if len(oldAcmeSites) > 0 { - log.Warn("certificate storage path has changed; attempting one-time auto-migration", + logger.Warn("certificate storage path has changed; attempting one-time auto-migration", zap.String("old_folder", oldAcmeSitesDir), zap.String("new_folder", newBaseDir), zap.String("details", "https://github.com/caddyserver/caddy/issues/2955")) @@ -775,13 +774,13 @@ func (t *TLS) moveCertificates() error { // move the folder oldPath := filepath.Join(oldAcmeSitesDir, siteInfo.Name()) newPath := filepath.Join(newBaseDir, siteInfo.Name()) - log.Info("moving certificate assets", + logger.Info("moving certificate assets", zap.String("ca", oldCA), zap.String("site", siteInfo.Name()), zap.String("destination", newPath)) err = os.Rename(oldPath, newPath) if err != nil { - log.Error("failed moving site to new path; skipping", + logger.Error("failed moving site to new path; skipping", zap.String("old_path", oldPath), zap.String("new_path", newPath), zap.Error(err)) @@ -792,7 +791,7 @@ func (t *TLS) moveCertificates() error { metaFilePath := filepath.Join(newPath, siteInfo.Name()+".json") metaContents, err := ioutil.ReadFile(metaFilePath) if err != nil { - log.Error("could not read metadata file", + logger.Error("could not read metadata file", zap.String("filename", metaFilePath), zap.Error(err)) continue @@ -806,12 +805,12 @@ func (t *TLS) moveCertificates() error { } newMeta, err := json.MarshalIndent(cr, "", "\t") if err != nil { - log.Error("encoding new metadata file", zap.Error(err)) + logger.Error("encoding new metadata file", zap.Error(err)) continue } err = ioutil.WriteFile(metaFilePath, newMeta, 0600) if err != nil { - log.Error("writing new metadata file", zap.Error(err)) + logger.Error("writing new metadata file", zap.Error(err)) continue } } |