Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal: add gosec to the linter checks #2526

Merged
merged 1 commit into from
May 15, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -3,11 +3,12 @@ run:

linters:
enable:
- bodyclose
- goimports
- gosec
- misspell
- unparam
- unconvert
- bodyclose
- unparam

# TODO(jpeach): enable these later:
# - gocyclo
2 changes: 1 addition & 1 deletion cmd/contour/servecontext_test.go
Original file line number Diff line number Diff line change
@@ -332,7 +332,7 @@ func tryConnect(address string, clientCredentialsDir string) (*x509.Certificate,
clientConfig := &tls.Config{
ServerName: "localhost",
Certificates: []tls.Certificate{cert},
InsecureSkipVerify: true,
InsecureSkipVerify: true, // nolint:gosec
}
conn, err := tls.Dial("tcp", address, clientConfig)
if err != nil {
20 changes: 10 additions & 10 deletions cmd/contour/shutdownmanager.go
Original file line number Diff line number Diff line change
@@ -79,7 +79,7 @@ func (s *shutdownmanagerContext) healthzHandler(w http.ResponseWriter, r *http.R
func (s *shutdownmanagerContext) shutdownHandler(w http.ResponseWriter, r *http.Request) {
// Send shutdown signal to Envoy to start draining connections
s.Infof("failing envoy healthchecks")
err := shutdownEnvoy(healthcheckFailURL)
err := shutdownEnvoy()
if err != nil {
s.Errorf("error sending envoy healthcheck fail: %v", err)
}
@@ -88,7 +88,7 @@ func (s *shutdownmanagerContext) shutdownHandler(w http.ResponseWriter, r *http.
time.Sleep(s.checkDelay)

for {
openConnections, err := getOpenConnections(prometheusURL)
openConnections, err := getOpenConnections()
if err != nil {
s.Error(err)
} else {
@@ -107,29 +107,29 @@ func (s *shutdownmanagerContext) shutdownHandler(w http.ResponseWriter, r *http.
}

// shutdownEnvoy sends a POST request to /healthcheck/fail to tell Envoy to start draining connections
func shutdownEnvoy(url string) error {
resp, err := http.Post(url, "", nil)
func shutdownEnvoy() error {
resp, err := http.Post(healthcheckFailURL, "", nil)
if err != nil {
return fmt.Errorf("creating healthcheck fail post request failed: %s", err)
return fmt.Errorf("creating healthcheck fail POST request failed: %s", err)
}

defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("post request for url %q returned http status %s", url, resp.Status)
return fmt.Errorf("POST for %q returned HTTP status %s", healthcheckFailURL, resp.Status)
}
return nil
}

// getOpenConnections parses a http request to a prometheus endpoint returning the sum of values found
func getOpenConnections(url string) (int, error) {
func getOpenConnections() (int, error) {
// Make request to Envoy Prometheus endpoint
resp, err := http.Get(url)
resp, err := http.Get(prometheusURL)
if err != nil {
return -1, fmt.Errorf("get request for metrics failed: %s", err)
return -1, fmt.Errorf("creating metrics GET request failed: %s", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return -1, fmt.Errorf("get request for metrics failed with http status %s", resp.Status)
return -1, fmt.Errorf("GET for %q returned HTTP status %s", prometheusURL, resp.Status)
}

// Parse Prometheus listener stats for open connections
2 changes: 1 addition & 1 deletion hack/golangci-lint
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ if command -v docker >/dev/null; then
--rm \
--volume $(pwd):/app \
--workdir /app \
golangci/golangci-lint:v1.23.8 ${PROGNAME} "$@"
golangci/golangci-lint:v1.27.0 ${PROGNAME} "$@"
fi

cat <<EOF
12 changes: 10 additions & 2 deletions internal/certgen/makecerts.go
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ package certgen
import (
"crypto/rand"
"crypto/rsa"
"crypto/sha1"
"crypto/sha1" // nolint:gosec
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
@@ -129,8 +129,16 @@ func newSerial(now time.Time) *big.Int {
return big.NewInt(int64(now.Nanosecond()))
}

// bigIntHash generates a SubjectKeyId by hashing the modulus of the private
// key. This isn't one of the methods listed in RFC 5280 4.2.1.2, but that also
// notes that other methods are acceptable.
//
// gosec makes a blanket claim that SHA-1 is unacceptable, which is
// false here. The core Go method of generations the SubjectKeyId (see
// https://github.com/golang/go/issues/26676) also uses SHA-1, as recommended
// by RFC 5280.
func bigIntHash(n *big.Int) []byte {
h := sha1.New()
h := sha1.New() // nolint:gosec
h.Write(n.Bytes()) // nolint:errcheck
return h.Sum(nil)
}
6 changes: 4 additions & 2 deletions internal/envoy/cluster.go
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@
package envoy

import (
"crypto/sha1"
"crypto/sha1" // nolint:gosec
"crypto/sha256"
"fmt"
"strconv"
@@ -182,7 +182,9 @@ func Clustername(cluster *dag.Cluster) string {
buf += uv.SubjectName
}

hash := sha1.Sum([]byte(buf))
// This isn't a crypto hash, we just want a unique name.
hash := sha1.Sum([]byte(buf)) // nolint:gosec

ns := service.Namespace
name := service.Name
return hashname(60, ns, name, strconv.Itoa(int(service.Port)), fmt.Sprintf("%x", hash[:5]))
5 changes: 3 additions & 2 deletions internal/envoy/secret.go
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@
package envoy

import (
"crypto/sha1"
"crypto/sha1" // nolint:gosec
"fmt"

envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth"
@@ -24,7 +24,8 @@ import (

// Secretname returns the name of the SDS secret for this secret.
func Secretname(s *dag.Secret) string {
hash := sha1.Sum(s.Cert())
// This isn't a crypto hash, we just want a unique name.
hash := sha1.Sum(s.Cert()) // nolint:gosec
ns := s.Namespace()
name := s.Name()
return hashname(60, ns, name, fmt.Sprintf("%x", hash[:5]))
4 changes: 2 additions & 2 deletions internal/workgroup/example_test.go
Original file line number Diff line number Diff line change
@@ -83,7 +83,7 @@ func ExampleGroup_Run_multipleListeners() {

// listen on port 80
g.Add(func(stop <-chan struct{}) error {
l, err := net.Listen("tcp", ":80")
l, err := net.Listen("tcp", ":80") // nolint:gosec
if err != nil {
return err
}
@@ -97,7 +97,7 @@ func ExampleGroup_Run_multipleListeners() {

// listen on port 443
g.Add(func(stop <-chan struct{}) error {
l, err := net.Listen("tcp", ":443")
l, err := net.Listen("tcp", ":443") // nolint:gosec
if err != nil {
return err
}