Introduce gosec for Static Application Security Testing (SAST)

.gitignore

@@ -1,4 +1,5 @@
 /bin
+hack/tools/bin
 /.kube-secrets
 /tmp/*
 /dev
@@ -11,3 +12,6 @@
 
 ./terraformer
 TODO
+
+# gosec
+gosec-report.sarif

Makefile

@@ -5,6 +5,7 @@
 GARDENER_HACK_DIR    		  := $(shell go list -m -f "{{.Dir}}" github.com/gardener/gardener)/hack
 VERSION                       := $(shell cat VERSION)
 REPO_ROOT                     := $(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))
+HACK_DIR                      := $(REPO_ROOT)/hack
 REGISTRY                      := europe-docker.pkg.dev/gardener-project/public/gardener
 VPN_SERVER_IMAGE_REPOSITORY  := $(REGISTRY)/vpn-server
 VPN_SERVER_IMAGE_TAG         := $(VERSION)
@@ -20,11 +21,15 @@ ARCH                ?= amd64
 
 PATH                          := $(GOBIN):$(PATH)
 
+TOOLS_DIR := $(HACK_DIR)/tools
+include $(GARDENER_HACK_DIR)/tools.mk
+
 export PATH
 
 .PHONY: tidy
 tidy:
 	@GO111MODULE=on go mod tidy
+	@mkdir -p $(HACK_DIR) && cp $(GARDENER_HACK_DIR)/sast.sh $(HACK_DIR)/sast.sh && chmod +xw $(HACK_DIR)/sast.sh
 
 .PHONY: vpn-server-docker-image
 vpn-server-docker-image:
@@ -67,10 +72,24 @@ docker-push:
 	@gcloud docker -- push $(VPN_CLIENT_IMAGE_REPOSITORY):$(VPN_CLIENT_IMAGE_TAG)
 
 .PHONY: check
-check:
+check: sast-report
 	go fmt ./...
 	go vet ./...
 
+# TODO(scheererj): Remove once https://github.com/gardener/gardener/pull/10642 is available as release.
+TOOLS_PKG_PATH := $(shell go list -tags tools -f '{{ .Dir }}' github.com/gardener/gardener/hack/tools 2>/dev/null)
+.PHONY: adjust-install-gosec.sh
+adjust-install-gosec.sh:
+	@chmod +xw $(TOOLS_PKG_PATH)/install-gosec.sh
+
+.PHONY: sast
+sast: adjust-install-gosec.sh $(GOSEC)
+	@./hack/sast.sh
+
+.PHONY: sast-report
+sast-report: adjust-install-gosec.sh $(GOSEC)
+	@./hack/sast.sh --gosec-report true
+
 .PHONY: test
 test:
 	go test ./...

cmd/vpn_server/app/firewall.go

@@ -50,7 +50,9 @@ func firewallCommand() *cobra.Command {
 func runFirewallCommand(log logr.Logger, device, mode string, networks []string) error {
 	// Firewall subcommand is called indirectly from openvpn. As PATH env variables seems not to be set,
 	// it is injected here.
-	os.Setenv("PATH", "/sbin")
+	if err := os.Setenv("PATH", "/sbin"); err != nil {
+		return fmt.Errorf("setting PATH environment variable failed: %w", err)
+	}
 	iptable4, err := network.NewIPTables(log, iptables.ProtocolIPv4)
 	if err != nil {
 		return err

go.sum

@@ -1,3 +1,4 @@
+github.com/ahmetb/gen-crd-api-reference-docs v0.3.0/go.mod h1:TdjdkYhlOifCQWPs1UdTma97kQQMozf5h26hTuG70u8=
 github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so=
 github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
@@ -182,6 +183,7 @@ go.mongodb.org/mongo-driver v1.13.1 h1:YIc7HTYsKndGK4RFzJ3covLz1byri52x0IoMB0Pt/
 go.mongodb.org/mongo-driver v1.13.1/go.mod h1:wcDf1JBCXy2mOW0bWHwO/IOYqdca1MPCwDtFu/Z9+eo=
 go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
 go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
+go.uber.org/mock v0.4.0/go.mod h1:a6FSlNadKUHUa9IP5Vyt1zh4fC7uAwxMutEAscFbkZc=
 go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
 go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
 go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8=
@@ -256,6 +258,7 @@ k8s.io/apimachinery v0.29.8 h1:uBHc9WuKiTHClIspJqtR84WNpG0aOGn45HWqxgXkk8Y=
 k8s.io/apimachinery v0.29.8/go.mod h1:i3FJVwhvSp/6n8Fl4K97PJEP8C+MM+aoDq4+ZJBf70Y=
 k8s.io/client-go v0.29.8 h1:QMRKcIzqE/qawknXcsi51GdIAYN8UP39S/M5KnFu/J0=
 k8s.io/client-go v0.29.8/go.mod h1:ZzrAAVrqO2jVXMb8My/jTke8n0a/mIynnA3y/1y1UB0=
+k8s.io/code-generator v0.29.7/go.mod h1:7TYnI0dYItL2cKuhhgPSuF3WED9uMdELgbVXFfn/joE=
 k8s.io/component-base v0.29.8 h1:4LJ94/eOJpDFZFbGbRH4CEyk29a7PZr8noVe9tBJUUY=
 k8s.io/component-base v0.29.8/go.mod h1:FYOQSsKgh9/+FNleq8m6cXH2Cq8fNiUnJzDROowLaqU=
 k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw=
@@ -266,6 +269,7 @@ k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 h1:pUdcCO1Lk/tbT5ztQWOBi5HBgbBP1
 k8s.io/utils v0.0.0-20240711033017-18e509b52bc8/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 sigs.k8s.io/controller-runtime v0.17.6 h1:12IXsozEsIXWAMRpgRlYS1jjAHQXHtWEOMdULh3DbEw=
 sigs.k8s.io/controller-runtime v0.17.6/go.mod h1:N0jpP5Lo7lMTF9aL56Z/B2oWBJjey6StQM0jRbKQXtY=
+sigs.k8s.io/controller-tools v0.14.0/go.mod h1:TV7uOtNNnnR72SpzhStvPkoS/U5ir0nMudrkrC4M9Sc=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=

hack/sast.sh

@@ -0,0 +1,44 @@
+#!/usr/bin/env bash
+#
+# SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+set -e
+
+root_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." &> /dev/null && pwd )"
+
+gosec_report="false"
+gosec_report_parse_flags=""
+
+parse_flags() {
+  while test $# -gt 1; do
+    case "$1" in
+      --gosec-report)
+        shift; gosec_report="$1"
+        ;;
+      *)
+        echo "Unknown argument: $1"
+        exit 1
+        ;;
+    esac
+    shift
+  done
+}
+
+parse_flags "$@"
+
+echo "> Running gosec"
+gosec --version
+if [[ "$gosec_report" != "false" ]]; then
+  echo "Exporting report to $root_dir/gosec-report.sarif"
+  gosec_report_parse_flags="-track-suppressions -fmt=sarif -out=gosec-report.sarif -stdout"
+fi
+
+# Gardener uses code-generators https://github.com/kubernetes/code-generator and https://github.com/protocolbuffers/protobuf
+# which create lots of G103 (CWE-242: Use of unsafe calls should be audited) & G104 (CWE-703: Errors unhandled) errors.
+# However, those generators are best-pratice in Kubernetes environment and their results are tested well.
+# Thus, generated code is excluded from gosec scan.
+# Nested go modules are not supported by gosec (see https://github.com/securego/gosec/issues/501), so the ./hack folder
+# is excluded too. It does not contain productive code anyway.
+gosec -exclude-generated -exclude-dir=hack $gosec_report_parse_flags ./...

pkg/ippool/broker.go

@@ -95,7 +95,7 @@ func (b *ipAddressBroker) announceIPAddress(ctx context.Context, used bool, look
 
 func (b *ipAddressBroker) findFreeIPAddress(lookupResult *IPPoolUsageLookupResult) string {
 	for i := 0; i < 1000; i++ {
-		index := rand.N(b.endIndex-b.startIndex+1) + b.startIndex
+		index := rand.N(b.endIndex-b.startIndex+1) + b.startIndex // #nosec: G404 -- No cryptographic context.
 		ip := make(net.IP, len(b.base))
 		copy(ip, b.base)
 		ip[len(ip)-1] = byte(index)
@@ -145,7 +145,7 @@ func (b *ipAddressBroker) AcquireIP(ctx context.Context) (string, error) {
 			break
 		}
 		b.log("conflict, retrying...")
-		time.Sleep(b.waitTime * time.Duration(rand.N(10)/10))
+		time.Sleep(b.waitTime * time.Duration(rand.N(10)/10)) // #nosec: G404 -- No cryptographic context.
 	}
 
 	if b.hasConflict(result) {

pkg/network/iptables.go

@@ -35,6 +35,5 @@ func adjustPath(path string, proto iptables.Protocol) string {
 
 func iptablesWorks(path string) bool {
 	// check both iptables and ip6tables
-	return exec.Command(path, "-L").Run() == nil &&
-		exec.Command(adjustPath(path, iptables.ProtocolIPv6), "-L").Run() == nil
+	return exec.Command(path, "-L").Run() == nil && exec.Command(adjustPath(path, iptables.ProtocolIPv6), "-L").Run() == nil // #nosec: G204 -- Command line is completely static "/sbin/(iptables|ip6tables)-(legacy|nft) -L".
 }

pkg/openvpn/config.go

@@ -13,6 +13,7 @@ import (
 )
 
 const defaultOpenVPNConfigFile = "/openvpn.config"
+const defaultConfigFilePermissions = 0o600
 
 func executeTemplate(name string, w io.Writer, templt string, data any) error {
 	cidrMaskFunc :=

pkg/openvpn/config_client.go

@@ -40,5 +40,5 @@ func WriteClientConfigFile(v ClientValues) error {
 	if err != nil {
 		return fmt.Errorf("error %w: Could not generate openvpn config from %v", err, v)
 	}
-	return os.WriteFile(defaultOpenVPNConfigFile, []byte(openvpnConfig), 0o644)
+	return os.WriteFile(defaultOpenVPNConfigFile, []byte(openvpnConfig), defaultConfigFilePermissions)
 }

pkg/openvpn/config_server.go

@@ -60,7 +60,7 @@ func WriteServerConfigFiles(v SeedServerValues) error {
 	if err != nil {
 		return fmt.Errorf("error %w: Could not generate openvpn config from %v", err, v)
 	}
-	if err := os.WriteFile(defaultOpenVPNConfigFile, []byte(openvpnConfig), 0o644); err != nil {
+	if err := os.WriteFile(defaultOpenVPNConfigFile, []byte(openvpnConfig), defaultConfigFilePermissions); err != nil {
 		return err
 	}
 
@@ -72,13 +72,13 @@ func WriteServerConfigFiles(v SeedServerValues) error {
 	if err != nil && !os.IsExist(err) {
 		return err
 	}
-	if err := os.WriteFile(path.Join(openvpnClientConfigDir, openvpnClientConfigPrefix), []byte(vpnShootClientConfig), 0o644); err != nil {
+	if err := os.WriteFile(path.Join(openvpnClientConfigDir, openvpnClientConfigPrefix), []byte(vpnShootClientConfig), defaultConfigFilePermissions); err != nil {
 		return err
 	}
 
 	if v.IsHA {
 		for i := 0; i < v.HAVPNClients; i++ {
-			if err := os.WriteFile(fmt.Sprintf("%s-%d", path.Join(openvpnClientConfigDir, openvpnClientConfigPrefix), i), []byte(""), 0o644); err != nil {
+			if err := os.WriteFile(fmt.Sprintf("%s-%d", path.Join(openvpnClientConfigDir, openvpnClientConfigPrefix), i), []byte(""), defaultConfigFilePermissions); err != nil {
 				return err
 			}
 		}

pkg/openvpn/exporter/exporter.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"net/http"
 	"strings"
+	"time"
 
 	"github.com/go-logr/logr"
 	"github.com/kumina/openvpn_exporter/exporters"
@@ -52,9 +53,11 @@ func Start(log logr.Logger, cfg Config) error {
 		return err
 	}
 
-	http.Handle(cfg.MetricsPath, promhttp.Handler())
-	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
-		w.Write([]byte(`
+	// Use non-default mux to avoid profiling being automatically enabled
+	handler := http.NewServeMux()
+	handler.Handle(cfg.MetricsPath, promhttp.Handler())
+	handler.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte(`
 			<html>
 			<head><title>OpenVPN Exporter</title></head>
 			<body>
@@ -64,5 +67,10 @@ func Start(log logr.Logger, cfg Config) error {
 			</html>`))
 	})
 
-	return http.ListenAndServe(cfg.ListenAddress, nil)
+	return (&http.Server{
+		Addr:         cfg.ListenAddress,
+		Handler:      handler,
+		ReadTimeout:  10 * time.Second,
+		WriteTimeout: 10 * time.Second,
+	}).ListenAndServe()
 }

pkg/pprof/pprof.go

@@ -8,15 +8,18 @@ import (
 	"context"
 	"errors"
 	"net/http"
-	_ "net/http/pprof"
+	_ "net/http/pprof" // #nosec: G108 -- default http mux is only used when profiling is enable, only other server (exporter) uses separate http mux.
+	"time"
 
 	"github.com/go-logr/logr"
 )
 
 // Serve starts a new http server serving pprof endpoints on :6060
 func Serve(ctx context.Context, log logr.Logger) {
 	server := &http.Server{
-		Addr: ":6060",
+		Addr:              ":6060",
+		ReadHeaderTimeout: 10 * time.Second,
+		WriteTimeout:      10 * time.Second,
 	}
 	go func() {
 		log.Info("serving on :6060")

pkg/vpn_client/bonding.go

@@ -52,7 +52,7 @@ func ConfigureBonding(ctx context.Context, log logr.Logger, cfg *config.VPNClien
 			return err
 		}
 
-		cmd := exec.CommandContext(ctx, "openvpn", "--mktun", "--dev", linkName)
+		cmd := exec.CommandContext(ctx, "openvpn", "--mktun", "--dev", linkName) // #nosec: G204 -- linkName is fairly static (see above) pointing to tap0/tap1.
 		err = cmd.Run()
 		if err != nil {
 			return err