Merge pull request #47 from jk-cluster/feat/samba-conventions

Samba chart: security hardening, configurable node ports (conventions batch 3)

ci/samba/fixtures.yaml

@@ -1,10 +1,10 @@
 # Fixtures for the samba install-test: the users secret the 1Password
 # operator would otherwise materialize (release name: ci). users.conf uses
-# the dockur/samba "user:password" format.
+# the dockur/samba "username:uid:groupname:gid:password[:homedir]" format.
 apiVersion: v1
 kind: Secret
 metadata:
   name: ci-samba-users-secret
 stringData:
   users.conf: |
-    ciuser:ci-password
+    ciuser:1000:smb:1000:ci-password

ci/samba/test-values.yaml

@@ -1,5 +1,5 @@
 secrets:
-  sambaUsersRef: op://ci/samba-users
+  sambaUsersSecretRef: op://ci/samba-users
 
 samba:
   storageClass: local-path

samba/Chart.yaml

@@ -5,6 +5,6 @@ icon: https://avatars.githubusercontent.com/u/13281359
 
 type: application
 
-version: 3.1.2+up4.23.10
+version: 4.0.0+up4.23.10
 
 appVersion: 4.23.10

samba/templates/samba-config.configmap.yaml

@@ -1,7 +1,7 @@
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: {{ .Release.Name }}-{{ .Chart.Name }}-config-cfm
+  name: {{ .Release.Name }}-{{ .Chart.Name }}-configmap
 data:
   smb.conf: |-
     [global]

samba/templates/samba-users.secret.yaml

@@ -3,4 +3,4 @@ kind: OnePasswordItem
 metadata:
   name: {{ .Release.Name }}-{{ .Chart.Name }}-users-secret
 spec:
-  itemPath: {{ required "A secret-reference for the samba-users-secret must be provided" .Values.secrets.sambaUsersRef }}
+  itemPath: {{ required "A secret-reference for the samba-users-secret must be provided" .Values.secrets.sambaUsersSecretRef }}

samba/templates/samba.service.yaml

@@ -9,11 +9,11 @@ spec:
     - protocol: TCP
       port: 445
       targetPort: 445
-      nodePort: 30445
+      nodePort: {{ .Values.samba.smbNodePort }}
       name: smb-port-445
     - protocol: TCP
       port: 139
       targetPort: 139
-      nodePort: 30139
+      nodePort: {{ .Values.samba.netbiosNodePort }}
       name: smb-port-139
   type: NodePort

samba/templates/samba.sfs.yaml

@@ -20,6 +20,29 @@ spec:
         - name: {{ .Release.Name }}-{{ .Chart.Name }}-sfs
           image: "ghcr.io/dockur/samba:{{ .Chart.AppVersion }}"
           imagePullPolicy: IfNotPresent
+          {{- with .Values.samba.env }}
+          env:
+            {{- range $value := . }}
+            {{- if $value.value }}
+            - name: {{ $value.name }}
+              value: {{ tpl $value.value $ | quote }}
+            {{- else if $value.valueFrom }}
+            {{- if $value.valueFrom.secretKeyRef }}
+            - name: {{ $value.name }}
+              valueFrom:
+                secretKeyRef:
+                  name: {{ tpl $value.valueFrom.secretKeyRef.name $ | quote }}
+                  key: {{ tpl $value.valueFrom.secretKeyRef.key $ | quote }}
+            {{- else if $value.valueFrom.configMapKeyRef }}
+            - name: {{ $value.name }}
+              valueFrom:
+                configMapKeyRef:
+                  name: {{ tpl $value.valueFrom.configMapKeyRef.name $ | quote }}
+                  key: {{ tpl $value.valueFrom.configMapKeyRef.key $ | quote }}
+            {{- end }}
+            {{- end }}
+            {{- end }}
+          {{- end }}
           ports:
             - containerPort: 445
               protocol: TCP
@@ -47,14 +70,16 @@ spec:
             {{- toYaml .Values.samba.startupProbe | nindent 12 }}
           resources:
             {{- include "samba.resources" .Values.samba.resources | nindent 12 }}
+          securityContext:
+            {{- toYaml .Values.samba.securityContext.container | nindent 12 }}
       restartPolicy: Always
       automountServiceAccountToken: false
       securityContext:
-        fsGroup: 1000
+        {{- toYaml .Values.samba.securityContext.pod | nindent 8 }}
       volumes:
         - name: smb-config
           configMap:
-            name: {{ .Release.Name }}-{{ .Chart.Name }}-config-cfm
+            name: {{ .Release.Name }}-{{ .Chart.Name }}-configmap
             items:
               - key: smb.conf
                 path: smb.conf

samba/values.yaml

@@ -5,17 +5,23 @@
 scaleDown: false
 
 secrets:
-  sambaUsersRef: null
+  sambaUsersSecretRef: null
 
 samba:
   # Replica count for this workload; 0 is allowed and scales it down.
   # The chart-global scaleDown flag takes precedence.
   replicas: 1
+  # NodePorts the SMB service is published on (service type NodePort).
+  smbNodePort: 30445
+  netbiosNodePort: 30139
   shares:
     - name: null
       validUsers: null
       description: null
       size: null
+  # Extra environment variables for the samba container. Values are rendered
+  # through tpl; value, secretKeyRef and configMapKeyRef forms are supported.
+  env: []
   # SMB is not an HTTP protocol, so probe the main SMB TCP port.
   livenessProbe:
     tcpSocket:
@@ -46,5 +52,35 @@ samba:
       cpu: 0.1
       ephemeralStorage: 50Mi
 
+  # Deviations from the repo's hardening baseline, forced by the
+  # ghcr.io/dockur/samba image (its entrypoint samba.sh):
+  # - no runAsNonRoot: the entrypoint creates the Unix users from users.conf
+  #   (groupadd/adduser/usermod) and smbd impersonates the authenticated
+  #   share user per session, both of which require starting as root.
+  # - readOnlyRootFilesystem: false: the entrypoint writes to the root
+  #   filesystem at startup (/etc/passwd + /etc/group for user creation,
+  #   the /etc/samba.conf healthcheck symlink, mkdir /shared and the Samba
+  #   TDB databases under /var/lib/samba).
+  # - capabilities: ALL dropped except what a multi-user root file server
+  #   needs: NET_BIND_SERVICE (ports 445/139), SETUID/SETGID (per-session
+  #   user impersonation), CHOWN/FOWNER/DAC_OVERRIDE (managing share files
+  #   on behalf of the users).
+  securityContext:
+    pod:
+      fsGroup: 1000
+    container:
+      allowPrivilegeEscalation: false
+      readOnlyRootFilesystem: false
+      capabilities:
+        drop:
+          - ALL
+        add:
+          - CHOWN
+          - DAC_OVERRIDE
+          - FOWNER
+          - NET_BIND_SERVICE
+          - SETGID
+          - SETUID
+
   globalConfig: null
   storageClass: longhorn