diff --git a/.golangci.yml b/.golangci.yml index 5c6faefb..30f9c356 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,42 +1,78 @@ +run: + # Timeout for analysis, e.g. 30s, 5m. + # Default: 1m + timeout: 3m + + skip-dirs: + - pkg + + + +# This file contains only configs which differ from defaults. +# All possible options can be found here https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml linters-settings: - govet: - check-shadowing: true - settings: - printf: - funcs: - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf - revive: - min-confidence: 0 - gocyclo: - min-complexity: 10 - maligned: - suggest-new: true - dupl: - threshold: 100 - goconst: - min-len: 2 - min-occurrences: 2 + cyclop: + # The maximal code complexity to report. + # Default: 10 + max-complexity: 30 + # The maximal average package complexity. + # If it's higher than 0.0 (float) the check is enabled + # Default: 0.0 + package-average: 10.0 + depguard: list-type: blacklist packages: # logging is allowed only by logutils.Log, logrus # is allowed to use only in logutils package - github.com/sirupsen/logrus - misspell: - locale: US - lll: - line-length: 140 - goimports: - local-prefixes: github.com/golangci/golangci-lint + + dupl: + threshold: 100 + + errcheck: + # Report about not checking of errors in type assertions: `a := b.(MyStruct)`. + # Such cases aren't reported by default. + # Default: false + check-type-assertions: true + + funlen: + # Checks the number of lines in a function. + # If lower than 0, disable the check. + # Default: 60 + lines: 100 + # Checks the number of statements in a function. + # If lower than 0, disable the check. + # Default: 40 + statements: 50 + + gocognit: + # Minimal code complexity to report + # Default: 30 (but we recommend 10-20) + min-complexity: 20 + + goconst: + min-len: 2 + min-occurrences: 2 + gocritic: enabled-tags: - performance - style - experimental - diagnostic + # Settings passed to gocritic. + # The settings key is the name of a supported gocritic checker. + # The list of supported checkers can be find in https://go-critic.github.io/overview. + settings: + captLocal: + # Whether to restrict checker to params only. + # Default: true + paramsOnly: false + underef: + # Whether to skip (*x).method() calls where x is a pointer receiver. + # Default: true + skipRecvDeref: false disabled-checks: - commentFormatting - commentedOutCode @@ -46,28 +82,246 @@ linters-settings: - tooManyResultsChecker - unnamedResult + gocyclo: + min-complexity: 10 + + gomnd: + # List of function patterns to exclude from analysis. + # Values always ignored: `time.Date` + # Default: [] + ignored-functions: + - os.Chmod + - os.Mkdir + - os.MkdirAll + - os.OpenFile + - os.WriteFile + - prometheus.ExponentialBuckets + - prometheus.ExponentialBucketsRange + - prometheus.LinearBuckets + - strconv.FormatFloat + - strconv.FormatInt + - strconv.FormatUint + - strconv.ParseFloat + - strconv.ParseInt + - strconv.ParseUint + + gomodguard: + blocked: + # List of blocked modules. + # Default: [] + modules: + - github.com/golang/protobuf: + recommendations: + - google.golang.org/protobuf + reason: "see https://developers.google.com/protocol-buffers/docs/reference/go/faq#modules" + - github.com/satori/go.uuid: + recommendations: + - github.com/google/uuid + reason: "satori's package is not maintained" + - github.com/gofrs/uuid: + recommendations: + - github.com/google/uuid + reason: "see recommendation from dev-infra team: https://confluence.gtforge.com/x/gQI6Aw" + + goimports: + local-prefixes: github.com/golangci/golangci-lint + + govet: + check-shadowing: true + # Enable all analyzers. + # Default: false + enable-all: true + # Disable analyzers by name. + # Run `go tool vet help` to see all analyzers. + # Default: [] + disable: + - fieldalignment # too strict + # Settings per analyzer. + settings: + printf: + funcs: + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf + + lll: + line-length: 140 + + maligned: + suggest-new: true + + misspell: + locale: US + + nakedret: + # Make an issue if func has more lines of code than this setting, and it has naked returns. + # Default: 30 + max-func-lines: 0 + + nolintlint: + # Exclude following linters from requiring an explanation. + # Default: [] + allow-no-explanation: [ funlen, gocognit, lll ] + # Enable to require an explanation of nonzero length after each nolint directive. + # Default: false + require-explanation: true + # Enable to require nolint directives to mention the specific linter being suppressed. + # Default: false + require-specific: true + + revive: + min-confidence: 0 + + rowserrcheck: + # database/sql is always checked + # Default: [] + packages: + - github.com/jmoiron/sqlx + + tenv: + # The option `all` will run against whole test files (`_test.go`) regardless of method/function signatures. + # Otherwise, only methods that take `*testing.T`, `*testing.B`, and `testing.TB` as arguments are checked. + # Default: false + all: true + + varcheck: + # Check usage of exported fields and variables. + # Default: false + exported-fields: false # default false # TODO: enable after fixing false positives + linters: disable-all: true enable: - - deadcode - - gocritic - - gofmt - - gosimple - - govet - - ineffassign - - misspell - - revive - - staticcheck - - structcheck - - unused + ## enabled by default + - deadcode # Finds unused code + - gosimple # Linter for Go source code that specializes in simplifying a code + - govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string + - ineffassign # Detects when assignments to existing variables are not used + - staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks + - structcheck # Finds unused struct fields + - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code + - unused # Checks Go code for unused constants, variables, functions and types + - varcheck # Finds unused global variables and constants + ## disabled by default + - gocritic # Provides diagnostics that check for bugs, performance and style issues. + - gofmt # [replaced by goimports] Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification + - goimports # In addition to fixing imports, goimports also formats your code in the same style as gofmt. + - misspell # Finds commonly misspelled English words in comments + - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. -run: - skip-dirs: - - pkg + ## Consider Enabling + #- asasalint # Check for pass []any as any in variadic func(...any) + #- asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers + #- bidichk # Checks for dangerous unicode character sequences + #- bodyclose # checks whether HTTP response body is closed successfully + #- contextcheck # check the function whether use a non-inherited context + #- cyclop # checks function and package cyclomatic complexity + #- dupl # Tool for code clone detection + #- durationcheck # check for two durations multiplied together + #- errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases + #- errname # Checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error. + #- errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. + #- execinquery # execinquery is a linter about query string checker in Query function which reads your Go src files and warning it finds + #- exhaustive # check exhaustiveness of enum switch statements + #- exportloopref # checks for pointers to enclosing loop variables + #- forbidigo # Forbids identifiers + #- funlen # Tool for detection of long functions + #- gochecknoglobals # check that no global variables exist + #- gochecknoinits # Checks that no init functions are present in Go code + #- gocognit # Computes and checks the cognitive complexity of functions + #- goconst # Finds repeated strings that could be replaced by a constant + #- gocyclo # Computes and checks the cyclomatic complexity of functions + #- godot # Check if comments end in a period + #- gomnd # An analyzer to detect magic numbers. + #- gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. + #- gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. + #- goprintffuncname # Checks that printf-like functions are named with f at the end + #- lll # Reports long lines + #- makezero # Finds slice declarations with non-zero initial length + #- nakedret # Finds naked returns in functions greater than a specified function length + #- nestif # Reports deeply nested if statements + #- nilerr # Finds the code that returns nil even if it checks that the error is not nil. + #- nilnil # Checks that there is no simultaneous return of nil error and an invalid value. + #- noctx # noctx finds sending http request without context.Context + #- nolintlint # Reports ill-formed or insufficient nolint directives + #- nonamedreturns # Reports all named returns + #- nosprintfhostport # Checks for misuse of Sprintf to construct a host with port in a URL. + #- predeclared # find code that shadows one of Go's predeclared identifiers + #- promlinter # Check Prometheus metrics naming via promlint + #- rowserrcheck # checks whether Err of rows is checked successfully + #- sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed. + #- stylecheck # Stylecheck is a replacement for golint + #- tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17 + #- testpackage # linter that makes you use a separate _test package + #- tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes + #- unconvert # Remove unnecessary type conversions + #- unparam # Reports unused function parameters + #- wastedassign # wastedassign finds wasted assignment statements. + #- whitespace # Tool for detection of leading and trailing whitespace + #- wrapcheck # Checks that errors returned from external packages are wrapped + ## you may want to enable + #- decorder # check declaration order and count of types, constants, variables and functions + #- exhaustruct # Checks if all structure fields are initialized + #- goheader # Checks is file header matches to pattern + #- ireturn # Accept Interfaces, Return Concrete Types + #- prealloc # [premature optimization, but can be used in some cases] Finds slice declarations that could potentially be preallocated + #- varnamelen # [great idea, but too many false positives] checks that the length of a variable's name matches its scope + # + ## disabled + #- containedctx # containedctx is a linter that detects struct contained context.Context field + #- depguard # [replaced by gomodguard] Go linter that checks if package imports are in a list of acceptable packages + #- dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) + #- errchkjson # [don't see profit + I'm against of omitting errors like in the first example https://github.com/breml/errchkjson] Checks types passed to the json encoding functions. Reports unsupported types and optionally reports occasions, where the check for the returned error can be omitted. + #- forcetypeassert # [replaced by errcheck] finds forced type assertions + #- gci # Gci controls golang package import order and makes it always deterministic. + #- godox # Tool for detection of FIXME, TODO and other comment keywords + #- goerr113 # [too strict] Golang linter to check the errors handling expressions + #- gofumpt # [replaced by goimports, gofumports is not available yet] Gofumpt checks whether code was gofumpt-ed. + #- grouper # An analyzer to analyze expression groups. + #- ifshort # Checks that your code uses short syntax for if-statements whenever possible + #- importas # Enforces consistent import aliases + #- maintidx # maintidx measures the maintainability index of each function. + #- nlreturn # [too strict and mostly code is not more readable] nlreturn checks for a new line before return and branch statements to increase code clarity + #- nosnakecase # Detects snake case of variable naming and function name. # TODO: maybe enable after https://github.com/sivchari/nosnakecase/issues/14 + #- paralleltest # [too many false positives] paralleltest detects missing usage of t.Parallel() method in your Go test + #- tagliatelle # Checks the struct tags. + #- thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers + #- wsl # [too strict and mostly code is not more readable] Whitespace Linter - Forces you to use empty lines! + ## deprecated + #- exhaustivestruct # [deprecated, replaced by exhaustruct] Checks if all struct's fields are initialized + #- golint # [deprecated, replaced by revive] Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes + #- interfacer # [deprecated] Linter that suggests narrower interface types + #- maligned # [deprecated, replaced by govet fieldalignment] Tool to detect Go structs that would take less memory if their fields were sorted + #- scopelint # [deprecated, replaced by exportloopref] Scopelint checks for unpinned variables in go programs issues: + # Maximum count of issues with the same text. + # Set to 0 to disable. + # Default: 3 + max-same-issues: 50 + exclude: - declaration of "err" shadows declaration at line - should have a package comment, unless it's in another file for this package - func `CLICommand. - error strings should not be capitalized or end with punctuation or a newline + + exclude-rules: + - source: "^//\\s*go:generate\\s" + linters: [ lll ] + - source: "(noinspection|TODO)" + linters: [ godot ] + - source: "//noinspection" + linters: [ gocritic ] + - source: "^\\s+if _, ok := err\\.\\([^.]+\\.InternalError\\); ok {" + linters: [ errorlint ] + - path: "_test\\.go" + linters: + - bodyclose + - dupl + - funlen + - goconst + - gosec + - noctx + - wrapcheck diff --git a/cmd/step/main.go b/cmd/step/main.go index 25fb4cf8..da865b85 100644 --- a/cmd/step/main.go +++ b/cmd/step/main.go @@ -41,9 +41,6 @@ import ( _ "github.com/smallstep/certificates/cas/cloudcas" _ "github.com/smallstep/certificates/cas/softcas" _ "github.com/smallstep/certificates/cas/stepcas" - - // Profiling and debugging - _ "net/http/pprof" ) // Version is set by an LDFLAG at build time representing the git tag or commit diff --git a/command/ca/rekey.go b/command/ca/rekey.go index fe41da45..c8c8f160 100644 --- a/command/ca/rekey.go +++ b/command/ca/rekey.go @@ -315,6 +315,7 @@ func rekeyCertificateAction(ctx *cli.Context) error { // Do not rekey if (cert.notAfter - now) > (expiresIn + jitter) if expiresIn > 0 { + // nolint:gosec // The random number below is not being used for crypto. jitter := rand.Int63n(int64(expiresIn / 20)) if d := time.Until(leaf.NotAfter); d > expiresIn+time.Duration(jitter) { ui.Printf("certificate not rekeyed: expires in %s\n", d.Round(time.Second)) diff --git a/command/ca/renew.go b/command/ca/renew.go index eeb7010d..f92938e6 100644 --- a/command/ca/renew.go +++ b/command/ca/renew.go @@ -315,6 +315,7 @@ func renewCertificateAction(ctx *cli.Context) error { // Do not renew if (cert.notAfter - now) > (expiresIn + jitter) if expiresIn > 0 { + // nolint:gosec // The random number below is not being used for crypto. jitter := rand.Int63n(int64(expiresIn / 20)) if d := time.Until(cert.Leaf.NotAfter); d > expiresIn+time.Duration(jitter) { ui.Printf("certificate not renewed: expires in %s\n", d.Round(time.Second)) @@ -348,8 +349,10 @@ func nextRenewDuration(leaf *x509.Certificate, expiresIn, renewPeriod time.Durat case d <= 0: return 0 case d < period/20: + // nolint:gosec // The random number below is not being used for crypto. return time.Duration(rand.Int63n(int64(d))) default: + // nolint:gosec // The random number below is not being used for crypto. n := rand.Int63n(int64(period / 20)) d -= time.Duration(n) return d @@ -381,6 +384,7 @@ func runExecCmd(execCmd string) error { return nil } parts := strings.Split(execCmd, " ") + // nolint:gosec // arguments controlled by step. cmd := exec.Command(parts[0], parts[1:]...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout @@ -413,6 +417,7 @@ func newRenewer(ctx *cli.Context, caURL string, cert tls.Certificate, rootFile s TLSClientConfig: &tls.Config{ RootCAs: rootCAs, PreferServerCipherSuites: true, + MinVersion: tls.VersionTLS12, }, } @@ -523,7 +528,7 @@ func (r *renewer) Rekey(priv interface{}, outCert, outKey string, writePrivateKe // NOTE: this function logs each time the certificate is successfully renewed. func (r *renewer) RenewAndPrepareNext(outFile string, expiresIn, renewPeriod time.Duration) (time.Duration, error) { const durationOnErrors = 1 * time.Minute - Info := log.New(os.Stdout, "INFO: ", log.LstdFlags) + infoLog := log.New(os.Stdout, "INFO: ", log.LstdFlags) resp, err := r.Renew(outFile) if err != nil { @@ -554,21 +559,21 @@ func (r *renewer) RenewAndPrepareNext(outFile string, expiresIn, renewPeriod tim // Get next renew duration next := nextRenewDuration(resp.ServerPEM.Certificate, expiresIn, renewPeriod) - Info.Printf("%s certificate renewed, next in %s", resp.ServerPEM.Certificate.Subject.CommonName, next.Round(time.Second)) + infoLog.Printf("%s certificate renewed, next in %s", resp.ServerPEM.Certificate.Subject.CommonName, next.Round(time.Second)) return next, nil } func (r *renewer) Daemon(outFile string, next, expiresIn, renewPeriod time.Duration, afterRenew func() error) error { // Loggers - Info := log.New(os.Stdout, "INFO: ", log.LstdFlags) - Error := log.New(os.Stderr, "ERROR: ", log.LstdFlags) + infoLog := log.New(os.Stdout, "INFO: ", log.LstdFlags) + errLog := log.New(os.Stderr, "ERROR: ", log.LstdFlags) // Daemon loop signals := make(chan os.Signal, 1) signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) defer signal.Stop(signals) - Info.Printf("first renewal in %s", next.Round(time.Second)) + infoLog.Printf("first renewal in %s", next.Round(time.Second)) var err error for { select { @@ -576,18 +581,18 @@ func (r *renewer) Daemon(outFile string, next, expiresIn, renewPeriod time.Durat switch sig { case syscall.SIGHUP: if next, err = r.RenewAndPrepareNext(outFile, expiresIn, renewPeriod); err != nil { - Error.Println(err) + errLog.Println(err) } else if err := afterRenew(); err != nil { - Error.Println(err) + errLog.Println(err) } case syscall.SIGINT, syscall.SIGTERM: return nil } case <-time.After(next): if next, err = r.RenewAndPrepareNext(outFile, expiresIn, renewPeriod); err != nil { - Error.Println(err) + errLog.Println(err) } else if err := afterRenew(); err != nil { - Error.Println(err) + errLog.Println(err) } } } diff --git a/command/ca/revoke.go b/command/ca/revoke.go index 58d3908c..76e1e866 100644 --- a/command/ca/revoke.go +++ b/command/ca/revoke.go @@ -463,6 +463,7 @@ func (f *revokeFlow) Revoke(ctx *cli.Context, serial, token string) error { RootCAs: rootCAs, PreferServerCipherSuites: true, Certificates: []tls.Certificate{cert}, + MinVersion: tls.VersionTLS12, }, } } diff --git a/command/certificate/p12.go b/command/certificate/p12.go index a9ce66f7..b999e54d 100644 --- a/command/certificate/p12.go +++ b/command/certificate/p12.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "crypto/x509" "fmt" + "github.com/pkg/errors" "github.com/smallstep/cli/crypto/fingerprint" "github.com/smallstep/cli/crypto/pemutil" diff --git a/command/certificate/remote.go b/command/certificate/remote.go index d8b9bab9..46592137 100644 --- a/command/certificate/remote.go +++ b/command/certificate/remote.go @@ -46,7 +46,10 @@ func getPeerCertificates(addr, serverName, roots string, insecure bool) ([]*x509 if _, _, err := net.SplitHostPort(addr); err != nil { addr = net.JoinHostPort(addr, "443") } - tlsConfig := &tls.Config{RootCAs: rootCAs} + tlsConfig := &tls.Config{ + MinVersion: tls.VersionTLS12, + RootCAs: rootCAs, + } if insecure { tlsConfig.InsecureSkipVerify = true } diff --git a/command/crl/inspect.go b/command/crl/inspect.go index d1dd9caf..8a2218ba 100644 --- a/command/crl/inspect.go +++ b/command/crl/inspect.go @@ -135,7 +135,8 @@ func inspectAction(ctx *cli.Context) error { return err } tlsConfig = &tls.Config{ - RootCAs: pool, + RootCAs: pool, + MinVersion: tls.VersionTLS12, } tr := http.DefaultTransport.(*http.Transport).Clone() tr.TLSClientConfig = tlsConfig diff --git a/command/crypto/hash/hash.go b/command/crypto/hash/hash.go index f7fcf573..ca27221c 100644 --- a/command/crypto/hash/hash.go +++ b/command/crypto/hash/hash.go @@ -1,8 +1,8 @@ package hash import ( - "crypto/md5" - "crypto/sha1" + "crypto/md5" // nolint:gosec // md5 can only be used with --insecure flag + "crypto/sha1" // nolint:gosec // sha1 is being used to calculate a hash, not a key "crypto/sha256" "crypto/sha512" "crypto/subtle" @@ -273,6 +273,7 @@ func compareAction(ctx *cli.Context) error { func getHash(ctx *cli.Context, alg string, insecure bool) (hashConstructor, error) { switch strings.ToLower(alg) { case "sha", "sha1": + // nolint:gosec // sha1 is being used to calculate a hash, not a key. return sha1.New, nil case "sha224": return sha256.New224, nil diff --git a/command/fileserver/fileserver.go b/command/fileserver/fileserver.go index 110b2650..a98b32f2 100644 --- a/command/fileserver/fileserver.go +++ b/command/fileserver/fileserver.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "os" + "time" "github.com/pkg/errors" @@ -121,6 +122,7 @@ func fileServerAction(ctx *cli.Context) error { tlsConfig = &tls.Config{ ClientCAs: pool, ClientAuth: tls.RequireAndVerifyClientCert, + MinVersion: tls.VersionTLS13, } } @@ -130,8 +132,9 @@ func fileServerAction(ctx *cli.Context) error { } srv := &http.Server{ - Handler: http.FileServer(http.Dir(root)), - TLSConfig: tlsConfig, + Handler: http.FileServer(http.Dir(root)), + TLSConfig: tlsConfig, + ReadHeaderTimeout: 15 * time.Second, } if cert != "" && key != "" { fmt.Printf("Serving HTTPS at %s ...\n", l.Addr().String()) diff --git a/command/oauth/cmd.go b/command/oauth/cmd.go index c11c5587..0fd65f2e 100644 --- a/command/oauth/cmd.go +++ b/command/oauth/cmd.go @@ -45,17 +45,18 @@ import ( // available here https://cloud.google.com/sdk/docs/quickstarts const ( defaultClientID = "1087160488420-8qt7bavg3qesdhs6it824mhnfgcfe8il.apps.googleusercontent.com" - defaultClientNotSoSecret = "udTrOT3gzrO7W9fDPgZQLfYJ" + defaultClientNotSoSecret = "udTrOT3gzrO7W9fDPgZQLfYJ" // nolint:gosec // This is a client meant for open source testing. The client has no security access or roles. defaultDeviceAuthzClientID = "1087160488420-1u0jqoulmv3mfomfh6fhkfs4vk4bdjih.apps.googleusercontent.com" - defaultDeviceAuthzClientNotSoSecret = "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" - defaultDeviceAuthzInterval = 5 - defaultDeviceAuthzExpiresIn = time.Minute * 5 + defaultDeviceAuthzClientNotSoSecret = "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" // nolint:gosec // This is a client meant for open source testing. The client has no security access or roles. + + defaultDeviceAuthzInterval = 5 + defaultDeviceAuthzExpiresIn = time.Minute * 5 // The URN for getting verification token offline oobCallbackUrn = "urn:ietf:wg:oauth:2.0:oob" // The URN for token request grant type jwt-bearer - jwtBearerUrn = "urn:ietf:params:oauth:grant-type:jwt-bearer" + jwtBearerUrn = "urn:ietf:params:oauth:grant-type:jwt-bearer" // nolint:gosec // This is a resource identifier (not a secret). ) type token struct { @@ -712,7 +713,10 @@ func (o *oauth) NewServer() (*httptest.Server, error) { } srv := &httptest.Server{ Listener: l, - Config: &http.Server{Handler: o}, + Config: &http.Server{ + Handler: o, + ReadHeaderTimeout: 15 * time.Second, + }, } srv.Start() @@ -1192,6 +1196,8 @@ func (o *oauth) Exchange(tokenEndpoint, code string) (*token, error) { data.Set("grant_type", "authorization_code") data.Set("code_verifier", o.codeChallenge) + // nolint:gosec // Tainted url deemed acceptable. Not used to store any + // backend data. resp, err := http.PostForm(tokenEndpoint, data) if err != nil { return nil, errors.WithStack(err) diff --git a/crypto/pemutil/pkcs8.go b/crypto/pemutil/pkcs8.go index a2b2f6e6..15189ea9 100644 --- a/crypto/pemutil/pkcs8.go +++ b/crypto/pemutil/pkcs8.go @@ -3,11 +3,11 @@ package pemutil import ( "crypto/aes" "crypto/cipher" - "crypto/des" + "crypto/des" // nolint:gosec // des is only being used for decryption, not encryption. "crypto/ecdsa" "crypto/ed25519" "crypto/rsa" - "crypto/sha1" + "crypto/sha1" // nolint:gosec // sha1 is only being used for decryption, not encryption. "crypto/sha256" "crypto/x509" "crypto/x509/pkix" @@ -115,15 +115,19 @@ type rfc1423Algo struct { // rfc1423Algos holds a slice of the possible ways to encrypt a PEM // block. The ivSize numbers were taken from the OpenSSL source. var rfc1423Algos = []rfc1423Algo{{ - cipher: x509.PEMCipherDES, - name: "DES-CBC", + cipher: x509.PEMCipherDES, + name: "DES-CBC", + // nolint:gosec // des is only being used for decryption, not encryption. + // Maintain support for legacy keys. cipherFunc: des.NewCipher, keySize: 8, blockSize: des.BlockSize, identifier: oidDESCBC, }, { - cipher: x509.PEMCipher3DES, - name: "DES-EDE3-CBC", + cipher: x509.PEMCipher3DES, + name: "DES-EDE3-CBC", + // nolint:gosec // des is only being used for decryption, not encryption. + // Maintain support for legacy keys. cipherFunc: des.NewTripleDESCipher, keySize: 24, blockSize: des.BlockSize, @@ -288,7 +292,7 @@ func MarshalPKCS8PrivateKey(key interface{}) ([]byte, error) { // key derived using PBKDF2 over the given password. func DecryptPEMBlock(block *pem.Block, password []byte) ([]byte, error) { if block.Headers["Proc-Type"] == "4,ENCRYPTED" { - //nolint + // nolint return x509.DecryptPEMBlock(block, password) } @@ -350,9 +354,13 @@ func DecryptPKCS8PrivateKey(data, password []byte) ([]byte, error) { // DES, TripleDES case encParam.EncryAlgo.Equal(oidDESCBC): symkey = pbkdf2.Key(password, salt, iter, 8, keyHash) + // nolint:gosec // des is only being used for decryption, not encryption. + // Maintain support for legacy keys. block, err = des.NewCipher(symkey) case encParam.EncryAlgo.Equal(oidD3DESCBC): symkey = pbkdf2.Key(password, salt, iter, 24, keyHash) + // nolint:gosec // des is only being used for decryption, not encryption. + // Maintain support for legacy keys. block, err = des.NewTripleDESCipher(symkey) default: return nil, errors.Errorf("unsupported encrypted PEM: unknown algorithm %v", encParam.EncryAlgo) diff --git a/crypto/pemutil/ssh.go b/crypto/pemutil/ssh.go index 42b14616..47dc37a5 100644 --- a/crypto/pemutil/ssh.go +++ b/crypto/pemutil/ssh.go @@ -220,7 +220,9 @@ func ParseOpenSSHPrivateKey(key []byte, opts ...Options) (crypto.PrivateKey, err return nil, errors.Errorf("error decoding key: unsupported elliptic curve %s", key.Curve) } + //nolint: gocritic // ignore capitalization N := curve.Params().N + //nolint: gocritic // ignore capitalization X, Y := elliptic.Unmarshal(curve, key.Pub) if X == nil || Y == nil { return nil, errors.New("error decoding key: failed to unmarshal public key") @@ -307,6 +309,7 @@ func SerializeOpenSSHPrivateKey(key crypto.PrivateKey, opts ...Options) (*pem.Bl switch k := key.(type) { case *rsa.PrivateKey: + //nolint: gocritic // ignore capitalization E := new(big.Int).SetInt64(int64(k.PublicKey.E)) // Marshal public key: // E and N are in reversed order in the public and private key. diff --git a/crypto/sshutil/shell.go b/crypto/sshutil/shell.go index c733dd67..a9766abd 100644 --- a/crypto/sshutil/shell.go +++ b/crypto/sshutil/shell.go @@ -5,7 +5,6 @@ import ( "log" "net" "os" - "os/exec" "path/filepath" "strings" @@ -69,69 +68,6 @@ func WithAddUser(user string, cert *ssh.Certificate, priv interface{}) ShellOpti } } -// WithBastion forward the connection through the given bastion address. -func WithBastion(user, address, command string) ShellOption { - return func(s *Shell) error { - s.dialer = func(callback ssh.HostKeyCallback) (*ssh.Client, error) { - // Connect to bastion - bastion, err := ssh.Dial("tcp", address, &ssh.ClientConfig{ - User: user, - Auth: s.authMethods, - HostKeyCallback: callback, - }) - if err != nil { - return nil, errors.Wrapf(err, "error connecting %s", address) - } - // Connect from bastion to final destination - conn, err := bastion.Dial("tcp", s.address) - if err != nil { - return nil, errors.Wrapf(err, "error connecting %s", s.address) - } - c, chans, reqs, err := ssh.NewClientConn(conn, s.address, &ssh.ClientConfig{ - User: s.user, - Auth: s.authMethods, - HostKeyCallback: callback, - }) - if err != nil { - return nil, err - } - return ssh.NewClient(c, chans, reqs), nil - } - return nil - } -} - -// WithProxyCommand forwards the connection through the given command -func WithProxyCommand(command string) ShellOption { - return func(s *Shell) error { - s.dialer = func(callback ssh.HostKeyCallback) (*ssh.Client, error) { - host, port, err := net.SplitHostPort(s.address) - if err != nil { - return nil, errors.Wrap(err, "error parsing address") - } - pr, pw := net.Pipe() - args := strings.Fields(ProxyCommand(command, s.user, host, port)) - cmd := exec.Command(args[0], args[1:]...) - cmd.Stdin = pw - cmd.Stdout = pw - cmd.Stderr = os.Stderr - if err := cmd.Start(); err != nil { - return nil, errors.Wrap(err, "error running proxy command") - } - c, chans, reqs, err := ssh.NewClientConn(pr, s.address, &ssh.ClientConfig{ - User: s.user, - Auth: s.authMethods, - HostKeyCallback: callback, - }) - if err != nil { - return nil, err - } - return ssh.NewClient(c, chans, reqs), nil - } - return nil - } -} - // withDefaultAuthMethod adds the ssh.Agent as an ssh.AuthMethod. func withDefaultAuthMethod() ShellOption { return func(s *Shell) error { diff --git a/crypto/tlsutil/options.go b/crypto/tlsutil/options.go deleted file mode 100644 index 002b8c32..00000000 --- a/crypto/tlsutil/options.go +++ /dev/null @@ -1,33 +0,0 @@ -package tlsutil - -import ( - "crypto/tls" - - "github.com/smallstep/cli/crypto/x509util" -) - -// TLSOptions represents the TLS options that can be specified on *tls.Config -// types to configure HTTPS servers and clients. -type TLSOptions struct { - CipherSuites x509util.CipherSuites `json:"cipherSuites" step:"cipherSuites"` - MinVersion x509util.TLSVersion `json:"minVersion" step:"minVersion"` - MaxVersion x509util.TLSVersion `json:"maxVersion" step:"maxVersion"` - Renegotiation bool `json:"renegotiation" step:"renegotiation"` -} - -// TLSConfig returns the tls.Config equivalent of the TLSOptions. -func (t *TLSOptions) TLSConfig() *tls.Config { - var rs tls.RenegotiationSupport - if t.Renegotiation { - rs = tls.RenegotiateFreelyAsClient - } else { - rs = tls.RenegotiateNever - } - - return &tls.Config{ - CipherSuites: t.CipherSuites.Value(), - MinVersion: t.MinVersion.Value(), - MaxVersion: t.MaxVersion.Value(), - Renegotiation: rs, - } -} diff --git a/crypto/x509util/crt.go b/crypto/x509util/crt.go index a587d631..09e110b4 100644 --- a/crypto/x509util/crt.go +++ b/crypto/x509util/crt.go @@ -1,6 +1,7 @@ package x509util import ( + // nolint:gosec // sha1 is being used to calculate an identifier, not a key. "crypto/sha1" "crypto/sha256" "crypto/x509" @@ -48,6 +49,7 @@ func EncodedFingerprint(cert *x509.Certificate, encoding FingerprintEncoding, if !insecure { return "", errors.New("sha1 requires '--insecure' flag") } + // nolint:gosec // sha1 is being used to calculate an identifier, not a key. sum := sha1.Sum(cert.Raw) return fingerprint.Fingerprint(sum[:], fingerprint.WithEncoding(fingerprint.Encoding(encoding))), nil } @@ -67,8 +69,8 @@ func SplitSANs(sans []string) (dnsNames []string, ips []net.IP, emails []string, return } for _, san := range sans { - // avoid ifelse -> switch statement linter suggestion - // nolint:gocritic + + // nolint:gocritic // avoid ifelse -> switch statement linter suggestion if ip := net.ParseIP(san); ip != nil { ips = append(ips, ip) } else if u, err := url.Parse(san); err == nil && u.Scheme != "" { diff --git a/crypto/x509util/profile.go b/crypto/x509util/profile.go index 76e7e105..827e62f6 100644 --- a/crypto/x509util/profile.go +++ b/crypto/x509util/profile.go @@ -4,7 +4,7 @@ import ( "crypto" "crypto/rand" "crypto/rsa" - "crypto/sha1" + "crypto/sha1" // nolint:gosec // sha1 is being used to calculate an identifier, not a key. "crypto/x509" "crypto/x509/pkix" "encoding/asn1" @@ -57,7 +57,7 @@ var ( // DefaultTLSMinVersion default minimum version of TLS. DefaultTLSMinVersion = TLSVersion(1.2) // DefaultTLSMaxVersion default maximum version of TLS. - DefaultTLSMaxVersion = TLSVersion(1.2) + DefaultTLSMaxVersion = TLSVersion(1.3) // DefaultTLSRenegotiation default TLS connection renegotiation policy. DefaultTLSRenegotiation = false // Never regnegotiate. // DefaultTLSCipherSuites specifies default step ciphersuite(s). @@ -67,18 +67,19 @@ var ( } // ApprovedTLSCipherSuites smallstep approved ciphersuites. ApprovedTLSCipherSuites = CipherSuites{ - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", + // AEADs w/ ECDHE "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305", + + // CBC w/ ECDHE + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", } // oidExtensionCTPoison is the OID for the certificate transparency poison @@ -519,6 +520,7 @@ func generateSubjectKeyID(pub crypto.PublicKey) ([]byte, error) { if _, err = asn1.Unmarshal(b, &info); err != nil { return nil, errors.Wrap(err, "error unmarshaling public key") } + // nolint:gosec // sha1 is being used to calculate an identifier, not a key. hash := sha1.Sum(info.SubjectPublicKey.Bytes) return hash[:], nil } diff --git a/exec/exec.go b/exec/exec.go index 6925e829..c42c9a78 100644 --- a/exec/exec.go +++ b/exec/exec.go @@ -127,6 +127,7 @@ func OpenInBrowser(url, browser string) error { // Step executes step with the given commands and returns the standard output. func Step(args ...string) ([]byte, error) { var stdout bytes.Buffer + // nolint:gosec // arguments controlled by step. cmd := exec.Command(os.Args[0], args...) cmd.Stdin = os.Stdin cmd.Stderr = os.Stderr diff --git a/internal/cryptoutil/cryptoutil.go b/internal/cryptoutil/cryptoutil.go index 34cf3fb7..2c02fc0d 100644 --- a/internal/cryptoutil/cryptoutil.go +++ b/internal/cryptoutil/cryptoutil.go @@ -87,6 +87,7 @@ func newKMSSigner(kms, key string) (crypto.Signer, error) { args = append(args, key) // Get public key + // nolint:gosec // arguments controlled by step. cmd := exec.Command(name, args...) out, err := cmd.Output() if err != nil { @@ -134,6 +135,7 @@ func (s *kmsSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) } args = append(args, s.key) + // nolint:gosec // arguments controlled by step. cmd := exec.Command(s.name, args...) stdin, err := cmd.StdinPipe() if err != nil { diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go index a3088d07..277f2fe3 100644 --- a/internal/plugin/plugin.go +++ b/internal/plugin/plugin.go @@ -25,6 +25,7 @@ func LookPath(name string) (string, error) { // it to complete. func Run(ctx *cli.Context, file string) error { args := ctx.Args() + // nolint:gosec // arguments controlled by step. cmd := exec.Command(file, args[1:]...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout diff --git a/jose/parse.go b/jose/parse.go index 4440485c..e28b71ec 100644 --- a/jose/parse.go +++ b/jose/parse.go @@ -160,6 +160,7 @@ func ParseKey(filename string, opts ...Option) (*JSONWebKey, error) { // ReadJWKSet reads a JWK Set from a URL or filename. URLs must start with "https://". func ReadJWKSet(filename string) ([]byte, error) { if strings.HasPrefix(filename, "https://") { + // nolint:gosec // This tainted URL is not considered a security risk. resp, err := http.Get(filename) if err != nil { return nil, errors.Wrapf(err, "error retrieving %s", filename) diff --git a/jose/validate.go b/jose/validate.go index 9a0f0bed..ff3eb4aa 100644 --- a/jose/validate.go +++ b/jose/validate.go @@ -4,7 +4,7 @@ import ( "crypto/ecdsa" "crypto/ed25519" "crypto/rsa" - "crypto/sha1" + "crypto/sha1" // nolint:gosec // sha1 is being used to calculate an identifier, not a key. "crypto/x509" "encoding/base64" "fmt" @@ -88,6 +88,7 @@ func ValidateX5T(certFile string, key interface{}) (string, error) { } // x5t is the base64 URL encoded SHA1 thumbprint // (see https://tools.ietf.org/html/rfc7515#section-4.1.7) + // nolint:gosec // sha1 is used to calculate an identifier, not a key. fingerprint := sha1.Sum(certs[0].Raw) return base64.URLEncoding.EncodeToString(fingerprint[:]), nil } diff --git a/make/common.mk b/make/common.mk index 18999304..c629d184 100644 --- a/make/common.mk +++ b/make/common.mk @@ -75,7 +75,7 @@ integration: bin/$(BINNAME) ######################################### fmt: - $Q gofmt -l -w $(SRC) + $Q goimports -l -w $(SRC) lint: $Q LOG_LEVEL=error golangci-lint run --timeout=30m diff --git a/usage/html.go b/usage/html.go index 1ede910f..f2d3b564 100644 --- a/usage/html.go +++ b/usage/html.go @@ -121,6 +121,7 @@ func htmlHelpAction(ctx *cli.Context) error { // css style cssFile := path.Join(dir, "style.css") + // nolint:gosec // Written file contains nothing sensitive. if err := os.WriteFile(cssFile, []byte(css), 0666); err != nil { return errs.FileError(err, cssFile) } @@ -219,11 +220,9 @@ func (h *htmlHelpHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { last := len(args) - 1 lastName := args[last] subcmd := ctx.App.Commands - parent := createParentCommand(ctx) for _, name := range args[:last] { for _, cmd := range subcmd { if cmd.HasName(name) { - parent = cmd subcmd = cmd.Subcommands break } @@ -235,7 +234,6 @@ func (h *htmlHelpHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { continue } cmd.HelpName = fmt.Sprintf("%s %s", ctx.App.HelpName, strings.Join(args, " ")) - parent.HelpName = fmt.Sprintf("%s %s", ctx.App.HelpName, strings.Join(args[:last], " ")) ctx.Command = cmd if len(cmd.Subcommands) == 0 { diff --git a/utils/cautils/acmeutils.go b/utils/cautils/acmeutils.go index 78532984..8d1e9acc 100644 --- a/utils/cautils/acmeutils.go +++ b/utils/cautils/acmeutils.go @@ -30,7 +30,10 @@ import ( ) func startHTTPServer(addr, token, keyAuth string) *http.Server { - srv := &http.Server{Addr: addr} + srv := &http.Server{ + Addr: addr, + ReadHeaderTimeout: 15 * time.Second, + } http.HandleFunc(fmt.Sprintf("/.well-known/acme-challenge/%s", token), func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/octet-stream") @@ -117,7 +120,7 @@ func (wm *webrootMode) Run() error { return errors.Wrapf(err, "error checking for directory %s", wm.dir) } - // Use 0755 and 0644 (rather than 0700 and 0600) for directory and file + // NOTE: Use 0755 and 0644 (rather than 0700 and 0600) for directory and file // respectively because the process running the file server, and therefore // reading/serving the file, may be owned by a different user than // the one running the `step` command that will write the file. @@ -128,6 +131,7 @@ func (wm *webrootMode) Run() error { } } + // nolint:gosec // See note above. return errors.Wrapf(os.WriteFile(fmt.Sprintf("%s/%s", chPath, wm.token), []byte(keyAuth), 0644), "error writing key authorization file %s", chPath+wm.token) } @@ -390,7 +394,12 @@ func (af *acmeFlow) getClientTruststoreOption(mergeRootCAs bool) (ca.ClientOptio return nil, errors.New("failed to append local root ca to system cert pool") } - return ca.WithTransport(&http.Transport{TLSClientConfig: &tls.Config{RootCAs: rootCAs}}), nil + return ca.WithTransport(&http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: rootCAs, + MinVersion: tls.VersionTLS12, + }, + }), nil } // Use local Root CA only diff --git a/utils/cautils/bootstrap.go b/utils/cautils/bootstrap.go index d9dfcfcb..0554c53f 100644 --- a/utils/cautils/bootstrap.go +++ b/utils/cautils/bootstrap.go @@ -219,6 +219,9 @@ func BootstrapTeamAuthority(ctx *cli.Context, team, teamAuthority string) error } // Using public PKI + // nolint:gosec // Variadic URL is considered safe here for the following reasons: + // 1) The input is from the command line, rather than a web form or publicly available API. + // 2) The command is expected to be used on a client, rather than a privileged backend host. resp, err := http.Get(apiEndpoint) if err != nil { return errors.Wrap(err, "error getting authority data")