Add security checks for go

- add security checks and github actions - correct complains Signed-off-by: Manuel Bluhm <[email protected]>
tiiuae · Aug 30, 2024 · 0f86ab5 · 0f86ab5
1 parent 4e5a6d9
commit 0f86ab5
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 20 deletions.
diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
@@ -1,12 +1,14 @@
 # SPDX-FileCopyrightText: 2022-2024 TII (SSRC) and the Ghaf contributors
-#
 # SPDX-License-Identifier: Apache-2.0
 
 name: check
 on:
   pull_request:
     branches:
       - main
+  push:
+    branches:
+      - main
 jobs:
   run-checks:
     runs-on: ubuntu-latest
@@ -20,5 +22,4 @@ jobs:
       - name: Check nix flake show runs successfully
         run: nix flake show
       - name: Run nix flake check
-        run: nix flake check
-
+        run: nix flake check
diff --git a/.github/workflows/go-sectest.yml b/.github/workflows/go-sectest.yml
@@ -0,0 +1,29 @@
+# SPDX-FileCopyrightText: 2022-2024 TII (SSRC) and the Ghaf contributors
+# SPDX-License-Identifier: Apache-2.0
+
+name: go-sectest
+on:
+    push:
+        paths:
+        - 'internal/**'
+        - 'api/**'
+    pull_request:
+        paths:
+        - 'internal/**'
+        - 'api/**'
+jobs:
+  tests:
+    runs-on: ubuntu-latest
+    env:
+      GO111MODULE: on
+    steps:
+      - name: Checkout Source
+        uses: actions/checkout@v3
+      - name: Run Gosec Security Scanner
+        uses: securego/gosec@master
+        with:
+          args: '-no-fail -fmt sarif -out results.sarif ./...'
+      - name: Upload SARIF file
+        uses: github/codeql-action/upload-sarif@v2
+        with:
+          sarif_file: results.sarif
diff --git a/internal/cmd/givc-agent/main.go b/internal/cmd/givc-agent/main.go
@@ -198,7 +198,10 @@ func main() {
 					},
 				}
 				log.Infof("Trying to register service: %s", service)
-				serviceclient.RegisterRemoteService(cfgAdminServer, serviceEntryRequest)
+				_, err := serviceclient.RegisterRemoteService(cfgAdminServer, serviceEntryRequest)
+				if err != nil {
+					log.Warnf("Error registering service: %s", err)
+				}
 			}
 		}
 	}()

diff --git a/internal/pkgs/utility/tls.go b/internal/pkgs/utility/tls.go
@@ -6,6 +6,7 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"os"
+	"path/filepath"
 
 	log "github.com/sirupsen/logrus"
 )
@@ -21,12 +22,12 @@ var (
 func TlsServerConfig(CACertFilePath string, CertFilePath string, KeyFilePath string, mutual bool) *tls.Config {
 
 	// Load TLS certificates and key
-	serverTLSCert, err := tls.LoadX509KeyPair(CertFilePath, KeyFilePath)
+	serverTLSCert, err := tls.LoadX509KeyPair(filepath.Clean(CertFilePath), filepath.Clean(KeyFilePath))
 	if err != nil {
 		log.Fatalf("[TlsServerConfig] Error loading server certificate and key file: %v", err)
 	}
 	certPool := x509.NewCertPool()
-	caCertPEM, err := os.ReadFile(CACertFilePath)
+	caCertPEM, err := os.ReadFile(filepath.Clean(CACertFilePath))
 	if err != nil {
 		log.Fatalf("[TlsServerConfig] Error loading CA certificate: %v", err)
 	}
@@ -63,7 +64,7 @@ func TlsClientConfig(CACertFilePath string, CertFilePath string, KeyFilePath str
 		log.Fatalf("[TlsClientConfig] Error loading client certificate and key file: %v", err)
 	}
 	certPool := x509.NewCertPool()
-	caCertPEM, err := os.ReadFile(CACertFilePath)
+	caCertPEM, err := os.ReadFile(filepath.Clean(CACertFilePath))
 	if err != nil {
 		log.Fatalf("[TlsClientConfig] Error loading CA certificate: %v", err)
 	}

diff --git a/internal/pkgs/utility/utility.go b/internal/pkgs/utility/utility.go
@@ -86,7 +86,7 @@ func GetCGroupPathForProcess(pid uint32) (string, error) {
 	cgroupFilePath := fmt.Sprintf("/proc/%d/cgroup", pid)
 
 	// Open the cgroup file
-	file, err := os.Open(cgroupFilePath)
+	file, err := os.Open(filepath.Clean(cgroupFilePath))
 	if err != nil {
 		return "", err
 	}

diff --git a/internal/pkgs/wifimanager/controller.go b/internal/pkgs/wifimanager/controller.go
@@ -110,7 +110,10 @@ func NewController() (*WifiController, error) {
 }
 
 func (c *WifiController) Close() {
-	c.conn.Close()
+	err := c.conn.Close()
+	if err != nil {
+		log.Warnf("[WifiController] failed to close connection: %s", err)
+	}
 }
 
 func (c *WifiController) GetNetworkList(ctx context.Context, NetworkInterface string) ([]*wifi_api.AccessPoint, error) {
@@ -167,8 +170,10 @@ func (c *WifiController) GetActiveConnection(ctx context.Context) (bool, string,
 		if err != nil {
 			return false, "", 0, "", fmt.Errorf("failed to get active access point path: %s", err)
 		}
-		activeAPPath.Store(&ap)
-
+		err = activeAPPath.Store(&ap)
+		if err != nil {
+			return false, "", 0, "", fmt.Errorf("failed to store active access point path: %s", err)
+		}
 		// No active connection
 		if ap == "/" {
 			return false, "", 0, "", nil
@@ -203,6 +208,10 @@ func (c *WifiController) Connect(ctx context.Context, SSID string, Password stri
 		if err != nil {
 			return "", fmt.Errorf("failed to get access points: %s", err)
 		}
+		if len(apPaths) < 1 {
+			continue
+		}
+
 		// Iterate over access points and append into output
 		for _, apPath := range apPaths {
 			apObject := c.conn.Object("org.freedesktop.NetworkManager", apPath)
@@ -250,7 +259,10 @@ func (c *WifiController) Connect(ctx context.Context, SSID string, Password stri
 		}
 
 		if keymgmt == NmAPSecConWPAEAP {
-			settings = MergeSettings(settings, extendSettings)
+			settings, err = MergeSettings(settings, extendSettings)
+			if err != nil {
+				return "", fmt.Errorf("failed to merge settings %s: %s", extendSettings, err)
+			}
 		}
 
 		// Add a new connection and connect
@@ -437,42 +449,61 @@ func GetAPData(ap dbus.BusObject) (AP, error) {
 	if err != nil {
 		return accesspoint, fmt.Errorf("failed to get SSID: %s", err)
 	}
-	ssid_variant.Store(&ssid)
+	err = ssid_variant.Store(&ssid)
+	if err != nil {
+		return accesspoint, fmt.Errorf("failed to store SSID: %s", err)
+	}
 	accesspoint.SSID = string(ssid)
 
 	strength_variant, err := ap.GetProperty("org.freedesktop.NetworkManager.AccessPoint.Strength")
 	if err != nil {
 		return accesspoint, fmt.Errorf("failed to get Strength: %s", err)
 	}
-	strength_variant.Store(&(accesspoint.Strength))
+	err = strength_variant.Store(&(accesspoint.Strength))
+	if err != nil {
+		return accesspoint, fmt.Errorf("failed to store Strength: %s", err)
+	}
 
 	flags_variant, err := ap.GetProperty("org.freedesktop.NetworkManager.AccessPoint.Flags")
 	if err != nil {
 		return accesspoint, fmt.Errorf("failed to get WPA flags: %s", err)
 	}
-	flags_variant.Store(&PrivacyFlag)
+	err = flags_variant.Store(&PrivacyFlag)
+	if err != nil {
+		return accesspoint, fmt.Errorf("failed to store flags: %s", err)
+	}
 	accesspoint.PrivacyFlag = PrivacyFlag != 0
 
 	wpaFlags_variant, err := ap.GetProperty("org.freedesktop.NetworkManager.AccessPoint.WpaFlags")
 	if err != nil {
 		return accesspoint, fmt.Errorf("failed to get WPA flags: %s", err)
 	}
-	wpaFlags_variant.Store(&(accesspoint.WPAFlags))
+	err = wpaFlags_variant.Store(&(accesspoint.WPAFlags))
+	if err != nil {
+		return accesspoint, fmt.Errorf("failed to store WPAFlags: %s", err)
+	}
 
 	rsnFlags_variant, err := ap.GetProperty("org.freedesktop.NetworkManager.AccessPoint.RsnFlags")
 	if err != nil {
 		return accesspoint, fmt.Errorf("failed to get RSN flags: %s", err)
 	}
-	rsnFlags_variant.Store(&(accesspoint.RSNFlags))
+	err = rsnFlags_variant.Store(&(accesspoint.RSNFlags))
+	if err != nil {
+		return accesspoint, fmt.Errorf("failed to store RSNFlags: %s", err)
+	}
 
 	return accesspoint, nil
 }
 
-func MergeSettings(baseSettings map[string]map[string]dbus.Variant, rawExtensionSettings string) map[string]map[string]dbus.Variant {
+func MergeSettings(baseSettings map[string]map[string]dbus.Variant, rawExtensionSettings string) (map[string]map[string]dbus.Variant, error) {
 	var settings map[string]any
 
 	// Parse the raw settings extension string
-	json.Unmarshal([]byte(rawExtensionSettings), &settings)
+	err := json.Unmarshal([]byte(rawExtensionSettings), &settings)
+	if err != nil {
+		log.Warnf("[WifiController] failed to parse extension settings: %s", err)
+		return nil, err
+	}
 
 	// Merge the two settings maps
 	for setting, keys := range settings {
@@ -486,5 +517,5 @@ func MergeSettings(baseSettings map[string]map[string]dbus.Variant, rawExtension
 			baseSettings[setting][key] = dbus.MakeVariant(value)
 		}
 	}
-	return baseSettings
+	return baseSettings, nil
 }