Merge pull request #1850 from shashankram/security

Add security code checker using gosec linter and
annotate false positives.

.golangci.yml

@@ -8,6 +8,7 @@ linters:
     - gofmt
     - goimports
     - golint
+    - gosec
     - govet
     - misspell
     - unused

demo/cmd/bookstore/bookstore.go

@@ -109,7 +109,7 @@ func sellBook(w http.ResponseWriter, r *http.Request) {
 	// Slow down the responses artificially.
 	maxNoiseMilliseconds := 750
 	minNoiseMilliseconds := 150
-	intNoise := rand.Intn(maxNoiseMilliseconds-minNoiseMilliseconds) + minNoiseMilliseconds
+	intNoise := rand.Intn(maxNoiseMilliseconds-minNoiseMilliseconds) + minNoiseMilliseconds // #nosec G404
 	pretendToBeBusy := time.Duration(intNoise) * time.Millisecond
 	log.Info().Msgf("Sleeping %+v", pretendToBeBusy)
 	time.Sleep(pretendToBeBusy)

demo/cmd/common/books.go

@@ -147,7 +147,7 @@ func GetBooks(participantName string, meshExpectedResponseCode int, booksCount *
 
 			// Create random URLs to test egress
 			if fetchURL == httpPrefix || fetchURL == httpsPrefix {
-				index := rand.Intn(len(egressURLs))
+				index := rand.Intn(len(egressURLs)) // #nosec G404
 				fetchURL = fmt.Sprintf("%s%s", url, egressURLs[index])
 			}
 

go.sum

@@ -471,6 +471,7 @@ github.com/googleapis/gnostic v0.1.0/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTV
 github.com/googleapis/gnostic v0.2.0/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTVRp3pOg5EKY=
 github.com/googleapis/gnostic v0.3.1 h1:WeAefnSUHlBb0iJKwxFDZdbfGwkd7xRNuV+IpXMJhYk=
 github.com/googleapis/gnostic v0.3.1/go.mod h1:on+2t9HRStVgn95RSsFWFz+6Q0Snyqv1awfrALZdbtU=
+github.com/gookit/color v1.2.5 h1:s1gzb/fg3HhkSLKyWVUsZcVBUo+R1TwEYTmmxH8gGFg=
 github.com/gookit/color v1.2.5/go.mod h1:AhIE+pS6D4Ql0SQWbBeXPHw7gY0/sjHoA4s/n1KB7xg=
 github.com/gophercloud/gophercloud v0.1.0 h1:P/nh25+rzXouhytV2pUHBb65fnds26Ghl8/391+sT5o=
 github.com/gophercloud/gophercloud v0.1.0/go.mod h1:vxM41WHh5uqHVBMZHzuwNOHh8XEoIEcSTewFxm1c5g8=
@@ -837,6 +838,7 @@ github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIH
 github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0=
 github.com/sclevine/agouti v3.0.0+incompatible/go.mod h1:b4WX9W9L1sfQKXeJf1mUTLZKJ48R1S7H23Ji7oFO5Bw=
 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
+github.com/securego/gosec v0.0.0-20200401082031-e946c8c39989 h1:rq2/kILQnPtq5oL4+IAjgVOjh5e2yj2aaCYi7squEvI=
 github.com/securego/gosec/v2 v2.4.0 h1:ivAoWcY5DMs9n04Abc1VkqZBO0FL0h4ShTcVsC53lCE=
 github.com/securego/gosec/v2 v2.4.0/go.mod h1:0/Q4cjmlFDfDUj1+Fib61sc+U5IQb2w+Iv9/C3wPVko=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=

pkg/catalog/xds_certificates.go

@@ -67,8 +67,8 @@ func (mc *MeshCatalog) filterTrafficSplitServices(services []v1.Service) []v1.Se
 	// These are the services except ones that are a root of a TrafficSplit policy
 	var filteredServices []v1.Service
 
-	for _, svc := range services {
-		nsSvc := utils.K8sSvcToMeshSvc(&svc)
+	for i, svc := range services {
+		nsSvc := utils.K8sSvcToMeshSvc(&services[i])
 		if _, shouldSkip := excludeTheseServices[nsSvc]; shouldSkip {
 			continue
 		}

pkg/certificate/file.go

@@ -15,6 +15,7 @@ func LoadCertificateFromFile(caPEMFile string) (tresorPem.Certificate, error) {
 		return nil, errors.Wrap(errInvalidFileName, caPEMFile)
 	}
 
+	// #nosec G304
 	caPEM, err := ioutil.ReadFile(caPEMFile)
 	if err != nil {
 		log.Error().Err(err).Msgf("Error reading file: %+v", caPEMFile)
@@ -37,6 +38,7 @@ func LoadPrivateKeyFromFile(caKeyPEMFile string) (tresorPem.PrivateKey, error) {
 		return nil, errInvalidFileName
 	}
 
+	// #nosec G304
 	caKeyPEM, err := ioutil.ReadFile(caKeyPEMFile)
 	if err != nil {
 		log.Error().Err(err).Msgf("Error reading file: %+v", caKeyPEMFile)

pkg/certificate/rotor/rotor.go

@@ -72,7 +72,8 @@ func ShouldRotate(cert certificate.Certificater) bool {
 	// We want to renew earlier. How much earlier is defined in renewBeforeCertExpires.
 	// We add a few seconds noise to the early renew period so that certificates that may have been
 	// created at the same time are not renewed at the exact same time.
-	intNoise := rand.Intn(maxNoiseSeconds-minNoiseSeconds) + minNoiseSeconds
+
+	intNoise := rand.Intn(maxNoiseSeconds-minNoiseSeconds) + minNoiseSeconds /* #nosec G404 */
 	secondsNoise := time.Duration(intNoise) * time.Second
 	return time.Until(cert.GetExpiration()) <= (renewBeforeCertExpires + secondsNoise)
 }

pkg/cli/chart_source.go

@@ -22,7 +22,7 @@ func GetChartSource(path string) (string, error) {
 		return "", err
 	}
 	defer os.Remove(packagedPath)
-	packaged, err := ioutil.ReadFile(packagedPath)
+	packaged, err := ioutil.ReadFile(packagedPath) // #nosec G304
 	if err != nil {
 		return "", err
 	}

pkg/debugger/envoy.go

@@ -16,6 +16,8 @@ func (ds debugServer) getEnvoyConfig(pod *v1.Pod, cn certificate.CommonName, url
 
 	minPort := 16000
 	maxPort := 18000
+
+	// #nosec G404
 	portFwdRequest := portForward{
 		Pod:       pod,
 		LocalPort: rand.Intn(maxPort-minPort) + minPort,

pkg/health/health.go

@@ -43,6 +43,9 @@ func (httpProbe HTTPProbe) Probe() (int, error) {
 	client := &http.Client{}
 
 	if httpProbe.Protocol == ProtocolHTTPS {
+		// Certificate validation is to be skipped for HTTPS probes
+		// similar to how k8s api server handles HTTPS probes.
+		// #nosec G402
 		transport := &http.Transport{
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
 		}

pkg/injector/webhook.go

@@ -98,6 +98,7 @@ func (wh *webhook) run(stop <-chan struct{}) {
 			return
 		}
 
+		// #nosec G402
 		server.TLSConfig = &tls.Config{
 			Certificates: []tls.Certificate{cert},
 		}

pkg/utils/mtls.go

@@ -28,6 +28,7 @@ func setupMutualTLS(insecure bool, serverName string, certPem []byte, keyPem []b
 		return nil, errors.Errorf("[grpc][mTLS][%s] Failed to append client certs", serverName)
 	}
 
+	// #nosec G402
 	tlsConfig := tls.Config{
 		InsecureSkipVerify: insecure,
 		ServerName:         serverName,

tests/e2e/common.go

@@ -542,7 +542,7 @@ func (td *OsmTestData) WaitForNamespacesDeleted(namespaces []string, timeout tim
 
 // RunLocal Executes command on local
 func (td *OsmTestData) RunLocal(path string, args []string) (*bytes.Buffer, *bytes.Buffer, error) {
-	cmd := exec.Command(path, args...)
+	cmd := exec.Command(path, args...) // #nosec G204
 	stdout := bytes.NewBuffer(nil)
 	stderr := bytes.NewBuffer(nil)
 	cmd.Stdout = stdout