Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Run Containers as Non-Root, and without Privilege Escalation by default. #400

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Run Containers as Non-Root, and without Privilege Escalation by default. #400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c54dbdb
Run containers as non-root w/o escalation privs
jk464 Feb 5, 2024
30ed80d
Do not preserve file attributes during pack file copy
Stealthii Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## Development
* Enable running all pods/containers as non-root without any escalation privileges (#400) (by @jk464 @Stealthii)

## v1.1.0
* Fix syntax with ensure-packs-volumes-are-writable job (#403, #411) (by @skiedude)
Expand Down
37 changes: 26 additions & 11 deletions templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,9 @@ Merge packs and virtualenvs from st2 with those from st2packs images
- 'sh'
- '-ec'
- |
/bin/cp -aR /opt/stackstorm/packs/. /opt/stackstorm/packs-shared &&
/bin/cp -aR /opt/stackstorm/virtualenvs/. /opt/stackstorm/virtualenvs-shared
{{- with .securityContext | default $.Values.st2actionrunner.securityContext | default $.Values.securityContext }}
{{/* st2actionrunner is likely the most permissive so use that if defined. */}}
/bin/cp -dR /opt/stackstorm/packs/. /opt/stackstorm/packs-shared &&
/bin/cp -dR /opt/stackstorm/virtualenvs/. /opt/stackstorm/virtualenvs-shared
Comment on lines +353 to +354
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-d only preserves symlinks as-is. -a also copied the mode, including execute bits.

We need to make sure to preserve the execute bit on files. Would we have to add --preserve=mode to copy that?

Also, how will this interact with #245 (Switch from cp to rsync with fallback to cp)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you please check out #415? I'd like to come up with the minimal set of flags for cp and rsync.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Stealthii made these changes (which remains me I should credit him as an author in the commits..), I believe this was to fix issues with running as a non-privileged user. @Stealthii can you comment here / have a look at the above PRs :D

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cognifloyd I've commented on #415 with a proposed change that should preserve all relevant attributes besides ownership. #415 (comment)

Once verified and if it becomes part of that PR, we can drop the change here and have this PR focus solely on the enablement of non-root runtimes.

{{- with $.Values.securityContext }}
securityContext: {{- toYaml . | nindent 8 }}
{{- end }}
{{- end }}
Expand All @@ -372,10 +371,9 @@ Merge packs and virtualenvs from st2 with those from st2packs images
- 'sh'
- '-ec'
- |
/bin/cp -aR /opt/stackstorm/packs/. /opt/stackstorm/packs-shared &&
/bin/cp -aR /opt/stackstorm/virtualenvs/. /opt/stackstorm/virtualenvs-shared
{{- with .Values.st2actionrunner.securityContext | default .Values.securityContext }}
{{/* st2actionrunner is likely the most permissive so use that if defined. */}}
/bin/cp -dR /opt/stackstorm/packs/. /opt/stackstorm/packs-shared &&
/bin/cp -dR /opt/stackstorm/virtualenvs/. /opt/stackstorm/virtualenvs-shared
{{- with .Values.securityContext }}
securityContext: {{- toYaml . | nindent 8 }}
{{- end }}
{{- end }}
Expand All @@ -393,9 +391,8 @@ Merge packs and virtualenvs from st2 with those from st2packs images
- 'sh'
- '-ec'
- |
/bin/cp -aR /opt/stackstorm/configs/. /opt/stackstorm/configs-shared
{{- with .Values.st2actionrunner.securityContext | default .Values.securityContext }}
{{/* st2actionrunner is likely the most permissive so use that if defined. */}}
/bin/cp -dR /opt/stackstorm/configs/. /opt/stackstorm/configs-shared
{{- with .Values.securityContext }}
securityContext: {{- toYaml . | nindent 8 }}
{{- end }}
{{- end }}
Expand All @@ -422,3 +419,21 @@ Create the custom env list for each deployment
value: {{ $value | quote }}
{{- end }}
{{- end -}}

{{/*
Define st2web ports
*/}}
{{- define "stackstorm-ha.st2web.http_port" -}}
{{- if ne (default 0 ((($.Values.st2web.securityContext).runAsUser) | int)) 0 -}}
8080
{{- else -}}
80
{{- end -}}
{{- end -}}
{{- define "stackstorm-ha.st2web.https_port" -}}
{{- if ne (default 0 ((($.Values.st2web.securityContext).runAsUser) | int)) 0 -}}
8443
{{- else -}}
443
{{- end -}}
{{- end -}}
14 changes: 10 additions & 4 deletions templates/deployments.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,13 +439,15 @@ spec:
securityContext: {{- toYaml . | nindent 10 }}
{{- end }}
ports:
- containerPort: {{ eq (get .Values.st2web.env "ST2WEB_HTTPS" | toString) "1" | ternary 443 80 }}
{{- $https_port := include "stackstorm-ha.st2web.https_port" . }}
{{- $http_port := include "stackstorm-ha.st2web.http_port" . }}
- containerPort: {{ eq (get .Values.st2web.env "ST2WEB_HTTPS" | toString) "1" | ternary $https_port $http_port }}
# Probe to check if app is running. Failure will lead to a pod restart.
livenessProbe:
httpGet:
scheme: {{ eq (get .Values.st2web.env "ST2WEB_HTTPS" | toString) "1" | ternary "HTTPS" "HTTP" }}
path: /
port: {{ eq (get .Values.st2web.env "ST2WEB_HTTPS" | toString) "1" | ternary 443 80 }}
port: {{ eq (get .Values.st2web.env "ST2WEB_HTTPS" | toString) "1" | ternary $https_port $http_port }}
initialDelaySeconds: 1
# Probe to check if app is ready to serve traffic. Failure will lead to temp stop serving traffic.
# TODO: Failing to add readinessProbe, since st2 requires authorization (401) and we don't have `/healthz` endpoints yet (https://github.com/StackStorm/st2/issues/4020)
Expand Down Expand Up @@ -1644,13 +1646,17 @@ spec:
{{- end }}
volumeMounts:
- name: st2client-config-vol
{{- if eq .Values.st2.system_user.user "root" }}
mountPath: /root/.st2/
{{- else }}
mountPath: /home/{{ .Values.st2.system_user.user }}/.st2/
{{- end }}
# `st2 login` doesn't exit on failure correctly, use old methods instead. See bug: https://github.com/StackStorm/st2/issues/4338
command:
- 'sh'
- '-ec'
- |
cat <<EOT > /root/.st2/config
cat <<EOT > /home/stanley/.st2/config
{{- tpl .Values.st2client.st2clientConfig . | nindent 12 }}
EOT
containers:
Expand Down Expand Up @@ -1683,7 +1689,7 @@ spec:
{{- end }}
{{- include "stackstorm-ha.overrides-config-mounts" . | nindent 8 }}
- name: st2client-config-vol
mountPath: /root/.st2/
mountPath: /home/stanley/.st2/
- name: st2-ssh-key-vol
mountPath: {{ tpl .Values.st2.system_user.ssh_key_file . | dir | dir }}/.ssh-key-vol/
{{- if ne "disable" (default "" .Values.st2.datastore_crypto_key) }}
Expand Down
18 changes: 9 additions & 9 deletions templates/jobs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,13 @@ spec:
{{- end }}
volumeMounts:
- name: st2client-config-vol
mountPath: /root/.st2/
mountPath: /home/stanley/.st2/
# `st2 login` doesn't exit on failure correctly, use old methods instead. See bug: https://github.com/StackStorm/st2/issues/4338
command:
- 'sh'
- '-ec'
- |
cat <<EOT > /root/.st2/config
cat <<EOT > /home/stanley/.st2/config
{{- tpl .Values.jobs.st2clientConfig . | nindent 12 }}
EOT
containers:
Expand All @@ -197,7 +197,7 @@ spec:
{{- end }}
volumeMounts:
- name: st2client-config-vol
mountPath: /root/.st2/
mountPath: /home/stanley/.st2/
- name: st2-apikeys-vol
mountPath: /etc/st2/apikeys.yaml
subPath: apikeys.yaml
Expand Down Expand Up @@ -290,13 +290,13 @@ spec:
{{- end }}
volumeMounts:
- name: st2client-config-vol
mountPath: /root/.st2/
mountPath: /home/stanley/.st2/
# `st2 login` doesn't exit on failure correctly, use old methods instead. See bug: https://github.com/StackStorm/st2/issues/4338
command:
- 'sh'
- '-ec'
- |
cat <<EOT > /root/.st2/config
cat <<EOT > /home/stanley/.st2/config
{{- tpl .Values.jobs.st2clientConfig . | nindent 12 }}
EOT
containers:
Expand Down Expand Up @@ -325,7 +325,7 @@ spec:
volumeMounts:
{{- include "stackstorm-ha.st2-config-volume-mounts" . | nindent 8 }}
- name: st2client-config-vol
mountPath: /root/.st2/
mountPath: /home/stanley/.st2/
- name: st2-kv-vol
mountPath: /etc/st2/st2kv.yaml
subPath: st2kv.yaml
Expand Down Expand Up @@ -660,13 +660,13 @@ spec:
{{- end }}
volumeMounts:
- name: st2client-config-vol
mountPath: /root/.st2/
mountPath: /home/stanley/.st2/
# `st2 login` doesn't exit on failure correctly, use old methods instead. See bug: https://github.com/StackStorm/st2/issues/4338
command:
- 'sh'
- '-ec'
- |
cat <<EOT > /root/.st2/config
cat <<EOT > /home/stanley/.st2/config
{{- tpl $.Values.jobs.st2clientConfig $ | nindent 12 }}
EOT
containers:
Expand All @@ -692,7 +692,7 @@ spec:
{{- end }}
volumeMounts:
- name: st2client-config-vol
mountPath: /root/.st2/
mountPath: /home/stanley/.st2/
{{- include "stackstorm-ha.overrides-config-mounts" $ | nindent 8 }}
{{- include "stackstorm-ha.st2-config-volume-mounts" $ | nindent 8 }}
{{- include "stackstorm-ha.packs-volume-mounts-for-register-job" $ | nindent 8 }}
Expand Down
4 changes: 3 additions & 1 deletion templates/services.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,10 @@ spec:
{{- end }}
{{- end }}
ports:
{{- $https_port := include "stackstorm-ha.st2web.https_port" . }}
{{- $http_port := include "stackstorm-ha.st2web.http_port" . }}
- protocol: TCP
port: {{ eq (get .Values.st2web.env "ST2WEB_HTTPS" | toString) "1" | ternary 443 80 }}
port: {{ eq (get .Values.st2web.env "ST2WEB_HTTPS" | toString) "1" | ternary $https_port $http_port }}

{{ if .Values.st2chatops.enabled -}}
---
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/ingress_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ tests:
servicePort: 80
- path: /fancy-sensor
serviceName: fancy-sensor
servicePort: 8080
servicePort: {{ template ".stackstorm-ha.st2web.http_port" . }}
tls: []
st2web:
service:
Expand Down Expand Up @@ -105,4 +105,4 @@ tests:
service:
name: fancy-sensor
port:
number: 8080
number: {{ template ".stackstorm-ha.st2web.http_port" . }}
36 changes: 32 additions & 4 deletions values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,11 @@ st2:
## Default SecurityContext for pods and containers.
## Overrides available for st2web, st2actionrunner, st2sensorcontainer, st2client pods, and custom packs images.
##
podSecurityContext: {}
securityContext: {}
podSecurityContext:
runAsNonRoot: true
securityContext:
runAsUser: 1000
allowPrivilegeEscalation: false
Comment on lines +303 to +307
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my cluster, I use drop capabilities, re-adding them only for the deployments that need it. So I have:

#podSecurityContext:
securityContext:
  capabilities:
    drop: ["ALL"]
st2actionrunner:
  #podSecurityContext:
  securityContext:
    capabilities:
      drop: ["ALL"]
      add:
        - chown
        - fowner # chmod
        - fsetid
        - setfcap
        - setgid
        - setuid
        - dac_override # allows root to bypass "discretionary access control" (file rwx permission checks)
        - kill
        - net_raw # ping
        - audit_write # sudo
        - setpcap
st2web:
  #podSecurityContext:
  securityContext:
    capabilities:
      drop: ["ALL"]
      add:
        - chown
        - setgid
        - setuid
        - dac_override # allows root to bypass "discretionary access control" (file rwx permission checks)
        - net_bind_service # bind privileged ports 80 or 443

So, that's a slightly different approach than just running as a non-root user. In particular, if st2web is not running as root, and is using unprivileged ports, then the config could be somewhat simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for st2web, the fact I don't bind privileged ports, and also I ensured any required files are chown by nginx in the Docker image removes any need for this capabilities?

For st2actionrunner do we have any documentation that points towards what st2actionrunner needs each of those capabilities for? I'm wondering if we can remove any of them by making changes elsewhere, w/o breaking compatibility ofc.


##
## StackStorm HA Ingress
Expand Down Expand Up @@ -377,7 +380,10 @@ st2web:
attach: false
# override the default .podSecurityContext or .securityContext here
podSecurityContext: {}
securityContext: {} # NB: nginx requires some capabilities, drop ALL will cause issues.
securityContext: # NB: nginx requires some capabilities, drop ALL will cause issues.
runAsUser: 999 # run as nginx user
runAsGroup: 999 # run as nginx group
allowPrivilegeEscalation: false
# mount extra volumes on the st2web pod(s) (primarily useful for k8s-provisioned secrets)
## Note that Helm templating is supported in 'mount' and 'volume'
extra_volumes: []
Expand Down Expand Up @@ -1050,6 +1056,15 @@ mongodb:
arbiter:
enabled: false
resources: {}
podSecurityContext:
enabled: true
fsGroup: 1001
sysctls: []
containerSecurityContext:
enabled: true
runAsUser: 1001
runAsNonRoot: true
allowPrivilegeEscalation: false

##
## RabbitMQ configuration (3rd party chart dependency)
Expand Down Expand Up @@ -1097,7 +1112,12 @@ rabbitmq:
# As RabbitMQ enabled prometheus operator monitoring by default, disable it for non-prometheus users
metrics:
enabled: false

podSecurityContext:
fsGroup: 1001
runAsUser: 1001
runAsNonRoot: true
containerSecurityContext:
allowPrivilegeEscalation: false
##
## Redis HA configuration (3rd party chart dependency)
##
Expand Down Expand Up @@ -1133,6 +1153,14 @@ redis:
usePassword: false
metrics:
enabled: false
securityContext:
enabled: true
fsGroup: 1001
runAsNonRoot: true
containerSecurityContext:
enabled: true
runAsUser: 1001
allowPrivilegeEscalation: false

##
## Settings to be applied to all stackstorm-ha pods
Expand Down
Loading