Run livenessprobe as non-root + Run all possible sidecars as nonRootGroup (#65)

* Create CSI socket file using non-root user

Signed-off-by: Prankul <[email protected]>

---------

Signed-off-by: Prankul <[email protected]>

Author: prankulmahajan

deploy/kubernetes/driver/kubernetes/manifests/config-map.yaml

@@ -16,4 +16,4 @@ data:
   VPC_RETRY_INTERVAL: "60" # This is max retry Gap in seconds even considering for exponential retry
   VPC_API_VERSION: "2021-04-20"
   VPC_API_GENERATION: "1"
-  IKS_ENABLED: "True"
+  IKS_ENABLED: "True"

deploy/kubernetes/driver/kubernetes/manifests/controller-server.yaml

@@ -25,6 +25,7 @@ spec:
       securityContext:
         runAsNonRoot: true
         runAsUser: 2121
+        runAsGroup: 2121
       containers:
         - name: csi-provisioner
           image: MUSTPATCHWITHKUSTOMIZE
@@ -187,6 +188,13 @@ spec:
                 configMapKeyRef:
                   name: ibm-vpc-file-csi-configmap
                   key: SIDECAR_ENDPOINT
+          resources:
+            limits:
+              cpu: 12m
+              memory: 20Mi
+            requests:
+              cpu: 3m
+              memory: 5Mi
           volumeMounts:
             - mountPath: /sidecardir
               name: socket-dir

deploy/kubernetes/driver/kubernetes/manifests/node-server.yaml

@@ -31,6 +31,7 @@ spec:
           securityContext:
             runAsNonRoot: false
             runAsUser: 0
+            runAsGroup: 0
             privileged: false
           args:
             - "--v=5"
@@ -64,6 +65,7 @@ spec:
           securityContext:
             runAsNonRoot: false
             runAsUser: 0
+            runAsGroup: 0
             privileged: true
           image: MUSTPATCHWITHKUSTOMIZE
           imagePullPolicy: Always
@@ -91,6 +93,10 @@ spec:
                   fieldPath: spec.nodeName
             - name: SOCKET_PATH
               value: "/var/lib/ibmshare.sock"
+            - name: IS_NODE_SERVER
+              value: "true"
+            - name: SIDECAR_GROUP_ID
+              value: "2121"
           resources:
             limits:
               cpu: 200m
@@ -139,11 +145,15 @@ spec:
             - mountPath: /tmp/mount-helper
               name: mh-logs
         - name: liveness-probe
-          image: MUSTPATCHWITHKUSTOMIZE
           securityContext:
-            runAsNonRoot: false
-            runAsUser: 0
+            runAsNonRoot: true
+            runAsUser: 2121
+            runAsGroup: 2121
             privileged: false
+            seLinuxOptions: # seLinux label is set as a precaution for accessing csi socket
+              type: spc_t
+              level: s0
+          image: MUSTPATCHWITHKUSTOMIZE
           args:
             - "--csi-address=$(CSI_ADDRESS)"
           env:
@@ -163,6 +173,11 @@ spec:
             - name: plugin-dir
               mountPath: /csi
         - name: storage-secret-sidecar
+          securityContext:
+            runAsNonRoot: true
+            runAsUser: 2121
+            runAsGroup: 2121
+            privileged: false
           image: MUSTPATCHWITHKUSTOMIZE
           imagePullPolicy: Always
           args:

pkg/ibmcsidriver/fileOps.go

@@ -0,0 +1,85 @@
+/**
+ *
+ * Copyright 2024- IBM Inc. All rights reserved
+ * SPDX-License-Identifier: Apache2.0
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Package ibmcsidriver ...
+package ibmcsidriver
+
+//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate
+
+import (
+	"os"
+	"strconv"
+
+	"go.uber.org/zap"
+)
+
+const (
+	filePermission = 0660
+)
+
+//counterfeiter:generate . socketPermission
+
+// socketPermission represents file system operations
+type socketPermission interface {
+	Chown(name string, uid, gid int) error
+	Chmod(name string, mode os.FileMode) error
+}
+
+// realSocketPermission implements socketPermission
+type opsSocketPermission struct{}
+
+func (f *opsSocketPermission) Chown(name string, uid, gid int) error {
+	return os.Chown(name, uid, gid)
+}
+
+func (f *opsSocketPermission) Chmod(name string, mode os.FileMode) error {
+	return os.Chmod(name, mode)
+}
+
+// setupSidecar updates owner/group and permission of the file given(addr)
+func setupSidecar(addr string, ops socketPermission, logger *zap.Logger) error {
+	groupSt := os.Getenv("SIDECAR_GROUP_ID")
+
+	logger.Info("Setting owner and permissions of csi socket file. SIDECAR_GROUP_ID env must match the 'livenessprobe' sidecar container groupID for csi socket connection.")
+
+	// If env is not set, set default to 0
+	if groupSt == "" {
+		logger.Warn("Unable to fetch SIDECAR_GROUP_ID environment variable. Sidecar container(s) might fail...")
+		groupSt = "0"
+	}
+
+	group, err := strconv.Atoi(groupSt)
+	if err != nil {
+		return err
+	}
+
+	// Change group of csi socket to non-root user for enabling the csi sidecar
+	if err := ops.Chown(addr, -1, group); err != nil {
+		return err
+	}
+
+	// Modify permissions of csi socket
+	// Only the users and the group owners will have read/write access to csi socket
+	if err := ops.Chmod(addr, filePermission); err != nil {
+		return err
+	}
+
+	logger.Info("Successfully set owner and permissions of csi socket file.")
+
+	return nil
+}

pkg/ibmcsidriver/fileOps_test.go

@@ -0,0 +1,125 @@
+/**
+ *
+ * Copyright 2024- IBM Inc. All rights reserved
+ * SPDX-License-Identifier: Apache2.0
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ibmcsidriver
+
+import (
+	"errors"
+	"os"
+	"testing"
+
+	"github.com/IBM/ibm-vpc-file-csi-driver/pkg/ibmcsidriver/ibmcsidriverfakes"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSetupSidecar(t *testing.T) {
+	tests := []struct {
+		name               string
+		groupID            string
+		expectedErr        bool
+		chownErr           error
+		chmodErr           error
+		expectedChownCalls int
+		expectedChmodCalls int
+		expectedGroupID    int
+	}{
+		{
+			name:               "ValidGroupID",
+			groupID:            "2121",
+			expectedErr:        false,
+			chownErr:           nil,
+			chmodErr:           nil,
+			expectedChownCalls: 1,
+			expectedChmodCalls: 1,
+			expectedGroupID:    2121,
+		},
+		{
+			name:               "EmptyGroupID",
+			groupID:            "",
+			expectedErr:        false,
+			chownErr:           nil,
+			chmodErr:           nil,
+			expectedChownCalls: 1,
+			expectedChmodCalls: 1,
+			expectedGroupID:    0, // Default to 0 if SIDECAR_GROUP_ID is empty
+		},
+		{
+			name:               "ChownError",
+			groupID:            "1000",
+			expectedErr:        true,
+			chownErr:           errors.New("chown error"),
+			chmodErr:           nil,
+			expectedChownCalls: 1,
+			expectedChmodCalls: 0, // No chmod expected if chown fails
+			expectedGroupID:    1000,
+		},
+		{
+			name:               "ChmodError",
+			groupID:            "1000",
+			expectedErr:        true,
+			chownErr:           nil,
+			chmodErr:           errors.New("chmod error"),
+			expectedChownCalls: 1,
+			expectedChmodCalls: 1,
+			expectedGroupID:    1000,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Set SIDECAR_GROUP_ID environment variable
+			if tc.groupID != "" {
+				os.Setenv("SIDECAR_GROUP_ID", tc.groupID)
+			} else {
+				os.Unsetenv("SIDECAR_GROUP_ID")
+			}
+			defer os.Unsetenv("SIDECAR_GROUP_ID")
+
+			// Create the fake object generated by counterfeiter
+			fakeSocketPermission := new(ibmcsidriverfakes.FakeSocketPermission)
+
+			// Set return values for chown and chmod methods
+			fakeSocketPermission.ChownReturns(tc.chownErr)
+			fakeSocketPermission.ChmodReturns(tc.chmodErr)
+
+			// Creating test logger
+			logger, teardown := GetTestLogger(t)
+			defer teardown()
+
+			// Call the function under test
+			err := setupSidecar("/path/to/socket", fakeSocketPermission, logger)
+
+			// Verify the result
+			if tc.expectedErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+
+			// Verify the number of times Chown and Chmod were called
+			assert.Equal(t, tc.expectedChownCalls, fakeSocketPermission.ChownCallCount())
+			assert.Equal(t, tc.expectedChmodCalls, fakeSocketPermission.ChmodCallCount())
+
+			// Verify the group ID passed to chown
+			if tc.expectedChownCalls > 0 {
+				_, _, actualGroupID := fakeSocketPermission.ChownArgsForCall(0)
+				assert.Equal(t, tc.expectedGroupID, actualGroupID)
+			}
+		})
+	}
+}