diff options
41 files changed, 256 insertions, 199 deletions
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7233365..476b908 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -148,20 +148,6 @@ jobs: env: SSH_KEY: ${{ secrets.S390X_SSH_KEY }} - # From https://github.com/reviewdog/action-golangci-lint - golangci-lint: - name: runner / golangci-lint - runs-on: ubuntu-latest - steps: - - name: Checkout code into the Go module directory - uses: actions/checkout@v2 - - - name: Run golangci-lint - uses: reviewdog/action-golangci-lint@v1 - # uses: docker://reviewdog/action-golangci-lint:v1 # pre-build docker image - with: - github_token: ${{ secrets.github_token }} - goreleaser-check: runs-on: ubuntu-latest steps: diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..ef69150 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,23 @@ +name: Lint + +on: + push: + branches: + - master + pull_request: + branches: + - master + +jobs: + # From https://github.com/golangci/golangci-lint-action + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: golangci-lint + uses: golangci/golangci-lint-action@v2 + with: + version: v1.31 + # Optional: show only new issues if it's a pull request. The default value is `false`. + # only-new-issues: true diff --git a/.golangci.yml b/.golangci.yml index f6d8322..2c6acca 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,21 +1,68 @@ linters-settings: errcheck: - ignore: fmt:.*,io/ioutil:^Read.*,github.com/caddyserver/caddy/v2/caddyconfig:RegisterAdapter,github.com/caddyserver/caddy/v2:RegisterModule + ignore: fmt:.*,io/ioutil:^Read.*,go.uber.org/zap/zapcore:^Add.* ignoretests: true - misspell: - locale: US linters: + disable-all: true enable: - bodyclose - - prealloc - - unconvert + - deadcode - errcheck - gofmt - goimports - gosec + - gosimple + - govet - ineffassign - misspell + - prealloc + - staticcheck + - structcheck + - typecheck + - unconvert + - unused + - varcheck + # these are implicitly disabled: + # - asciicheck + # - depguard + # - dogsled + # - dupl + # - exhaustive + # - exportloopref + # - funlen + # - gci + # - gochecknoglobals + # - gochecknoinits + # - gocognit + # - goconst + # - gocritic + # - gocyclo + # - godot + # - godox + # - goerr113 + # - gofumpt + # - goheader + # - golint + # - gomnd + # - gomodguard + # - goprintffuncname + # - interfacer + # - lll + # - maligned + # - nakedret + # - nestif + # - nlreturn + # - noctx + # - nolintlint + # - rowserrcheck + # - scopelint + # - sqlclosecheck + # - stylecheck + # - testpackage + # - unparam + # - whitespace + # - wsl run: # default concurrency is a available CPU number. @@ -398,7 +398,10 @@ func (h adminHandler) handleError(w http.ResponseWriter, r *http.Request, err er w.Header().Set("Content-Type", "application/json") w.WriteHeader(apiErr.Code) - json.NewEncoder(w).Encode(apiErr) + encErr := json.NewEncoder(w).Encode(apiErr) + if encErr != nil { + Log().Named("admin.api").Error("failed to encode error response", zap.Error(encErr)) + } } // checkHost returns a handler that wraps next such that @@ -838,7 +841,7 @@ var ( // will get deleted before the process gracefully exits. func PIDFile(filename string) error { pid := []byte(strconv.Itoa(os.Getpid()) + "\n") - err := ioutil.WriteFile(filename, pid, 0644) + err := ioutil.WriteFile(filename, pid, 0600) if err != nil { return err } diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index f8b5e2c..4a0b335 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/caddyconfig/httpcaddyfile/builtins.go @@ -356,7 +356,7 @@ func parseTLS(h Helper) ([]ConfigValue, error) { } // begin building the final config values - var configVals []ConfigValue + configVals := []ConfigValue{} // certificate loaders if len(fileLoader) > 0 { diff --git a/caddytest/caddytest.go b/caddytest/caddytest.go index 85a6373..d3c70b6 100644 --- a/caddytest/caddytest.go +++ b/caddytest/caddytest.go @@ -124,10 +124,10 @@ func (tc *Tester) initServer(rawConfig string, configType string) error { return } defer res.Body.Close() - body, err := ioutil.ReadAll(res.Body) + body, _ := ioutil.ReadAll(res.Body) var out bytes.Buffer - json.Indent(&out, body, "", " ") + _ = json.Indent(&out, body, "", " ") tc.t.Logf("----------- failed with config -----------\n%s", out.String()) } }) @@ -221,10 +221,11 @@ func isCaddyAdminRunning() error { client := &http.Client{ Timeout: Default.LoadRequestTimeout, } - _, err := client.Get(fmt.Sprintf("http://localhost:%d/config/", Default.AdminPort)) + resp, err := client.Get(fmt.Sprintf("http://localhost:%d/config/", Default.AdminPort)) if err != nil { return errors.New("caddy integration test caddy server not running. Expected to be listening on localhost:2019") } + resp.Body.Close() return nil } @@ -272,7 +273,7 @@ func CreateTestingTransport() *http.Transport { IdleConnTimeout: 90 * time.Second, TLSHandshakeTimeout: 5 * time.Second, ExpectContinueTimeout: 1 * time.Second, - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec } } diff --git a/cmd/commandfuncs.go b/cmd/commandfuncs.go index 772fe01..28fa26e 100644 --- a/cmd/commandfuncs.go +++ b/cmd/commandfuncs.go @@ -96,7 +96,7 @@ func cmdStart(fl Flags) (int, error) { // started yet, and writing synchronously would result // in a deadlock go func() { - stdinpipe.Write(expect) + _, _ = stdinpipe.Write(expect) stdinpipe.Close() }() diff --git a/cmd/main.go b/cmd/main.go index 914df21..e92e71f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -236,6 +236,7 @@ func watchConfigFile(filename, adapterName string) { } // begin poller + //nolint:staticcheck for range time.Tick(1 * time.Second) { // get the file info info, err := os.Stat(filename) diff --git a/cmd/proc_posix.go b/cmd/proc_posix.go deleted file mode 100644 index 9ca589f..0000000 --- a/cmd/proc_posix.go +++ /dev/null @@ -1,37 +0,0 @@ -// 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. - -// +build !windows - -package caddycmd - -import ( - "fmt" - "os" - "path/filepath" - "syscall" -) - -func gracefullyStopProcess(pid int) error { - fmt.Print("Graceful stop... ") - err := syscall.Kill(pid, syscall.SIGINT) - if err != nil { - return fmt.Errorf("kill: %v", err) - } - return nil -} - -func getProcessName() string { - return filepath.Base(os.Args[0]) -} diff --git a/cmd/proc_windows.go b/cmd/proc_windows.go deleted file mode 100644 index 4a62c27..0000000 --- a/cmd/proc_windows.go +++ /dev/null @@ -1,44 +0,0 @@ -// 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 caddycmd - -import ( - "fmt" - "os" - "os/exec" - "path/filepath" - "strconv" -) - -func gracefullyStopProcess(pid int) error { - fmt.Print("Forceful stop... ") - // process on windows will not stop unless forced with /f - cmd := exec.Command("taskkill", "/pid", strconv.Itoa(pid), "/f") - if err := cmd.Run(); err != nil { - return fmt.Errorf("taskkill: %v", err) - } - return nil -} - -// On Windows the app name passed in os.Args[0] will match how -// caddy was started eg will match caddy or caddy.exe. -// So return appname with .exe for consistency -func getProcessName() string { - base := filepath.Base(os.Args[0]) - if filepath.Ext(base) == "" { - return base + ".exe" - } - return base -} @@ -354,6 +354,8 @@ github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/libdns/libdns v0.1.0 h1:0ctCOrVJsVzj53mop1angHp/pE3hmAhP7KiHvR0HD04= github.com/libdns/libdns v0.1.0/go.mod h1:yQCXzk1lEZmmCPa857bnk4TsOiqYasqpyOEeSObbb40= github.com/logrusorgru/aurora v0.0.0-20181002194514-a7b3b318ed4e/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4= +github.com/lucas-clemente/quic-go v0.19.2 h1:w8BBYUx5Z+kNpeaOeQW/KzcNsKWhh4O6PeQhb0nURPg= +github.com/lucas-clemente/quic-go v0.19.2/go.mod h1:ZUygOqIoai0ASXXLJ92LTnKdbqh9MHCLTX6Nr1jUrK0= github.com/lunixbochs/vtclean v1.0.0 h1:xu2sLAri4lGiovBDQKxl5mrXyESr3gUr5m5SM5+LVb8= github.com/lunixbochs/vtclean v1.0.0/go.mod h1:pHhQNgMf3btfWnGBVipUOjRYhoOsdGqdm/+2c2E2WMI= github.com/magiconair/properties v1.7.6/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= diff --git a/listeners.go b/listeners.go index 39bd811..2c52d12 100644 --- a/listeners.go +++ b/listeners.go @@ -129,9 +129,9 @@ func (fcl *fakeCloseListener) Accept() (net.Conn, error) { if *fcl.deadline { switch ln := fcl.Listener.(type) { case *net.TCPListener: - ln.SetDeadline(time.Time{}) + _ = ln.SetDeadline(time.Time{}) case *net.UnixListener: - ln.SetDeadline(time.Time{}) + _ = ln.SetDeadline(time.Time{}) } *fcl.deadline = false } @@ -167,9 +167,9 @@ func (fcl *fakeCloseListener) Close() error { if !*fcl.deadline { switch ln := fcl.Listener.(type) { case *net.TCPListener: - ln.SetDeadline(time.Now().Add(-1 * time.Minute)) + _ = ln.SetDeadline(time.Now().Add(-1 * time.Minute)) case *net.UnixListener: - ln.SetDeadline(time.Now().Add(-1 * time.Minute)) + _ = ln.SetDeadline(time.Now().Add(-1 * time.Minute)) } *fcl.deadline = true } @@ -458,7 +458,7 @@ func (cl *CustomLog) buildCore() { if cl.Sampling.Thereafter == 0 { cl.Sampling.Thereafter = 100 } - c = zapcore.NewSampler(c, cl.Sampling.Interval, + c = zapcore.NewSamplerWithOptions(c, cl.Sampling.Interval, cl.Sampling.First, cl.Sampling.Thereafter) } cl.core = c diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index 43cc6f7..42e7725 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -363,6 +363,7 @@ func (app *App) Start() error { ErrorLog: serverLogger, }, } + //nolint:errcheck go h3srv.Serve(h3ln) app.h3servers = append(app.h3servers, h3srv) app.h3listeners = append(app.h3listeners, h3ln) @@ -391,6 +392,7 @@ func (app *App) Start() error { zap.Bool("tls", useTLS), ) + //nolint:errcheck go s.Serve(ln) app.servers = append(app.servers, s) } diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index 805a37c..fff4c46 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -523,7 +523,10 @@ func (app *App) createAutomationPolicies(ctx caddy.Context, internalNames []stri // our base/catch-all policy - this will serve the // public-looking names as well as any other names // that don't match any other policy - app.tlsApp.AddAutomationPolicy(basePolicy) + err := app.tlsApp.AddAutomationPolicy(basePolicy) + if err != nil { + return err + } } else { // a base policy already existed; we might have // changed it, so re-provision it diff --git a/modules/caddyhttp/caddyauth/basicauth.go b/modules/caddyhttp/caddyauth/basicauth.go index f383199..33be70d 100644 --- a/modules/caddyhttp/caddyauth/basicauth.go +++ b/modules/caddyhttp/caddyauth/basicauth.go @@ -240,6 +240,7 @@ func (c *Cache) makeRoom() { // map with less code, this is a heavily skewed eviction // strategy; generating random numbers is cheap and // ensures a much better distribution. + //nolint:gosec rnd := weakrand.Intn(len(c.cache)) i := 0 for key := range c.cache { diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go index a7ac889..485afe0 100644 --- a/modules/caddyhttp/caddyhttp.go +++ b/modules/caddyhttp/caddyhttp.go @@ -18,18 +18,14 @@ import ( "bytes" "encoding/json" "io" - weakrand "math/rand" "net" "net/http" "strconv" - "time" "github.com/caddyserver/caddy/v2" ) func init() { - weakrand.Seed(time.Now().UnixNano()) - caddy.RegisterModule(tlsPlaceholderWrapper{}) } diff --git a/modules/caddyhttp/encode/encode.go b/modules/caddyhttp/encode/encode.go index 52205aa..e42eeed 100644 --- a/modules/caddyhttp/encode/encode.go +++ b/modules/caddyhttp/encode/encode.go @@ -262,7 +262,7 @@ func acceptedEncodings(r *http.Request) []string { return []string{} } - var prefs []encodingPreference + prefs := []encodingPreference{} for _, accepted := range strings.Split(acceptEncHeader, ",") { parts := strings.Split(accepted, ";") diff --git a/modules/caddyhttp/errors.go b/modules/caddyhttp/errors.go index 0593063..85dc3df 100644 --- a/modules/caddyhttp/errors.go +++ b/modules/caddyhttp/errors.go @@ -16,14 +16,19 @@ package caddyhttp import ( "fmt" - mathrand "math/rand" + weakrand "math/rand" "path" "runtime" "strings" + "time" "github.com/caddyserver/caddy/v2" ) +func init() { + weakrand.Seed(time.Now().UnixNano()) +} + // Error is a convenient way for a Handler to populate the // essential fields of a HandlerError. If err is itself a // HandlerError, then any essential fields that are not @@ -92,7 +97,8 @@ func randString(n int, sameCase bool) string { } b := make([]byte, n) for i := range b { - b[i] = dict[mathrand.Int63()%int64(len(dict))] + //nolint:gosec + b[i] = dict[weakrand.Int63()%int64(len(dict))] } return string(b) } diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go index 3fecad9..cd41ea5 100644 --- a/modules/caddyhttp/fileserver/browse.go +++ b/modules/caddyhttp/fileserver/browse.go @@ -82,7 +82,7 @@ func (fsrv *FileServer) serveBrowse(dirPath string, w http.ResponseWriter, r *ht w.Header().Set("Content-Type", "text/html; charset=utf-8") } - buf.WriteTo(w) + _, _ = buf.WriteTo(w) return nil } diff --git a/modules/caddyhttp/fileserver/browselisting.go b/modules/caddyhttp/fileserver/browselisting.go index 79944f9..f3f85a3 100644 --- a/modules/caddyhttp/fileserver/browselisting.go +++ b/modules/caddyhttp/fileserver/browselisting.go @@ -30,10 +30,8 @@ import ( func (fsrv *FileServer) directoryListing(files []os.FileInfo, canGoUp bool, urlPath string, repl *caddy.Replacer) browseListing { filesToHide := fsrv.transformHidePaths(repl) - var ( - fileInfos []fileInfo - dirCount, fileCount int - ) + var dirCount, fileCount int + fileInfos := []fileInfo{} for _, f := range files { name := f.Name() @@ -109,10 +107,8 @@ type browseListing struct { // Breadcrumbs returns l.Path where every element maps // the link to the text to display. func (l browseListing) Breadcrumbs() []crumb { - var result []crumb - if len(l.Path) == 0 { - return result + return []crumb{} } // skip trailing slash @@ -122,13 +118,13 @@ func (l browseListing) Breadcrumbs() []crumb { } parts := strings.Split(lpath, "/") - for i := range parts { - txt := parts[i] - if i == 0 && parts[i] == "" { - txt = "/" + result := make([]crumb, len(parts)) + for i, p := range parts { + if i == 0 && p == "" { + p = "/" } lnk := strings.Repeat("../", len(parts)-i-1) - result = append(result, crumb{Link: lnk, Text: txt}) + result[i] = crumb{Link: lnk, Text: p} } return result diff --git a/modules/caddyhttp/fileserver/browselisting_test.go b/modules/caddyhttp/fileserver/browselisting_test.go new file mode 100644 index 0000000..6d58b7e --- /dev/null +++ b/modules/caddyhttp/fileserver/browselisting_test.go @@ -0,0 +1,40 @@ +package fileserver + +import ( + "testing" +) + +func TestBreadcrumbs(t *testing.T) { + testdata := []struct { + path string + expected []crumb + }{ + {"", []crumb{}}, + {"/", []crumb{{Text: "/"}}}, + {"foo/bar/baz", []crumb{ + {Link: "../../", Text: "foo"}, + {Link: "../", Text: "bar"}, + {Link: "", Text: "baz"}, + }}, + {"/qux/quux/corge/", []crumb{ + {Link: "../../../", Text: "/"}, + {Link: "../../", Text: "qux"}, + {Link: "../", Text: "quux"}, + {Link: "", Text: "corge"}, + }}, + } + + for _, d := range testdata { + l := browseListing{Path: d.path} + actual := l.Breadcrumbs() + if len(actual) != len(d.expected) { + t.Errorf("wrong size output, got %d elements but expected %d", len(actual), len(d.expected)) + continue + } + for i, c := range actual { + if c != d.expected[i] { + t.Errorf("got %#v but expected %#v at index %d", c, d.expected[i], i) + } + } + } +} diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index b9f2b23..caad0b4 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -249,7 +249,7 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c } w.WriteHeader(statusCode) if r.Method != http.MethodHead { - io.Copy(w, file) + _, _ = io.Copy(w, file) } return nil } @@ -278,6 +278,7 @@ func (fsrv *FileServer) openFile(filename string, w http.ResponseWriter) (*os.Fi } // maybe the server is under load and ran out of file descriptors? // have client wait arbitrary seconds to help prevent a stampede + //nolint:gosec backoff := weakrand.Intn(maxBackoff-minBackoff) + minBackoff w.Header().Set("Retry-After", strconv.Itoa(backoff)) return nil, caddyhttp.Error(http.StatusServiceUnavailable, err) diff --git a/modules/caddyhttp/marshalers.go b/modules/caddyhttp/marshalers.go index 9e8bb9f..8001bd8 100644 --- a/modules/caddyhttp/marshalers.go +++ b/modules/caddyhttp/marshalers.go @@ -75,7 +75,8 @@ func (t LoggableTLSConnState) MarshalLogObject(enc zapcore.ObjectEncoder) error enc.AddUint16("version", t.Version) enc.AddUint16("cipher_suite", t.CipherSuite) enc.AddString("proto", t.NegotiatedProtocol) - enc.AddBool("proto_mutual", t.NegotiatedProtocolIsMutual) + // NegotiatedProtocolIsMutual is deprecated - it's always true + enc.AddBool("proto_mutual", true) enc.AddString("server_name", t.ServerName) if len(t.PeerCertificates) > 0 { enc.AddString("client_common_name", t.PeerCertificates[0].Subject.CommonName) diff --git a/modules/caddyhttp/replacer.go b/modules/caddyhttp/replacer.go index 3993433..5a0efce 100644 --- a/modules/caddyhttp/replacer.go +++ b/modules/caddyhttp/replacer.go @@ -362,7 +362,8 @@ func getReqTLSReplacement(req *http.Request, key string) (interface{}, bool) { case "proto": return req.TLS.NegotiatedProtocol, true case "proto_mutual": - return req.TLS.NegotiatedProtocolIsMutual, true + // req.TLS.NegotiatedProtocolIsMutual is deprecated - it's always true. + return true, true case "server_name": return req.TLS.ServerName, true } diff --git a/modules/caddyhttp/reverseproxy/fastcgi/client.go b/modules/caddyhttp/reverseproxy/fastcgi/client.go index ae0de00..94df0c7 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/client.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/client.go @@ -242,13 +242,6 @@ func (c *FCGIClient) writeBeginRequest(role uint16, flags uint8) error { return c.writeRecord(BeginRequest, b[:]) } -func (c *FCGIClient) writeEndRequest(appStatus int, protocolStatus uint8) error { - b := make([]byte, 8) - binary.BigEndian.PutUint32(b, uint32(appStatus)) - b[4] = protocolStatus - return c.writeRecord(EndRequest, b) -} - func (c *FCGIClient) writePairs(recType uint8, pairs map[string]string) error { w := newWriter(c, recType) b := make([]byte, 8) diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index 4e93320..285834b 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -263,7 +263,7 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, host H } defer func() { // drain any remaining body so connection could be re-used - io.Copy(ioutil.Discard, body) + _, _ = io.Copy(ioutil.Discard, body) resp.Body.Close() }() diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index d2a9951..61e9054 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -173,6 +173,7 @@ func (h *HTTPTransport) NewTransport(ctx caddy.Context) (*http.Transport, error) dialer.Resolver = &net.Resolver{ PreferGo: true, Dial: func(ctx context.Context, _, _ string) (net.Conn, error) { + //nolint:gosec addr := h.Resolver.netAddrs[weakrand.Intn(len(h.Resolver.netAddrs))] return d.DialContext(ctx, addr.Network, addr.JoinHostPort(0)) }, diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 7fc61ae..bed5289 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -314,7 +314,7 @@ func (h *Handler) Cleanup() error { // remove hosts from our config from the pool for _, upstream := range h.Upstreams { - hosts.Delete(upstream.String()) + _, _ = hosts.Delete(upstream.String()) } return nil @@ -339,7 +339,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht buf := bufPool.Get().(*bytes.Buffer) buf.Reset() defer bufPool.Put(buf) - io.Copy(buf, r.Body) + _, _ = io.Copy(buf, r.Body) r.Body.Close() r.Body = ioutil.NopCloser(buf) } @@ -518,7 +518,8 @@ func (h Handler) prepareRequest(req *http.Request) error { // (This method is mostly the beginning of what was borrowed from the net/http/httputil package in the // Go standard library which was used as the foundation.) func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, di DialInfo, next caddyhttp.Handler) error { - di.Upstream.Host.CountRequest(1) + _ = di.Upstream.Host.CountRequest(1) + //nolint:errcheck defer di.Upstream.Host.CountRequest(-1) // point the request to this upstream @@ -742,16 +743,6 @@ func copyHeader(dst, src http.Header) { } } -func cloneHeader(h http.Header) http.Header { - h2 := make(http.Header, len(h)) - for k, vv := range h { - vv2 := make([]string, len(vv)) - copy(vv2, vv) - h2[k] = vv2 - } - return h2 -} - func upgradeType(h http.Header) string { if !httpguts.HeaderValuesContainsToken(h["Connection"], "Upgrade") { return "" @@ -759,18 +750,6 @@ func upgradeType(h http.Header) string { return strings.ToLower(h.Get("Upgrade")) } -func singleJoiningSlash(a, b string) string { - aslash := strings.HasSuffix(a, "/") - bslash := strings.HasPrefix(b, "/") - switch { - case aslash && bslash: - return a + b[1:] - case !aslash && !bslash: - return a + "/" + b - } - return a + b -} - // removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h. // See RFC 7230, section 6.1 func removeConnectionHeaders(h http.Header) { diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies.go b/modules/caddyhttp/reverseproxy/selectionpolicies.go index a1010f4..a391b2f 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies.go @@ -536,7 +536,7 @@ func hostByHashing(pool []*Upstream, s string) *Upstream { // hash calculates a fast hash based on s. func hash(s string) uint32 { h := fnv.New32a() - h.Write([]byte(s)) + _, _ = h.Write([]byte(s)) return h.Sum32() } diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go index 4004b7a..f108a97 100644 --- a/modules/caddyhttp/reverseproxy/streaming.go +++ b/modules/caddyhttp/reverseproxy/streaming.go @@ -138,9 +138,9 @@ func (h Handler) copyResponse(dst io.Writer, src io.Reader, flushInterval time.D } } - buf := streamingBufPool.Get().([]byte) + buf := streamingBufPool.Get().(*[]byte) defer streamingBufPool.Put(buf) - _, err := h.copyBuffer(dst, src, buf) + _, err := h.copyBuffer(dst, src, *buf) return err } @@ -255,7 +255,12 @@ func (c switchProtocolCopier) copyToBackend(errc chan<- error) { var streamingBufPool = sync.Pool{ New: func() interface{} { - return make([]byte, defaultBufferSize) + // The Pool's New function should generally only return pointer + // types, since a pointer can be put into the return interface + // value without an allocation + // - (from the package docs) + b := make([]byte, defaultBufferSize) + return &b }, } diff --git a/modules/caddyhttp/reverseproxy/streaming_test.go b/modules/caddyhttp/reverseproxy/streaming_test.go new file mode 100644 index 0000000..4ed1f1e --- /dev/null +++ b/modules/caddyhttp/reverseproxy/streaming_test.go @@ -0,0 +1,30 @@ +package reverseproxy + +import ( + "bytes" + "strings" + "testing" +) + +func TestHandlerCopyResponse(t *testing.T) { + h := Handler{} + testdata := []string{ + "", + strings.Repeat("a", defaultBufferSize), + strings.Repeat("123456789 123456789 123456789 12", 3000), + } + dst := bytes.NewBuffer(nil) + + for _, d := range testdata { + src := bytes.NewBuffer([]byte(d)) + dst.Reset() + err := h.copyResponse(dst, src, 0) + if err != nil { + t.Errorf("failed with error: %v", err) + } + out := dst.String() + if out != d { + t.Errorf("bad read: got %q", out) + } + } +} diff --git a/modules/caddyhttp/templates/tplcontext.go b/modules/caddyhttp/templates/tplcontext.go index 7bc0ce7..8bdaeec 100644 --- a/modules/caddyhttp/templates/tplcontext.go +++ b/modules/caddyhttp/templates/tplcontext.go @@ -270,7 +270,10 @@ func (templateContext) funcMarkdown(input interface{}) (string, error) { buf.Reset() defer bufPool.Put(buf) - md.Convert([]byte(inputStr), buf) + err := md.Convert([]byte(inputStr), buf) + if err != nil { + return "", err + } return buf.String(), nil } diff --git a/modules/caddypki/acmeserver/acmeserver.go b/modules/caddypki/acmeserver/acmeserver.go index 6023e06..9d8a6fc 100644 --- a/modules/caddypki/acmeserver/acmeserver.go +++ b/modules/caddypki/acmeserver/acmeserver.go @@ -132,11 +132,11 @@ func (ash *Handler) Provision(ctx caddy.Context) error { return err } - acmeAuth, err := acme.NewAuthority( - auth.GetDatabase().(nosql.DB), // stores all the server state - ash.Host, // used for directory links; TODO: not needed - strings.Trim(ash.PathPrefix, "/"), // used for directory links - auth) // configures the signing authority + acmeAuth, err := acme.New(auth, acme.AuthorityOptions{ + DB: auth.GetDatabase().(nosql.DB), // stores all the server state + DNS: ash.Host, // used for directory links; TODO: not needed + Prefix: strings.Trim(ash.PathPrefix, "/"), // used for directory links + }) if err != nil { return err } diff --git a/modules/caddypki/ca.go b/modules/caddypki/ca.go index c0a0096..f95c9a0 100644 --- a/modules/caddypki/ca.go +++ b/modules/caddypki/ca.go @@ -309,14 +309,6 @@ func (ca CA) loadOrGenIntermediate(rootCert *x509.Certificate, rootKey interface func (ca CA) genIntermediate(rootCert *x509.Certificate, rootKey interface{}) (interCert *x509.Certificate, interKey interface{}, err error) { repl := ca.newReplacer() - rootKeyPEM, err := ca.storage.Load(ca.storageKeyRootKey()) - if err != nil { - return nil, nil, fmt.Errorf("loading root key to sign new intermediate: %v", err) - } - rootKey, err = pemDecodePrivateKey(rootKeyPEM) - if err != nil { - return nil, nil, fmt.Errorf("decoding root key: %v", err) - } interCert, interKey, err = generateIntermediate(repl.ReplaceAll(ca.IntermediateCommonName, ""), rootCert, rootKey) if err != nil { return nil, nil, fmt.Errorf("generating CA intermediate: %v", err) diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go index 7eda002..de929cc 100644 --- a/modules/caddytls/connpolicy.go +++ b/modules/caddytls/connpolicy.go @@ -80,6 +80,7 @@ func (cp ConnectionPolicies) TLSConfig(ctx caddy.Context) *tls.Config { } return &tls.Config{ + MinVersion: tls.VersionTLS12, GetConfigForClient: func(hello *tls.ClientHelloInfo) (*tls.Config, error) { // filter policies by SNI first, if possible, to speed things up // when there may be lots of policies diff --git a/modules/caddytls/distributedstek/distributedstek.go b/modules/caddytls/distributedstek/distributedstek.go index f29db29..e76fc47 100644 --- a/modules/caddytls/distributedstek/distributedstek.go +++ b/modules/caddytls/distributedstek/distributedstek.go @@ -145,7 +145,12 @@ func (s *Provider) storeSTEK(dstek distributedSTEK) error { // current STEK is outdated (NextRotation time is in the past), // then it is rotated and persisted. The resulting STEK is returned. func (s *Provider) getSTEK() (distributedSTEK, error) { - s.storage.Lock(s.ctx, stekLockName) + err := s.storage.Lock(s.ctx, stekLockName) + if err != nil { + return distributedSTEK{}, fmt.Errorf("failed to acquire storage lock: %v", err) + } + + //nolint:errcheck defer s.storage.Unlock(stekLockName) // load the current STEKs from storage diff --git a/modules/caddytls/folderloader.go b/modules/caddytls/folderloader.go index f1a742d..10b017e 100644 --- a/modules/caddytls/folderloader.go +++ b/modules/caddytls/folderloader.go @@ -97,26 +97,38 @@ func x509CertFromCertAndKeyPEMFile(fpath string) (tls.Certificate, error) { if derBlock.Type == "CERTIFICATE" { // Re-encode certificate as PEM, appending to certificate chain - pem.Encode(certBuilder, derBlock) + err = pem.Encode(certBuilder, derBlock) + if err != nil { + return tls.Certificate{}, err + } } else if derBlock.Type == "EC PARAMETERS" { // EC keys generated from openssl can be composed of two blocks: // parameters and key (parameter block should come first) if !foundKey { // Encode parameters - pem.Encode(keyBuilder, derBlock) + err = pem.Encode(keyBuilder, derBlock) + if err != nil { + return tls.Certificate{}, err + } // Key must immediately follow derBlock, bundle = pem.Decode(bundle) if derBlock == nil || derBlock.Type != "EC PRIVATE KEY" { return tls.Certificate{}, fmt.Errorf("%s: expected elliptic private key to immediately follow EC parameters", fpath) } - pem.Encode(keyBuilder, derBlock) + err = pem.Encode(keyBuilder, derBlock) + if err != nil { + return tls.Certificate{}, err + } foundKey = true } } else if derBlock.Type == "PRIVATE KEY" || strings.HasSuffix(derBlock.Type, " PRIVATE KEY") { // RSA key if !foundKey { - pem.Encode(keyBuilder, derBlock) + err = pem.Encode(keyBuilder, derBlock) + if err != nil { + return tls.Certificate{}, err + } foundKey = true } } else { diff --git a/modules/caddytls/internalissuer.go b/modules/caddytls/internalissuer.go index 6f228ea..416369f 100644 --- a/modules/caddytls/internalissuer.go +++ b/modules/caddytls/internalissuer.go @@ -27,6 +27,7 @@ import ( "github.com/caddyserver/caddy/v2/modules/caddypki" "github.com/caddyserver/certmagic" "github.com/smallstep/certificates/authority/provisioner" + "go.uber.org/zap" ) func init() { @@ -51,7 +52,8 @@ type InternalIssuer struct { // validate certificate chains. SignWithRoot bool `json:"sign_with_root,omitempty"` - ca *caddypki.CA + ca *caddypki.CA + logger *zap.Logger } // CaddyModule returns the Caddy module information. @@ -64,6 +66,8 @@ func (InternalIssuer) CaddyModule() caddy.ModuleInfo { // Provision sets up the issuer. func (iss *InternalIssuer) Provision(ctx caddy.Context) error { + iss.logger = ctx.Logger(iss) + // get a reference to the configured CA appModule, err := ctx.App("pki") if err != nil { @@ -115,11 +119,15 @@ func (iss InternalIssuer) Issue(ctx context.Context, csr *x509.CertificateReques // ensure issued certificate does not expire later than its issuer lifetime := time.Duration(iss.Lifetime) if time.Now().Add(lifetime).After(issuerCert.NotAfter) { - // TODO: log this - lifetime = issuerCert.NotAfter.Sub(time.Now()) + lifetime = time.Until(issuerCert.NotAfter) + iss.logger.Warn("cert lifetime would exceed issuer NotAfter, clamping lifetime", + zap.Duration("orig_lifetime", time.Duration(iss.Lifetime)), + zap.Duration("lifetime", lifetime), + zap.Time("not_after", issuerCert.NotAfter), + ) } - certChain, err := auth.Sign(csr, provisioner.SignOptions{}, customCertLifetime(iss.Lifetime)) + certChain, err := auth.Sign(csr, provisioner.SignOptions{}, customCertLifetime(caddy.Duration(lifetime))) if err != nil { return nil, err } diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 146eed4..fd3473e 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -498,8 +498,6 @@ var ( storageCleanMu sync.Mutex ) -const automateKey = "automate" - // Interface guards var ( _ caddy.App = (*TLS)(nil) diff --git a/modules/caddytls/values.go b/modules/caddytls/values.go index f0944a3..dea0013 100644 --- a/modules/caddytls/values.go +++ b/modules/caddytls/values.go @@ -122,6 +122,7 @@ var SupportedProtocols = map[string]uint16{ // unsupportedProtocols is a map of unsupported protocols. // Used for logging only, not enforcement. var unsupportedProtocols = map[string]uint16{ + //nolint:staticcheck "ssl3.0": tls.VersionSSL30, "tls1.0": tls.VersionTLS10, "tls1.1": tls.VersionTLS11, |