Fix/security issues fix (#617)

* fix: enable running otel and fluentd with the following security settings: readOnlyRootFilesystem, allowPrivilegeEscalation, seccompProfile, runAsNonRoot. * fix: add condition for adding fluentd-checkpoint-dir volume in post delete hook * fix: comment about issues with cri-o runtime * Update helm-charts/splunk-otel-collector/templates/configmap-fluentd.yaml Co-authored-by: harshit-splunk <[email protected]> * fix: run pre-commit * Address review comments * fix pre-commit * fix: change default user and group to 999 in patch-log-dirs init container Co-authored-by: harshit-splunk <[email protected]> Co-authored-by: harshit-splunk <[email protected]> Co-authored-by: Antoine Toulme <[email protected]>
signalfx · Jan 18, 2023 · a9f24c0 · a9f24c0
1 parent 921de28
commit a9f24c0
Show file tree

Hide file tree

Showing 13 changed files with 335 additions and 39 deletions.
diff --git a/.DS_Store b/.DS_Store
diff --git a/helm-charts/splunk-otel-collector/templates/configmap-fluentd.yaml b/helm-charts/splunk-otel-collector/templates/configmap-fluentd.yaml
@@ -139,7 +139,7 @@ data:
       <storage>
         @type local
         persistent true
-        path /var/log/splunkd-fluentd-journald-{{ $name }}.pos.json
+        path {{ $.Values.fluentd.config.posFilePrefix }}-journald-{{ $name }}.pos.json
       </storage>
       <entry>
         field_map {"MESSAGE": "log", "_SYSTEMD_UNIT": "source"}

diff --git a/helm-charts/splunk-otel-collector/templates/daemonset.yaml b/helm-charts/splunk-otel-collector/templates/daemonset.yaml
@@ -70,7 +70,31 @@ spec:
       {{- end }}
       {{- if and (eq (include "splunk-otel-collector.logsEnabled" .) "true") (not .Values.isWindows) }}
       initContainers:
-        {{- if and (eq .Values.logsEngine "fluentd") (not (eq .Values.distribution "gke/autopilot")) }}
+        {{- if ne .Values.distribution "gke/autopilot" }}
+        # Previously, fluentd checkpoints were written to /var/log directory.
+        # So, /var/log directory could not be mounted as read-only.
+        # Now, default fluentd checkpoint is moved to /var/addon/splunk/fluent_pos directory
+        # move-fluent-checkpoint will handle 2 scenarios
+        # - When upgrading from fluentd to fluentd, it will move checkpoint, so fluentd can continue from where it left
+        # - When migrating from fluentd to otel, it will move checkpoint, and then migrate to otel checkpoint. So, it
+        #   covers both previous and current fluentd checkpoint directory.
+        {{- if ne .Values.fluentd.config.posFilePrefix "/var/log/splunk-fluentd" }}
+        - name: move-fluent-checkpoint
+          image: {{ template "splunk-otel-collector.image.initPatchLogDirs" . }}
+          imagePullPolicy: {{ .Values.image.initPatchLogDirs.pullPolicy }}
+          command: ['sh', '-c', '
+          mkdir -p {{ dir .Values.fluentd.config.posFilePrefix }};
+          find /var/log -maxdepth 1 -name *.json | xargs -I{} mv {} -t {{ dir .Values.fluentd.config.posFilePrefix }};
+          find /var/log -maxdepth 1 -name *.pos | xargs -I{} mv {} -t {{ dir .Values.fluentd.config.posFilePrefix }};']
+          securityContext:
+            runAsUser: 0
+          volumeMounts:
+            - name: fluentd-checkpoint-dir
+              mountPath: {{ dir .Values.fluentd.config.posFilePrefix }}
+            - name: varlog
+              mountPath: /var/log
+        {{- end }}
+        {{- if eq .Values.logsEngine "fluentd" }}
         - name: prepare-fluentd-config
           image: {{ template "splunk-otel-collector.image.fluentd" . }}
           imagePullPolicy: {{ .Values.image.fluentd.pullPolicy }}
@@ -97,7 +121,6 @@ spec:
             - name: fluentd-config-json
               mountPath: /fluentd/etc/json
         {{- else }}
-        {{- if not (eq .Values.distribution "gke/autopilot") }}
         - name: migrate-checkpoint
           image: {{ template "splunk-otel-collector.image.otelcol" . }}
           imagePullPolicy: {{ .Values.image.otelcol.pullPolicy }}
@@ -128,41 +151,61 @@ spec:
               mountPath: /var/log
             - name: varlibdockercontainers
               mountPath: /var/lib/docker/containers
+            {{- if ne .Values.fluentd.config.posFilePrefix "/var/log/splunk-fluentd" }}
+            - name: fluentd-checkpoint-dir
+              mountPath: {{ dir .Values.fluentd.config.posFilePrefix -}}
+            {{- end }}
         {{- end }}
-        {{- if and $agent.securityContext.runAsUser $agent.securityContext.runAsGroup }}
+        {{- end }}
+        {{- if or (and (.Values.fluentd.securityContext.runAsUser) (.Values.fluentd.securityContext.runAsGroup)) (and ($agent.securityContext.runAsUser) ($agent.securityContext.runAsGroup)) }}
         - name: patch-log-dirs
           image: {{ template "splunk-otel-collector.image.initPatchLogDirs" . }}
           imagePullPolicy: {{ .Values.image.initPatchLogDirs.pullPolicy }}
           command: ['sh', '-c', '
           mkdir -p {{ .Values.logsCollection.checkpointPath }};
-          chown -Rv {{ $agent.securityContext.runAsUser | default 20000 }}:{{ $agent.securityContext.runAsGroup | default 20000 }} {{ .Values.logsCollection.checkpointPath }};
-          chmod -v g+rwxs {{ .Values.logsCollection.checkpointPath }};
+          setfacl -n -Rm d:m::rwx,m::rwx,d:g:{{ $agent.securityContext.runAsGroup | default 999 }}:rwx,g:{{ $agent.securityContext.runAsGroup | default 999 }}:rwx {{ .Values.logsCollection.checkpointPath }};
+          {{- if ne .Values.fluentd.config.posFilePrefix "/var/log/splunk-fluentd" }}
+          setfacl -n -Rm d:m::rwx,m::rwx,d:g:{{ .Values.fluentd.securityContext.runAsGroup | default 999 }}:rwx,g:{{ .Values.fluentd.securityContext.runAsGroup | default 999 }}:rwx {{ dir .Values.fluentd.config.posFilePrefix }};
+          {{- end }}
           {{ if .Values.logsCollection.containers.enabled -}}
           if [ -d "/var/lib/docker/containers" ];
           then
-              setfacl -n -Rm d:g:{{ $agent.securityContext.runAsGroup | default 20000 }}:rx,g:{{ $agent.securityContext.runAsGroup | default 20000 }}:rx /var/lib/docker/containers;
+              setfacl -n -Rm d:m::rx,m::rx,d:g:{{ $agent.securityContext.runAsGroup | default 999 }}:rx,g:{{ $agent.securityContext.runAsGroup | default 999 }}:rx /var/lib/docker/containers;
           fi;
           if [ -d "/var/log/crio/pods" ];
           then
-              setfacl -n -Rm d:g:{{ $agent.securityContext.runAsGroup | default 20000 }}:rx,g:{{ $agent.securityContext.runAsGroup | default 20000 }}:rx /var/log/crio/pods;
+              setfacl -n -Rm d:m::rx,m::rx,d:g:{{ $agent.securityContext.runAsGroup | default 999 }}:rx,g:{{ $agent.securityContext.runAsGroup | default 999 }}:rx /var/log/crio/pods;
           fi;
           if [ -d "/var/log/pods" ];
           then
-              setfacl -n -Rm d:g:{{ $agent.securityContext.runAsGroup | default 20000 }}:rx,g:{{ $agent.securityContext.runAsGroup | default 20000 }}:rx /var/log/pods;
+              setfacl -n -Rm d:m::rx,m::rx,d:g:{{ $agent.securityContext.runAsGroup | default 999 }}:rx,g:{{ $agent.securityContext.runAsGroup | default 999 }}:rx /var/log/pods;
+          fi;
+          {{- end }}
+          {{- if .Values.logsCollection.journald.enabled }}
+          if [ -d "{{ .Values.logsCollection.journald.directory }}" ];
+          then
+              setfacl -n -Rm d:m::rx,m::rx,d:g:{{ $agent.securityContext.runAsGroup | default 999 }}:rx,g:{{ $agent.securityContext.runAsGroup | default 999 }}:rx {{ .Values.logsCollection.journald.directory }};
           fi;
           {{- end }}']
           securityContext:
             runAsUser: 0
           volumeMounts:
             - name: checkpoint
               mountPath: {{ .Values.logsCollection.checkpointPath }}
+            {{- if ne .Values.fluentd.config.posFilePrefix "/var/log/splunk-fluentd" }}
+            - name: fluentd-checkpoint-dir
+              mountPath: {{ dir .Values.fluentd.config.posFilePrefix }}
+            {{- end }}
             {{- if .Values.logsCollection.containers.enabled }}
             - name: varlog
               mountPath: /var/log
             - name: varlibdockercontainers
               mountPath: /var/lib/docker/containers
             {{- end }}
-        {{- end }}
+            {{- if .Values.logsCollection.journald.enabled }}
+            - name: journaldlogs
+              mountPath: {{.Values.logsCollection.journald.directory}}
+            {{- end }}
         {{- end }}
       {{- end }}
       containers:
@@ -195,6 +238,7 @@ spec:
         volumeMounts:
         - name: varlog
           mountPath: {{ .Values.fluentd.config.containers.path }}
+          readOnly: true
         - name: varlogdest
           mountPath: {{ .Values.fluentd.config.containers.pathDest }}
           readOnly: true
@@ -203,6 +247,10 @@ spec:
           readOnly: true
         - name: fluentd-config
           mountPath: /fluentd/etc
+        {{- if ne .Values.fluentd.config.posFilePrefix "/var/log/splunk-fluentd" }}
+        - name: fluentd-checkpoint-dir
+          mountPath: {{ dir .Values.fluentd.config.posFilePrefix }}
+        {{- end }}
         - name: tmp
           mountPath: /tmp
       {{- end }}
@@ -396,7 +444,24 @@ spec:
       terminationGracePeriodSeconds: {{ .Values.terminationGracePeriodSeconds }}
       volumes:
       {{- if (eq (include "splunk-otel-collector.logsEnabled" .) "true") }}
+      {{- if not .Values.isWindows }}
+      - name: checkpoint
+        hostPath:
+          path: {{ .Values.logsCollection.checkpointPath }}
+          type: DirectoryOrCreate
+      {{- if ne .Values.fluentd.config.posFilePrefix "/var/log/splunk-fluentd" }}
+      - name: fluentd-checkpoint-dir
+        hostPath:
+          path: {{ dir .Values.fluentd.config.posFilePrefix }}
+          type: DirectoryOrCreate
+      {{- end }}
+      {{- end }}
       {{- if eq .Values.logsEngine "fluentd" }}
+      {{- if or (and (.Values.fluentd.securityContext.runAsUser) (.Values.fluentd.securityContext.runAsGroup)) (and ($agent.securityContext.runAsUser) ($agent.securityContext.runAsGroup)) }}
+      - name: varlibdockercontainers
+        hostPath:
+          path: /var/lib/docker/containers
+      {{- end }}
       - name: varlog
         hostPath:
           path: {{ .Values.fluentd.config.containers.path }}
@@ -438,10 +503,6 @@ spec:
         hostPath:
           path: /var/lib/docker/containers
       {{- end }}
-      - name: checkpoint
-        hostPath:
-          path: {{ .Values.logsCollection.checkpointPath }}
-          type: DirectoryOrCreate
       {{- if .Values.logsCollection.journald.enabled}}
       - name: journaldlogs
         hostPath:

diff --git a/helm-charts/splunk-otel-collector/templates/revert-patch-log-dirs-hook.yaml b/helm-charts/splunk-otel-collector/templates/revert-patch-log-dirs-hook.yaml
@@ -0,0 +1,82 @@
+{{- if or (and (.Values.fluentd.securityContext.runAsUser) (.Values.fluentd.securityContext.runAsGroup)) (and (.Values.agent.securityContext.runAsUser) (.Values.agent.securityContext.runAsGroup)) }}
+apiVersion: v1
+kind: Pod
+metadata:
+  name: {{ template "splunk-otel-collector.fullname" . }}-revert-patch-log-dir
+  labels:
+    {{- include "splunk-otel-collector.commonLabels" . | nindent 4 }}
+  annotations:
+    "helm.sh/hook": post-delete
+    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
+spec:
+  restartPolicy: Never
+  containers:
+  - name: revert-patch-log-dirs
+    image: {{ template "splunk-otel-collector.image.initPatchLogDirs" . }}
+    imagePullPolicy: {{ .Values.image.initPatchLogDirs.pullPolicy }}
+    securityContext:
+      runAsUser: 0
+    command: ['sh', '-c', '
+    setfacl --recursive --remove-all  {{ .Values.logsCollection.checkpointPath }};
+    {{- if ne .Values.fluentd.config.posFilePrefix "/var/log/splunk-fluentd" }}
+    setfacl --recursive --remove-all  {{ dir .Values.fluentd.config.posFilePrefix }};
+    {{- end }}
+    {{ if .Values.logsCollection.containers.enabled -}}
+    if [ -d "/var/lib/docker/containers" ];
+    then
+        setfacl --recursive --remove-all  /var/lib/docker/containers;
+    fi;
+    if [ -d "/var/log/crio/pods" ];
+    then
+        setfacl --recursive --remove-all  /var/log/crio/pods;
+    fi;
+    if [ -d "/var/log/pods" ];
+    then
+        setfacl --recursive --remove-all  /var/log/pods;
+    fi;
+    {{- end }}
+    {{- if .Values.logsCollection.journald.enabled }}
+    if [ -d "{{ .Values.logsCollection.journald.directory }}" ];
+    then
+        setfacl --recursive --remove-all d:m::rx,m::rx,d:g:{{ .Values.agent.securityContext.runAsGroup | default 999 }}:rx,g:{{ .Values.agent.securityContext.runAsGroup | default 999 }}:rx {{ .Values.logsCollection.journald.directory }};
+    fi;
+    {{- end }}']
+    volumeMounts:
+      - name: checkpoint
+        mountPath: {{ .Values.logsCollection.checkpointPath }}
+      {{- if ne .Values.fluentd.config.posFilePrefix "/var/log/splunk-fluentd" }}
+      - name: fluentd-checkpoint-dir
+        mountPath: {{ dir .Values.fluentd.config.posFilePrefix }}
+      {{- end }}
+      {{- if .Values.logsCollection.containers.enabled }}
+      - name: varlog
+        mountPath: /var/log
+      - name: varlibdockercontainers
+        mountPath: /var/lib/docker/containers
+      {{- end }}
+      {{- if .Values.logsCollection.journald.enabled }}
+      - name: journaldlogs
+        mountPath: {{.Values.logsCollection.journald.directory}}
+      {{- end }}
+  volumes:
+    - name: checkpoint
+      hostPath:
+        path: {{ .Values.logsCollection.checkpointPath }}
+    - name: varlog
+      hostPath:
+        path: /var/log
+    - name: varlibdockercontainers
+      hostPath:
+        path: /var/lib/docker/containers
+    {{- if ne .Values.fluentd.config.posFilePrefix "/var/log/splunk-fluentd" }}
+    - name: fluentd-checkpoint-dir
+      hostPath:
+        path: {{ dir .Values.fluentd.config.posFilePrefix }}
+        type: DirectoryOrCreate
+    {{- end }}
+    {{- if .Values.logsCollection.journald.enabled}}
+    - name: journaldlogs
+      hostPath:
+        path: {{.Values.logsCollection.journald.directory}}
+    {{- end}}
+{{- end }}
diff --git a/helm-charts/splunk-otel-collector/templates/securityContextConstraints.yaml b/helm-charts/splunk-otel-collector/templates/securityContextConstraints.yaml
@@ -42,6 +42,8 @@ runAsUser:
   type: RunAsAny
 supplementalGroups:
   type: RunAsAny
+seccompProfiles:
+- runtime/default
 requiredDropCapabilities:
 - ALL
 {{- end }}
diff --git a/helm-charts/splunk-otel-collector/values.yaml b/helm-charts/splunk-otel-collector/values.yaml
@@ -350,13 +350,19 @@ agent:
       memory: 500Mi
 
   # To collect container logs and journald logs, it will run the agent as a root user.
-  # To run it as non root user, uncomment below `securityContext` options.
+  # To run it as non root user, change runAsUser and runAsGroup to non-zero value (e.g. 999) and change runAsNonRoot to true.
   # Setting runAsUser and runAsGroup to a non root user enables an init container that patches group
   # permissions of container logs directories on the host filesystem to make logs readable by this non root user.
-
-  securityContext: {}
-  #   runAsUser: 20000
-  #   runAsGroup: 20000
+  # NOTE: Running this container as a non-root user doesn't work with openshift cluser or cri-o runntime.
+  # See https://github.com/cri-o/cri-o/issues/6519 for more details.
+  securityContext:
+    readOnlyRootFilesystem: true
+    allowPrivilegeEscalation: false
+    seccompProfile:
+      type: RuntimeDefault
+    runAsUser: 0
+    runAsGroup: 0
+    runAsNonRoot: false
 
   # Specifies DaemonSet update strategy.
   # Possible values: "OnDelete" and "RollingUpdate".
@@ -606,8 +612,23 @@ fluentd:
       cpu: 100m
       memory: 200Mi
 
+  # To collect container logs and journald logs, it will run the fluentd sidecar as a root user.
+  # To run it as non root user, change runAsUser and runAsGroup to non-zero value (e.g. 999) and change runAsNonRoot to true.
+  # Setting runAsUser and runAsGroup to a non root user enables an init container that patches group
+  # permissions of container logs directories on the host filesystem to make logs readable by this non root user.
+  # NOTE: Running this container as a non-root user doesn't work with openshift cluser or cri-o runntime.
+  # See https://github.com/cri-o/cri-o/issues/6519 for more details.
+
   securityContext:
     runAsUser: 0
+  #  securityContext:
+  #  readOnlyRootFilesystem: true
+  #  allowPrivilegeEscalation: false
+  #  seccompProfile:
+  #    type: RuntimeDefault
+  #  runAsUser: 999
+  #  runAsGroup: 999
+  #  runAsNonRoot: true
 
   # Extra enviroment variables to be set in the FluentD container
   extraEnvs: []
@@ -662,7 +683,7 @@ fluentd:
     # Prefix for pos_file tail source parameter
     # Can be used if you want to run multiple instances of fluentd on the same host
     # https://docs.fluentd.org/input/tail#pos_file-highly-recommended
-    posFilePrefix: /var/log/splunk-fluentd
+    posFilePrefix: /var/addon/splunk/fluent_pos/splunk-fluentd
 
     # `customFilters` defines the custom filters to be used.
     # This section can be used to define custom filters using plugins like https://github.com/splunk/fluent-plugin-jq

diff --git a/rendered/manifests/agent-only/daemonset.yaml b/rendered/manifests/agent-only/daemonset.yaml
@@ -79,6 +79,14 @@ spec:
           protocol: TCP
         image: quay.io/signalfx/splunk-otel-collector:0.67.0
         imagePullPolicy: IfNotPresent
+        securityContext:
+          allowPrivilegeEscalation: false
+          readOnlyRootFilesystem: true
+          runAsGroup: 0
+          runAsNonRoot: false
+          runAsUser: 0
+          seccompProfile:
+            type: RuntimeDefault
         env:
           - name: SPLUNK_MEMORY_TOTAL_MIB
             value: "500"