From b41d36c6217e46f3d420e0ccf7acab5eeebff9e0 Mon Sep 17 00:00:00 2001
From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com>
Date: Fri, 18 Oct 2019 12:42:25 -0400
Subject: [PATCH] Require vault to run as non root (#80)

* Require vault to run as non root

* Fix unit tests

* Make uid/gid configurable, remove home emptydir
---
 templates/server-config-configmap.yaml |  2 -
 templates/server-statefulset.yaml      | 10 +--
 test/unit/server-configmap.bats        | 49 ---------------
 test/unit/server-dev-statefulset.bats  | 85 +++++++++++++++++++++++---
 test/unit/server-ha-statefulset.bats   | 85 +++++++++++++++++++++++---
 test/unit/server-statefulset.bats      | 76 ++++++++++++++++++++---
 values.yaml                            |  5 --
 7 files changed, 223 insertions(+), 89 deletions(-)

diff --git a/templates/server-config-configmap.yaml b/templates/server-config-configmap.yaml
index 0a0df7c15..811500b39 100644
--- a/templates/server-config-configmap.yaml
+++ b/templates/server-config-configmap.yaml
@@ -13,9 +13,7 @@ metadata:
     app.kubernetes.io/managed-by: {{ .Release.Service }}
 data:
   extraconfig-from-values.hcl: |-
-  {{- if eq (.Values.server.mlock.enabled | toString) "false" }}
     disable_mlock = true
-  {{- end }}
   {{- if eq .mode "standalone" }}
     {{ tpl .Values.server.standalone.config . | nindent 4 | trim }}
   {{- else if eq .mode "ha" }}
diff --git a/templates/server-statefulset.yaml b/templates/server-statefulset.yaml
index 075b6e919..4a8c8e66c 100644
--- a/templates/server-statefulset.yaml
+++ b/templates/server-statefulset.yaml
@@ -41,17 +41,19 @@ spec:
       terminationGracePeriodSeconds: 10
       serviceAccountName: {{ template "vault.fullname" . }}
       securityContext:
-        fsGroup: 1000
+        readOnlyRootFilesystem: true
+        runAsNonRoot: true
+        runAsGroup: {{ .Values.server.gid | default 1000 }}
+        runAsUser: {{ .Values.server.uid | default 100 }}
+        fsGroup: {{ .Values.server.gid | default 1000 }}
       volumes:
         {{ template "vault.volumes" . }}
       containers:
         - name: vault
           {{ template "vault.resources" . }}
-          {{- if eq (.Values.server.mlock.enabled | toString) "true" }}
           securityContext:
             capabilities:
               add: ["IPC_LOCK"]
-          {{- end }}
           image: "{{ .Values.global.image }}"
           command: {{ template "vault.command" . }}
           args: {{ template "vault.args" . }}
@@ -70,10 +72,8 @@ spec:
               value: "{{ include "vault.scheme" . }}://$(POD_IP):8200"
             - name: SKIP_CHOWN
               value: "true"
-            {{- if eq (.Values.server.mlock.enabled | toString) "false" }}
             - name: SKIP_SETCAP
               value: "true"
-            {{- end }}
             {{ template "vault.envs" . }}
             {{- include "vault.extraEnvironmentVars" .Values.server | nindent 12 }}
             {{- include "vault.extraSecretEnvironmentVars" .Values.server | nindent 12 }}
diff --git a/test/unit/server-configmap.bats b/test/unit/server-configmap.bats
index 0629028ac..7a66c53f9 100755
--- a/test/unit/server-configmap.bats
+++ b/test/unit/server-configmap.bats
@@ -82,52 +82,3 @@ load _helpers
       yq '.data["extraconfig-from-values.hcl"] | match("bar") | length' | tee /dev/stderr)
   [ ! -z "${actual}" ]
 }
-
-@test "server/ConfigMap: mlock by default" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -x templates/server-config-configmap.yaml \
-      . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | contains("disable_mlock") | not)' | tee /dev/stderr)
-  [ -z "${actual}" ]
-
-  local actual=$(helm template \
-      -x templates/server-config-configmap.yaml \
-      --set 'server.standalone.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | contains("disable_mlock") | not)' | tee /dev/stderr)
-  [ -z "${actual}" ]
-
-  local actual=$(helm template \
-      -x templates/server-config-configmap.yaml \
-      --set 'server.ha.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | contains("disable_mlock") | not)' | tee /dev/stderr)
-  [ -z "${actual}" ]
-}
-
-@test "server/ConfigMap: disable mlock" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -x templates/server-config-configmap.yaml \
-      --set 'server.mlock.enabled=false' \
-      . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | contains("disable_mlock")' | tee /dev/stderr)
-  [ ! -z "${actual}" ]
-
-  local actual=$(helm template \
-      -x templates/server-config-configmap.yaml \
-      --set 'server.mlock.enabled=false' \
-      --set 'server.standalone.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | contains("disable_mlock")' | tee /dev/stderr)
-  [ ! -z "${actual}" ]
-
-  local actual=$(helm template \
-      -x templates/server-config-configmap.yaml \
-      --set 'server.mlock.enabled=false' \
-      --set 'server.ha.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.data["extraconfig-from-values.hcl"] | contains("disable_mlock")' | tee /dev/stderr)
-  [ ! -z "${actual}" ]
-}
diff --git a/test/unit/server-dev-statefulset.bats b/test/unit/server-dev-statefulset.bats
index ff06fc1eb..6af6d897f 100755
--- a/test/unit/server-dev-statefulset.bats
+++ b/test/unit/server-dev-statefulset.bats
@@ -224,19 +224,19 @@ load _helpers
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
   local actual=$(echo $object |
-     yq -r '.[6].name' | tee /dev/stderr)
+     yq -r '.[7].name' | tee /dev/stderr)
   [ "${actual}" = "FOO" ]
 
   local actual=$(echo $object |
-      yq -r '.[6].value' | tee /dev/stderr)
+      yq -r '.[7].value' | tee /dev/stderr)
   [ "${actual}" = "bar" ]
 
   local actual=$(echo $object |
-      yq -r '.[7].name' | tee /dev/stderr)
+      yq -r '.[8].name' | tee /dev/stderr)
   [ "${actual}" = "FOOBAR" ]
 
   local actual=$(echo $object |
-      yq -r '.[7].value' | tee /dev/stderr)
+      yq -r '.[8].value' | tee /dev/stderr)
   [ "${actual}" = "foobar" ]
 }
 
@@ -257,23 +257,23 @@ load _helpers
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
   local actual=$(echo $object |
-      yq -r '.[5].name' | tee /dev/stderr)
+      yq -r '.[6].name' | tee /dev/stderr)
   [ "${actual}" = "ENV_FOO_0" ]
   local actual=$(echo $object |
-      yq -r '.[5].valueFrom.secretKeyRef.name' | tee /dev/stderr)
+      yq -r '.[6].valueFrom.secretKeyRef.name' | tee /dev/stderr)
   [ "${actual}" = "secret_name_0" ]
   local actual=$(echo $object |
-      yq -r '.[5].valueFrom.secretKeyRef.key' | tee /dev/stderr)
+      yq -r '.[6].valueFrom.secretKeyRef.key' | tee /dev/stderr)
   [ "${actual}" = "secret_key_0" ]
 
   local actual=$(echo $object |
-      yq -r '.[6].name' | tee /dev/stderr)
+      yq -r '.[7].name' | tee /dev/stderr)
   [ "${actual}" = "ENV_FOO_1" ]
   local actual=$(echo $object |
-      yq -r '.[6].valueFrom.secretKeyRef.name' | tee /dev/stderr)
+      yq -r '.[7].valueFrom.secretKeyRef.name' | tee /dev/stderr)
   [ "${actual}" = "secret_name_1" ]
   local actual=$(echo $object |
-      yq -r '.[6].valueFrom.secretKeyRef.key' | tee /dev/stderr)
+      yq -r '.[7].valueFrom.secretKeyRef.key' | tee /dev/stderr)
   [ "${actual}" = "secret_key_1" ]
 }
 
@@ -311,3 +311,68 @@ load _helpers
       yq -r '.spec.volumeClaimTemplates' | tee /dev/stderr)
   [ "${actual}" = "null" ]
 }
+
+#--------------------------------------------------------------------
+# Security Contexts
+@test "server/standalone-StatefulSet: uid default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.dev.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsUser' | tee /dev/stderr)
+  [ "${actual}" = "100" ]
+}
+
+@test "server/standalone-StatefulSet: uid configurable" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.uid=2000' \
+      --set 'server.dev.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsUser' | tee /dev/stderr)
+  [ "${actual}" = "2000" ]
+}
+
+@test "server/standalone-StatefulSet: gid default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.dev.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsGroup' | tee /dev/stderr)
+  [ "${actual}" = "1000" ]
+}
+
+@test "server/standalone-StatefulSet: gid configurable" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.gid=2000' \
+      --set 'server.dev.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsGroup' | tee /dev/stderr)
+  [ "${actual}" = "2000" ]
+}
+
+@test "server/standalone-StatefulSet: fsgroup default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.dev.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.fsGroup' | tee /dev/stderr)
+  [ "${actual}" = "1000" ]
+}
+
+@test "server/standalone-StatefulSet: fsgroup configurable" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.gid=2000' \
+      --set 'server.dev.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.fsGroup' | tee /dev/stderr)
+  [ "${actual}" = "2000" ]
+}
diff --git a/test/unit/server-ha-statefulset.bats b/test/unit/server-ha-statefulset.bats
index a750e1dec..06c747f62 100755
--- a/test/unit/server-ha-statefulset.bats
+++ b/test/unit/server-ha-statefulset.bats
@@ -320,19 +320,19 @@ load _helpers
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
   local actual=$(echo $object |
-     yq -r '.[5].name' | tee /dev/stderr)
+     yq -r '.[6].name' | tee /dev/stderr)
   [ "${actual}" = "FOO" ]
 
   local actual=$(echo $object |
-      yq -r '.[5].value' | tee /dev/stderr)
+      yq -r '.[6].value' | tee /dev/stderr)
   [ "${actual}" = "bar" ]
 
   local actual=$(echo $object |
-      yq -r '.[6].name' | tee /dev/stderr)
+      yq -r '.[7].name' | tee /dev/stderr)
   [ "${actual}" = "FOOBAR" ]
 
   local actual=$(echo $object |
-      yq -r '.[6].value' | tee /dev/stderr)
+      yq -r '.[7].value' | tee /dev/stderr)
   [ "${actual}" = "foobar" ]
 }
 
@@ -354,23 +354,23 @@ load _helpers
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
   local actual=$(echo $object |
-      yq -r '.[5].name' | tee /dev/stderr)
+      yq -r '.[6].name' | tee /dev/stderr)
   [ "${actual}" = "ENV_FOO_0" ]
   local actual=$(echo $object |
-      yq -r '.[5].valueFrom.secretKeyRef.name' | tee /dev/stderr)
+      yq -r '.[6].valueFrom.secretKeyRef.name' | tee /dev/stderr)
   [ "${actual}" = "secret_name_0" ]
   local actual=$(echo $object |
-      yq -r '.[5].valueFrom.secretKeyRef.key' | tee /dev/stderr)
+      yq -r '.[6].valueFrom.secretKeyRef.key' | tee /dev/stderr)
   [ "${actual}" = "secret_key_0" ]
 
   local actual=$(echo $object |
-      yq -r '.[6].name' | tee /dev/stderr)
+      yq -r '.[7].name' | tee /dev/stderr)
   [ "${actual}" = "ENV_FOO_1" ]
   local actual=$(echo $object |
-      yq -r '.[6].valueFrom.secretKeyRef.name' | tee /dev/stderr)
+      yq -r '.[7].valueFrom.secretKeyRef.name' | tee /dev/stderr)
   [ "${actual}" = "secret_name_1" ]
   local actual=$(echo $object |
-      yq -r '.[6].valueFrom.secretKeyRef.key' | tee /dev/stderr)
+      yq -r '.[7].valueFrom.secretKeyRef.key' | tee /dev/stderr)
   [ "${actual}" = "secret_key_1" ]
 }
 
@@ -506,3 +506,68 @@ load _helpers
       yq -r '.spec.template.spec.nodeSelector' | tee /dev/stderr)
   [ "${actual}" = "testing" ]
 }
+
+#--------------------------------------------------------------------
+# Security Contexts
+@test "server/standalone-StatefulSet: uid default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsUser' | tee /dev/stderr)
+  [ "${actual}" = "100" ]
+}
+
+@test "server/standalone-StatefulSet: uid configurable" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.uid=2000' \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsUser' | tee /dev/stderr)
+  [ "${actual}" = "2000" ]
+}
+
+@test "server/standalone-StatefulSet: gid default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsGroup' | tee /dev/stderr)
+  [ "${actual}" = "1000" ]
+}
+
+@test "server/standalone-StatefulSet: gid configurable" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.gid=2000' \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsGroup' | tee /dev/stderr)
+  [ "${actual}" = "2000" ]
+}
+
+@test "server/standalone-StatefulSet: fsgroup default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.fsGroup' | tee /dev/stderr)
+  [ "${actual}" = "1000" ]
+}
+
+@test "server/standalone-StatefulSet: fsgroup configurable" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.gid=2000' \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.fsGroup' | tee /dev/stderr)
+  [ "${actual}" = "2000" ]
+}
diff --git a/test/unit/server-statefulset.bats b/test/unit/server-statefulset.bats
index 7a9f53b0a..fd0876c74 100755
--- a/test/unit/server-statefulset.bats
+++ b/test/unit/server-statefulset.bats
@@ -305,19 +305,19 @@ load _helpers
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
   local actual=$(echo $object |
-     yq -r '.[5].name' | tee /dev/stderr)
+     yq -r '.[6].name' | tee /dev/stderr)
   [ "${actual}" = "FOO" ]
 
   local actual=$(echo $object |
-      yq -r '.[5].value' | tee /dev/stderr)
+      yq -r '.[6].value' | tee /dev/stderr)
   [ "${actual}" = "bar" ]
 
   local actual=$(echo $object |
-      yq -r '.[6].name' | tee /dev/stderr)
+      yq -r '.[7].name' | tee /dev/stderr)
   [ "${actual}" = "FOOBAR" ]
 
   local actual=$(echo $object |
-      yq -r '.[6].value' | tee /dev/stderr)
+      yq -r '.[7].value' | tee /dev/stderr)
   [ "${actual}" = "foobar" ]
 
   local object=$(helm template \
@@ -328,19 +328,19 @@ load _helpers
       yq -r '.spec.template.spec.containers[0].env' | tee /dev/stderr)
 
   local actual=$(echo $object |
-     yq -r '.[5].name' | tee /dev/stderr)
+     yq -r '.[6].name' | tee /dev/stderr)
   [ "${actual}" = "FOO" ]
 
   local actual=$(echo $object |
-      yq -r '.[5].value' | tee /dev/stderr)
+      yq -r '.[6].value' | tee /dev/stderr)
   [ "${actual}" = "bar" ]
 
   local actual=$(echo $object |
-      yq -r '.[6].name' | tee /dev/stderr)
+      yq -r '.[7].name' | tee /dev/stderr)
   [ "${actual}" = "FOOBAR" ]
 
   local actual=$(echo $object |
-      yq -r '.[6].value' | tee /dev/stderr)
+      yq -r '.[7].value' | tee /dev/stderr)
   [ "${actual}" = "foobar" ]
 }
 
@@ -532,3 +532,63 @@ load _helpers
       yq -r '.spec.template.metadata.labels.foo' | tee /dev/stderr)
   [ "${actual}" = "bar" ]
 }
+
+
+#--------------------------------------------------------------------
+# Security Contexts
+@test "server/standalone-StatefulSet: uid default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsUser' | tee /dev/stderr)
+  [ "${actual}" = "100" ]
+}
+
+@test "server/standalone-StatefulSet: uid configurable" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.uid=2000' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsUser' | tee /dev/stderr)
+  [ "${actual}" = "2000" ]
+}
+
+@test "server/standalone-StatefulSet: gid default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsGroup' | tee /dev/stderr)
+  [ "${actual}" = "1000" ]
+}
+
+@test "server/standalone-StatefulSet: gid configurable" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.gid=2000' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.runAsGroup' | tee /dev/stderr)
+  [ "${actual}" = "2000" ]
+}
+
+@test "server/standalone-StatefulSet: fsgroup default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.fsGroup' | tee /dev/stderr)
+  [ "${actual}" = "1000" ]
+}
+
+@test "server/standalone-StatefulSet: fsgroup configurable" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-statefulset.yaml \
+      --set 'server.gid=2000' \
+      . | tee /dev/stderr |
+      yq -r '.spec.template.spec.securityContext.fsGroup' | tee /dev/stderr)
+  [ "${actual}" = "2000" ]
+}
diff --git a/values.yaml b/values.yaml
index 4e0ec41d1..70a09ef3f 100644
--- a/values.yaml
+++ b/values.yaml
@@ -240,11 +240,6 @@ server:
   serviceaccount:
     annotations: {}
   
-  # mlock prevents memory from being swapped to disk.  If swap is enabled this should 
-  # be true.
-  mlock:
-    enabled: true
-
 # Vault UI
 ui:
   # True if you want to create a Service entry for the Vault UI.