diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c5c89b502..426b2ba74 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -51,7 +51,7 @@ jobs: check-latest: true - name: golangci-lint - uses: golangci/golangci-lint-action@v6 + uses: golangci/golangci-lint-action@v8 with: version: latest diff --git a/.golangci.yml b/.golangci.yml index aecff563e..0f1082e72 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,27 +1,15 @@ -linters-settings: - errcheck: - exclude-functions: - - fmt.* - - (go.uber.org/zap/zapcore.ObjectEncoder).AddObject - - (go.uber.org/zap/zapcore.ObjectEncoder).AddArray - gci: - sections: - - standard # Standard section: captures all standard packages. - - default # Default section: contains all imports that could not be matched to another section type. - - prefix(github.com/caddyserver/caddy/v2/cmd) # ensure that this is always at the top and always has a line break. - - prefix(github.com/caddyserver/caddy) # Custom section: groups all imports with the specified Prefix. - # Skip generated files. - # Default: true - skip-generated: true - # Enable custom order of sections. - # If `true`, make the section order the same as the order of `sections`. - # Default: false - custom-order: true - exhaustive: - ignore-enum-types: reflect.Kind|svc.Cmd - +version: "2" +run: + issues-exit-code: 1 + tests: false +output: + formats: + text: + path: stdout + print-linter-name: true + print-issued-lines: true linters: - disable-all: true + default: none enable: - asasalint - asciicheck @@ -35,148 +23,96 @@ linters: - errcheck - errname - exhaustive - - gci - - gofmt - - goimports - - gofumpt - gosec - - gosimple - govet - - ineffassign - importas + - ineffassign - misspell - prealloc - promlinter - sloglint - sqlclosecheck - staticcheck - - tenv - testableexamples - testifylint - tparallel - - typecheck - unconvert - unused - wastedassign - whitespace - zerologlint - # these are implicitly disabled: - # - containedctx - # - contextcheck - # - cyclop - # - depguard - # - errchkjson - # - errorlint - # - exhaustruct - # - execinquery - # - exhaustruct - # - forbidigo - # - forcetypeassert - # - funlen - # - ginkgolinter - # - gocheckcompilerdirectives - # - gochecknoglobals - # - gochecknoinits - # - gochecksumtype - # - gocognit - # - goconst - # - gocritic - # - gocyclo - # - godot - # - godox - # - goerr113 - # - goheader - # - gomnd - # - gomoddirectives - # - gomodguard - # - goprintffuncname - # - gosmopolitan - # - grouper - # - inamedparam - # - interfacebloat - # - ireturn - # - lll - # - loggercheck - # - maintidx - # - makezero - # - mirror - # - musttag - # - nakedret - # - nestif - # - nilerr - # - nilnil - # - nlreturn - # - noctx - # - nolintlint - # - nonamedreturns - # - nosprintfhostport - # - paralleltest - # - perfsprint - # - predeclared - # - protogetter - # - reassign - # - revive - # - rowserrcheck - # - stylecheck - # - tagalign - # - tagliatelle - # - testpackage - # - thelper - # - unparam - # - usestdlibvars - # - varnamelen - # - wrapcheck - # - wsl - -run: - # default concurrency is a available CPU number. - # concurrency: 4 # explicitly omit this value to fully utilize available resources. - timeout: 5m - issues-exit-code: 1 - tests: false - -# output configuration options -output: - formats: - - format: 'colored-line-number' - print-issued-lines: true - print-linter-name: true - -issues: - exclude-rules: - - text: 'G115' # TODO: Either we should fix the issues or nuke the linter if it's bad - linters: - - gosec - # we aren't calling unknown URL - - text: 'G107' # G107: Url provided to HTTP request as taint input - linters: - - gosec - # as a web server that's expected to handle any template, this is totally in the hands of the user. - - text: 'G203' # G203: Use of unescaped data in HTML templates - linters: - - gosec - # we're shelling out to known commands, not relying on user-defined input. - - text: 'G204' # G204: Audit use of command execution - linters: - - gosec - # the choice of weakrand is deliberate, hence the named import "weakrand" - - path: modules/caddyhttp/reverseproxy/selectionpolicies.go - text: 'G404' # G404: Insecure random number source (rand) - linters: - - gosec - - path: modules/caddyhttp/reverseproxy/streaming.go - text: 'G404' # G404: Insecure random number source (rand) - linters: - - gosec - - path: modules/logging/filters.go - linters: - - dupl - - path: modules/caddyhttp/matchers.go - linters: - - dupl - - path: modules/caddyhttp/vars.go - linters: - - dupl - - path: _test\.go - linters: - - errcheck + settings: + staticcheck: + checks: ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-QF1006", "-QF1008"] # default, and exclude 1 more undesired check + errcheck: + exclude-functions: + - fmt.* + - (go.uber.org/zap/zapcore.ObjectEncoder).AddObject + - (go.uber.org/zap/zapcore.ObjectEncoder).AddArray + exhaustive: + ignore-enum-types: reflect.Kind|svc.Cmd + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - gosec + text: G115 # TODO: Either we should fix the issues or nuke the linter if it's bad + - linters: + - gosec + text: G107 # we aren't calling unknown URL + - linters: + - gosec + text: G203 # as a web server that's expected to handle any template, this is totally in the hands of the user. + - linters: + - gosec + text: G204 # we're shelling out to known commands, not relying on user-defined input. + - linters: + - gosec + # the choice of weakrand is deliberate, hence the named import "weakrand" + path: modules/caddyhttp/reverseproxy/selectionpolicies.go + text: G404 + - linters: + - gosec + path: modules/caddyhttp/reverseproxy/streaming.go + text: G404 + - linters: + - dupl + path: modules/logging/filters.go + - linters: + - dupl + path: modules/caddyhttp/matchers.go + - linters: + - dupl + path: modules/caddyhttp/vars.go + - linters: + - errcheck + path: _test\.go + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - gci + - gofmt + - gofumpt + - goimports + settings: + gci: + sections: + - standard # Standard section: captures all standard packages. + - default # Default section: contains all imports that could not be matched to another section type. + - prefix(github.com/caddyserver/caddy/v2/cmd) # ensure that this is always at the top and always has a line break. + - prefix(github.com/caddyserver/caddy) # Custom section: groups all imports with the specified Prefix. + custom-order: true + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/admin.go b/admin.go index 244c7d719..45bd574a1 100644 --- a/admin.go +++ b/admin.go @@ -424,6 +424,13 @@ func replaceLocalAdminServer(cfg *Config, ctx Context) error { handler := cfg.Admin.newAdminHandler(addr, false, ctx) + // run the provisioners for loaded modules to make sure local + // state is properly re-initialized in the new admin server + err = cfg.Admin.provisionAdminRouters(ctx) + if err != nil { + return err + } + ln, err := addr.Listen(context.TODO(), 0, net.ListenConfig{}) if err != nil { return err @@ -545,6 +552,13 @@ func replaceRemoteAdminServer(ctx Context, cfg *Config) error { // because we are using TLS authentication instead handler := cfg.Admin.newAdminHandler(addr, true, ctx) + // run the provisioners for loaded modules to make sure local + // state is properly re-initialized in the new admin server + err = cfg.Admin.provisionAdminRouters(ctx) + if err != nil { + return err + } + // create client certificate pool for TLS mutual auth, and extract public keys // so that we can enforce access controls at the application layer clientCertPool := x509.NewCertPool() diff --git a/admin_test.go b/admin_test.go index e3d235b66..c637f92a9 100644 --- a/admin_test.go +++ b/admin_test.go @@ -19,6 +19,7 @@ import ( "crypto/x509" "encoding/json" "fmt" + "maps" "net/http" "net/http/httptest" "reflect" @@ -335,9 +336,7 @@ func TestAdminHandlerBuiltinRouteErrors(t *testing.T) { func testGetMetricValue(labels map[string]string) float64 { promLabels := prometheus.Labels{} - for k, v := range labels { - promLabels[k] = v - } + maps.Copy(promLabels, labels) metric, err := adminMetrics.requestErrors.GetMetricWith(promLabels) if err != nil { @@ -377,9 +376,7 @@ func (m *mockModule) CaddyModule() ModuleInfo { func TestNewAdminHandlerRouterRegistration(t *testing.T) { originalModules := make(map[string]ModuleInfo) - for k, v := range modules { - originalModules[k] = v - } + maps.Copy(originalModules, modules) defer func() { modules = originalModules }() @@ -479,9 +476,7 @@ func TestAdminRouterProvisioning(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { originalModules := make(map[string]ModuleInfo) - for k, v := range modules { - originalModules[k] = v - } + maps.Copy(originalModules, modules) defer func() { modules = originalModules }() @@ -774,9 +769,7 @@ func (m *mockIssuerModule) CaddyModule() ModuleInfo { func TestManageIdentity(t *testing.T) { originalModules := make(map[string]ModuleInfo) - for k, v := range modules { - originalModules[k] = v - } + maps.Copy(originalModules, modules) defer func() { modules = originalModules }() diff --git a/caddy.go b/caddy.go index d6a2ae0b3..80290cee9 100644 --- a/caddy.go +++ b/caddy.go @@ -505,14 +505,6 @@ func provisionContext(newCfg *Config, replaceAdminServer bool) (Context, error) return ctx, err } - // start the admin endpoint (and stop any prior one) - if replaceAdminServer { - err = replaceLocalAdminServer(newCfg, ctx) - if err != nil { - return ctx, fmt.Errorf("starting caddy administration endpoint: %v", err) - } - } - // create the new filesystem map newCfg.fileSystems = &filesystems.FileSystemMap{} @@ -544,6 +536,14 @@ func provisionContext(newCfg *Config, replaceAdminServer bool) (Context, error) return ctx, err } + // start the admin endpoint (and stop any prior one) + if replaceAdminServer { + err = replaceLocalAdminServer(newCfg, ctx) + if err != nil { + return ctx, fmt.Errorf("starting caddy administration endpoint: %v", err) + } + } + // Load and Provision each app and their submodules err = func() error { for appName := range newCfg.AppsRaw { @@ -1104,9 +1104,15 @@ func (e Event) Origin() Module { return e.origin } // Returns the module t // CloudEvents spec. func (e Event) CloudEvent() CloudEvent { dataJSON, _ := json.Marshal(e.Data) + var source string + if e.Origin() == nil { + source = "caddy" + } else { + source = string(e.Origin().CaddyModule().ID) + } return CloudEvent{ ID: e.id.String(), - Source: e.origin.CaddyModule().String(), + Source: source, SpecVersion: "1.0", Type: e.name, Time: e.ts, diff --git a/caddy_test.go b/caddy_test.go index adf14350e..08fa5c0d0 100644 --- a/caddy_test.go +++ b/caddy_test.go @@ -15,6 +15,7 @@ package caddy import ( + "context" "testing" "time" ) @@ -72,3 +73,21 @@ func TestParseDuration(t *testing.T) { } } } + +func TestEvent_CloudEvent_NilOrigin(t *testing.T) { + ctx, _ := NewContext(Context{Context: context.Background()}) // module will be nil by default + event, err := NewEvent(ctx, "started", nil) + if err != nil { + t.Fatalf("NewEvent() error = %v", err) + } + + // This should not panic + ce := event.CloudEvent() + + if ce.Source != "caddy" { + t.Errorf("Expected CloudEvent Source to be 'caddy', got '%s'", ce.Source) + } + if ce.Type != "started" { + t.Errorf("Expected CloudEvent Type to be 'started', got '%s'", ce.Type) + } +} diff --git a/caddyconfig/caddyfile/adapter.go b/caddyconfig/caddyfile/adapter.go index da4f98337..449370dc6 100644 --- a/caddyconfig/caddyfile/adapter.go +++ b/caddyconfig/caddyfile/adapter.go @@ -68,7 +68,7 @@ func (a Adapter) Adapt(body []byte, options map[string]any) ([]byte, []caddyconf // TODO: also perform this check on imported files func FormattingDifference(filename string, body []byte) (caddyconfig.Warning, bool) { // replace windows-style newlines to normalize comparison - normalizedBody := bytes.Replace(body, []byte("\r\n"), []byte("\n"), -1) + normalizedBody := bytes.ReplaceAll(body, []byte("\r\n"), []byte("\n")) formatted := Format(normalizedBody) if bytes.Equal(formatted, normalizedBody) { diff --git a/caddyconfig/caddyfile/formatter.go b/caddyconfig/caddyfile/formatter.go index f1d12fa51..0476a9b93 100644 --- a/caddyconfig/caddyfile/formatter.go +++ b/caddyconfig/caddyfile/formatter.go @@ -94,7 +94,7 @@ func Format(input []byte) []byte { } // detect whether we have the start of a heredoc - if !quoted && !(heredoc != heredocClosed || heredocEscaped) && + if !quoted && (heredoc == heredocClosed && !heredocEscaped) && space && last == '<' && ch == '<' { write(ch) heredoc = heredocOpening diff --git a/caddyconfig/caddyfile/lexer.go b/caddyconfig/caddyfile/lexer.go index 9b523f397..a3b1fc7db 100644 --- a/caddyconfig/caddyfile/lexer.go +++ b/caddyconfig/caddyfile/lexer.go @@ -137,7 +137,7 @@ func (l *lexer) next() (bool, error) { } // detect whether we have the start of a heredoc - if !(quoted || btQuoted) && !(inHeredoc || heredocEscaped) && + if (!quoted && !btQuoted) && (!inHeredoc && !heredocEscaped) && len(val) > 1 && string(val[:2]) == "<<" { // a space means it's just a regular token and not a heredoc if ch == ' ' { diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index 3fc08b2c8..d41ef413d 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/caddyconfig/httpcaddyfile/builtins.go @@ -15,6 +15,7 @@ package httpcaddyfile import ( + "encoding/json" "fmt" "html" "net/http" @@ -843,13 +844,18 @@ func parseHandleErrors(h Helper) ([]ConfigValue, error) { return nil, h.Errf("segment was not parsed as a subroute") } + // wrap the subroutes + wrappingRoute := caddyhttp.Route{ + HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject(subroute, "handler", "subroute", nil)}, + } + subroute = &caddyhttp.Subroute{ + Routes: []caddyhttp.Route{wrappingRoute}, + } if expression != "" { statusMatcher := caddy.ModuleMap{ "expression": h.JSON(caddyhttp.MatchExpression{Expr: expression}), } - for i := range subroute.Routes { - subroute.Routes[i].MatcherSetsRaw = []caddy.ModuleMap{statusMatcher} - } + subroute.Routes[0].MatcherSetsRaw = []caddy.ModuleMap{statusMatcher} } return []ConfigValue{ { diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go index f0687a7e9..eac7f5dc2 100644 --- a/caddyconfig/httpcaddyfile/directives.go +++ b/caddyconfig/httpcaddyfile/directives.go @@ -16,6 +16,7 @@ package httpcaddyfile import ( "encoding/json" + "maps" "net" "slices" "sort" @@ -173,10 +174,12 @@ func RegisterDirectiveOrder(dir string, position Positional, standardDir string) if d != standardDir { continue } - if position == Before { + switch position { + case Before: newOrder = append(newOrder[:i], append([]string{dir}, newOrder[i:]...)...) - } else if position == After { + case After: newOrder = append(newOrder[:i+1], append([]string{dir}, newOrder[i+1:]...)...) + case First, Last: } break } @@ -365,9 +368,7 @@ func parseSegmentAsConfig(h Helper) ([]ConfigValue, error) { // copy existing matcher definitions so we can augment // new ones that are defined only in this scope matcherDefs := make(map[string]caddy.ModuleMap, len(h.matcherDefs)) - for key, val := range h.matcherDefs { - matcherDefs[key] = val - } + maps.Copy(matcherDefs, h.matcherDefs) // find and extract any embedded matcher definitions in this scope for i := 0; i < len(segments); i++ { @@ -483,12 +484,29 @@ func sortRoutes(routes []ConfigValue) { // we can only confidently compare path lengths if both // directives have a single path to match (issue #5037) if iPathLen > 0 && jPathLen > 0 { + // trim the trailing wildcard if there is one + iPathTrimmed := strings.TrimSuffix(iPM[0], "*") + jPathTrimmed := strings.TrimSuffix(jPM[0], "*") + // if both paths are the same except for a trailing wildcard, // sort by the shorter path first (which is more specific) - if strings.TrimSuffix(iPM[0], "*") == strings.TrimSuffix(jPM[0], "*") { + if iPathTrimmed == jPathTrimmed { return iPathLen < jPathLen } + // we use the trimmed length to compare the paths + // https://github.com/caddyserver/caddy/issues/7012#issuecomment-2870142195 + // credit to https://github.com/Hellio404 + // for sorts with many items, mixing matchers w/ and w/o wildcards will confuse the sort and result in incorrect orders + iPathLen = len(iPathTrimmed) + jPathLen = len(jPathTrimmed) + + // if both paths have the same length, sort lexically + // https://github.com/caddyserver/caddy/pull/7015#issuecomment-2871993588 + if iPathLen == jPathLen { + return iPathTrimmed < jPathTrimmed + } + // sort most-specific (longest) path first return iPathLen > jPathLen } diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index ae6f5ddee..3dcd3ea5b 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -633,12 +633,6 @@ func (st *ServerType) serversFromPairings( srv.AutoHTTPS = new(caddyhttp.AutoHTTPSConfig) } srv.AutoHTTPS.IgnoreLoadedCerts = true - - case "prefer_wildcard": - if srv.AutoHTTPS == nil { - srv.AutoHTTPS = new(caddyhttp.AutoHTTPSConfig) - } - srv.AutoHTTPS.PreferWildcard = true } } @@ -706,16 +700,6 @@ func (st *ServerType) serversFromPairings( return specificity(iLongestHost) > specificity(jLongestHost) }) - // collect all hosts that have a wildcard in them - wildcardHosts := []string{} - for _, sblock := range p.serverBlocks { - for _, addr := range sblock.parsedKeys { - if strings.HasPrefix(addr.Host, "*.") { - wildcardHosts = append(wildcardHosts, addr.Host[2:]) - } - } - } - var hasCatchAllTLSConnPolicy, addressQualifiesForTLS bool autoHTTPSWillAddConnPolicy := srv.AutoHTTPS == nil || !srv.AutoHTTPS.Disabled @@ -801,7 +785,13 @@ func (st *ServerType) serversFromPairings( cp.FallbackSNI = fallbackSNI } - // only append this policy if it actually changes something + // only append this policy if it actually changes something, + // or if the configuration explicitly automates certs for + // these names (this is necessary to hoist a connection policy + // above one that may manually load a wildcard cert that would + // otherwise clobber the automated one; the code that appends + // policies that manually load certs comes later, so they're + // lower in the list) if !cp.SettingsEmpty() || mapContains(forceAutomatedNames, hosts) { srv.TLSConnPolicies = append(srv.TLSConnPolicies, cp) hasCatchAllTLSConnPolicy = len(hosts) == 0 @@ -841,18 +831,6 @@ func (st *ServerType) serversFromPairings( addressQualifiesForTLS = true } - // If prefer wildcard is enabled, then we add hosts that are - // already covered by the wildcard to the skip list - if addressQualifiesForTLS && srv.AutoHTTPS != nil && srv.AutoHTTPS.PreferWildcard { - baseDomain := addr.Host - if idx := strings.Index(baseDomain, "."); idx != -1 { - baseDomain = baseDomain[idx+1:] - } - if !strings.HasPrefix(addr.Host, "*.") && slices.Contains(wildcardHosts, baseDomain) { - srv.AutoHTTPS.SkipCerts = append(srv.AutoHTTPS.SkipCerts, addr.Host) - } - } - // predict whether auto-HTTPS will add the conn policy for us; if so, we // may not need to add one for this server autoHTTPSWillAddConnPolicy = autoHTTPSWillAddConnPolicy && @@ -1083,11 +1061,40 @@ func consolidateConnPolicies(cps caddytls.ConnectionPolicies) (caddytls.Connecti // if they're exactly equal in every way, just keep one of them if reflect.DeepEqual(cps[i], cps[j]) { - cps = append(cps[:j], cps[j+1:]...) + cps = slices.Delete(cps, j, j+1) i-- break } + // as a special case, if there are adjacent TLS conn policies that are identical except + // by their matchers, and the matchers are specifically just ServerName ("sni") matchers + // (by far the most common), we can combine them into a single policy + if i == j-1 && len(cps[i].MatchersRaw) == 1 && len(cps[j].MatchersRaw) == 1 { + if iSNIMatcherJSON, ok := cps[i].MatchersRaw["sni"]; ok { + if jSNIMatcherJSON, ok := cps[j].MatchersRaw["sni"]; ok { + // position of policies and the matcher criteria check out; if settings are + // the same, then we can combine the policies; we have to unmarshal and + // remarshal the matchers though + if cps[i].SettingsEqual(*cps[j]) { + var iSNIMatcher caddytls.MatchServerName + if err := json.Unmarshal(iSNIMatcherJSON, &iSNIMatcher); err == nil { + var jSNIMatcher caddytls.MatchServerName + if err := json.Unmarshal(jSNIMatcherJSON, &jSNIMatcher); err == nil { + iSNIMatcher = append(iSNIMatcher, jSNIMatcher...) + cps[i].MatchersRaw["sni"], err = json.Marshal(iSNIMatcher) + if err != nil { + return nil, fmt.Errorf("recombining SNI matchers: %v", err) + } + cps = slices.Delete(cps, j, j+1) + i-- + break + } + } + } + } + } + } + // if they have the same matcher, try to reconcile each field: either they must // be identical, or we have to be able to combine them safely if reflect.DeepEqual(cps[i].MatchersRaw, cps[j].MatchersRaw) { @@ -1189,12 +1196,13 @@ func consolidateConnPolicies(cps caddytls.ConnectionPolicies) (caddytls.Connecti } } - cps = append(cps[:j], cps[j+1:]...) + cps = slices.Delete(cps, j, j+1) i-- break } } } + return cps, nil } diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go index 2eedca453..a0899dd8d 100644 --- a/caddyconfig/httpcaddyfile/tlsapp.go +++ b/caddyconfig/httpcaddyfile/tlsapp.go @@ -92,11 +92,9 @@ func (st ServerType) buildTLSApp( tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, catchAllAP) } - // collect all hosts that have a wildcard in them, and arent HTTP - wildcardHosts := []string{} - // hosts that have been explicitly marked to be automated, - // even if covered by another wildcard - forcedAutomatedNames := make(map[string]struct{}) + var wildcardHosts []string // collect all hosts that have a wildcard in them, and aren't HTTP + forcedAutomatedNames := make(map[string]struct{}) // explicitly configured to be automated, even if covered by a wildcard + for _, p := range pairings { var addresses []string for _, addressWithProtocols := range p.addressesWithProtocols { @@ -153,7 +151,7 @@ func (st ServerType) buildTLSApp( ap.OnDemand = true } - // collect hosts that are forced to be automated + // collect hosts that are forced to have certs automated for their specific name if _, ok := sblock.pile["tls.force_automate"]; ok { for _, host := range sblockHosts { forcedAutomatedNames[host] = struct{}{} @@ -375,7 +373,9 @@ func (st ServerType) buildTLSApp( return nil, warnings, err } for _, cfg := range ech.Configs { - ap.SubjectsRaw = append(ap.SubjectsRaw, cfg.PublicName) + if cfg.PublicName != "" { + ap.SubjectsRaw = append(ap.SubjectsRaw, cfg.PublicName) + } } if tlsApp.Automation == nil { tlsApp.Automation = new(caddytls.AutomationConfig) diff --git a/caddytest/caddytest.go b/caddytest/caddytest.go index 623c45e5e..7b56bb281 100644 --- a/caddytest/caddytest.go +++ b/caddytest/caddytest.go @@ -281,7 +281,7 @@ func validateTestPrerequisites(tc *Tester) error { tc.t.Cleanup(func() { os.Remove(f.Name()) }) - if _, err := f.WriteString(fmt.Sprintf(initConfig, tc.config.AdminPort)); err != nil { + if _, err := fmt.Fprintf(f, initConfig, tc.config.AdminPort); err != nil { return err } diff --git a/caddytest/integration/acme_test.go b/caddytest/integration/acme_test.go index d7e4c296d..f10aef6a8 100644 --- a/caddytest/integration/acme_test.go +++ b/caddytest/integration/acme_test.go @@ -12,13 +12,14 @@ import ( "strings" "testing" - "github.com/caddyserver/caddy/v2" - "github.com/caddyserver/caddy/v2/caddytest" "github.com/mholt/acmez/v3" "github.com/mholt/acmez/v3/acme" smallstepacme "github.com/smallstep/certificates/acme" "go.uber.org/zap" "go.uber.org/zap/exp/zapslog" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/caddytest" ) const acmeChallengePort = 9081 diff --git a/caddytest/integration/acmeserver_test.go b/caddytest/integration/acmeserver_test.go index ca5845f87..d6a9ba005 100644 --- a/caddytest/integration/acmeserver_test.go +++ b/caddytest/integration/acmeserver_test.go @@ -9,11 +9,12 @@ import ( "strings" "testing" - "github.com/caddyserver/caddy/v2/caddytest" "github.com/mholt/acmez/v3" "github.com/mholt/acmez/v3/acme" "go.uber.org/zap" "go.uber.org/zap/exp/zapslog" + + "github.com/caddyserver/caddy/v2/caddytest" ) func TestACMEServerDirectory(t *testing.T) { diff --git a/caddytest/integration/caddyfile_adapt/acme_server_policy-allow.caddyfiletest b/caddytest/integration/caddyfile_adapt/acme_server_policy-allow.caddyfiletest new file mode 100644 index 000000000..5d1d8a3bc --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/acme_server_policy-allow.caddyfiletest @@ -0,0 +1,72 @@ +{ + pki { + ca custom-ca { + name "Custom CA" + } + } +} + +acme.example.com { + acme_server { + ca custom-ca + allow { + domains host-1.internal.example.com host-2.internal.example.com + } + } +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "acme.example.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "ca": "custom-ca", + "handler": "acme_server", + "policy": { + "allow": { + "domains": [ + "host-1.internal.example.com", + "host-2.internal.example.com" + ] + } + } + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + }, + "pki": { + "certificate_authorities": { + "custom-ca": { + "name": "Custom CA" + } + } + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/acme_server_policy-both.caddyfiletest b/caddytest/integration/caddyfile_adapt/acme_server_policy-both.caddyfiletest new file mode 100644 index 000000000..15cdbba90 --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/acme_server_policy-both.caddyfiletest @@ -0,0 +1,80 @@ +{ + pki { + ca custom-ca { + name "Custom CA" + } + } +} + +acme.example.com { + acme_server { + ca custom-ca + allow { + domains host-1.internal.example.com host-2.internal.example.com + } + deny { + domains dc.internal.example.com + } + } +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "acme.example.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "ca": "custom-ca", + "handler": "acme_server", + "policy": { + "allow": { + "domains": [ + "host-1.internal.example.com", + "host-2.internal.example.com" + ] + }, + "deny": { + "domains": [ + "dc.internal.example.com" + ] + } + } + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + }, + "pki": { + "certificate_authorities": { + "custom-ca": { + "name": "Custom CA" + } + } + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/acme_server_policy-deny.caddyfiletest b/caddytest/integration/caddyfile_adapt/acme_server_policy-deny.caddyfiletest new file mode 100644 index 000000000..0478088c9 --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/acme_server_policy-deny.caddyfiletest @@ -0,0 +1,71 @@ +{ + pki { + ca custom-ca { + name "Custom CA" + } + } +} + +acme.example.com { + acme_server { + ca custom-ca + deny { + domains dc.internal.example.com + } + } +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "acme.example.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "ca": "custom-ca", + "handler": "acme_server", + "policy": { + "deny": { + "domains": [ + "dc.internal.example.com" + ] + } + } + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + }, + "pki": { + "certificate_authorities": { + "custom-ca": { + "name": "Custom CA" + } + } + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard.caddyfiletest b/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard.caddyfiletest deleted file mode 100644 index 04f2c4665..000000000 --- a/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard.caddyfiletest +++ /dev/null @@ -1,109 +0,0 @@ -{ - auto_https prefer_wildcard -} - -*.example.com { - tls { - dns mock - } - respond "fallback" -} - -foo.example.com { - respond "foo" -} ----------- -{ - "apps": { - "http": { - "servers": { - "srv0": { - "listen": [ - ":443" - ], - "routes": [ - { - "match": [ - { - "host": [ - "foo.example.com" - ] - } - ], - "handle": [ - { - "handler": "subroute", - "routes": [ - { - "handle": [ - { - "body": "foo", - "handler": "static_response" - } - ] - } - ] - } - ], - "terminal": true - }, - { - "match": [ - { - "host": [ - "*.example.com" - ] - } - ], - "handle": [ - { - "handler": "subroute", - "routes": [ - { - "handle": [ - { - "body": "fallback", - "handler": "static_response" - } - ] - } - ] - } - ], - "terminal": true - } - ], - "automatic_https": { - "skip_certificates": [ - "foo.example.com" - ], - "prefer_wildcard": true - } - } - } - }, - "tls": { - "automation": { - "policies": [ - { - "subjects": [ - "*.example.com" - ], - "issuers": [ - { - "challenges": { - "dns": { - "provider": { - "name": "mock" - } - } - }, - "module": "acme" - } - ] - } - ] - } - } - } -} \ No newline at end of file diff --git a/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard_multi.caddyfiletest b/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard_multi.caddyfiletest deleted file mode 100644 index 4f8c26a5d..000000000 --- a/caddytest/integration/caddyfile_adapt/auto_https_prefer_wildcard_multi.caddyfiletest +++ /dev/null @@ -1,268 +0,0 @@ -{ - auto_https prefer_wildcard -} - -# Covers two domains -*.one.example.com { - tls { - dns mock - } - respond "one fallback" -} - -# Is covered, should not get its own AP -foo.one.example.com { - respond "foo one" -} - -# This one has its own tls config so it doesn't get covered (escape hatch) -bar.one.example.com { - respond "bar one" - tls bar@bar.com -} - -# Covers nothing but AP gets consolidated with the first -*.two.example.com { - tls { - dns mock - } - respond "two fallback" -} - -# Is HTTP so it should not cover -http://*.three.example.com { - respond "three fallback" -} - -# Has no wildcard coverage so it gets an AP -foo.three.example.com { - respond "foo three" -} ----------- -{ - "apps": { - "http": { - "servers": { - "srv0": { - "listen": [ - ":443" - ], - "routes": [ - { - "match": [ - { - "host": [ - "foo.three.example.com" - ] - } - ], - "handle": [ - { - "handler": "subroute", - "routes": [ - { - "handle": [ - { - "body": "foo three", - "handler": "static_response" - } - ] - } - ] - } - ], - "terminal": true - }, - { - "match": [ - { - "host": [ - "foo.one.example.com" - ] - } - ], - "handle": [ - { - "handler": "subroute", - "routes": [ - { - "handle": [ - { - "body": "foo one", - "handler": "static_response" - } - ] - } - ] - } - ], - "terminal": true - }, - { - "match": [ - { - "host": [ - "bar.one.example.com" - ] - } - ], - "handle": [ - { - "handler": "subroute", - "routes": [ - { - "handle": [ - { - "body": "bar one", - "handler": "static_response" - } - ] - } - ] - } - ], - "terminal": true - }, - { - "match": [ - { - "host": [ - "*.one.example.com" - ] - } - ], - "handle": [ - { - "handler": "subroute", - "routes": [ - { - "handle": [ - { - "body": "one fallback", - "handler": "static_response" - } - ] - } - ] - } - ], - "terminal": true - }, - { - "match": [ - { - "host": [ - "*.two.example.com" - ] - } - ], - "handle": [ - { - "handler": "subroute", - "routes": [ - { - "handle": [ - { - "body": "two fallback", - "handler": "static_response" - } - ] - } - ] - } - ], - "terminal": true - } - ], - "automatic_https": { - "skip_certificates": [ - "foo.one.example.com", - "bar.one.example.com" - ], - "prefer_wildcard": true - } - }, - "srv1": { - "listen": [ - ":80" - ], - "routes": [ - { - "match": [ - { - "host": [ - "*.three.example.com" - ] - } - ], - "handle": [ - { - "handler": "subroute", - "routes": [ - { - "handle": [ - { - "body": "three fallback", - "handler": "static_response" - } - ] - } - ] - } - ], - "terminal": true - } - ], - "automatic_https": { - "prefer_wildcard": true - } - } - } - }, - "tls": { - "automation": { - "policies": [ - { - "subjects": [ - "foo.three.example.com" - ] - }, - { - "subjects": [ - "bar.one.example.com" - ], - "issuers": [ - { - "email": "bar@bar.com", - "module": "acme" - }, - { - "ca": "https://acme.zerossl.com/v2/DV90", - "email": "bar@bar.com", - "module": "acme" - } - ] - }, - { - "subjects": [ - "*.one.example.com", - "*.two.example.com" - ], - "issuers": [ - { - "challenges": { - "dns": { - "provider": { - "name": "mock" - } - } - }, - "module": "acme" - } - ] - } - ] - } - } - } -} \ No newline at end of file diff --git a/caddytest/integration/caddyfile_adapt/error_example.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_example.caddyfiletest index bd42aee55..6f3059ab2 100644 --- a/caddytest/integration/caddyfile_adapt/error_example.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_example.caddyfiletest @@ -106,20 +106,29 @@ example.com { "handler": "subroute", "routes": [ { - "group": "group0", "handle": [ { - "handler": "rewrite", - "uri": "/{http.error.status_code}.html" - } - ] - }, - { - "handle": [ - { - "handler": "file_server", - "hide": [ - "./Caddyfile" + "handler": "subroute", + "routes": [ + { + "group": "group0", + "handle": [ + { + "handler": "rewrite", + "uri": "/{http.error.status_code}.html" + } + ] + }, + { + "handle": [ + { + "handler": "file_server", + "hide": [ + "./Caddyfile" + ] + } + ] + } ] } ] diff --git a/caddytest/integration/caddyfile_adapt/error_multi_site_blocks.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_multi_site_blocks.caddyfiletest index 0e84a13c2..1bec4b3e8 100644 --- a/caddytest/integration/caddyfile_adapt/error_multi_site_blocks.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_multi_site_blocks.caddyfiletest @@ -165,8 +165,17 @@ bar.localhost { { "handle": [ { - "body": "404 or 410 error", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "404 or 410 error", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -178,8 +187,17 @@ bar.localhost { { "handle": [ { - "body": "Error In range [500 .. 599]", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error In range [500 .. 599]", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -208,8 +226,17 @@ bar.localhost { { "handle": [ { - "body": "404 or 410 error from second site", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "404 or 410 error from second site", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -221,8 +248,17 @@ bar.localhost { { "handle": [ { - "body": "Error In range [500 .. 599] from second site", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error In range [500 .. 599] from second site", + "handler": "static_response" + } + ] + } + ] } ], "match": [ diff --git a/caddytest/integration/caddyfile_adapt/error_range_codes.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_range_codes.caddyfiletest index 46b70c8e3..299abc5f1 100644 --- a/caddytest/integration/caddyfile_adapt/error_range_codes.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_range_codes.caddyfiletest @@ -96,8 +96,17 @@ localhost:3010 { { "handle": [ { - "body": "Error in the [400 .. 499] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error in the [400 .. 499] range", + "handler": "static_response" + } + ] + } + ] } ], "match": [ diff --git a/caddytest/integration/caddyfile_adapt/error_range_simple_codes.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_range_simple_codes.caddyfiletest index 70158830c..9d4d2645c 100644 --- a/caddytest/integration/caddyfile_adapt/error_range_simple_codes.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_range_simple_codes.caddyfiletest @@ -116,8 +116,17 @@ localhost:2099 { { "handle": [ { - "body": "Error in the [400 .. 499] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error in the [400 .. 499] range", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -129,8 +138,17 @@ localhost:2099 { { "handle": [ { - "body": "Error code is equal to 500 or in the [300..399] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error code is equal to 500 or in the [300..399] range", + "handler": "static_response" + } + ] + } + ] } ], "match": [ diff --git a/caddytest/integration/caddyfile_adapt/error_simple_codes.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_simple_codes.caddyfiletest index 5ac5863e3..29a51ffad 100644 --- a/caddytest/integration/caddyfile_adapt/error_simple_codes.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_simple_codes.caddyfiletest @@ -96,8 +96,17 @@ localhost:3010 { { "handle": [ { - "body": "404 or 410 error", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "404 or 410 error", + "handler": "static_response" + } + ] + } + ] } ], "match": [ diff --git a/caddytest/integration/caddyfile_adapt/error_sort.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_sort.caddyfiletest index 63701cccb..1faf9b3bd 100644 --- a/caddytest/integration/caddyfile_adapt/error_sort.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_sort.caddyfiletest @@ -116,8 +116,17 @@ localhost:2099 { { "handle": [ { - "body": "Error in the [400 .. 499] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error in the [400 .. 499] range", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -129,8 +138,17 @@ localhost:2099 { { "handle": [ { - "body": "Fallback route: code outside the [400..499] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Fallback route: code outside the [400..499] range", + "handler": "static_response" + } + ] + } + ] } ] } diff --git a/caddytest/integration/caddyfile_adapt/error_subhandlers.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_subhandlers.caddyfiletest new file mode 100644 index 000000000..54429a73e --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/error_subhandlers.caddyfiletest @@ -0,0 +1,260 @@ +{ + http_port 2099 +} +localhost:2099 { + root * /var/www/ + file_server + + handle_errors 404 { + handle /en/* { + respond "not found" 404 + } + handle /es/* { + respond "no encontrado" + } + handle { + respond "default not found" + } + } + handle_errors { + handle /en/* { + respond "English error" + } + handle /es/* { + respond "Spanish error" + } + handle { + respond "Default error" + } + } +} +---------- +{ + "apps": { + "http": { + "http_port": 2099, + "servers": { + "srv0": { + "listen": [ + ":2099" + ], + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "handler": "vars", + "root": "/var/www/" + }, + { + "handler": "file_server", + "hide": [ + "./Caddyfile" + ] + } + ] + } + ] + } + ], + "terminal": true + } + ], + "errors": { + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "group": "group3", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "not found", + "handler": "static_response", + "status_code": 404 + } + ] + } + ] + } + ], + "match": [ + { + "path": [ + "/en/*" + ] + } + ] + }, + { + "group": "group3", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "no encontrado", + "handler": "static_response" + } + ] + } + ] + } + ], + "match": [ + { + "path": [ + "/es/*" + ] + } + ] + }, + { + "group": "group3", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "default not found", + "handler": "static_response" + } + ] + } + ] + } + ] + } + ] + } + ], + "match": [ + { + "expression": "{http.error.status_code} in [404]" + } + ] + }, + { + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "group": "group8", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "English error", + "handler": "static_response" + } + ] + } + ] + } + ], + "match": [ + { + "path": [ + "/en/*" + ] + } + ] + }, + { + "group": "group8", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Spanish error", + "handler": "static_response" + } + ] + } + ] + } + ], + "match": [ + { + "path": [ + "/es/*" + ] + } + ] + }, + { + "group": "group8", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Default error", + "handler": "static_response" + } + ] + } + ] + } + ] + } + ] + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + } + } + } +} + diff --git a/caddytest/integration/caddyfile_adapt/tls_automation_wildcard_force_automate.caddyfiletest b/caddytest/integration/caddyfile_adapt/tls_automation_wildcard_force_automate.caddyfiletest index 4eb6c4f1c..623bafd70 100644 --- a/caddytest/integration/caddyfile_adapt/tls_automation_wildcard_force_automate.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/tls_automation_wildcard_force_automate.caddyfiletest @@ -131,13 +131,7 @@ shadowed.example.com { { "match": { "sni": [ - "automated1.example.com" - ] - } - }, - { - "match": { - "sni": [ + "automated1.example.com", "automated2.example.com" ] } diff --git a/caddytest/integration/caddyfile_adapt_test.go b/caddytest/integration/caddyfile_adapt_test.go index 0d9f0fa47..674036e17 100644 --- a/caddytest/integration/caddyfile_adapt_test.go +++ b/caddytest/integration/caddyfile_adapt_test.go @@ -10,7 +10,6 @@ import ( "testing" "github.com/caddyserver/caddy/v2/caddytest" - _ "github.com/caddyserver/caddy/v2/internal/testmocks" ) diff --git a/caddytest/integration/caddyfile_test.go b/caddytest/integration/caddyfile_test.go index 11ffc08ae..d45d5a5e9 100644 --- a/caddytest/integration/caddyfile_test.go +++ b/caddytest/integration/caddyfile_test.go @@ -615,7 +615,6 @@ func TestReplaceWithReplacementPlaceholder(t *testing.T) { respond "{query}"`, "caddyfile") tester.AssertGetResponse("http://localhost:9080/endpoint?placeholder=baz&foo=bar", 200, "foo=baz&placeholder=baz") - } func TestReplaceWithKeyPlaceholder(t *testing.T) { @@ -783,6 +782,46 @@ func TestHandleErrorRangeAndCodes(t *testing.T) { tester.AssertGetResponse("http://localhost:9080/private", 410, "Error in the [400 .. 499] range") } +func TestHandleErrorSubHandlers(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(`{ + admin localhost:2999 + http_port 9080 + } + localhost:9080 { + root * /srv + file_server + error /*/internalerr* "Internal Server Error" 500 + + handle_errors 404 { + handle /en/* { + respond "not found" 404 + } + handle /es/* { + respond "no encontrado" 404 + } + handle { + respond "default not found" + } + } + handle_errors { + handle { + respond "Default error" + } + handle /en/* { + respond "English error" + } + } + } + `, "caddyfile") + // act and assert + tester.AssertGetResponse("http://localhost:9080/en/notfound", 404, "not found") + tester.AssertGetResponse("http://localhost:9080/es/notfound", 404, "no encontrado") + tester.AssertGetResponse("http://localhost:9080/notfound", 404, "default not found") + tester.AssertGetResponse("http://localhost:9080/es/internalerr", 500, "Default error") + tester.AssertGetResponse("http://localhost:9080/en/internalerr", 500, "English error") +} + func TestInvalidSiteAddressesAsDirectives(t *testing.T) { type testCase struct { config, expectedError string diff --git a/caddytest/integration/mockdns_test.go b/caddytest/integration/mockdns_test.go index 615116a3a..31dc4be7b 100644 --- a/caddytest/integration/mockdns_test.go +++ b/caddytest/integration/mockdns_test.go @@ -3,10 +3,11 @@ package integration import ( "context" - "github.com/caddyserver/caddy/v2" - "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/certmagic" "github.com/libdns/libdns" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" ) func init() { @@ -55,7 +56,9 @@ func (MockDNSProvider) SetRecords(ctx context.Context, zone string, recs []libdn } // Interface guard -var _ caddyfile.Unmarshaler = (*MockDNSProvider)(nil) -var _ certmagic.DNSProvider = (*MockDNSProvider)(nil) -var _ caddy.Provisioner = (*MockDNSProvider)(nil) -var _ caddy.Module = (*MockDNSProvider)(nil) +var ( + _ caddyfile.Unmarshaler = (*MockDNSProvider)(nil) + _ certmagic.DNSProvider = (*MockDNSProvider)(nil) + _ caddy.Provisioner = (*MockDNSProvider)(nil) + _ caddy.Module = (*MockDNSProvider)(nil) +) diff --git a/caddytest/integration/stream_test.go b/caddytest/integration/stream_test.go index d2f2fd79b..57231a527 100644 --- a/caddytest/integration/stream_test.go +++ b/caddytest/integration/stream_test.go @@ -13,9 +13,10 @@ import ( "testing" "time" - "github.com/caddyserver/caddy/v2/caddytest" "golang.org/x/net/http2" "golang.org/x/net/http2/h2c" + + "github.com/caddyserver/caddy/v2/caddytest" ) // (see https://github.com/caddyserver/caddy/issues/3556 for use case) diff --git a/cmd/commandfuncs.go b/cmd/commandfuncs.go index 5127c0f90..1660e0f6f 100644 --- a/cmd/commandfuncs.go +++ b/cmd/commandfuncs.go @@ -24,6 +24,7 @@ import ( "io" "io/fs" "log" + "maps" "net" "net/http" "os" @@ -703,9 +704,7 @@ func AdminAPIRequest(adminAddr, method, uri string, headers http.Header, body io if body != nil { req.Header.Set("Content-Type", "application/json") } - for k, v := range headers { - req.Header[k] = v - } + maps.Copy(req.Header, headers) // make an HTTP client that dials our network type, since admin // endpoints aren't always TCP, which is what the default transport diff --git a/cmd/main.go b/cmd/main.go index 87fa9fb95..47d702ca7 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -418,7 +418,7 @@ func parseEnvFile(envInput io.Reader) (map[string]string, error) { // quoted value: support newlines if strings.HasPrefix(val, `"`) || strings.HasPrefix(val, "'") { quote := string(val[0]) - for !(strings.HasSuffix(line, quote) && !strings.HasSuffix(line, `\`+quote)) { + for !strings.HasSuffix(line, quote) || strings.HasSuffix(line, `\`+quote) { val = strings.ReplaceAll(val, `\`+quote, quote) if !scanner.Scan() { break diff --git a/cmd/main_test.go b/cmd/main_test.go index 3b2412c57..bff34f443 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -235,7 +235,6 @@ func Test_isCaddyfile(t *testing.T) { wantErr: false, }, { - name: "json is not caddyfile but not error", args: args{ configFile: "./Caddyfile.json", @@ -245,7 +244,6 @@ func Test_isCaddyfile(t *testing.T) { wantErr: false, }, { - name: "prefix of Caddyfile and ./ with any extension is Caddyfile", args: args{ configFile: "./Caddyfile.prd", @@ -255,7 +253,6 @@ func Test_isCaddyfile(t *testing.T) { wantErr: false, }, { - name: "prefix of Caddyfile without ./ with any extension is Caddyfile", args: args{ configFile: "Caddyfile.prd", diff --git a/cmd/packagesfuncs.go b/cmd/packagesfuncs.go index 695232001..cda6f31f6 100644 --- a/cmd/packagesfuncs.go +++ b/cmd/packagesfuncs.go @@ -84,7 +84,7 @@ func cmdAddPackage(fl Flags) (int, error) { return caddy.ExitCodeFailedStartup, fmt.Errorf("invalid module name: %v", err) } // only allow a version to be specified if it's different from the existing version - if _, ok := pluginPkgs[module]; ok && !(version != "" && pluginPkgs[module].Version != version) { + if _, ok := pluginPkgs[module]; ok && (version == "" || pluginPkgs[module].Version == version) { return caddy.ExitCodeFailedStartup, fmt.Errorf("package is already added") } pluginPkgs[module] = pluginPackage{Version: version, Path: module} diff --git a/context.go b/context.go index a65814f03..eb0979f3a 100644 --- a/context.go +++ b/context.go @@ -577,11 +577,11 @@ func (ctx Context) Slogger() *slog.Logger { if err != nil { panic("config missing, unable to create dev logger: " + err.Error()) } - return slog.New(zapslog.NewHandler(l.Core(), nil)) + return slog.New(zapslog.NewHandler(l.Core())) } mod := ctx.Module() if mod == nil { - return slog.New(zapslog.NewHandler(Log().Core(), nil)) + return slog.New(zapslog.NewHandler(Log().Core())) } return slog.New(zapslog.NewHandler(ctx.cfg.Logging.Logger(mod).Core(), zapslog.WithName(string(mod.CaddyModule().ID)), diff --git a/go.mod b/go.mod index f272fb02a..2a2993aa9 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,7 @@ require ( github.com/klauspost/cpuid/v2 v2.2.10 github.com/mholt/acmez/v3 v3.1.2 github.com/prometheus/client_golang v1.19.1 - github.com/quic-go/quic-go v0.50.1 + github.com/quic-go/quic-go v0.51.0 github.com/smallstep/certificates v0.26.1 github.com/smallstep/nosql v0.6.1 github.com/smallstep/truststore v0.13.0 @@ -39,7 +39,7 @@ require ( go.uber.org/zap/exp v0.3.0 golang.org/x/crypto v0.36.0 golang.org/x/crypto/x509roots/fallback v0.0.0-20250305170421-49bf5b80c810 - golang.org/x/net v0.37.0 + golang.org/x/net v0.38.0 golang.org/x/sync v0.12.0 golang.org/x/term v0.30.0 golang.org/x/time v0.11.0 diff --git a/go.sum b/go.sum index 807f6144c..a53eacf81 100644 --- a/go.sum +++ b/go.sum @@ -397,8 +397,8 @@ github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo= github.com/quic-go/qpack v0.5.1 h1:giqksBPnT/HDtZ6VhtFKgoLOWmlyo9Ei6u9PqzIMbhI= github.com/quic-go/qpack v0.5.1/go.mod h1:+PC4XFrEskIVkcLzpEkbLqq1uCoxPhQuvK5rH1ZgaEg= -github.com/quic-go/quic-go v0.50.1 h1:unsgjFIUqW8a2oopkY7YNONpV1gYND6Nt9hnt1PN94Q= -github.com/quic-go/quic-go v0.50.1/go.mod h1:Vim6OmUvlYdwBhXP9ZVrtGmCMWa3wEqhq3NgYrI8b4E= +github.com/quic-go/quic-go v0.51.0 h1:K8exxe9zXxeRKxaXxi/GpUqYiTrtdiWP8bo1KFya6Wc= +github.com/quic-go/quic-go v0.51.0/go.mod h1:MFlGGpcpJqRAfmYi6NC2cptDPSxRWTOGNuP4wqrWmzQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= @@ -633,8 +633,8 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= -golang.org/x/net v0.37.0 h1:1zLorHbz+LYj7MQlSf1+2tPIIgibq2eL5xkrGk6f+2c= -golang.org/x/net v0.37.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8= +golang.org/x/net v0.38.0 h1:vRMAPTMaeGqVhG5QyLJHqNDwecKTomGeqbnfZyKlBI8= +golang.org/x/net v0.38.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= diff --git a/internal/logs.go b/internal/logs.go new file mode 100644 index 000000000..4ed4a572e --- /dev/null +++ b/internal/logs.go @@ -0,0 +1,22 @@ +package internal + +import "fmt" + +// MaxSizeSubjectsListForLog returns the keys in the map as a slice of maximum length +// maxToDisplay. It is useful for logging domains being managed, for example, since a +// map is typically needed for quick lookup, but a slice is needed for logging, and this +// can be quite a doozy since there may be a huge amount (hundreds of thousands). +func MaxSizeSubjectsListForLog(subjects map[string]struct{}, maxToDisplay int) []string { + numberOfNamesToDisplay := min(len(subjects), maxToDisplay) + domainsToDisplay := make([]string, 0, numberOfNamesToDisplay) + for domain := range subjects { + domainsToDisplay = append(domainsToDisplay, domain) + if len(domainsToDisplay) >= numberOfNamesToDisplay { + break + } + } + if len(subjects) > maxToDisplay { + domainsToDisplay = append(domainsToDisplay, fmt.Sprintf("(and %d more...)", len(subjects)-maxToDisplay)) + } + return domainsToDisplay +} diff --git a/listeners_test.go b/listeners_test.go index 03945308e..a4cadd3aa 100644 --- a/listeners_test.go +++ b/listeners_test.go @@ -30,7 +30,7 @@ func TestSplitNetworkAddress(t *testing.T) { expectErr bool }{ { - input: "", + input: "", expectHost: "", }, { @@ -41,7 +41,7 @@ func TestSplitNetworkAddress(t *testing.T) { input: ":", // empty host & empty port }, { - input: "::", + input: "::", expectHost: "::", }, { @@ -184,9 +184,8 @@ func TestParseNetworkAddress(t *testing.T) { expectErr bool }{ { - input: "", - expectAddr: NetworkAddress{ - }, + input: "", + expectAddr: NetworkAddress{}, }, { input: ":", @@ -311,9 +310,8 @@ func TestParseNetworkAddressWithDefaults(t *testing.T) { expectErr bool }{ { - input: "", - expectAddr: NetworkAddress{ - }, + input: "", + expectAddr: NetworkAddress{}, }, { input: ":", diff --git a/logging.go b/logging.go index ca10beeed..1a7b0ce29 100644 --- a/logging.go +++ b/logging.go @@ -20,6 +20,7 @@ import ( "io" "log" "os" + "slices" "strings" "sync" "time" @@ -161,7 +162,9 @@ func (logging *Logging) setupNewDefault(ctx Context) error { if err != nil { return fmt.Errorf("setting up default log: %v", err) } - newDefault.logger = zap.New(newDefault.CustomLog.core, options...) + + filteringCore := &filteringCore{newDefault.CustomLog.core, newDefault.CustomLog} + newDefault.logger = zap.New(filteringCore, options...) // redirect the default caddy logs defaultLoggerMu.Lock() @@ -490,10 +493,8 @@ func (cl *CustomLog) provision(ctx Context, logging *Logging) error { if len(cl.Include) > 0 && len(cl.Exclude) > 0 { // prevent intersections for _, allow := range cl.Include { - for _, deny := range cl.Exclude { - if allow == deny { - return fmt.Errorf("include and exclude must not intersect, but found %s in both lists", allow) - } + if slices.Contains(cl.Exclude, allow) { + return fmt.Errorf("include and exclude must not intersect, but found %s in both lists", allow) } } diff --git a/logging_test.go b/logging_test.go new file mode 100644 index 000000000..293591fbb --- /dev/null +++ b/logging_test.go @@ -0,0 +1,106 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package caddy + +import "testing" + +func TestCustomLog_loggerAllowed(t *testing.T) { + type fields struct { + BaseLog BaseLog + Include []string + Exclude []string + } + type args struct { + name string + isModule bool + } + tests := []struct { + name string + fields fields + args args + want bool + }{ + { + name: "include", + fields: fields{ + Include: []string{"foo"}, + }, + args: args{ + name: "foo", + isModule: true, + }, + want: true, + }, + { + name: "exclude", + fields: fields{ + Exclude: []string{"foo"}, + }, + args: args{ + name: "foo", + isModule: true, + }, + want: false, + }, + { + name: "include and exclude", + fields: fields{ + Include: []string{"foo"}, + Exclude: []string{"foo"}, + }, + args: args{ + name: "foo", + isModule: true, + }, + want: false, + }, + { + name: "include and exclude (longer namespace)", + fields: fields{ + Include: []string{"foo.bar"}, + Exclude: []string{"foo"}, + }, + args: args{ + name: "foo.bar", + isModule: true, + }, + want: true, + }, + { + name: "excluded module is not printed", + fields: fields{ + Include: []string{"admin.api.load"}, + Exclude: []string{"admin.api"}, + }, + args: args{ + name: "admin.api", + isModule: false, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cl := &CustomLog{ + BaseLog: tt.fields.BaseLog, + Include: tt.fields.Include, + Exclude: tt.fields.Exclude, + } + if got := cl.loggerAllowed(tt.args.name, tt.args.isModule); got != tt.want { + t.Errorf("CustomLog.loggerAllowed() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index 317825ac5..b550904e2 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -152,7 +152,7 @@ type App struct { tlsApp *caddytls.TLS // used temporarily between phases 1 and 2 of auto HTTPS - allCertDomains []string + allCertDomains map[string]struct{} } // CaddyModule returns the Caddy module information. diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index 769cfd4ef..c34954f92 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -25,6 +25,7 @@ import ( "go.uber.org/zap" "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/internal" "github.com/caddyserver/caddy/v2/modules/caddytls" ) @@ -65,12 +66,6 @@ type AutoHTTPSConfig struct { // enabled. To force automated certificate management // regardless of loaded certificates, set this to true. IgnoreLoadedCerts bool `json:"ignore_loaded_certificates,omitempty"` - - // If true, automatic HTTPS will prefer wildcard names - // and ignore non-wildcard names if both are available. - // This allows for writing a config with top-level host - // matchers without having those names produce certificates. - PreferWildcard bool `json:"prefer_wildcard,omitempty"` } // automaticHTTPSPhase1 provisions all route matchers, determines @@ -163,33 +158,8 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er } } - // trim the list of domains covered by wildcards, if configured - if srv.AutoHTTPS.PreferWildcard { - wildcards := make(map[string]struct{}) - for d := range serverDomainSet { - if strings.HasPrefix(d, "*.") { - wildcards[d[2:]] = struct{}{} - } - } - for d := range serverDomainSet { - if strings.HasPrefix(d, "*.") { - continue - } - base := d - if idx := strings.Index(d, "."); idx != -1 { - base = d[idx+1:] - } - if _, ok := wildcards[base]; ok { - delete(serverDomainSet, d) - } - } - } - // build the list of domains that could be used with ECH (if enabled) - // so the TLS app can know to publish ECH configs for them; we do this - // after trimming domains covered by wildcards because, presumably, - // if the user wants to use wildcard certs, they also want to use the - // wildcard for ECH, rather than individual subdomains + // so the TLS app can know to publish ECH configs for them echDomains := make([]string, 0, len(serverDomainSet)) for d := range serverDomainSet { echDomains = append(echDomains, d) @@ -295,19 +265,10 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er } } - // we now have a list of all the unique names for which we need certs; - // turn the set into a slice so that phase 2 can use it - app.allCertDomains = make([]string, 0, len(uniqueDomainsForCerts)) + // we now have a list of all the unique names for which we need certs var internal, tailscale []string uniqueDomainsLoop: for d := range uniqueDomainsForCerts { - if !isTailscaleDomain(d) { - // 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, - // unless it's a Tailscale domain, because we don't manage those - 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 @@ -346,6 +307,7 @@ uniqueDomainsLoop: } if isTailscaleDomain(d) { tailscale = append(tailscale, d) + delete(uniqueDomainsForCerts, d) // not managed by us; handled separately } else if shouldUseInternal(d) { internal = append(internal, d) } @@ -381,7 +343,7 @@ uniqueDomainsLoop: // match on known domain names, unless it's our special case of a // catch-all which is an empty string (common among catch-all sites // that enable on-demand TLS for yet-unknown domain names) - if !(len(domains) == 1 && domains[0] == "") { + if len(domains) != 1 || domains[0] != "" { matcherSet = append(matcherSet, MatchHost(domains)) } @@ -475,6 +437,9 @@ redirServersLoop: } } + // persist the domains/IPs we're managing certs for through provisioning/startup + app.allCertDomains = uniqueDomainsForCerts + logger.Debug("adjusted config", zap.Reflect("tls", app.tlsApp), zap.Reflect("http", app)) @@ -777,7 +742,7 @@ func (app *App) automaticHTTPSPhase2() error { return nil } app.logger.Info("enabling automatic TLS certificate management", - zap.Strings("domains", app.allCertDomains), + zap.Strings("domains", internal.MaxSizeSubjectsListForLog(app.allCertDomains, 1000)), ) err := app.tlsApp.Manage(app.allCertDomains) if err != nil { diff --git a/modules/caddyhttp/fileserver/browse.html b/modules/caddyhttp/fileserver/browse.html index d2d698197..704661f21 100644 --- a/modules/caddyhttp/fileserver/browse.html +++ b/modules/caddyhttp/fileserver/browse.html @@ -26,7 +26,7 @@ - {{- else if .HasExt ".jpg" ".jpeg" ".png" ".gif" ".webp" ".tiff" ".bmp" ".heif" ".heic" ".svg"}} + {{- else if .HasExt ".jpg" ".jpeg" ".png" ".gif" ".webp" ".tiff" ".bmp" ".heif" ".heic" ".svg" ".avif"}} {{- if eq .Tpl.Layout "grid"}} {{- else}} @@ -802,7 +802,7 @@ footer { {{.NumFiles}} file{{if ne 1 .NumFiles}}s{{end}} - {{.HumanTotalFileSize}} total + {{.HumanTotalFileSize}} total {{- if ne 0 .Limit}} @@ -868,7 +868,7 @@ footer { {{- end}} - + {{- if and (eq .Sort "name") (ne .Order "desc")}} Name diff --git a/modules/caddyhttp/fileserver/matcher.go b/modules/caddyhttp/fileserver/matcher.go index b5b4c9f0f..fbcd36e0a 100644 --- a/modules/caddyhttp/fileserver/matcher.go +++ b/modules/caddyhttp/fileserver/matcher.go @@ -252,7 +252,7 @@ func celFileMatcherMacroExpander() parser.MacroExpander { } for _, arg := range args { - if !(isCELStringLiteral(arg) || isCELCaddyPlaceholderCall(arg)) { + if !isCELStringLiteral(arg) && !isCELCaddyPlaceholderCall(arg) { return nil, &common.Error{ Location: eh.OffsetLocation(arg.ID()), Message: "matcher only supports repeated string literal arguments", @@ -616,15 +616,16 @@ func isCELTryFilesLiteral(e ast.Expr) bool { return false } mapKeyStr := mapKey.AsLiteral().ConvertToType(types.StringType).Value() - if mapKeyStr == "try_files" || mapKeyStr == "split_path" { + switch mapKeyStr { + case "try_files", "split_path": if !isCELStringListLiteral(mapVal) { return false } - } else if mapKeyStr == "try_policy" || mapKeyStr == "root" { + case "try_policy", "root": if !(isCELStringExpr(mapVal)) { return false } - } else { + default: return false } } diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index 1072d1878..5c2ea7018 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -300,8 +300,10 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c info, err := fs.Stat(fileSystem, filename) if err != nil { err = fsrv.mapDirOpenError(fileSystem, err, filename) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, fs.ErrInvalid) { + if errors.Is(err, fs.ErrNotExist) { return fsrv.notFound(w, r, next) + } else if errors.Is(err, fs.ErrInvalid) { + return caddyhttp.Error(http.StatusBadRequest, err) } else if errors.Is(err, fs.ErrPermission) { return caddyhttp.Error(http.StatusForbidden, err) } @@ -611,6 +613,11 @@ func (fsrv *FileServer) mapDirOpenError(fileSystem fs.FS, originalErr error, nam return originalErr } + var pathErr *fs.PathError + if errors.As(originalErr, &pathErr) { + return fs.ErrInvalid + } + parts := strings.Split(name, separator) for i := range parts { if parts[i] == "" { diff --git a/modules/caddyhttp/intercept/intercept.go b/modules/caddyhttp/intercept/intercept.go index 29889dcc0..cb23adf0a 100644 --- a/modules/caddyhttp/intercept/intercept.go +++ b/modules/caddyhttp/intercept/intercept.go @@ -118,6 +118,11 @@ func (irh interceptedResponseHandler) WriteHeader(statusCode int) { irh.ResponseRecorder.WriteHeader(statusCode) } +// EXPERIMENTAL: Subject to change or removal. +func (irh interceptedResponseHandler) Unwrap() http.ResponseWriter { + return irh.ResponseRecorder +} + // EXPERIMENTAL: Subject to change or removal. func (ir Intercept) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error { buf := bufPool.Get().(*bytes.Buffer) diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index 01bd7b8b4..48c92f174 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -552,7 +552,6 @@ func (MatchPath) matchPatternWithEscapeSequence(escapedPath, matchPath string) b if iPattern >= len(matchPath) || iPath >= len(escapedPath) { break } - // get the next character from the request path pathCh := string(escapedPath[iPath]) diff --git a/modules/caddyhttp/metrics_test.go b/modules/caddyhttp/metrics_test.go index 4a0519b87..4e1aa8b30 100644 --- a/modules/caddyhttp/metrics_test.go +++ b/modules/caddyhttp/metrics_test.go @@ -9,8 +9,9 @@ import ( "sync" "testing" - "github.com/caddyserver/caddy/v2" "github.com/prometheus/client_golang/prometheus/testutil" + + "github.com/caddyserver/caddy/v2" ) func TestServerNameFromContext(t *testing.T) { diff --git a/modules/caddyhttp/replacer.go b/modules/caddyhttp/replacer.go index 776aa6294..69779e6ed 100644 --- a/modules/caddyhttp/replacer.go +++ b/modules/caddyhttp/replacer.go @@ -363,13 +363,13 @@ func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.Respo } } - switch { - case key == "http.shutting_down": + switch key { + case "http.shutting_down": server := req.Context().Value(ServerCtxKey).(*Server) server.shutdownAtMu.RLock() defer server.shutdownAtMu.RUnlock() return !server.shutdownAt.IsZero(), true - case key == "http.time_until_shutdown": + case "http.time_until_shutdown": server := req.Context().Value(ServerCtxKey).(*Server) server.shutdownAtMu.RLock() defer server.shutdownAtMu.RUnlock() diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index d0947197a..8439d1d51 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -665,9 +665,10 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if d.NextArg() { return d.ArgErr() } - if subdir == "request_buffers" { + switch subdir { + case "request_buffers": h.RequestBuffers = size - } else if subdir == "response_buffers" { + case "response_buffers": h.ResponseBuffers = size } diff --git a/modules/caddyhttp/reverseproxy/command.go b/modules/caddyhttp/reverseproxy/command.go index f9304efa2..54955bcf7 100644 --- a/modules/caddyhttp/reverseproxy/command.go +++ b/modules/caddyhttp/reverseproxy/command.go @@ -122,9 +122,10 @@ func cmdReverseProxy(fs caddycmd.Flags) (int, error) { } } if fromAddr.Port == "" { - if fromAddr.Scheme == "http" { + switch fromAddr.Scheme { + case "http": fromAddr.Port = httpPort - } else if fromAddr.Scheme == "https" { + case "https": fromAddr.Port = httpsPort } } diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index 5db73a4a2..2325af9a7 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -17,6 +17,7 @@ package fastcgi import ( "encoding/json" "net/http" + "slices" "strconv" "strings" @@ -314,7 +315,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error // if the index is turned off, we skip the redirect and try_files if indexFile != "off" { - dirRedir := false + var dirRedir bool dirIndex := "{http.request.uri.path}/" + indexFile tryPolicy := "first_exist_fallback" @@ -328,13 +329,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error tryPolicy = "" } - for _, tf := range tryFiles { - if tf == dirIndex { - dirRedir = true - - break - } - } + dirRedir = slices.Contains(tryFiles, dirIndex) } if dirRedir { diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index f018f40ca..ac42570b2 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -484,7 +484,7 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, networ markHealthy := func() { // increment passes and then check if it has reached the threshold to be healthy - err := upstream.Host.countHealthPass(1) + err := upstream.countHealthPass(1) if err != nil { if c := h.HealthChecks.Active.logger.Check(zapcore.ErrorLevel, "could not count active health pass"); c != nil { c.Write( diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index 92fe9ab7c..24407df2b 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -353,7 +353,7 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e h.NetworkProxyRaw = caddyconfig.JSONModuleObject(u, "from", "url", nil) } if len(h.NetworkProxyRaw) != 0 { - proxyMod, err := caddyCtx.LoadModule(h, "ForwardProxyRaw") + proxyMod, err := caddyCtx.LoadModule(h, "NetworkProxyRaw") if err != nil { return nil, fmt.Errorf("failed to load network_proxy module: %v", err) } @@ -382,6 +382,36 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e if err != nil { return nil, fmt.Errorf("making TLS client config: %v", err) } + + // servername has a placeholder, so we need to replace it + if strings.Contains(h.TLS.ServerName, "{") { + rt.DialTLSContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + // reuses the dialer from above to establish a plaintext connection + conn, err := dialContext(ctx, network, addr) + if err != nil { + return nil, err + } + + // but add our own handshake logic + repl := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer) + tlsConfig := rt.TLSClientConfig.Clone() + tlsConfig.ServerName = repl.ReplaceAll(tlsConfig.ServerName, "") + tlsConn := tls.Client(conn, tlsConfig) + + // complete the handshake before returning the connection + if rt.TLSHandshakeTimeout != 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, rt.TLSHandshakeTimeout) + defer cancel() + } + err = tlsConn.HandshakeContext(ctx) + if err != nil { + _ = tlsConn.Close() + return nil, err + } + return tlsConn, nil + } + } } if h.KeepAlive != nil { @@ -453,45 +483,9 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e return rt, nil } -// replaceTLSServername checks TLS servername to see if it needs replacing -// if it does need replacing, it creates a new cloned HTTPTransport object to avoid any races -// and does the replacing of the TLS servername on that and returns the new object -// if no replacement is necessary it returns the original -func (h *HTTPTransport) replaceTLSServername(repl *caddy.Replacer) *HTTPTransport { - // check whether we have TLS and need to replace the servername in the TLSClientConfig - if h.TLSEnabled() && strings.Contains(h.TLS.ServerName, "{") { - // make a new h, "copy" the parts we don't need to touch, add a new *tls.Config and replace servername - newtransport := &HTTPTransport{ - Resolver: h.Resolver, - TLS: h.TLS, - KeepAlive: h.KeepAlive, - Compression: h.Compression, - MaxConnsPerHost: h.MaxConnsPerHost, - DialTimeout: h.DialTimeout, - FallbackDelay: h.FallbackDelay, - ResponseHeaderTimeout: h.ResponseHeaderTimeout, - ExpectContinueTimeout: h.ExpectContinueTimeout, - MaxResponseHeaderSize: h.MaxResponseHeaderSize, - WriteBufferSize: h.WriteBufferSize, - ReadBufferSize: h.ReadBufferSize, - Versions: h.Versions, - Transport: h.Transport.Clone(), - h2cTransport: h.h2cTransport, - } - newtransport.Transport.TLSClientConfig.ServerName = repl.ReplaceAll(newtransport.Transport.TLSClientConfig.ServerName, "") - return newtransport - } - - return h -} - // RoundTrip implements http.RoundTripper. func (h *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) { - // Try to replace TLS servername if needed - repl := req.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) - transport := h.replaceTLSServername(repl) - - transport.SetScheme(req) + h.SetScheme(req) // use HTTP/3 if enabled (TODO: This is EXPERIMENTAL) if h.h3Transport != nil { @@ -507,7 +501,7 @@ func (h *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) { return h.h2cTransport.RoundTrip(req) } - return transport.Transport.RoundTrip(req) + return h.Transport.RoundTrip(req) } // SetScheme ensures that the outbound request req @@ -534,13 +528,7 @@ func (h *HTTPTransport) shouldUseTLS(req *http.Request) bool { } port := req.URL.Port() - for i := range h.TLS.ExceptPorts { - if h.TLS.ExceptPorts[i] == port { - return false - } - } - - return true + return !slices.Contains(h.TLS.ExceptPorts, port) } // TLSEnabled returns true if TLS is enabled. @@ -652,7 +640,7 @@ func (t *TLSConfig) MakeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) return nil, fmt.Errorf("getting tls app: %v", err) } tlsApp := tlsAppIface.(*caddytls.TLS) - err = tlsApp.Manage([]string{t.ClientCertificateAutomate}) + err = tlsApp.Manage(map[string]struct{}{t.ClientCertificateAutomate: {}}) if err != nil { return nil, fmt.Errorf("managing client certificate: %v", err) } diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index f07d3daeb..88fba55a1 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -1150,7 +1150,7 @@ func (lb LoadBalancing) tryAgain(ctx caddy.Context, start time.Time, retries int // we have to assume the upstream received the request, and // retries need to be carefully decided, because some requests // are not idempotent - if !isDialError && !(isHandlerError && errors.Is(herr, errNoUpstream)) { + if !isDialError && (!isHandlerError || !errors.Is(herr, errNoUpstream)) { if lb.RetryMatch == nil && req.Method != "GET" { // by default, don't retry requests if they aren't GET return false diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index fcf7f90f6..a9c036596 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -808,7 +808,7 @@ func leastRequests(upstreams []*Upstream) *Upstream { return nil } var best []*Upstream - var bestReqs int = -1 + bestReqs := -1 for _, upstream := range upstreams { if upstream == nil { continue diff --git a/modules/caddyhttp/reverseproxy/upstreams_test.go b/modules/caddyhttp/reverseproxy/upstreams_test.go index 48e2d2a63..8caf8696a 100644 --- a/modules/caddyhttp/reverseproxy/upstreams_test.go +++ b/modules/caddyhttp/reverseproxy/upstreams_test.go @@ -52,5 +52,4 @@ func TestResolveIpVersion(t *testing.T) { t.Errorf("resolveIpVersion(): Expected %s got %s", test.expectedIpVersion, ipVersion) } } - } diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index 31ebfb430..2b18744db 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -377,11 +377,7 @@ func buildQueryString(qs string, repl *caddy.Replacer) string { // performed in normalized/unescaped space. func trimPathPrefix(escapedPath, prefix string) string { var iPath, iPrefix int - for { - if iPath >= len(escapedPath) || iPrefix >= len(prefix) { - break - } - + for iPath < len(escapedPath) && iPrefix < len(prefix) { prefixCh := prefix[iPrefix] ch := string(escapedPath[iPath]) diff --git a/modules/caddyhttp/server_test.go b/modules/caddyhttp/server_test.go index 53f35368f..6ce09974b 100644 --- a/modules/caddyhttp/server_test.go +++ b/modules/caddyhttp/server_test.go @@ -171,6 +171,7 @@ func BenchmarkServer_LogRequest_WithTrace(b *testing.B) { s.logRequest(accLog, req, wrec, &duration, repl, bodyReader, false) } } + func TestServer_TrustedRealClientIP_NoTrustedHeaders(t *testing.T) { req := httptest.NewRequest("GET", "/", nil) req.RemoteAddr = "192.0.2.1:12345" diff --git a/modules/caddyhttp/staticresp.go b/modules/caddyhttp/staticresp.go index 1b93ede4b..12108ac03 100644 --- a/modules/caddyhttp/staticresp.go +++ b/modules/caddyhttp/staticresp.go @@ -22,6 +22,7 @@ import ( "net/http" "net/textproto" "os" + "slices" "strconv" "strings" "text/template" @@ -323,13 +324,7 @@ func cmdRespond(fl caddycmd.Flags) (int, error) { // figure out if status code was explicitly specified; this lets // us set a non-zero value as the default but is a little hacky - var statusCodeFlagSpecified bool - for _, fl := range os.Args { - if fl == "--status" { - statusCodeFlagSpecified = true - break - } - } + statusCodeFlagSpecified := slices.Contains(os.Args, "--status") // try to determine what kind of parameter the unnamed argument is if arg != "" { diff --git a/modules/caddypki/acmeserver/caddyfile.go b/modules/caddypki/acmeserver/caddyfile.go index c4d85716f..a7dc8e337 100644 --- a/modules/caddypki/acmeserver/caddyfile.go +++ b/modules/caddypki/acmeserver/caddyfile.go @@ -91,19 +91,17 @@ func parseACMEServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error acmeServer.Policy.AllowWildcardNames = true case "allow": r := &RuleSet{} - for h.Next() { - for h.NextBlock(h.Nesting() - 1) { - if h.CountRemainingArgs() == 0 { - return nil, h.ArgErr() // TODO: - } - switch h.Val() { - case "domains": - r.Domains = append(r.Domains, h.RemainingArgs()...) - case "ip_ranges": - r.IPRanges = append(r.IPRanges, h.RemainingArgs()...) - default: - return nil, h.Errf("unrecognized 'allow' subdirective: %s", h.Val()) - } + for nesting := h.Nesting(); h.NextBlock(nesting); { + if h.CountRemainingArgs() == 0 { + return nil, h.ArgErr() // TODO: + } + switch h.Val() { + case "domains": + r.Domains = append(r.Domains, h.RemainingArgs()...) + case "ip_ranges": + r.IPRanges = append(r.IPRanges, h.RemainingArgs()...) + default: + return nil, h.Errf("unrecognized 'allow' subdirective: %s", h.Val()) } } if acmeServer.Policy == nil { @@ -112,19 +110,17 @@ func parseACMEServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error acmeServer.Policy.Allow = r case "deny": r := &RuleSet{} - for h.Next() { - for h.NextBlock(h.Nesting() - 1) { - if h.CountRemainingArgs() == 0 { - return nil, h.ArgErr() // TODO: - } - switch h.Val() { - case "domains": - r.Domains = append(r.Domains, h.RemainingArgs()...) - case "ip_ranges": - r.IPRanges = append(r.IPRanges, h.RemainingArgs()...) - default: - return nil, h.Errf("unrecognized 'deny' subdirective: %s", h.Val()) - } + for nesting := h.Nesting(); h.NextBlock(nesting); { + if h.CountRemainingArgs() == 0 { + return nil, h.ArgErr() // TODO: + } + switch h.Val() { + case "domains": + r.Domains = append(r.Domains, h.RemainingArgs()...) + case "ip_ranges": + r.IPRanges = append(r.IPRanges, h.RemainingArgs()...) + default: + return nil, h.Errf("unrecognized 'deny' subdirective: %s", h.Val()) } } if acmeServer.Policy == nil { diff --git a/modules/caddytls/acmeissuer.go b/modules/caddytls/acmeissuer.go index bf2ebeacc..4830570bf 100644 --- a/modules/caddytls/acmeissuer.go +++ b/modules/caddytls/acmeissuer.go @@ -220,7 +220,7 @@ func (iss *ACMEIssuer) makeIssuerTemplate(ctx caddy.Context) (certmagic.ACMEIssu } if len(iss.NetworkProxyRaw) != 0 { - proxyMod, err := ctx.LoadModule(iss, "ForwardProxyRaw") + proxyMod, err := ctx.LoadModule(iss, "NetworkProxyRaw") if err != nil { return template, fmt.Errorf("failed to load network_proxy module: %v", err) } diff --git a/modules/caddytls/automation.go b/modules/caddytls/automation.go index 6f3b98a3e..c9274878a 100644 --- a/modules/caddytls/automation.go +++ b/modules/caddytls/automation.go @@ -388,10 +388,8 @@ func (ap *AutomationPolicy) onlyInternalIssuer() bool { // isWildcardOrDefault determines if the subjects include any wildcard domains, // or is the "default" policy (i.e. no subjects) which is unbounded. func (ap *AutomationPolicy) isWildcardOrDefault() bool { - isWildcardOrDefault := false - if len(ap.subjects) == 0 { - isWildcardOrDefault = true - } + isWildcardOrDefault := len(ap.subjects) == 0 + for _, sub := range ap.subjects { if strings.HasPrefix(sub, "*") { isWildcardOrDefault = true diff --git a/modules/caddytls/certmanagers.go b/modules/caddytls/certmanagers.go index 56950bc84..0a9d459df 100644 --- a/modules/caddytls/certmanagers.go +++ b/modules/caddytls/certmanagers.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "fmt" "io" + "net" "net/http" "net/url" "strings" @@ -143,6 +144,10 @@ func (hcg HTTPCertGetter) GetCertificate(ctx context.Context, hello *tls.ClientH qs.Set("server_name", hello.ServerName) qs.Set("signature_schemes", strings.Join(sigs, ",")) qs.Set("cipher_suites", strings.Join(suites, ",")) + localIP, _, err := net.SplitHostPort(hello.Conn.LocalAddr().String()) + if err == nil && localIP != "" { + qs.Set("local_ip", localIP) + } parsed.RawQuery = qs.Encode() req, err := http.NewRequestWithContext(hcg.ctx, http.MethodGet, parsed.String(), nil) diff --git a/modules/caddytls/certselection.go b/modules/caddytls/certselection.go index a561e3a1d..ac210d3b6 100644 --- a/modules/caddytls/certselection.go +++ b/modules/caddytls/certselection.go @@ -87,13 +87,7 @@ nextChoice: } if len(p.AnyTag) > 0 { - var found bool - for _, tag := range p.AnyTag { - if cert.HasTag(tag) { - found = true - break - } - } + found := slices.ContainsFunc(p.AnyTag, cert.HasTag) if !found { continue } diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go index d3d90ff55..018ef243f 100644 --- a/modules/caddytls/connpolicy.go +++ b/modules/caddytls/connpolicy.go @@ -24,6 +24,8 @@ import ( "fmt" "io" "os" + "reflect" + "slices" "strings" "github.com/mholt/acmez/v3" @@ -368,13 +370,7 @@ func (p *ConnectionPolicy) buildStandardTLSConfig(ctx caddy.Context) error { } // ensure ALPN includes the ACME TLS-ALPN protocol - var alpnFound bool - for _, a := range p.ALPN { - if a == acmez.ACMETLS1Protocol { - alpnFound = true - break - } - } + alpnFound := slices.Contains(p.ALPN, acmez.ACMETLS1Protocol) if !alpnFound && (cfg.NextProtos == nil || len(cfg.NextProtos) > 0) { cfg.NextProtos = append(cfg.NextProtos, acmez.ACMETLS1Protocol) } @@ -461,6 +457,14 @@ func (p ConnectionPolicy) SettingsEmpty() bool { p.InsecureSecretsLog == "" } +// SettingsEmpty returns true if p's settings (fields +// except the matchers) are the same as q. +func (p ConnectionPolicy) SettingsEqual(q ConnectionPolicy) bool { + p.MatchersRaw = nil + q.MatchersRaw = nil + return reflect.DeepEqual(p, q) +} + // UnmarshalCaddyfile sets up the ConnectionPolicy from Caddyfile tokens. Syntax: // // connection_policy { @@ -1037,10 +1041,8 @@ func (l LeafCertClientAuth) VerifyClientCertificate(rawCerts [][]byte, _ [][]*x5 return fmt.Errorf("can't parse the given certificate: %s", err.Error()) } - for _, trustedLeafCert := range l.trustedLeafCerts { - if remoteLeafCert.Equal(trustedLeafCert) { - return nil - } + if slices.ContainsFunc(l.trustedLeafCerts, remoteLeafCert.Equal) { + return nil } return fmt.Errorf("client leaf certificate failed validation") diff --git a/modules/caddytls/ech.go b/modules/caddytls/ech.go index fe0ba93b7..7329bf1f2 100644 --- a/modules/caddytls/ech.go +++ b/modules/caddytls/ech.go @@ -138,7 +138,6 @@ func (ech *ECH) Provision(ctx caddy.Context) ([]string, error) { // all existing configs are now loaded; see if we need to make any new ones // based on the input configuration, and also mark the most recent one(s) as // current/active, so they can be used for ECH retries - for _, cfg := range ech.Configs { publicName := strings.ToLower(strings.TrimSpace(cfg.PublicName)) @@ -279,7 +278,7 @@ func (t *TLS) publishECHConfigs() error { // if all the (inner) domains have had this ECH config list published // by this publisher, then try the next publication config if len(serverNamesSet) == 0 { - logger.Debug("ECH config list already published by publisher for associated domains", + logger.Debug("ECH config list already published by publisher for associated domains (or no domains to publish for)", zap.Uint8s("config_ids", configIDs), zap.String("publisher", publisherKey)) continue @@ -300,7 +299,7 @@ func (t *TLS) publishECHConfigs() error { err := publisher.PublishECHConfigList(t.ctx, dnsNamesToPublish, echCfgListBin) if err == nil { t.logger.Info("published ECH configuration list", - zap.Strings("domains", publication.Domains), + zap.Strings("domains", dnsNamesToPublish), zap.Uint8s("config_ids", configIDs), zap.Error(err)) // update publication history, so that we don't unnecessarily republish every time @@ -390,27 +389,33 @@ func loadECHConfig(ctx caddy.Context, configID string) (echConfig, error) { return echConfig{}, nil } metaBytes, err := storage.Load(ctx, metaKey) - if err != nil { + if errors.Is(err, fs.ErrNotExist) { + logger.Warn("ECH config metadata file missing; will recreate at next publication", + zap.String("config_id", configID), + zap.Error(err)) + } else if err != nil { delErr := storage.Delete(ctx, cfgIDKey) if delErr != nil { - return echConfig{}, fmt.Errorf("error loading ECH metadata (%v) and cleaning up parent storage key %s: %v", err, cfgIDKey, delErr) + return echConfig{}, fmt.Errorf("error loading ECH config metadata (%v) and cleaning up parent storage key %s: %v", err, cfgIDKey, delErr) } - logger.Warn("could not load ECH metadata; deleted its config folder", + logger.Warn("could not load ECH config metadata; deleted its folder", zap.String("config_id", configID), zap.Error(err)) return echConfig{}, nil } var meta echConfigMeta - if err := json.Unmarshal(metaBytes, &meta); err != nil { - // even though it's just metadata, reset the whole config since we can't reliably maintain it - delErr := storage.Delete(ctx, cfgIDKey) - if delErr != nil { - return echConfig{}, fmt.Errorf("error decoding ECH metadata (%v) and cleaning up parent storage key %s: %v", err, cfgIDKey, delErr) + if len(metaBytes) > 0 { + if err := json.Unmarshal(metaBytes, &meta); err != nil { + // even though it's just metadata, reset the whole config since we can't reliably maintain it + delErr := storage.Delete(ctx, cfgIDKey) + if delErr != nil { + return echConfig{}, fmt.Errorf("error decoding ECH metadata (%v) and cleaning up parent storage key %s: %v", err, cfgIDKey, delErr) + } + logger.Warn("could not JSON-decode ECH metadata; deleted its config folder", + zap.String("config_id", configID), + zap.Error(err)) + return echConfig{}, nil } - logger.Warn("could not JSON-decode ECH metadata; deleted its config folder", - zap.String("config_id", configID), - zap.Error(err)) - return echConfig{}, nil } cfg.privKeyBin = privKeyBytes @@ -701,7 +706,7 @@ nextName: // HTTPS and SVCB RRs: RFC 9460 (https://www.rfc-editor.org/rfc/rfc9460) Scheme: "https", Name: relName, - TTL: 1 * time.Minute, // TODO: for testing only + TTL: 5 * time.Minute, // TODO: low hard-coded value only temporary; change to a higher value once more field-tested and key rotation is implemented Priority: 2, // allows a manual override with priority 1 Target: ".", Params: params, diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 0f8433960..7b49c0208 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -33,6 +33,7 @@ import ( "go.uber.org/zap/zapcore" "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/internal" "github.com/caddyserver/caddy/v2/modules/caddyevents" ) @@ -55,8 +56,10 @@ type TLS struct { // // The "automate" certificate loader module can be used to // specify a list of subjects that need certificates to be - // managed automatically. The first matching automation - // policy will be applied to manage the certificate(s). + // managed automatically, including subdomains that may + // already be covered by a managed wildcard certificate. + // The first matching automation policy will be used + // to manage automated certificate(s). // // All loaded certificates get pooled // into the same cache and may be used to complete TLS @@ -123,7 +126,7 @@ type TLS struct { dns any // technically, it should be any/all of the libdns interfaces (RecordSetter, RecordAppender, etc.) certificateLoaders []CertificateLoader - automateNames []string + automateNames map[string]struct{} ctx caddy.Context storageCleanTicker *time.Ticker storageCleanStop chan struct{} @@ -218,12 +221,13 @@ func (t *TLS) Provision(ctx caddy.Context) error { // special case; these will be loaded in later using our automation facilities, // which we want to avoid doing during provisioning if automateNames, ok := modIface.(*AutomateLoader); ok && automateNames != nil { - repl := caddy.NewReplacer() - subjects := make([]string, len(*automateNames)) - for i, sub := range *automateNames { - subjects[i] = repl.ReplaceAll(sub, "") + if t.automateNames == nil { + t.automateNames = make(map[string]struct{}) + } + repl := caddy.NewReplacer() + for _, sub := range *automateNames { + t.automateNames[repl.ReplaceAll(sub, "")] = struct{}{} } - t.automateNames = append(t.automateNames, subjects...) } else { return fmt.Errorf("loading certificates with 'automate' requires array of strings, got: %T", modIface) } @@ -283,7 +287,7 @@ func (t *TLS) Provision(ctx caddy.Context) error { if err != nil { return fmt.Errorf("provisioning default public automation policy: %v", err) } - for _, n := range t.automateNames { + for n := range t.automateNames { // if any names specified by the "automate" loader do not qualify for a public // certificate, we should initialize a default internal automation policy // (but we don't want to do this unnecessarily, since it may prompt for password!) @@ -339,8 +343,14 @@ func (t *TLS) Provision(ctx caddy.Context) error { // outer names should have certificates to reduce client brittleness for _, outerName := range outerNames { + if outerName == "" { + continue + } if !t.HasCertificateForSubject(outerName) { - t.automateNames = append(t.automateNames, outerNames...) + if t.automateNames == nil { + t.automateNames = make(map[string]struct{}) + } + t.automateNames[outerName] = struct{}{} } } } @@ -449,7 +459,8 @@ func (t *TLS) Cleanup() error { // app instance (which is being stopped) that are not managed or loaded by the // new app instance (which just started), and remove them from the cache var noLongerManaged []certmagic.SubjectIssuer - var reManage, noLongerLoaded []string + var noLongerLoaded []string + reManage := make(map[string]struct{}) for subj, currentIssuerKey := range t.managing { // It's a bit nuanced: managed certs can sometimes be different enough that we have to // swap them out for a different one, even if they are for the same subject/domain. @@ -467,7 +478,7 @@ func (t *TLS) Cleanup() error { // then, if the next app is managing a cert for this name, but with a different issuer, re-manage it if ok && nextIssuerKey != currentIssuerKey { - reManage = append(reManage, subj) + reManage[subj] = struct{}{} } } } @@ -488,7 +499,7 @@ func (t *TLS) Cleanup() error { if err := nextTLSApp.Manage(reManage); err != nil { if c := t.logger.Check(zapcore.ErrorLevel, "re-managing unloaded certificates with new config"); c != nil { c.Write( - zap.Strings("subjects", reManage), + zap.Strings("subjects", internal.MaxSizeSubjectsListForLog(reManage, 1000)), zap.Error(err), ) } @@ -509,17 +520,31 @@ func (t *TLS) Cleanup() error { return nil } -// Manage immediately begins managing names according to the -// matching automation policy. -func (t *TLS) Manage(names []string) error { +// Manage immediately begins managing subjects according to the +// matching automation policy. The subjects are given in a map +// to prevent duplication and also because quick lookups are +// needed to assess wildcard coverage, if any, depending on +// certain config parameters (with lots of subjects, computing +// wildcard coverage over a slice can be highly inefficient). +func (t *TLS) Manage(subjects map[string]struct{}) error { // for a large number of names, we can be more memory-efficient // by making only one certmagic.Config for all the names that // use that config, rather than calling ManageAsync once for // every name; so first, bin names by AutomationPolicy policyToNames := make(map[*AutomationPolicy][]string) - for _, name := range names { - ap := t.getAutomationPolicyForName(name) - policyToNames[ap] = append(policyToNames[ap], name) + for subj := range subjects { + ap := t.getAutomationPolicyForName(subj) + // by default, if a wildcard that covers the subj is also being + // managed, either by a previous call to Manage or by this one, + // prefer using that over individual certs for its subdomains; + // but users can disable this and force getting a certificate for + // subdomains by adding the name to the 'automate' cert loader + if t.managingWildcardFor(subj, subjects) { + if _, ok := t.automateNames[subj]; !ok { + continue + } + } + policyToNames[ap] = append(policyToNames[ap], subj) } // now that names are grouped by policy, we can simply make one @@ -530,7 +555,7 @@ func (t *TLS) Manage(names []string) error { if err != nil { const maxNamesToDisplay = 100 if len(names) > maxNamesToDisplay { - names = append(names[:maxNamesToDisplay], fmt.Sprintf("(%d more...)", len(names)-maxNamesToDisplay)) + names = append(names[:maxNamesToDisplay], fmt.Sprintf("(and %d more...)", len(names)-maxNamesToDisplay)) } return fmt.Errorf("automate: manage %v: %v", names, err) } @@ -555,6 +580,43 @@ func (t *TLS) Manage(names []string) error { return nil } +// managingWildcardFor returns true if the app is managing a certificate that covers that +// subject name (including consideration of wildcards), either from its internal list of +// names that it IS managing certs for, or from the otherSubjsToManage which includes names +// that WILL be managed. +func (t *TLS) managingWildcardFor(subj string, otherSubjsToManage map[string]struct{}) bool { + // TODO: we could also consider manually-loaded certs using t.HasCertificateForSubject(), + // but that does not account for how manually-loaded certs may be restricted as to which + // hostnames or ClientHellos they can be used with by tags, etc; I don't *think* anyone + // necessarily wants this anyway, but I thought I'd note this here for now (if we did + // consider manually-loaded certs, we'd probably want to rename the method since it + // wouldn't be just about managed certs anymore) + + // IP addresses must match exactly + if ip := net.ParseIP(subj); ip != nil { + _, managing := t.managing[subj] + return managing + } + + // replace labels of the domain with wildcards until we get a match + labels := strings.Split(subj, ".") + for i := range labels { + if labels[i] == "*" { + continue + } + labels[i] = "*" + candidate := strings.Join(labels, ".") + if _, ok := t.managing[candidate]; ok { + return true + } + if _, ok := otherSubjsToManage[candidate]; ok { + return true + } + } + + return false +} + // RegisterServerNames registers the provided DNS names with the TLS app. // This is currently used to auto-publish Encrypted ClientHello (ECH) // configurations, if enabled. Use of this function by apps using the TLS diff --git a/modules/logging/filewriter_test.go b/modules/logging/filewriter_test.go index f9072f98a..2a246156c 100644 --- a/modules/logging/filewriter_test.go +++ b/modules/logging/filewriter_test.go @@ -317,7 +317,7 @@ func TestFileModeToJSON(t *testing.T) { }{ { name: "none zero", - mode: 0644, + mode: 0o644, want: `"0644"`, wantErr: false, }, @@ -358,7 +358,7 @@ func TestFileModeModification(t *testing.T) { defer os.RemoveAll(dir) fpath := path.Join(dir, "test.log") - f_tmp, err := os.OpenFile(fpath, os.O_WRONLY|os.O_APPEND|os.O_CREATE, os.FileMode(0600)) + f_tmp, err := os.OpenFile(fpath, os.O_WRONLY|os.O_APPEND|os.O_CREATE, os.FileMode(0o600)) if err != nil { t.Fatalf("failed to create test file: %v", err) } diff --git a/modules/logging/filters_test.go b/modules/logging/filters_test.go index 8f7ba0d70..a929617d7 100644 --- a/modules/logging/filters_test.go +++ b/modules/logging/filters_test.go @@ -3,9 +3,10 @@ package logging import ( "testing" + "go.uber.org/zap/zapcore" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/modules/caddyhttp" - "go.uber.org/zap/zapcore" ) func TestIPMaskSingleValue(t *testing.T) {